Bug 1510569 - Implement serializers for nsITransportSecurityInfo, nsIX509Cert, and nsIX509CertList r=froydnj,keeler,mayhemer

As part of the ongoing effort to port the nsIWebProgress events from
RemoteWebProgress / WebProgressChild to BrowserParent / BrowserChild, we need
to (de)serialize the nsITransportSecurityInfo instance across the IPC layer.
The existing code was calling `NS_SerializeToString` which has the overhead of
(a) allocating a buffer and also performing base64 encoding/decoding. This
patch adds `IPC::ParamTraits` implementations for `nsITransportSecurityInfo`,
`nsIX509Certificate`, and `nsIX509CertList` that (de)serializes the params
directly onto and off of the IPC message so that we don't go through the
overhead of allocating and encoding/decoding an additional buffer.

This (de)serialization will address the performance issues present in the
current implementation.

As a side effect, I also make nsITransportSecurityInfo a builtinclass XPCOM
interface, since the existing serialization code was assuming it was, there is
only one implementation, and it is in C++.

Differential Revision: https://phabricator.services.mozilla.com/D35090

--HG--
extra : moz-landing-system : lando

ipc/glue/TransportSecurityInfoUtils.cpp

@@ -0,0 +1,112 @@
+/* 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 "TransportSecurityInfoUtils.h"
+
+#include "ipc/IPCMessageUtils.h"
+#include "mozilla/psm/TransportSecurityInfo.h"
+
+namespace IPC {
+
+void ParamTraits<nsITransportSecurityInfo*>::Write(
+    Message* aMsg, nsITransportSecurityInfo* aParam) {
+  bool nonNull = !!aParam;
+  WriteParam(aMsg, nonNull);
+  if (!nonNull) {
+    return;
+  }
+
+  aParam->SerializeToIPC(aMsg);
+}
+
+bool ParamTraits<nsITransportSecurityInfo*>::Read(
+    const Message* aMsg, PickleIterator* aIter,
+    RefPtr<nsITransportSecurityInfo>* aResult) {
+  *aResult = nullptr;
+
+  bool nonNull = false;
+  if (!ReadParam(aMsg, aIter, &nonNull)) {
+    return false;
+  }
+
+  if (!nonNull) {
+    return true;
+  }
+
+  RefPtr<nsITransportSecurityInfo> info =
+      new mozilla::psm::TransportSecurityInfo();
+  if (!info->DeserializeFromIPC(aMsg, aIter)) {
+    return false;
+  }
+
+  *aResult = info.forget();
+  return true;
+}
+
+void ParamTraits<nsIX509Cert*>::Write(Message* aMsg, nsIX509Cert* aParam) {
+  bool nonNull = !!aParam;
+  WriteParam(aMsg, nonNull);
+  if (!nonNull) {
+    return;
+  }
+
+  aParam->SerializeToIPC(aMsg);
+}
+
+bool ParamTraits<nsIX509Cert*>::Read(const Message* aMsg, PickleIterator* aIter,
+                                     RefPtr<nsIX509Cert>* aResult) {
+  *aResult = nullptr;
+
+  bool nonNull = false;
+  if (!ReadParam(aMsg, aIter, &nonNull)) {
+    return false;
+  }
+
+  if (!nonNull) {
+    return true;
+  }
+
+  RefPtr<nsIX509Cert> cert = new nsNSSCertificate();
+  if (!cert->DeserializeFromIPC(aMsg, aIter)) {
+    return false;
+  }
+
+  *aResult = cert.forget();
+  return true;
+}
+
+void ParamTraits<nsIX509CertList*>::Write(Message* aMsg,
+                                          nsIX509CertList* aParam) {
+  bool nonNull = !!aParam;
+  WriteParam(aMsg, nonNull);
+  if (!nonNull) {
+    return;
+  }
+
+  aParam->SerializeToIPC(aMsg);
+}
+
+bool ParamTraits<nsIX509CertList*>::Read(const Message* aMsg,
+                                         PickleIterator* aIter,
+                                         RefPtr<nsIX509CertList>* aResult) {
+  bool nonNull = false;
+  if (!ReadParam(aMsg, aIter, &nonNull)) {
+    return false;
+  }
+
+  if (!nonNull) {
+    *aResult = nullptr;
+    return true;
+  }
+
+  RefPtr<nsIX509CertList> certList = new nsNSSCertList();
+  if (!certList->DeserializeFromIPC(aMsg, aIter)) {
+    return false;
+  }
+
+  *aResult = certList.forget();
+  return true;
+}
+
+}  // namespace IPC

ipc/glue/TransportSecurityInfoUtils.h

@@ -0,0 +1,36 @@
+/* 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/. */
+
+#ifndef mozilla_ipc_TransportSecurityInfoUtils_h
+#define mozilla_ipc_TransportSecurityInfoUtils_h
+
+#include "nsCOMPtr.h"
+#include "nsITransportSecurityInfo.h"
+
+namespace IPC {
+
+template <>
+struct ParamTraits<nsITransportSecurityInfo*> {
+  static void Write(Message* aMsg, nsITransportSecurityInfo* aParam);
+  static bool Read(const Message* aMsg, PickleIterator* aIter,
+                   RefPtr<nsITransportSecurityInfo>* aResult);
+};
+
+template <>
+struct ParamTraits<nsIX509Cert*> {
+  static void Write(Message* aMsg, nsIX509Cert* aCert);
+  static bool Read(const Message* aMsg, PickleIterator* aIter,
+                   RefPtr<nsIX509Cert>* aResult);
+};
+
+template <>
+struct ParamTraits<nsIX509CertList*> {
+  static void Write(Message* aMsg, nsIX509CertList* aCertList);
+  static bool Read(const Message* aMsg, PickleIterator* aIter,
+                   RefPtr<nsIX509CertList>* aResult);
+};
+
+}  // namespace IPC
+
+#endif  // mozilla_ipc_TransportSecurityInfoUtils_h

