Extract Certificate Transparency SCTs from stapled OCSP responses

net/socket/ssl_client_socket_nss.cc

Index: net/socket/ssl_client_socket_nss.cc
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 13890ab076761722910253fe4cc845de4fc9707e..bb6f865439608bfca0a8f65ff495002c6290215a 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -93,6 +93,7 @@
 #include "net/cert/asn1_util.h"
 #include "net/cert/cert_status_flags.h"
 #include "net/cert/cert_verifier.h"
+#include "net/cert/ct_objects_extractor.h"
 #include "net/cert/ct_verifier.h"
 #include "net/cert/ct_verify_result.h"
 #include "net/cert/scoped_nss_types.h"
@@ -418,6 +419,7 @@ struct HandshakeState {
     server_cert_chain.Reset(NULL);
     server_cert = NULL;
     sct_list_from_tls_extension.clear();
+    sct_list_from_ocsp_stapling.clear();
     resumed_handshake = false;
     ssl_connection_status = 0;
   }
@@ -449,6 +451,9 @@ struct HandshakeState {
   scoped_refptr<X509Certificate> server_cert;
   // SignedCertificateTimestampList received via TLS extension (RFC 6962).
   std::string sct_list_from_tls_extension;
+  // SignedCertificateTimestampList received in a stapled OCSP response
+  // (RFC 6962).
+  std::string sct_list_from_ocsp_stapling;
 
   // True if the current handshake was the result of TLS session resumption.
   bool resumed_handshake;
@@ -760,10 +765,15 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe<Core> {
 
   // Updates the NSS and platform specific certificates.
   void UpdateServerCert();
-  // Update the nss_handshake_state_ with SignedCertificateTimestampLists
-  // received in the handshake, via a TLS extension or (to be implemented)
-  // OCSP stapling.
+  // Update the nss_handshake_state_ with the SignedCertificateTimestampList
+  // received in the handshake via a TLS extension.
   void UpdateSignedCertTimestamps();
+#ifdef SSL_ENABLE_OCSP_STAPLING

[Ryan Sleevi, 2013/12/03 21:03:18]

1) This should be #if defined
2) We shouldn't be using an ifdef here, because this will always be true given
our build time requirements (NSS 3.14.3+, but always using our own libssl)

[wtc, 2013/12/03 21:04:25]

Nit: can you take the opportunity to remove all #ifdef SSL_ENABLE_OCSP_STAPLING
from this file?

We used to be careful about that, so that we can compile this file against an
older version of the system NSS SSL library. But we no longer support that.

[ekasper, 2013/12/04 19:25:15]

Done.

+  // Update the OCSP response cache with the stapled response received in the
+  // handshake, and update nss_handshake_state_ with
+  // the SignedCertificateTimestampList received in the stapled OCSP response.
+  void UpdateStapledOCSPResponse();
+#endif
   // Updates the nss_handshake_state_ with the negotiated security parameters.
   void UpdateConnectionStatus();
   // Record histograms for channel id support during full handshakes - resumed
@@ -1663,6 +1673,9 @@ void SSLClientSocketNSS::Core::HandshakeSucceeded() {
   RecordChannelIDSupportOnNSSTaskRunner();
   UpdateServerCert();
   UpdateSignedCertTimestamps();
+#ifdef SSL_ENABLE_OCSP_STAPLING
+  UpdateStapledOCSPResponse();
+#endif
   UpdateConnectionStatus();
   UpdateNextProto();
 
@@ -1834,43 +1847,6 @@ int SSLClientSocketNSS::Core::DoHandshake() {
       false_started_ = true;
       HandshakeSucceeded();
     }
-
-    // TODO(wtc): move this block of code to OwnAuthCertHandler.
-  #if defined(SSL_ENABLE_OCSP_STAPLING)
-    // TODO(agl): figure out how to plumb an OCSP response into the Mac
-    // system library and update IsOCSPStaplingSupported for Mac.
-    if (IsOCSPStaplingSupported()) {
-      const SECItemArray* ocsp_responses =
-          SSL_PeerStapledOCSPResponses(nss_fd_);
-      if (ocsp_responses->len) {
-  #if defined(OS_WIN)
-        if (nss_handshake_state_.server_cert) {
-          CRYPT_DATA_BLOB ocsp_response_blob;
-          ocsp_response_blob.cbData = ocsp_responses->items[0].len;
-          ocsp_response_blob.pbData = ocsp_responses->items[0].data;
-          BOOL ok = CertSetCertificateContextProperty(
-              nss_handshake_state_.server_cert->os_cert_handle(),
-              CERT_OCSP_RESPONSE_PROP_ID,
-              CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG,
-              &ocsp_response_blob);
-          if (!ok) {
-            VLOG(1) << "Failed to set OCSP response property: "
-                    << GetLastError();
-          }
-        }
-  #elif defined(USE_NSS)
-        CacheOCSPResponseFromSideChannelFunction cache_ocsp_response =
-            GetCacheOCSPResponseFromSideChannelFunction();
-
-        cache_ocsp_response(
-            CERT_GetDefaultCertDB(),
-            nss_handshake_state_.server_cert_chain[0], PR_Now(),
-            &ocsp_responses->items[0], NULL);
-  #endif
-      }
-    }
-  #endif
-    // Done!
   } else {
     PRErrorCode prerr = PR_GetError();
     net_error = HandleNSSError(prerr, true);
@@ -2436,6 +2412,54 @@ void SSLClientSocketNSS::Core::UpdateSignedCertTimestamps() {
       signed_cert_timestamps->len);
 }
 
