bug 1049095 - re-verify joinee certificate with joining hostname when joining connections r=briansmith r=mcmanus r=cviecco r=mmc r=rbarnes
authorDavid Keeler <dkeeler@mozilla.com>
Thu, 21 Aug 2014 10:37:23 -0700
changeset 200930 c4d1c00413479524bbd1f9ef4ba1f0809099af65
parent 200929 feb73cba8e7fbe149b37eb13cb820bad34b0ab85
child 200931 1903ba337cdd13dbaccf8d6d33282f32f5762e6f
push id8326
push usernigelbabu@gmail.com
push dateFri, 22 Aug 2014 01:48:25 +0000
treeherderfx-team@68af8c8ba44f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbriansmith, mcmanus, cviecco, mmc, rbarnes
bugs1049095
milestone34.0a1
bug 1049095 - re-verify joinee certificate with joining hostname when joining connections r=briansmith r=mcmanus r=cviecco r=mmc r=rbarnes
security/certverifier/CertVerifier.cpp
security/certverifier/CertVerifier.h
security/manager/ssl/src/SSLServerCertVerification.cpp
security/manager/ssl/src/nsNSSIOLayer.cpp
--- a/security/certverifier/CertVerifier.cpp
+++ b/security/certverifier/CertVerifier.cpp
@@ -427,16 +427,17 @@ CertVerifier::VerifyCert(CERTCertificate
 
 SECStatus
 CertVerifier::VerifySSLServerCert(CERTCertificate* peerCert,
                      /*optional*/ const SECItem* stapledOCSPResponse,
                                   Time time,
                      /*optional*/ void* pinarg,
                                   const char* hostname,
                                   bool saveIntermediatesInPermanentDatabase,
+                                  Flags flags,
                  /*optional out*/ ScopedCERTCertList* builtChain,
                  /*optional out*/ SECOidTag* evOidPolicy)
 {
   PR_ASSERT(peerCert);
   // XXX: PR_ASSERT(pinarg)
   PR_ASSERT(hostname);
   PR_ASSERT(hostname[0]);
 
@@ -451,18 +452,18 @@ CertVerifier::VerifySSLServerCert(CERTCe
     PR_SetError(SSL_ERROR_BAD_CERT_DOMAIN, 0);
     return SECFailure;
   }
 
   ScopedCERTCertList builtChainTemp;
   // CreateCertErrorRunnable assumes that CERT_VerifyCertName is only called
   // if VerifyCert succeeded.
   SECStatus rv = VerifyCert(peerCert, certificateUsageSSLServer, time, pinarg,
-                            hostname, 0, stapledOCSPResponse, &builtChainTemp,
-                            evOidPolicy);
+                            hostname, flags, stapledOCSPResponse,
+                            &builtChainTemp, evOidPolicy);
   if (rv != SECSuccess) {
     return rv;
   }
 
   rv = CERT_VerifyCertName(peerCert, hostname);
   if (rv != SECSuccess) {
     return rv;
   }
--- a/security/certverifier/CertVerifier.h
+++ b/security/certverifier/CertVerifier.h
@@ -38,16 +38,17 @@ public:
 
   SECStatus VerifySSLServerCert(
                     CERTCertificate* peerCert,
        /*optional*/ const SECItem* stapledOCSPResponse,
                     mozilla::pkix::Time time,
        /*optional*/ void* pinarg,
                     const char* hostname,
                     bool saveIntermediatesInPermanentDatabase = false,
+                    Flags flags = 0,
    /*optional out*/ ScopedCERTCertList* builtChain = nullptr,
    /*optional out*/ SECOidTag* evOidPolicy = nullptr);
 
   enum pinning_enforcement_config {
     pinningDisabled = 0,
     pinningAllowUserCAMITM = 1,
     pinningStrict = 2,
     pinningEnforceTestMode = 3
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -754,17 +754,17 @@ AuthCertificate(CertVerifier& certVerifi
   // in private, transient contexts.
   bool saveIntermediates =
     !(providerFlags & nsISocketProvider::NO_PERMANENT_STORAGE);
 
   SECOidTag evOidPolicy;
   rv = certVerifier.VerifySSLServerCert(cert, stapledOCSPResponse,
                                         time, infoObject,
                                         infoObject->GetHostNameRaw(),
-                                        saveIntermediates, nullptr,
+                                        saveIntermediates, 0, nullptr,
                                         &evOidPolicy);
 
   // We want to remember the CA certs in the temp db, so that the application can find the
   // complete chain at any time it might need it.
   // But we keep only those CA certs in the temp db, that we didn't already know.
 
   RefPtr<nsSSLStatus> status(infoObject->SSLStatus());
   RefPtr<nsNSSCertificate> nsc;
--- a/security/manager/ssl/src/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/src/nsNSSIOLayer.cpp
@@ -427,19 +427,45 @@ nsNSSSocketInfo::JoinConnection(const ns
   if (cert) {
     nssCert = cert->GetCert();
   }
 
   if (!nssCert) {
     return NS_OK;
   }
 
-  if (CERT_VerifyCertName(nssCert, PromiseFlatCString(hostname).get()) !=
-      SECSuccess) {
-      return NS_OK;
+  // Attempt to verify the joinee's certificate using the joining hostname.
+  // This ensures that any hostname-specific verification logic (e.g. key
+  // pinning) is satisfied by the joinee's certificate chain.
+  // This verification only uses local information; since we're on the network
+  // thread, we would be blocking on ourselves if we attempted any network i/o.
+  // TODO(bug 1056935): The certificate chain built by this verification may be
+  // different than the certificate chain originally built during the joined
+  // connection's TLS handshake. Consequently, we may report a wrong and/or
+  // misleading certificate chain for HTTP transactions coalesced onto this
+  // connection. This may become problematic in the future. For example,
+  // if/when we begin relying on intermediate certificates being stored in the
+  // securityInfo of a cached HTTPS response, that cached certificate chain may
+  // actually be the wrong chain. We should consider having JoinConnection
+  // return the certificate chain built here, so that the calling Necko code
+  // can associate the correct certificate chain with the HTTP transactions it
+  // is trying to join onto this connection.
+  RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
+  if (!certVerifier) {
+    return NS_OK;
+  }
+  nsAutoCString hostnameFlat(PromiseFlatCString(hostname));
+  CertVerifier::Flags flags = CertVerifier::FLAG_LOCAL_ONLY;
+  SECStatus rv = certVerifier->VerifySSLServerCert(nssCert, nullptr,
+                                                   mozilla::pkix::Now(),
+                                                   nullptr, hostnameFlat.get(),
+                                                   false, flags, nullptr,
+                                                   nullptr);
+  if (rv != SECSuccess) {
+    return NS_OK;
   }
 
   // All tests pass - this is joinable
   mJoined = true;
   *_retval = true;
   return NS_OK;
 }