bug 528288 spdy - nsisslsocketcontrol::mayjoinconnection() r=honzab r=bsmith sr=honzab sr=biesi
authorPatrick McManus <mcmanus@ducksong.com>
Fri, 02 Dec 2011 10:28:57 -0500
changeset 81191 86e3d2e80614aace6d638b7da82bf777e7a803ac
parent 81190 2cb0358aa68b1ad502a6f6c4efe88fe9308259ae
child 81192 04afb38d54ee14c3830f0d909daf90a35cb57dbd
push id21564
push usermak77@bonardo.net
push dateSat, 03 Dec 2011 11:10:17 +0000
treeherdermozilla-central@a68c96c1d8e0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab, bsmith, honzab, biesi
bugs528288
milestone11.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 528288 spdy - nsisslsocketcontrol::mayjoinconnection() r=honzab r=bsmith sr=honzab sr=biesi patch 17
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
netwerk/socket/nsISSLSocketControl.idl
security/manager/ssl/public/nsIX509Cert3.idl
security/manager/ssl/src/nsNSSCallbacks.cpp
security/manager/ssl/src/nsNSSCertificate.cpp
security/manager/ssl/src/nsNSSIOLayer.cpp
security/manager/ssl/src/nsNSSIOLayer.h
security/manager/ssl/src/nsSSLStatus.h
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -43,18 +43,17 @@
 #include "nsNetCID.h"
 #include "nsCOMPtr.h"
 #include "nsNetUtil.h"
 
 #include "nsIServiceManager.h"
 
 #include "nsIObserverService.h"
 
-#include "nsISSLStatusProvider.h"
-#include "nsISSLStatus.h"
+#include "nsISSLSocketControl.h"
 #include "prnetdb.h"
 
 using namespace mozilla;
 
 // defined by the socket transport service while active
 extern PRThread *gSocketThread;
 
 static NS_DEFINE_CID(kSocketTransportServiceCID, NS_SOCKETTRANSPORTSERVICE_CID);
@@ -512,49 +511,18 @@ nsHttpConnectionMgr::ReportSpdyConnectio
         SetSpdyPreferred(ent);
         preferred = ent;
     }
     else if (preferred != ent) {
         // A different hostname is the preferred spdy host for this
         // IP address.
         ent->mUsingSpdy = true;
         conn->DontReuse();
-        ent->mCert = nsnull;
     }
 