ipc/glue/moz.build

@@ -47,6 +47,7 @@ EXPORTS.mozilla.ipc += [
     'Shmem.h',
     'TaskFactory.h',
     'Transport.h',
+    'TransportSecurityInfoUtils.h',
     'URIUtils.h',
     'WindowsMessageLoop.h',
 ]
@@ -167,6 +168,7 @@ UNIFIED_SOURCES += [
     'SharedMemory.cpp',
     'Shmem.cpp',
     'StringUtil.cpp',
+    'TransportSecurityInfoUtils.cpp',
     'URIUtils.cpp',
 ]
 

netwerk/base/FuzzySecurityInfo.cpp

@@ -323,5 +323,15 @@ FuzzySecurityInfo::GetServerRootCertIsBuiltInRoot(bool* aIsBuiltInRoot) {
   return NS_OK;
 }
 
+void FuzzySecurityInfo::SerializeToIPC(IPC::Message* aMsg) {
+  MOZ_CRASH("Unused");
+}
+
+bool FuzzySecurityInfo::DeserializeFromIPC(const IPC::Message* aMsg,
+                                           PickleIterator* aIter) {
+  MOZ_CRASH("Unused");
+  return false;
+}
+
 }  // namespace net
 }  // namespace mozilla

netwerk/socket/nsITransportSecurityInfo.idl

@@ -9,6 +9,16 @@
 interface nsIX509Cert;
 interface nsIX509CertList;
 