+#ifdef SSL_ENABLE_OCSP_STAPLING
+void SSLClientSocketNSS::Core::UpdateStapledOCSPResponse() {
+  const SECItemArray* ocsp_responses =
+      SSL_PeerStapledOCSPResponses(nss_fd_);
+  if (!ocsp_responses || !ocsp_responses->len)
+    return;
+
+  if (ssl_config_.signed_cert_timestamps_enabled &&
+      nss_handshake_state_.server_cert) {
+    std::string ocsp_response(
+        reinterpret_cast<char*>(ocsp_responses->items[0].data),
+        ocsp_responses->items[0].len);

[wtc, 2013/12/03 21:04:25]

We are copying an OCSP response here. This seems to argue for changing
ct::ExtractSCTListFromOCSPResponse() to take a base::StringPiece instead of a
std::string as the |ocsp_response| input.

[ekasper, 2013/12/04 19:25:15]

This has changed - I now store the raw response and parse it differently, later
on.

+    ct::ExtractSCTListFromOCSPResponse(
+        nss_handshake_state_.server_cert->os_cert_handle(),
+        ocsp_response,
+        &nss_handshake_state_.sct_list_from_ocsp_stapling);
+  }
+  // TODO(agl): figure out how to plumb an OCSP response into the Mac
+  // system library and update IsOCSPStaplingSupported for Mac.
+  if (IsOCSPStaplingSupported()) {
+  #if defined(OS_WIN)
+    if (nss_handshake_state_.server_cert) {

[wtc, 2013/12/03 21:04:25]

Hmm... I wonder why we check nss_handshake_state_.server_cert for null in the
OS_WIN case but not in the USE_NSS case.

This comes from the original code, so you don't need to deal with this.

[Ryan Sleevi, 2013/12/03 21:07:15]

Because Windows will fail to parse certificates that don't map into Windows
structures (eg: FILETIME date ranges), but NSS will.

http://crbug.com/91341

Also, Chromoting (which doesn't have and can't use CAPI), but did use this code
with pseudotcpsocket

+      CRYPT_DATA_BLOB ocsp_response_blob;
+      ocsp_response_blob.cbData = ocsp_responses->items[0].len;
+      ocsp_response_blob.pbData = ocsp_responses->items[0].data;
+      BOOL ok = CertSetCertificateContextProperty(
+          nss_handshake_state_.server_cert->os_cert_handle(),
+          CERT_OCSP_RESPONSE_PROP_ID,
+          CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG,
+          &ocsp_response_blob);
+      if (!ok) {
+        VLOG(1) << "Failed to set OCSP response property: "
+                << GetLastError();
+      }
+    }
+  #elif defined(USE_NSS)
+    CacheOCSPResponseFromSideChannelFunction cache_ocsp_response =
+        GetCacheOCSPResponseFromSideChannelFunction();
+
+    cache_ocsp_response(
+        CERT_GetDefaultCertDB(),
+        nss_handshake_state_.server_cert_chain[0], PR_Now(),
+        &ocsp_responses->items[0], NULL);
+  #endif
+  }  // IsOCSPStaplingSupported()
+}
+#endif
+
 void SSLClientSocketNSS::Core::UpdateConnectionStatus() {
   SSLChannelInfo channel_info;
   SECStatus ok = SSL_GetChannelInfo(nss_fd_,
@@ -3193,12 +3217,14 @@ int SSLClientSocketNSS::InitializeSSLOptions() {
 
 // Added in NSS 3.15
 #ifdef SSL_ENABLE_OCSP_STAPLING
-  if (IsOCSPStaplingSupported()) {
-    rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_OCSP_STAPLING, PR_TRUE);
-    if (rv != SECSuccess) {
-      LogFailedNSSFunction(net_log_, "SSL_OptionSet",
-                           "SSL_ENABLE_OCSP_STAPLING");
-    }
+  // Request OCSP stapling even on platforms that don't support it, in
+  // order to extract Certificate Transparency information.
+  rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_OCSP_STAPLING,
+                     (IsOCSPStaplingSupported() ||
+                      ssl_config_.signed_cert_timestamps_enabled));
+  if (rv != SECSuccess) {
+    LogFailedNSSFunction(net_log_, "SSL_OptionSet",
+                         "SSL_ENABLE_OCSP_STAPLING");
   }
 #endif
 
@@ -3355,7 +3381,8 @@ int SSLClientSocketNSS::DoHandshakeComplete(int result) {
   }
   set_channel_id_sent(core_->state().channel_id_sent);
   set_signed_cert_timestamps_received(
-      !core_->state().sct_list_from_tls_extension.empty());
+      !core_->state().sct_list_from_tls_extension.empty() ||
+      !core_->state().sct_list_from_ocsp_stapling.empty());
 
   LeaveFunction(result);
   return result;
@@ -3513,8 +3540,8 @@ void SSLClientSocketNSS::VerifyCT() {
   // external communication.
   int result = cert_transparency_verifier_->Verify(
       server_cert_verify_result_.verified_cert,
-      std::string(), // SCT list from OCSP response
-      std::string(),  // SCT list from TLS extension
+      core_->state().sct_list_from_ocsp_stapling,
+      core_->state().sct_list_from_tls_extension,
       &ct_verify_result_,
       net_log_);