Implement Expect-Staple

net/socket/ssl_client_socket_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/ssl_client_socket_impl.h"
 
 #include <errno.h>
 #include <openssl/bio.h>
 #include <openssl/bytestring.h>
 #include <openssl/err.h>
@@ skipping 67 matching lines @@
 
 // TLS extension number use for Token Binding.
 const unsigned int kTbExtNum = 24;
 
 // Token Binding ProtocolVersions supported.
 const uint8_t kTbProtocolVersionMajor = 0;
 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:
       *hash = SSLPrivateKey::Hash::MD5_SHA1;
       return true;
     case NID_sha1:
       *hash = SSLPrivateKey::Hash::SHA1;
       return true;
     case NID_sha256:
       *hash = SSLPrivateKey::Hash::SHA256;
@@ skipping 1244 matching lines @@
       result = ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN;
     else
       pkp_bypassed_ = true;
   }
 
   if (result == OK) {
     // Only check Certificate Transparency if there were no other errors with
     // the connection.
     VerifyCT();
 
+    ReportOCSP();
+
     DCHECK(!certificate_verified_);
     certificate_verified_ = true;
     MaybeCacheSession();
   }
 
   completed_connect_ = true;
   // Exit DoHandshakeLoop and return the result to the caller to Connect.
   DCHECK_EQ(STATE_NONE, next_handshake_state_);
   return result;
 }
@@ skipping 66 matching lines @@
         server_cert_verify_result_.cert_status &= ~CERT_STATUS_IS_EV;
       }
     }
     ct_verify_result_.cert_policy_compliance =
         policy_enforcer_->DoesConformToCertPolicy(
             server_cert_verify_result_.verified_cert.get(),
             ct_verify_result_.verified_scts, net_log_);
   }
 }
 
+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) {
     LogConnectEndEvent(rv);
     DoConnectCallback(rv);
   }
 }
 
 void SSLClientSocketImpl::OnSendComplete(int result) {
   if (next_handshake_state_ == STATE_HANDSHAKE) {
@@ skipping 862 matching lines @@
   if (rv != OK) {
     net_log_.EndEventWithNetErrorCode(NetLog::TYPE_SSL_CONNECT, rv);
     return;
   }
 
   net_log_.EndEvent(NetLog::TYPE_SSL_CONNECT,
                     base::Bind(&NetLogSSLInfoCallback, base::Unretained(this)));
 }
 
 }  // namespace net