bug 528288 spdy - nsisslsocketcontrol::mayjoinconnection() r=honzab r=bsmith sr=honzab sr=biesi

patch 17

netwerk/protocol/http/nsHttpConnectionMgr.cpp

@@ -48,8 +48,7 @@
 
 #include "nsIObserverService.h"
 
-#include "nsISSLStatusProvider.h"
-#include "nsISSLStatus.h"
+#include "nsISSLSocketControl.h"
 #include "prnetdb.h"
 
 using namespace mozilla;
@@ -517,39 +516,8 @@ nsHttpConnectionMgr::ReportSpdyConnection(nsHttpConnection *conn,
         // 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();
 }
 
@@ -634,9 +602,9 @@ nsHttpConnectionMgr::GetSpdyPreferred(nsConnectionEntry *aOriginalEntry)
     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
@@ -644,14 +612,15 @@ nsHttpConnectionMgr::GetSpdyPreferred(nsConnectionEntry *aOriginalEntry)
     // 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.
@@ -667,14 +636,28 @@ nsHttpConnectionMgr::GetSpdyPreferred(nsConnectionEntry *aOriginalEntry)
 
     // Check that the server cert supports redirection
     nsresult rv;
-    bool validCert = false;
+    bool isJoined = false;
 
-    rv = preferred->mCert->IsValidForHostname(
-        aOriginalEntry->mConnInfo->GetHost(), &validCert);
+    nsCOMPtr<nsISupports> securityInfo;
+    nsCOMPtr<nsISSLSocketControl> sslSocketControl;
+    nsCAutoString negotiatedNPN;
+    
+    activeSpdy->GetSecurityInfo(getter_AddRefs(securityInfo));
+    if (!securityInfo)
+        return nsnull;
 
-    if (NS_FAILED(rv) || !validCert) {
+    sslSocketControl = do_QueryInterface(securityInfo, &rv);
+    if (NS_FAILED(rv))
+        return nsnull;
+
+    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;

netwerk/protocol/http/nsHttpConnectionMgr.h

@@ -207,7 +207,6 @@ private:
 
         bool mTestedSpdy;
         bool mSpdyPreferred;
-        nsCOMPtr<nsIX509Cert3> mCert;
     };
 
     // nsConnectionHandle

netwerk/socket/nsISSLSocketControl.idl

@@ -48,7 +48,7 @@ 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;
 
@@ -74,5 +74,15 @@ interface nsISSLSocketControl : nsISupports {
      * 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);
+    
 };
 

security/manager/ssl/public/nsIX509Cert3.idl

@@ -43,7 +43,7 @@ interface nsICertVerificationListener;
 /**
  * Extending nsIX509Cert
  */
-[scriptable, uuid(09143cd9-dee3-4870-8450-8440c87e40e2)]
+[scriptable, uuid(399004d8-b8c7-4eb9-8362-d99f4c0161fd)]
 interface nsIX509Cert3 : nsIX509Cert2 {
 
   /**
@@ -88,15 +88,6 @@ interface nsIX509Cert3 : nsIX509Cert2 {
   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)]

security/manager/ssl/src/nsNSSCallbacks.cpp

@@ -966,6 +966,8 @@ void PR_CALLBACK HandshakeCallback(PRFileDesc* fd, void* client_data) {
       infoObject->SetNegotiatedNPN(reinterpret_cast<char *>(npnbuf), npnlen);
     else
       infoObject->SetNegotiatedNPN(nsnull, 0);
+
+    infoObject->SetHandshakeCompleted();
   }
 
   PORT_Free(cipherName);

security/manager/ssl/src/nsNSSCertificate.cpp

@@ -938,26 +938,6 @@ nsNSSCertificate::GetAllTokenNames(PRUint32 *aLength, PRUnichar*** aTokenNames)
   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)
 {

security/manager/ssl/src/nsNSSIOLayer.cpp

@@ -170,7 +170,8 @@ nsNSSSocketInfo::nsNSSSocketInfo()
     mHandshakeStartTime(0),
     mPort(0),
     mIsCertIssuerBlacklisted(false),
-    mNPNCompleted(false)
+    mNPNCompleted(false),
+    mHandshakeCompleted(false)
 {
 }
 
@@ -456,6 +457,62 @@ nsNSSSocketInfo::GetNegotiatedNPN(nsACString &aNegotiatedNPN)
   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);

security/manager/ssl/src/nsNSSIOLayer.h

@@ -154,6 +154,7 @@ public:
   }
 
   void SetNegotiatedNPN(const char *value, PRUint32 length);
+  void SetHandshakeCompleted() { mHandshakeCompleted = true; }
 
   // XXX: These are only used on for diagnostic purposes
   enum CertVerificationState {
@@ -212,6 +213,7 @@ protected:
 
   nsCString mNegotiatedNPN;
   bool      mNPNCompleted;
+  bool      mHandshakeCompleted;
 
 private:
   virtual void virtualDestroyNSSReference();

security/manager/ssl/src/nsSSLStatus.h

@@ -74,6 +74,9 @@ public:
   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;
 };