Bug 995801 - Cache nsNSSCertificate::mCachedEVStatus on disk. r=mayhemer, a=sledru
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 10 Jan 2014 11:13:03 -0800
changeset 192940 3253bb6f2a28
parent 192939 e79674fe4a79
child 192941 065972d5ac21
push id5971
push userryanvm@gmail.com
push dateWed, 04 Jun 2014 16:14:30 +0000
treeherdermozilla-aurora@a132629caa5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer, sledru
bugs995801
milestone31.0a2
Bug 995801 - Cache nsNSSCertificate::mCachedEVStatus on disk. r=mayhemer, a=sledru
security/manager/ssl/src/TransportSecurityInfo.cpp
security/manager/ssl/src/nsNSSCertificate.cpp
security/manager/ssl/src/nsNSSCertificate.h
security/manager/ssl/src/nsNSSCertificateFakeTransport.cpp
--- 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,35 +1772,54 @@ nsNSSCertListEnumerator::GetNext(nsISupp
 
   *_retval = nssCert;
   NS_ADDREF(*_retval);
 
   CERT_RemoveCertListNode(node);
   return NS_OK;
 }
 
+// NB: This serialization must match that of nsNSSCertificateFakeTransport.
 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
@@ -38,16 +38,18 @@ public:
   NS_DECL_NSIX509CERT
   NS_DECL_NSIX509CERT2
   NS_DECL_NSIX509CERT3
   NS_DECL_NSIIDENTITYINFO
   NS_DECL_NSISMIMECERT
   NS_DECL_NSISERIALIZABLE
   NS_DECL_NSICLASSINFO
 
+  friend class nsNSSCertificateFakeTransport;
+
   nsNSSCertificate(CERTCertificate* cert, SECOidTag* evOidPolicy = nullptr);
   nsNSSCertificate();
   virtual ~nsNSSCertificate();
   nsresult FormatUIStrings(const nsAutoString& nickname,
                            nsAutoString& nickWithSerial,
                            nsAutoString& details);
   static nsNSSCertificate* Create(CERTCertificate*cert = nullptr,
                                   SECOidTag* evOidPolicy = nullptr);
@@ -61,17 +63,17 @@ private:
   nsresult CreateTBSCertificateASN1Struct(nsIASN1Sequence** retSequence,
                                           nsINSSComponent* nssComponent);
   nsresult GetSortableDate(PRTime aTime, nsAString& _aSortableDate);
   virtual void virtualDestroyNSSReference();
   void destructorSafeDestroyNSSReference();
   bool InitFromDER(char* certDER, int derLen);  // return false on failure
 
   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
--- a/security/manager/ssl/src/nsNSSCertificateFakeTransport.cpp
+++ b/security/manager/ssl/src/nsNSSCertificateFakeTransport.cpp
@@ -1,23 +1,25 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsNSSCertificateFakeTransport.h"
+
 #include "nsCOMPtr.h"
+#include "nsIObjectInputStream.h"
+#include "nsIObjectOutputStream.h"
+#include "nsIProgrammingLanguage.h"
+#include "nsISupportsPrimitives.h"
+#include "nsIX509Cert.h"
 #include "nsNSSCertificate.h"
-#include "nsIX509Cert.h"
+#include "nsNSSCertificate.h"
 #include "nsString.h"
 #include "nsXPIDLString.h"
-#include "nsISupportsPrimitives.h"
-#include "nsIProgrammingLanguage.h"
-#include "nsIObjectOutputStream.h"
-#include "nsIObjectInputStream.h"
 
 #ifdef PR_LOGGING
 extern PRLogModuleInfo* gPIPNSSLog;
 #endif
 
 /* nsNSSCertificateFakeTransport */
 
 NS_IMPL_ISUPPORTS(nsNSSCertificateFakeTransport,
@@ -232,36 +234,53 @@ nsNSSCertificateFakeTransport::Equals(ns
 
 NS_IMETHODIMP
 nsNSSCertificateFakeTransport::GetSha256SubjectPublicKeyInfoDigest(nsACString_internal&)
 {
   NS_NOTREACHED("Unimplemented on content process");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
+// NB: This serialization must match that of nsNSSCertificate.
 NS_IMETHODIMP
 nsNSSCertificateFakeTransport::Write(nsIObjectOutputStream* aStream)
 {
   // On a non-chrome process we don't have mCert because we lack
   // nsNSSComponent.  nsNSSCertificateFakeTransport object is used only to carry the
   // certificate serialization.
 
-  nsresult rv = aStream->Write32(mCertSerialization->len);
+  // This serialization has to match that of nsNSSCertificate,
+  // so write a fake cached EV Status.
+  uint32_t status = static_cast<uint32_t>(nsNSSCertificate::ev_status_unknown);
+  nsresult rv = aStream->Write32(status);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  rv = aStream->Write32(mCertSerialization->len);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   return aStream->WriteByteArray(mCertSerialization->data, mCertSerialization->len);
 }
 
 NS_IMETHODIMP
 nsNSSCertificateFakeTransport::Read(nsIObjectInputStream* aStream)
 {
+  // This serialization has to match that of nsNSSCertificate,
+  // so read the cachedEVStatus but don't actually use it.
+  uint32_t cachedEVStatus;
+  nsresult rv = aStream->Read32(&cachedEVStatus);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
   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;