-    // If this is a preferred host for coalescing (aka ip pooling) then
-    // keep a reference to the server SSL cert. This will be used as an
-    // extra level of verification when deciding that
-    // connections from other hostnames are redirected to the preferred host.
-    //
-    if (preferred == ent) {
-        // Even if mCert is already set update the reference in case the
-        // reference is changing.
-        ent->mCert = nsnull;
-
-        nsCOMPtr<nsISupports> securityInfo;
-        nsCOMPtr<nsISSLStatusProvider> sslStatusProvider;
-        nsCOMPtr<nsISSLStatus> sslStatus;
-        nsCOMPtr<nsIX509Cert> cert;
-
-        conn->GetSecurityInfo(getter_AddRefs(securityInfo));
-        if (securityInfo)
-            sslStatusProvider = do_QueryInterface(securityInfo);
-
-        if (sslStatusProvider)
-            sslStatusProvider->
-                GetSSLStatus(getter_AddRefs(sslStatus));
-
-        if (sslStatus)
-            sslStatus->GetServerCert(getter_AddRefs(cert));
-
-        if (cert)
-            ent->mCert = do_QueryInterface(cert);
-    }
-    
     ProcessSpdyPendingQ();
 }
 
 bool
 nsHttpConnectionMgr::GetSpdyAlternateProtocol(nsACString &hostPortKey)
 {
     // The Alternate Protocol hash is protected under the monitor because
     // it is read from both the main and the network thread.
@@ -629,57 +597,72 @@ nsHttpConnectionMgr::GetSpdyPreferred(ns
 
     nsConnectionEntry *preferred =
         mSpdyPreferredHash.Get(aOriginalEntry->mCoalescingKey);
 
     // if there is no redirection no cert validation is required
     if (preferred == aOriginalEntry)
         return aOriginalEntry;
 
-    // if there is no server cert stored for the destination host
-    // then no validation can be made and we need to skip pooling
-    if (!preferred || !preferred->mCert || !preferred->mUsingSpdy)
+    // if there is no preferred host or it is no longer using spdy
+    // then skip pooling
+    if (!preferred || !preferred->mUsingSpdy)
         return nsnull;                         
 
     // if there is not an active spdy session in this entry then
     // we cannot pool because the cert upon activation may not
     // be the same as the old one. Active sessions are prohibited
     // from changing certs.
 
-    bool activeSpdy = false;
+    nsHttpConnection *activeSpdy = nsnull;
 
-    for (PRUint32 index = 0; index < preferred->mActiveConns.Length(); ++index)
+    for (PRUint32 index = 0; index < preferred->mActiveConns.Length(); ++index) {
         if (preferred->mActiveConns[index]->CanDirectlyActivate()) {
-            activeSpdy = true;
+            activeSpdy = preferred->mActiveConns[index];
             break;
         }
-    
+    }
+
     if (!activeSpdy) {
         // remove the preferred status of this entry if it cannot be
         // used for pooling.
         preferred->mSpdyPreferred = false;
         RemoveSpdyPreferred(preferred->mCoalescingKey);
         LOG(("nsHttpConnectionMgr::GetSpdyPreferredConnection "
              "preferred host mapping %s to %s removed due to inactivity.\n",
              aOriginalEntry->mConnInfo->Host(),
              preferred->mConnInfo->Host()));
 
         return nsnull;
     }
 
     // Check that the server cert supports redirection
     nsresult rv;
-    bool validCert = false;
+    bool isJoined = false;
+
+    nsCOMPtr<nsISupports> securityInfo;
+    nsCOMPtr<nsISSLSocketControl> sslSocketControl;
+    nsCAutoString negotiatedNPN;
+    
+    activeSpdy->GetSecurityInfo(getter_AddRefs(securityInfo));
+    if (!securityInfo)
+        return nsnull;
 
-    rv = preferred->mCert->IsValidForHostname(
-        aOriginalEntry->mConnInfo->GetHost(), &validCert);
+    sslSocketControl = do_QueryInterface(securityInfo, &rv);
+    if (NS_FAILED(rv))
+        return nsnull;
 
-    if (NS_FAILED(rv) || !validCert) {
+    rv = sslSocketControl->JoinConnection(NS_LITERAL_CSTRING("spdy/2"),
+                                          aOriginalEntry->mConnInfo->GetHost(),
+                                          aOriginalEntry->mConnInfo->Port(),
+                                          &isJoined);
+
+    if (NS_FAILED(rv) || !isJoined) {
         LOG(("nsHttpConnectionMgr::GetSpdyPreferredConnection "
-             "Host %s has cert which cannot be confirmed to use "
+             "Host %s cannot be confirmed to be joined "
              "with %s connections",
              preferred->mConnInfo->Host(), aOriginalEntry->mConnInfo->Host()));
         return nsnull;
     }
 
     // IP pooling confirmed
     LOG(("nsHttpConnectionMgr::GetSpdyPreferredConnection "
          "Host %s has cert valid for %s connections",
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -202,17 +202,16 @@ private:
 
         // To have the UsingSpdy flag means some host with the same hash information
         // has done NPN=spdy/2 at some point. It does not mean every connection
         // is currently using spdy.
         bool mUsingSpdy;
 
         bool mTestedSpdy;
         bool mSpdyPreferred;
-        nsCOMPtr<nsIX509Cert3> mCert;
     };
 
     // nsConnectionHandle
     //
     // thin wrapper around a real connection, used to keep track of references
     // to the connection to determine when the connection may be reused.  the
     // transaction (or pipeline) owns a reference to this handle.  this extra
     // layer of indirection greatly simplifies consumer code, avoiding the
--- a/netwerk/socket/nsISSLSocketControl.idl
+++ b/netwerk/socket/nsISSLSocketControl.idl
@@ -43,17 +43,17 @@
 interface nsIInterfaceRequestor;
 
 %{C++
 #include "nsTArray.h"
 class nsCString;
 %}
 [ref] native nsCStringTArrayRef(nsTArray<nsCString>);
 
-[scriptable, uuid(87fe5d5a-7f72-48d7-8a13-ef992e0cad43)]
+[scriptable, uuid(753f0f13-681d-4de3-a6c6-11aa7e0b3afd)]
 interface nsISSLSocketControl : nsISupports {
     attribute nsIInterfaceRequestor     notificationCallbacks;
 
     void proxyStartSSL();
     void StartTLS();
 
     /* NPN (Next Protocol Negotiation) is a mechanism for
        negotiating the protocol to be spoken inside the SSL
@@ -69,10 +69,20 @@ interface nsISSLSocketControl : nsISuppo
      * list. That also includes the case where the server does not
      * implement NPN.
      *
      * If negotiatedNPN is read before NPN has progressed to the point
      * where this information is available NS_ERROR_NOT_CONNECTED is
      * raised.
      */
     readonly attribute ACString negotiatedNPN;
+
+    /* Determine if a potential SSL connection to hostname:port with
+     * a desired NPN negotiated protocol of npnProtocol can use the socket
+     * associated with this object instead of making a new one.
+     */
+    boolean joinConnection(
+      in ACString npnProtocol, /* e.g. "spdy/2" */
+      in ACString hostname,
+      in long port);
+    
 };
 
--- a/security/manager/ssl/public/nsIX509Cert3.idl
+++ b/security/manager/ssl/public/nsIX509Cert3.idl
@@ -38,17 +38,17 @@
 
 #include "nsIX509Cert2.idl"
 
 interface nsICertVerificationListener;
 
 /**
  * Extending nsIX509Cert
  */
-[scriptable, uuid(09143cd9-dee3-4870-8450-8440c87e40e2)]
+[scriptable, uuid(399004d8-b8c7-4eb9-8362-d99f4c0161fd)]
 interface nsIX509Cert3 : nsIX509Cert2 {
 
   /**
    *  Constants for specifying the chain mode when exporting a certificate
    */
   const unsigned long CMS_CHAIN_MODE_CertOnly = 1;
   const unsigned long CMS_CHAIN_MODE_CertChain = 2;
   const unsigned long CMS_CHAIN_MODE_CertChainWithRoot = 3;
@@ -83,25 +83,16 @@ interface nsIX509Cert3 : nsIX509Cert2 {
    * @param length On success, the number of entries in the returned array.
    * @return On success, an array containing the names of all tokens 
    *         the certificate is stored on (may be empty).
    *         On failure the function throws/returns an error.
    */
   void getAllTokenNames(out unsigned long length,
                        [retval, array, size_is(length)] out wstring
                        tokenNames);
-
-   /**
-   * Determine if the certificate can be verified for specific host name
-   *
-   * @param aHostName the hostname to be verified
-   * @return a boolean successful verification
-   */
-    bool isValidForHostname(in AUTF8String aHostName);
-
 };
 
 [scriptable, uuid(2fd0a785-9f2d-4327-8871-8c3e0783891d)]
 interface nsICertVerificationResult : nsISupports {
 
   /**
    *  This interface reflects a container of
    *  verification results. Call will not block.
--- a/security/manager/ssl/src/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/src/nsNSSCallbacks.cpp
@@ -961,16 +961,18 @@ void PR_CALLBACK HandshakeCallback(PRFil
     unsigned char npnbuf[256];
     unsigned int npnlen;
     
     if (SSL_GetNextProto(fd, &state, npnbuf, &npnlen, 256) == SECSuccess &&
         state == SSL_NEXT_PROTO_NEGOTIATED)
       infoObject->SetNegotiatedNPN(reinterpret_cast<char *>(npnbuf), npnlen);
     else
       infoObject->SetNegotiatedNPN(nsnull, 0);
+
+    infoObject->SetHandshakeCompleted();
   }
 
   PORT_Free(cipherName);
   PR_FREEIF(certOrgName);
   PR_Free(signer);
 }
 
 struct OCSPDefaultResponders {
--- a/security/manager/ssl/src/nsNSSCertificate.cpp
+++ b/security/manager/ssl/src/nsNSSCertificate.cpp
@@ -934,36 +934,16 @@ nsNSSCertificate::GetAllTokenNames(PRUin
       return NS_ERROR_OUT_OF_MEMORY;
     }
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsNSSCertificate::IsValidForHostname(const nsACString & aHostName,
-                                     bool *retval)
-{
-  *retval = false;
-
-  nsNSSShutDownPreventionLock locker;
-  if (isAlreadyShutDown())
-    return NS_ERROR_NOT_AVAILABLE;
-
-  if (!mCert)
-    return NS_OK;
-    
-  if (CERT_VerifyCertName(mCert,
-                          PromiseFlatCString(aHostName).get()) == SECSuccess)
-    *retval = true;
-
-  return NS_OK;
-}
-
-NS_IMETHODIMP
 nsNSSCertificate::GetSubjectName(nsAString &_subjectName)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   _subjectName.Truncate();
   if (mCert->subjectName) {
--- a/security/manager/ssl/src/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/src/nsNSSIOLayer.cpp
@@ -165,17 +165,18 @@ nsNSSSocketInfo::nsNSSSocketInfo()
     mHandshakePending(true),
     mHasCleartextPhase(false),
     mHandshakeInProgress(false),
     mAllowTLSIntoleranceTimeout(true),
     mRememberClientAuthCertificate(false),
     mHandshakeStartTime(0),
     mPort(0),
     mIsCertIssuerBlacklisted(false),
-    mNPNCompleted(false)
+    mNPNCompleted(false),
+    mHandshakeCompleted(false)
 {
 }
 
 nsNSSSocketInfo::~nsNSSSocketInfo()
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return;
@@ -451,16 +452,72 @@ nsNSSSocketInfo::GetNegotiatedNPN(nsACSt
 {
   if (!mNPNCompleted)
     return NS_ERROR_NOT_CONNECTED;
 
   aNegotiatedNPN = mNegotiatedNPN;
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsNSSSocketInfo::JoinConnection(const nsACString & npnProtocol,
+                                const nsACString & hostname,
+                                PRInt32 port,
+                                bool *_retval NS_OUTPARAM)
+{
+  *_retval = false;
+
+  // Different ports may not be joined together
+  if (port != mPort)
+    return NS_OK;
+
+  // Make sure NPN has been completed and matches requested npnProtocol
+  if (!mNPNCompleted || !mNegotiatedNPN.Equals(npnProtocol))
+    return NS_OK;
+
+  // If this is the same hostname then the certicate status does not
+  // need to be considered. They are joinable.
+  if (mHostName && hostname.Equals(mHostName)) {
+    *_retval = true;
+    return NS_OK;
+  }
+
+  // Before checking the server certificate we need to make sure the
+  // handshake has completed.
+  if (!mHandshakeCompleted || !SSLStatus() || !SSLStatus()->mServerCert)
+    return NS_OK;
+
+  // If the cert has error bits (e.g. it is untrusted) then do not join.
+  // The value of mHaveCertErrorBits is only reliable because we know that
+  // the handshake completed.
+  if (SSLStatus()->mHaveCertErrorBits)
+    return NS_OK;
+
+  // Ensure that the server certificate covers the hostname that would
+  // like to join this connection
+
+  CERTCertificate *nssCert = nsnull;
+  CERTCertificateCleaner nsscertCleaner(nssCert);
+
+  nsCOMPtr<nsIX509Cert2> cert2 = do_QueryInterface(SSLStatus()->mServerCert);
+  if (cert2)
+    nssCert = cert2->GetCert();
+
+  if (!nssCert)
+    return NS_OK;
+
+  if (CERT_VerifyCertName(nssCert, PromiseFlatCString(hostname).get()) !=
+      SECSuccess)
+    return NS_OK;
+
+  // All tests pass - this is joinable
+  *_retval = true;
+  return NS_OK;
+}
+
 static nsresult
 formatPlainErrorMessage(nsXPIDLCString const & host, PRInt32 port,
                         PRErrorCode err, nsString &returnedMessage);
 
 static nsresult
 formatOverridableCertErrorMessage(nsISSLStatus & sslStatus,
                                   PRErrorCode errorCodeToReport, 
                                   const nsXPIDLCString & host, PRInt32 port,
--- a/security/manager/ssl/src/nsNSSIOLayer.h
+++ b/security/manager/ssl/src/nsNSSIOLayer.h
@@ -149,16 +149,17 @@ public:
   bool IsCertIssuerBlacklisted() const {
     return mIsCertIssuerBlacklisted;
   }
   void SetCertIssuerBlacklisted() {
     mIsCertIssuerBlacklisted = true;
   }
 
   void SetNegotiatedNPN(const char *value, PRUint32 length);
+  void SetHandshakeCompleted() { mHandshakeCompleted = true; }
 
   // XXX: These are only used on for diagnostic purposes
   enum CertVerificationState {
     before_cert_verification,
     waiting_for_cert_verification,
     after_cert_verification
   };
   void SetCertVerificationWaiting();
@@ -207,16 +208,17 @@ protected:
 
   /* SSL Status */
   nsRefPtr<nsSSLStatus> mSSLStatus;
 
   nsresult ActivateSSL();
 
   nsCString mNegotiatedNPN;
   bool      mNPNCompleted;
+  bool      mHandshakeCompleted;
 
 private:
   virtual void virtualDestroyNSSReference();
   void destructorSafeDestroyNSSReference();
 };
 
 class nsCStringHashSet;
 
--- a/security/manager/ssl/src/nsSSLStatus.h
+++ b/security/manager/ssl/src/nsSSLStatus.h
@@ -69,16 +69,19 @@ public:
   PRUint32 mSecretKeyLength;
   nsXPIDLCString mCipherName;
 
   bool mIsDomainMismatch;
   bool mIsNotValidAtThisTime;
   bool mIsUntrusted;
 
   bool mHaveKeyLengthAndCipher;
+
+  /* mHaveCertErrrorBits is relied on to determine whether or not a SPDY
+     connection is eligible for joining in nsNSSSocketInfo::JoinConnection() */
   bool mHaveCertErrorBits;
 };
 
 // 2c3837af-8b85-4a68-b0d8-0aed88985b32
 #define NS_SSLSTATUS_CID \
 { 0x2c3837af, 0x8b85, 0x4a68, \
   { 0xb0, 0xd8, 0x0a, 0xed, 0x88, 0x98, 0x5b, 0x32 } }