+%{ C++
+namespace IPC {
+  class Message;
+}
+class PickleIterator;
+%}
+
+[ptr] native IpcMessagePtr(IPC::Message);
+[ptr] native PickleIteratorPtr(PickleIterator);
+
 [builtinclass, scriptable, uuid(216112d3-28bc-4671-b057-f98cc09ba1ea)]
 interface nsITransportSecurityInfo : nsISupports {
     readonly attribute unsigned long securityState;
@@ -71,4 +81,10 @@ interface nsITransportSecurityInfo : nsISupports {
      */
     [must_use]
     readonly attribute boolean isExtendedValidation;
+
+    [notxpcom, noscript]
+    void SerializeToIPC(in IpcMessagePtr aMsg);
+
+    [notxpcom, noscript]
+    bool DeserializeFromIPC([const] in IpcMessagePtr aMsg, in PickleIteratorPtr aIter);
 };

security/apps/moz.build

@@ -12,6 +12,8 @@ UNIFIED_SOURCES += [
     'AppTrustDomain.cpp',
 ]
 
+include('/ipc/chromium/chromium-config.mozbuild')
+
 FINAL_LIBRARY = 'xul'
 
 LOCAL_INCLUDES += [

security/certverifier/moz.build

@@ -55,6 +55,8 @@ CXXFLAGS += [
     '-Wno-unused-parameter',
 ]
 
+include('/ipc/chromium/chromium-config.mozbuild')
+
 FINAL_LIBRARY = 'xul'
 
 if CONFIG['CC_TYPE'] == 'clang-cl':

security/certverifier/tests/gtest/moz.build

@@ -13,4 +13,6 @@ LOCAL_INCLUDES += [
     '/security/manager/ssl',
 ]
 
+include('/ipc/chromium/chromium-config.mozbuild')
+
 FINAL_LIBRARY = 'xul-gtest'

security/manager/ssl/TransportSecurityInfo.cpp

@@ -8,6 +8,7 @@
 
 #include "DateTimeFormat.h"
 #include "PSMRunnable.h"
+#include "ipc/IPCMessageUtils.h"
 #include "mozilla/Casting.h"
 #include "nsComponentManagerUtils.h"
 #include "nsIArray.h"
@@ -157,6 +158,8 @@ TransportSecurityInfo::GetInterface(const nsIID& uuid, void** result) {
   }
 static NS_DEFINE_CID(kTransportSecurityInfoMagic, TRANSPORTSECURITYINFOMAGIC);
 
+// NB: Any updates (except disk-only fields) must be kept in sync with
+//     |SerializeToIPC|.
 NS_IMETHODIMP
 TransportSecurityInfo::Write(nsIObjectOutputStream* aStream) {
   nsresult rv = aStream->WriteID(kTransportSecurityInfoMagic);
@@ -381,6 +384,8 @@ nsresult TransportSecurityInfo::ReadSSLStatus(nsIObjectInputStream* aStream) {
   return rv;
 }
 
+// NB: Any updates (except disk-only fields) must be kept in sync with
+//     |DeserializeFromIPC|.
 NS_IMETHODIMP
 TransportSecurityInfo::Read(nsIObjectInputStream* aStream) {
   nsID id;
@@ -542,6 +547,64 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) {
 
 #undef CHILD_DIAGNOSTIC_ASSERT
 
+void TransportSecurityInfo::SerializeToIPC(IPC::Message* aMsg) {
+  MutexAutoLock guard(mMutex);
+
+  int32_t errorCode = static_cast<int32_t>(mErrorCode);
+
+  WriteParam(aMsg, mSecurityState);
+  WriteParam(aMsg, errorCode);
+  WriteParam(aMsg, mServerCert);
+  WriteParam(aMsg, mCipherSuite);
+  WriteParam(aMsg, mProtocolVersion);
+  WriteParam(aMsg, mIsDomainMismatch);
+  WriteParam(aMsg, mIsNotValidAtThisTime);
+  WriteParam(aMsg, mIsUntrusted);
+  WriteParam(aMsg, mIsEV);
+  WriteParam(aMsg, mHasIsEVStatus);
+  WriteParam(aMsg, mHaveCipherSuiteAndProtocol);
+  WriteParam(aMsg, mHaveCertErrorBits);
+  WriteParam(aMsg, mCertificateTransparencyStatus);
+  WriteParam(aMsg, mKeaGroup);
+  WriteParam(aMsg, mSignatureSchemeName);
+  WriteParam(aMsg, mSucceededCertChain);
+  WriteParam(aMsg, mFailedCertChain);
+}
+
+bool TransportSecurityInfo::DeserializeFromIPC(const IPC::Message* aMsg,
+                                               PickleIterator* aIter) {
+  MutexAutoLock guard(mMutex);
+
+  int32_t errorCode = 0;
+
+  if (!ReadParam(aMsg, aIter, &mSecurityState) ||
+      !ReadParam(aMsg, aIter, &errorCode) ||
+      !ReadParam(aMsg, aIter, &mServerCert) ||
+      !ReadParam(aMsg, aIter, &mCipherSuite) ||
+      !ReadParam(aMsg, aIter, &mProtocolVersion) ||
+      !ReadParam(aMsg, aIter, &mIsDomainMismatch) ||
+      !ReadParam(aMsg, aIter, &mIsNotValidAtThisTime) ||
+      !ReadParam(aMsg, aIter, &mIsUntrusted) ||
+      !ReadParam(aMsg, aIter, &mIsEV) ||
+      !ReadParam(aMsg, aIter, &mHasIsEVStatus) ||
+      !ReadParam(aMsg, aIter, &mHaveCipherSuiteAndProtocol) ||
+      !ReadParam(aMsg, aIter, &mHaveCertErrorBits) ||
+      !ReadParam(aMsg, aIter, &mCertificateTransparencyStatus) ||
+      !ReadParam(aMsg, aIter, &mKeaGroup) ||
+      !ReadParam(aMsg, aIter, &mSignatureSchemeName) ||
+      !ReadParam(aMsg, aIter, &mSucceededCertChain) ||
+      !ReadParam(aMsg, aIter, &mFailedCertChain)) {
+    return false;
+  }
+
+  mErrorCode = static_cast<PRErrorCode>(errorCode);
+  if (mErrorCode != 0) {
+    mCanceled = true;
+  }
+
+  return true;
+}
+
 NS_IMETHODIMP
 TransportSecurityInfo::GetInterfaces(nsTArray<nsIID>& array) {
   array.Clear();

security/manager/ssl/TransportSecurityInfo.h

@@ -14,13 +14,14 @@
 #include "mozilla/BasePrincipal.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/RefPtr.h"
+#include "mozilla/ipc/TransportSecurityInfoUtils.h"
+#include "mozpkix/pkixtypes.h"
 #include "nsDataHashtable.h"
 #include "nsIClassInfo.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsITransportSecurityInfo.h"
 #include "nsNSSCertificate.h"
 #include "nsString.h"
-#include "mozpkix/pkixtypes.h"
 
 namespace mozilla {
 namespace psm {

security/manager/ssl/moz.build

@@ -84,6 +84,7 @@ EXPORTS.mozilla += [
 
 EXPORTS.mozilla.psm += [
     'PSMContentListener.h',
+    'TransportSecurityInfo.h',
 ]
 
 EXPORTS.ipc += [

security/manager/ssl/nsIX509Cert.idl

@@ -12,10 +12,18 @@ interface nsIASN1Object;
 interface nsICertVerificationListener;
 
 %{ C++
+namespace IPC {
+  class Message;
+}
+class PickleIterator;
+
  /* forward declaration */
  typedef struct CERTCertificateStr CERTCertificate;
 %}
+
 [ptr] native CERTCertificatePtr(CERTCertificate);
+[ptr] native IpcMessagePtr(IPC::Message);
+[ptr] native PickleIteratorPtr(PickleIterator);
 
 /**
  * This represents a X.509 certificate.
@@ -235,4 +243,10 @@ interface nsIX509Cert : nsISupports {
    */
   [must_use]
   void markForPermDeletion();
+
+  [notxpcom, noscript]
+  void SerializeToIPC(in IpcMessagePtr aMsg);
+
+  [notxpcom, noscript]
+  bool DeserializeFromIPC([const] in IpcMessagePtr aMsg, in PickleIteratorPtr aIter);
 };

security/manager/ssl/nsIX509CertList.idl

@@ -8,9 +8,16 @@ interface nsISimpleEnumerator;
 interface nsIX509Cert;
 
 %{C++
+namespace IPC {
+  class Message;
+}
+class PickleIterator;
 class nsNSSCertList;
 %}
+
 [ptr] native nsNSSCertListPtr(nsNSSCertList);
+[ptr] native IpcMessagePtr(IPC::Message);
+[ptr] native PickleIteratorPtr(PickleIterator);
 
 [scriptable, builtinclass, uuid(ae74cda5-cd2f-473f-96f5-f0b7fff62c68)]
 interface nsIX509CertList : nsISupports {
@@ -42,6 +49,11 @@ interface nsIX509CertList : nsISupports {
   [must_use]
   ACString asPKCS7Blob();
 
+  [notxpcom, noscript]
+  void SerializeToIPC(in IpcMessagePtr aMsg);
+
+  [notxpcom, noscript]
+  bool DeserializeFromIPC([const] in IpcMessagePtr aMsg, in PickleIteratorPtr aIter);
 };
 
 %{C++

security/manager/ssl/nsNSSCertificate.cpp

@@ -941,6 +941,8 @@ nsNSSCertList::AsPKCS7Blob(/*out*/ nsACString& result) {
   return NS_OK;
 }
 
+// NB: Any updates (except disk-only fields) must be kept in sync with
+//     |SerializeToIPC|.
 NS_IMETHODIMP
 nsNSSCertList::Write(nsIObjectOutputStream* aStream) {
   // Write the length of the list
@@ -965,6 +967,8 @@ nsNSSCertList::Write(nsIObjectOutputStream* aStream) {
   return rv;
 }
 
+// NB: Any updates (except disk-only fields) must be kept in sync with
+//     |DeserializeFromIPC|.
 NS_IMETHODIMP
 nsNSSCertList::Read(nsIObjectInputStream* aStream) {
   uint32_t certListLen;
@@ -992,6 +996,35 @@ nsNSSCertList::Read(nsIObjectInputStream* aStream) {
   return NS_OK;
 }
 
+void nsNSSCertList::SerializeToIPC(IPC::Message* aMsg) {
+  const size_t certCount = static_cast<size_t>(mCerts.size());
+  WriteParam(aMsg, certCount);
+
+  for (const auto& certRef : mCerts) {
+    RefPtr<nsIX509Cert> cert = nsNSSCertificate::Create(certRef.get());
+    MOZ_RELEASE_ASSERT(cert);
+
+    WriteParam(aMsg, cert);
+  }
+}
+
+bool nsNSSCertList::DeserializeFromIPC(const IPC::Message* aMsg,
+                                       PickleIterator* aIter) {
+  size_t count = 0;
+  if (!ReadParam(aMsg, aIter, &count)) {
+    return false;
+  }
+
+  for (size_t i = 0; i < count; i++) {
+    RefPtr<nsIX509Cert> cert;
+    if (!ReadParam(aMsg, aIter, &cert) || !cert || NS_FAILED(AddCert(cert))) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 NS_IMETHODIMP
 nsNSSCertList::GetEnumerator(nsISimpleEnumerator** _retval) {
   nsCOMPtr<nsISimpleEnumerator> enumerator(new nsNSSCertListEnumerator(mCerts));
@@ -1197,6 +1230,8 @@ nsNSSCertListEnumerator::GetNext(nsISupports** _retval) {
   return NS_OK;
 }
 
+// NB: Any updates (except disk-only fields) must be kept in sync with
+//     |SerializeToIPC|.
 NS_IMETHODIMP
 nsNSSCertificate::Write(nsIObjectOutputStream* aStream) {
   NS_ENSURE_STATE(mCert);
@@ -1213,6 +1248,8 @@ nsNSSCertificate::Write(nsIObjectOutputStream* aStream) {
       AsBytes(MakeSpan(mCert->derCert.data, mCert->derCert.len)));
 }
 
+// NB: Any updates (except disk-only fields) must be kept in sync with
+//     |DeserializeFromIPC|.
 NS_IMETHODIMP
 nsNSSCertificate::Read(nsIObjectInputStream* aStream) {
   NS_ENSURE_STATE(!mCert);
@@ -1243,6 +1280,45 @@ nsNSSCertificate::Read(nsIObjectInputStream* aStream) {
   return NS_OK;
 }
 
+void nsNSSCertificate::SerializeToIPC(IPC::Message* aMsg) {
+  bool hasCert = static_cast<bool>(mCert);
+  WriteParam(aMsg, hasCert);
+
+  if (!hasCert) {
+    return;
+  }
+
+  const nsDependentCSubstring certBytes(
+      reinterpret_cast<char*>(mCert->derCert.data), mCert->derCert.len);
+
+  WriteParam(aMsg, certBytes);
+}
+
+bool nsNSSCertificate::DeserializeFromIPC(const IPC::Message* aMsg,
+                                          PickleIterator* aIter) {
+  bool hasCert = false;
+  if (!ReadParam(aMsg, aIter, &hasCert)) {
+    return false;
+  }
+
+  if (!hasCert) {
+    return true;
+  }
+
+  nsCString derBytes;
+  if (!ReadParam(aMsg, aIter, &derBytes)) {
+    return false;
+  }
+
+  if (derBytes.Length() == 0) {
+    return false;
+  }
+
+  // NSS accepts a |char*| here, but doesn't modify the contents of the array
+  // and casts it back to an |unsigned char*|.
+  return InitFromDER(const_cast<char*>(derBytes.get()), derBytes.Length());
+}
+
 NS_IMETHODIMP
 nsNSSCertificate::GetInterfaces(nsTArray<nsIID>& array) {
   array.Clear();