bug 995801 - cache nsNSSCertificate::mCachedEVStatus on disk r=mayhemer
☠☠ backed out by 1ba1d7518428 ☠ ☠
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 10 Jan 2014 11:13:03 -0800
changeset 185042 d5da62e82faf
parent 185041 c9cb265b05f3
child 185043 3e60960771e5
push id44014
push userdkeeler@mozilla.com
push dateTue, 27 May 2014 17:08:28 +0000
treeherdermozilla-inbound@d5da62e82faf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs995801
milestone32.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 995801 - cache nsNSSCertificate::mCachedEVStatus on disk r=mayhemer
security/manager/ssl/src/TransportSecurityInfo.cpp
security/manager/ssl/src/nsNSSCertificate.cpp
security/manager/ssl/src/nsNSSCertificate.h
--- a/security/manager/ssl/src/TransportSecurityInfo.cpp
+++ b/security/manager/ssl/src/TransportSecurityInfo.cpp
@@ -279,192 +279,116 @@ TransportSecurityInfo::GetInterface(cons
     nsCOMPtr<nsIInterfaceRequestor> ir = new PipUIContext();
     rv = ir->GetInterface(uuid, result);
   } else {
     rv = mCallbacks->GetInterface(uuid, result);
   }
   return rv;
 }
 
