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 81990 86e3d2e80614aace6d638b7da82bf777e7a803ac
parent 81989 2cb0358aa68b1ad502a6f6c4efe88fe9308259ae
child 81991 04afb38d54ee14c3830f0d909daf90a35cb57dbd
push id628
push userclegnitto@mozilla.com
push dateWed, 21 Dec 2011 14:41:57 +0000
treeherdermozilla-aurora@24a61ad789e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab, bsmith, honzab, biesi
bugs528288
milestone11.0a1
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 } }