Implement Expect-Staple

net/socket/ssl_client_socket_impl.cc

Index: net/socket/ssl_client_socket_impl.cc
diff --git a/net/socket/ssl_client_socket_impl.cc b/net/socket/ssl_client_socket_impl.cc
index bcd66037690e803e7ded9f276660fcd24d328cea..a517ce50d244abd439b4c00f7c4546f5f438941b 100644
--- a/net/socket/ssl_client_socket_impl.cc
+++ b/net/socket/ssl_client_socket_impl.cc
@@ -85,6 +85,9 @@ const uint8_t kTbProtocolVersionMinor = 5;
 const uint8_t kTbMinProtocolVersionMajor = 0;
 const uint8_t kTbMinProtocolVersionMinor = 3;
 
+// Max age for OCSP responses
+const base::TimeDelta kOCSPResponseMaxAge = base::TimeDelta::FromDays(7);

[Ryan Sleevi, 2016/06/16 21:49:30]

Why is this an aspect of the socket? This is about cryptographic policy, not
related to SSL.

+
 bool EVP_MDToPrivateKeyHash(const EVP_MD* md, SSLPrivateKey::Hash* hash) {
   switch (EVP_MD_type(md)) {
     case NID_md5_sha1:
@@ -1349,6 +1352,8 @@ int SSLClientSocketImpl::DoVerifyCertComplete(int result) {
     // the connection.
     VerifyCT();
 
+    ReportOCSP();
+
     DCHECK(!certificate_verified_);
     certificate_verified_ = true;
     MaybeCacheSession();
@@ -1435,6 +1440,14 @@ void SSLClientSocketImpl::VerifyCT() {
   }
 }
 
+void SSLClientSocketImpl::ReportOCSP() {
+  base::Time verify_time = base::Time::Now();
+  transport_security_state_->CheckExpectStaple(

[Ryan Sleevi, 2016/06/16 21:49:30]

Putting it at this layer creates issues when multiplexing sockets.

If I have Expect-Staple for mail.google.com but not google.com, this won't
trigger if those connections ar emultiplexed.

[dadrian, 2016/06/17 17:26:55]

Why isn't this an issue for Expect CT?

[estark, 2016/06/17 18:33:02]

Expect-CT is processed per-request at the URLRequestHTTPJob layer. That's
more-or-less out of necessity for Expect-CT, because we require an Expect-CT
HTTP response header.

Another interesting question is "Why isn't this an issue for PKP reporting?",
and IIRC the answer to that is "It is." That is, for PKP reporting, we don't
always check pins when pooling (only for QUIC and SPDY), so if a request for
mail.google.com gets sent over a connection that was originally made for a
request to google.com, we won't send a pinning report (or enforce pins, for that
matter). Is my understanding correct, Ryan? (This is all for PKP
enforcing-with-reporting, not PKP-Report-Only.)

The downside to doing reporting at the URLRequestHTTPJob layer is that we will
send a lot more reports (every request to google.com instead of every connection
to google.com).

(I'm pondering whether it is useful to a site operator to receive a report in
Ryan's example of multiplexing mail.google.com and google.com requests, but have
to run to an interview, will ponder more later.)

+      host_and_port_, *server_cert_verify_result_.verified_cert, *server_cert_,
+      server_cert_verify_result_.is_issued_by_known_root, verify_time,
+      kOCSPResponseMaxAge, ocsp_response_);

[Ryan Sleevi, 2016/06/16 21:49:30]

BUG: You never set |ocsp_response_|
DESIGN: Why even have it as a member?

[svaldez, 2016/06/17 13:33:51]

CT and ExpectStaple should probably use a shared ocsp_response_, instead of
calling out to the SSL layer multiple times for it.

+}
+
 void SSLClientSocketImpl::OnHandshakeIOComplete(int result) {
   int rv = DoHandshakeLoop(result);
   if (rv != ERR_IO_PENDING) {