Backed out changeset d5da62e82faf (bug 995801) for test_browserElement_oop_SecurityChange.html failures.
authorRyan VanderMeulen <ryanvm@gmail.com>
Tue, 27 May 2014 14:27:40 -0400
changeset 185045 1ba1d7518428
parent 185044 7be685b59845
child 185167 448f2153d6d3
push id44017
push userryanvm@gmail.com
push dateTue, 27 May 2014 18:27:46 +0000
treeherdermozilla-inbound@1ba1d7518428 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs995801
milestone32.0a1
backs outd5da62e82faf
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
Backed out changeset d5da62e82faf (bug 995801) for test_browserElement_oop_SecurityChange.html failures. CLOSED TREE
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,116 +279,192 @@ TransportSecurityInfo::GetInterface(cons
     nsCOMPtr<nsIInterfaceRequestor> ir = new PipUIContext();
     rv = ir->GetInterface(uuid, result);
   } else {
     rv = mCallbacks->GetInterface(uuid, result);
   }
   return rv;
 }
 
-// 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(kNSSCertificateCID, NS_X509CERT_CID);
+#define TRANSPORTSECURITYINFOMAGIC { 0xa9863a23, 0x26b8, 0x4a9c, \
+  { 0x83, 0xf1, 0xe9, 0xda, 0xdb, 0x36, 0xb8, 0x30 } }
 static NS_DEFINE_CID(kTransportSecurityInfoMagic, TRANSPORTSECURITYINFOMAGIC);
 
 NS_IMETHODIMP
 TransportSecurityInfo::Write(nsIObjectOutputStream* stream)
 {
-  nsresult rv = stream->WriteID(kTransportSecurityInfoMagic);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
+  stream->WriteID(kTransportSecurityInfoMagic);
 
   MutexAutoLock lock(mMutex);
 
-  rv = stream->Write32(mSecurityState);
-  if (NS_FAILED(rv)) {
-    return rv;
+  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(mSubRequestsBrokenSecurity);
-  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(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
-  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;
-  }
+  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);
   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)
 {
-  nsID id;
-  nsresult rv = stream->ReadID(&id);
-  if (NS_FAILED(rv)) {
-    return rv;
+  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;
   }
-  if (!id.Equals(kTransportSecurityInfoMagic)) {
-    return NS_ERROR_UNEXPECTED;
+
+  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;
   }
 
   MutexAutoLock lock(mMutex);
 
-  rv = stream->Read32(&mSecurityState);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  uint32_t subRequestsBrokenSecurity;
-  rv = stream->Read32(&subRequestsBrokenSecurity);
-  if (NS_FAILED(rv)) {
-    return rv;
+  // 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);
   }
-  if (subRequestsBrokenSecurity >
-      static_cast<uint32_t>(std::numeric_limits<int32_t>::max())) {
-    return NS_ERROR_UNEXPECTED;
-  }
-  mSubRequestsBrokenSecurity = subRequestsBrokenSecurity;
-  uint32_t subRequestsNoSecurity;
-  rv = stream->Read32(&subRequestsNoSecurity);
-  if (NS_FAILED(rv)) {
-    return rv;
+  else {
+    mSecurityState = version;
+    version = 1;
   }
-  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;
+  nsAutoString dummyShortDesc;
+  stream->ReadString(dummyShortDesc);
+  stream->ReadString(mErrorMessageCached);
+  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");
   }
-  mErrorCode = 0;
-  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);
   }
-  mSSLStatus = reinterpret_cast<nsSSLStatus*>(supports.get());
-  if (!mSSLStatus) {
-    return NS_ERROR_FAILURE;
+  else {
+    mSubRequestsBrokenSecurity = 0;
+    mSubRequestsNoSecurity = 0;
   }
   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,49 +1772,31 @@ nsNSSCertListEnumerator::GetNext(nsISupp
   CERT_RemoveCertListNode(node);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSCertificate::Write(nsIObjectOutputStream* aStream)
 {
   NS_ENSURE_STATE(mCert);
-  nsresult rv = aStream->Write32(static_cast<uint32_t>(mCachedEVStatus));
+  nsresult rv = aStream->Write32(mCert->derCert.len);
   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;
-  rv = aStream->Read32(&len);
+  nsresult 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_invalid = 0, ev_status_valid = 1, ev_status_unknown = 2
+    ev_status_unknown = -1, ev_status_invalid = 0, ev_status_valid = 1
   } mCachedEVStatus;
   SECOidTag mCachedEVOidTag;
   nsresult hasValidEVOidTag(SECOidTag& resultOidTag, bool& validEV);
   nsresult getValidEVOidTag(SECOidTag& resultOidTag, bool& validEV);
 };
 
 class nsNSSCertList: public nsIX509CertList,
                      public nsNSSShutDownObject