Commit 40b8004e authored by Sean Feng's avatar Sean Feng
Browse files

Bug 1580304 - Remove nsNSSCertList/nsIX509CertList r=keeler 

nsNSSCertList/nsIX509CertList are redundant, and also contructing
them are expensive. so it is replaced by Array<nsIX509Cert>

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

--HG--
extra : moz-landing-system : lando
parent 622054c8

security/apps/AppSignatureVerification.cpp

+0 −1
Original line number Diff line number Diff line
@@ -1282,7 +1282,6 @@ nsresult OpenSignedAppFile(AppTrustedRoot aTrustedRoot, nsIFile* aJarFile, 
  }



  // Return the signer's certificate to the reader if they want it.

  // XXX: We should return an nsIX509CertList with the whole validated chain.

  if (aSignerCert) {

    // The COSE certificate is authoritative.

    if (aPolicy.COSERequired() || (coseCertItem && coseCertItem->len != 0)) {

security/manager/ssl/SSLServerCertVerification.cpp

+3 −10
Original line number Diff line number Diff line
@@ -459,7 +459,7 @@ class SSLServerCertVerificationJob : public Runnable { 
                            const void* fdForLogging,

                            TransportSecurityInfo* infoObject,

                            const UniqueCERTCertificate& serverCert,

                            const UniqueCERTCertList& peerCertChain,

                            UniqueCERTCertList& peerCertChain,

                            Maybe<nsTArray<uint8_t>>& stapledOCSPResponse,

                            Maybe<nsTArray<uint8_t>>& sctsFromTLSExtension,

                            Maybe<DelegatedCredentialInfo>& dcInfo,
@@ -1150,7 +1150,7 @@ Result AuthCertificate(CertVerifier& certVerifier, 
SECStatus SSLServerCertVerificationJob::Dispatch(

    const RefPtr<SharedCertVerifier>& certVerifier, const void* fdForLogging,

    TransportSecurityInfo* infoObject, const UniqueCERTCertificate& serverCert,

    const UniqueCERTCertList& peerCertChain,

    UniqueCERTCertList& peerCertChain,

    Maybe<nsTArray<uint8_t>>& stapledOCSPResponse,

    Maybe<nsTArray<uint8_t>>& sctsFromTLSExtension,

    Maybe<DelegatedCredentialInfo>& dcInfo, uint32_t providerFlags, Time time,
@@ -1167,14 +1167,7 @@ SECStatus SSLServerCertVerificationJob::Dispatch( 
    return SECFailure;

  }



  // Copy the certificate list so the runnable can take ownership of it in the

  // constructor.

  UniqueCERTCertList peerCertChainCopy =

      nsNSSCertList::DupCertList(peerCertChain);

  if (!peerCertChainCopy) {

    PR_SetError(SEC_ERROR_NO_MEMORY, 0);

    return SECFailure;

  }

  UniqueCERTCertList peerCertChainCopy = std::move(peerCertChain);



  RefPtr<SSLServerCertVerificationJob> job(new SSLServerCertVerificationJob(

      certVerifier, fdForLogging, infoObject, serverCert,

security/manager/ssl/TransportSecurityInfo.cpp

+0 −45
Original line number Diff line number Diff line
@@ -455,51 +455,6 @@ nsresult TransportSecurityInfo::ReadCertificatesFromStream( 
  return NS_OK;

}



nsresult TransportSecurityInfo::ConvertCertArrayToCertList(

    const nsTArray<RefPtr<nsIX509Cert>>& aCertArray,

    nsIX509CertList** aCertList) {

  NS_ENSURE_ARG_POINTER(aCertList);

  *aCertList = nullptr;



  // aCertList will be null if aCertArray is empty, this also matches

  // the original certList behaviour

  if (aCertArray.IsEmpty()) {

    return NS_OK;

  }



  nsCOMPtr<nsIX509CertList> certList = new nsNSSCertList();

  for (const auto& cert : aCertArray) {

    nsresult rv = certList->AddCert(cert);

    if (NS_FAILED(rv)) {

      return rv;

    }

  }



  certList.forget(aCertList);



  return NS_OK;

}



nsresult TransportSecurityInfo::ConvertCertListToCertArray(

    const nsCOMPtr<nsIX509CertList>& aCertList,

    nsTArray<RefPtr<nsIX509Cert>>& aCertArray) {

  MOZ_ASSERT(aCertList);

  if (!aCertList) {

    return NS_ERROR_INVALID_ARG;

  }



  aCertArray.Clear();

  RefPtr<nsNSSCertList> certList = aCertList->GetCertList();



  return certList->ForEachCertificateInChain(

      [&aCertArray](nsCOMPtr<nsIX509Cert>& aCert, bool aHasMore,

                    bool& aContinue) {

        RefPtr<nsIX509Cert> cert(aCert.get());

        aCertArray.AppendElement(cert);

        return NS_OK;

      });

}



// NB: Any updates (except disk-only fields) must be kept in sync with

//     |DeserializeFromIPC|.

NS_IMETHODIMP

security/manager/ssl/TransportSecurityInfo.h

+0 −7
Original line number Diff line number Diff line
@@ -110,10 +110,6 @@ class TransportSecurityInfo : public nsITransportSecurityInfo, 
  /* mHaveCertErrrorBits is relied on to determine whether or not a SPDY

     connection is eligible for joining in nsNSSSocketInfo::JoinConnection() */

  bool mHaveCertErrorBits;



  static nsresult ConvertCertArrayToCertList(

      const nsTArray<RefPtr<nsIX509Cert>>& aCertArray,

      nsIX509CertList** aCertList);

 private:

  // True if SetCanceled has been called (or if this was deserialized with a

  // non-zero mErrorCode, which can only be the case if SetCanceled was called
@@ -141,9 +137,6 @@ class TransportSecurityInfo : public nsITransportSecurityInfo, 
  nsTArray<RefPtr<nsIX509Cert>> mFailedCertChain;



  nsresult ReadSSLStatus(nsIObjectInputStream* aStream);

  static nsresult ConvertCertListToCertArray(

      const nsCOMPtr<nsIX509CertList>& aCertList,

      nsTArray<RefPtr<nsIX509Cert>>& aCertArray);



  // This function is used to read the binary that are serialized

  // by using nsIX509CertList

security/manager/ssl/components.conf

+0 −6
Original line number Diff line number Diff line
@@ -89,12 +89,6 @@ Classes = [ 
        'type': 'nsNSSCertificateDB',

        'legacy_constructor': 'mozilla::psm::NSSConstructor<nsNSSCertificateDB>',

    },

    {

        'cid': '{959fb165-6517-487f-ab9b-d8913be53197}',

        'contract_ids': ['@mozilla.org/security/x509certlist;1'],

        'type': 'nsNSSCertList',

        'legacy_constructor': 'mozilla::psm::NSSConstructor<nsNSSCertList>',

    },

    {

        'cid': '{36a1d3b3-d886-4317-96ff-87b0005cfef7}',

        'contract_ids': ['@mozilla.org/security/hash;1'],