-static NS_DEFINE_CID(kNSSCertificateCID, NS_X509CERT_CID);
-#define TRANSPORTSECURITYINFOMAGIC { 0xa9863a23, 0x26b8, 0x4a9c, \
-  { 0x83, 0xf1, 0xe9, 0xda, 0xdb, 0x36, 0xb8, 0x30 } }
+// This is a new magic value. However, it re-uses the first 4 bytes
+// of the previous value. This is so when older versions attempt to
+// read a newer serialized TransportSecurityInfo, they will actually
+// fail and return NS_ERROR_FAILURE instead of silently failing.
+#define TRANSPORTSECURITYINFOMAGIC { 0xa9863a23, 0x28ea, 0x45d2, \
+  { 0xa2, 0x5a, 0x35, 0x7c, 0xae, 0xfa, 0x7f, 0x82 } }
 static NS_DEFINE_CID(kTransportSecurityInfoMagic, TRANSPORTSECURITYINFOMAGIC);
 
 NS_IMETHODIMP
 TransportSecurityInfo::Write(nsIObjectOutputStream* stream)
 {
-  stream->WriteID(kTransportSecurityInfoMagic);
+  nsresult rv = stream->WriteID(kTransportSecurityInfoMagic);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
   MutexAutoLock lock(mMutex);
 
-  RefPtr<nsSSLStatus> status(mSSLStatus);
-  nsCOMPtr<nsISerializable> certSerializable;
-
-  // Write a redundant copy of the certificate for backward compatibility
-  // with previous versions, which also unnecessarily wrote it.
-  //
-  // As we are reading the object our self, not using ReadObject, we have
-  // to store it here 'manually' as well, mimicking our object stream
-  // implementation.
-
-  if (status) {
-    nsCOMPtr<nsIX509Cert> cert = status->mServerCert;
-    certSerializable = do_QueryInterface(cert);
-
-    if (!certSerializable) {
-      NS_ERROR("certificate is missing or isn't serializable");
-      return NS_ERROR_UNEXPECTED;
-    }
-  } else {
-    NS_WARNING("Serializing nsNSSSocketInfo without mSSLStatus");
+  rv = stream->Write32(mSecurityState);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
-
-  // Store the flag if there is the certificate present
-  stream->WriteBoolean(certSerializable);
-  if (certSerializable) {
-    stream->WriteID(kNSSCertificateCID);
-    stream->WriteID(NS_GET_IID(nsISupports));
-    certSerializable->Write(stream);
+  rv = stream->Write32(mSubRequestsBrokenSecurity);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  rv = stream->Write32(mSubRequestsNoSecurity);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
-
-  // Store the version number of the binary stream data format.
-  // The 0xFFFF0000 mask is included to the version number
-  // to distinguish version number from mSecurityState
-  // field stored in times before versioning has been introduced.
-  // This mask value has been chosen as mSecurityState could
-  // never be assigned such value.
-  uint32_t version = 3;
-  stream->Write32(version | 0xFFFF0000);
-  stream->Write32(mSecurityState);
-  stream->WriteWStringZ(EmptyString().get()); 
-
   // XXX: uses nsNSSComponent string bundles off the main thread
-  nsresult rv = formatErrorMessage(lock, 
-                                   mErrorCode, mErrorMessageType,
-                                   true, true, mErrorMessageCached);
-  NS_ENSURE_SUCCESS(rv, rv);
-  stream->WriteWStringZ(mErrorMessageCached.get());
-
-  stream->WriteCompoundObject(NS_ISUPPORTS_CAST(nsISSLStatus*, status),
-                              NS_GET_IID(nsISupports), true);
-
-  stream->Write32((uint32_t)0);
-  stream->Write32((uint32_t)0);
-  stream->Write32((uint32_t)mSubRequestsBrokenSecurity);
-  stream->Write32((uint32_t)mSubRequestsNoSecurity);
+  rv = formatErrorMessage(lock, mErrorCode, mErrorMessageType, true, true,
+                          mErrorMessageCached);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  rv = stream->WriteWStringZ(mErrorMessageCached.get());
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  nsCOMPtr<nsISerializable> serializable(mSSLStatus);
+  rv = stream->WriteCompoundObject(serializable, NS_GET_IID(nsISSLStatus),
+                                   true);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
   return NS_OK;
 }
 
-static bool CheckUUIDEquals(uint32_t m0,
-                            nsIObjectInputStream* stream,
-                            const nsCID& id)
-{
-  nsID tempID;
-  tempID.m0 = m0;
-  stream->Read16(&tempID.m1);
-  stream->Read16(&tempID.m2);
-  for (int i = 0; i < 8; ++i)
-    stream->Read8(&tempID.m3[i]);
-  return tempID.Equals(id);
-}
-
 NS_IMETHODIMP
 TransportSecurityInfo::Read(nsIObjectInputStream* stream)
 {
-  nsresult rv;
-
-  uint32_t version;
-  bool certificatePresent;
-
-  // Check what we have here...
-  uint32_t UUID_0;
-  stream->Read32(&UUID_0);
-  if (UUID_0 == kTransportSecurityInfoMagic.m0) {
-    // It seems this stream begins with our magic ID, check it really is there
-    if (!CheckUUIDEquals(UUID_0, stream, kTransportSecurityInfoMagic))
-      return NS_ERROR_FAILURE;
-
-    // OK, this seems to be our stream, now continue to check there is
-    // the certificate
-    stream->ReadBoolean(&certificatePresent);
-    stream->Read32(&UUID_0);
-  }
-  else {
-    // There is no magic, assume there is a certificate present as in versions
-    // prior to those with the magic didn't store that flag; we check the 
-    // certificate is present by cheking the CID then
-    certificatePresent = true;
+  nsID id;
+  nsresult rv = stream->ReadID(&id);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
-
-  if (certificatePresent && UUID_0 == kNSSCertificateCID.m0) {
-    // It seems there is the certificate CID present, check it now; we only
-    // have this single certificate implementation at this time.
-    if (!CheckUUIDEquals(UUID_0, stream, kNSSCertificateCID))
-      return NS_ERROR_FAILURE;
-
-    // OK, we have read the CID of the certificate, check the interface ID
-    nsID tempID;
-    stream->ReadID(&tempID);
-    if (!tempID.Equals(NS_GET_IID(nsISupports)))
-      return NS_ERROR_FAILURE;
-
-    nsCOMPtr<nsISerializable> serializable =
-        do_CreateInstance(kNSSCertificateCID, &rv);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    // This is the redundant copy of the certificate; just ignore it
-    serializable->Read(stream);
-
-    // We are done with reading the certificate, now read the version
-    // as we did before.
-    stream->Read32(&version);
-  }
-  else {
-    // There seems not to be the certificate present in the stream.
-    version = UUID_0;
+  if (!id.Equals(kTransportSecurityInfoMagic)) {
+    return NS_ERROR_UNEXPECTED;
   }
 
   MutexAutoLock lock(mMutex);
 
-  // If the version field we have just read is not masked with 0xFFFF0000
-  // then it is stored mSecurityState field and this is version 1 of
-  // the binary data stream format.
-  if ((version & 0xFFFF0000) == 0xFFFF0000) {
-    version &= ~0xFFFF0000;
-    stream->Read32(&mSecurityState);
+  rv = stream->Read32(&mSecurityState);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  uint32_t subRequestsBrokenSecurity;
+  rv = stream->Read32(&subRequestsBrokenSecurity);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  if (subRequestsBrokenSecurity >
+      static_cast<uint32_t>(std::numeric_limits<int32_t>::max())) {
+    return NS_ERROR_UNEXPECTED;
   }
-  else {
-    mSecurityState = version;
-    version = 1;
+  mSubRequestsBrokenSecurity = subRequestsBrokenSecurity;
+  uint32_t subRequestsNoSecurity;
+  rv = stream->Read32(&subRequestsNoSecurity);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
-  nsAutoString dummyShortDesc;
-  stream->ReadString(dummyShortDesc);
-  stream->ReadString(mErrorMessageCached);
+  if (subRequestsNoSecurity >
+      static_cast<uint32_t>(std::numeric_limits<int32_t>::max())) {
+    return NS_ERROR_UNEXPECTED;
+  }
+  mSubRequestsNoSecurity = subRequestsNoSecurity;
+  rv = stream->ReadString(mErrorMessageCached);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
   mErrorCode = 0;
-
-  nsCOMPtr<nsISupports> obj;
-  stream->ReadObject(true, getter_AddRefs(obj));
-  
-  mSSLStatus = reinterpret_cast<nsSSLStatus*>(obj.get());
-
-  if (!mSSLStatus) {
-    NS_WARNING("deserializing nsNSSSocketInfo without mSSLStatus");
+  nsCOMPtr<nsISupports> supports;
+  rv = stream->ReadObject(true, getter_AddRefs(supports));
+  if (NS_FAILED(rv)) {
+    return rv;
   }
-
-  if (version >= 2) {
-    uint32_t dummySubRequests;
-    stream->Read32((uint32_t*)&dummySubRequests);
-    stream->Read32((uint32_t*)&dummySubRequests);
-    stream->Read32((uint32_t*)&mSubRequestsBrokenSecurity);
-    stream->Read32((uint32_t*)&mSubRequestsNoSecurity);
-  }
-  else {
-    mSubRequestsBrokenSecurity = 0;
-    mSubRequestsNoSecurity = 0;
+  mSSLStatus = reinterpret_cast<nsSSLStatus*>(supports.get());
+  if (!mSSLStatus) {
+    return NS_ERROR_FAILURE;
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 TransportSecurityInfo::GetInterfaces(uint32_t *count, nsIID * **array)
 {
   *count = 0;
--- a/security/manager/ssl/src/nsNSSCertificate.cpp
+++ b/security/manager/ssl/src/nsNSSCertificate.cpp
@@ -1772,31 +1772,49 @@ nsNSSCertListEnumerator::GetNext(nsISupp
   CERT_RemoveCertListNode(node);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSCertificate::Write(nsIObjectOutputStream* aStream)
 {
   NS_ENSURE_STATE(mCert);
-  nsresult rv = aStream->Write32(mCert->derCert.len);
+  nsresult rv = aStream->Write32(static_cast<uint32_t>(mCachedEVStatus));
   if (NS_FAILED(rv)) {
     return rv;
   }
-
+  rv = aStream->Write32(mCert->derCert.len);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
   return aStream->WriteByteArray(mCert->derCert.data, mCert->derCert.len);
 }
 
 NS_IMETHODIMP
 nsNSSCertificate::Read(nsIObjectInputStream* aStream)
 {
   NS_ENSURE_STATE(!mCert);
 
+  uint32_t cachedEVStatus;
+  nsresult rv = aStream->Read32(&cachedEVStatus);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  if (cachedEVStatus == static_cast<uint32_t>(ev_status_unknown)) {
+    mCachedEVStatus = ev_status_unknown;
+  } else if (cachedEVStatus == static_cast<uint32_t>(ev_status_valid)) {
+    mCachedEVStatus = ev_status_valid;
+  } else if (cachedEVStatus == static_cast<uint32_t>(ev_status_invalid)) {
+    mCachedEVStatus = ev_status_invalid;
+  } else {
+    return NS_ERROR_UNEXPECTED;
+  }
+
   uint32_t len;
-  nsresult rv = aStream->Read32(&len);
+  rv = aStream->Read32(&len);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   nsXPIDLCString str;
   rv = aStream->ReadBytes(len, getter_Copies(str));
   if (NS_FAILED(rv)) {
     return rv;
--- a/security/manager/ssl/src/nsNSSCertificate.h
+++ b/security/manager/ssl/src/nsNSSCertificate.h
@@ -63,17 +63,17 @@ private:
   nsresult GetSortableDate(PRTime aTime, nsAString& _aSortableDate);
   virtual void virtualDestroyNSSReference();
   void destructorSafeDestroyNSSReference();
   bool InitFromDER(char* certDER, int derLen);  // return false on failure
 
   nsresult GetCertificateHash(nsAString& aFingerprint, SECOidTag aHashAlg);
 
   enum {
-    ev_status_unknown = -1, ev_status_invalid = 0, ev_status_valid = 1
+    ev_status_invalid = 0, ev_status_valid = 1, ev_status_unknown = 2
   } mCachedEVStatus;
   SECOidTag mCachedEVOidTag;
   nsresult hasValidEVOidTag(SECOidTag& resultOidTag, bool& validEV);
   nsresult getValidEVOidTag(SECOidTag& resultOidTag, bool& validEV);
 };
 
 class nsNSSCertList: public nsIX509CertList,
                      public nsNSSShutDownObject