Bug 1715142 - convert pinning to use a static pref r=rmf

This patch converts the pinning preference
"security.cert_pinning.enforcement_level" to be static. It also removes some
unused pinning preferences and parameters.

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

modules/libpref/init/StaticPrefList.yaml

@@ -10581,6 +10581,13 @@
   value: false
   mirror: always
 
+# Disable preloaded static key pins by default.
+- name: security.cert_pinning.enforcement_level
+  type: ReleaseAcquireAtomicUint32
+  value: 0
+  mirror: always
+  do_not_use_directly: true
+
 #---------------------------------------------------------------------------
 # Prefs starting with "slider."
 #---------------------------------------------------------------------------

modules/libpref/init/all.js

@@ -156,11 +156,6 @@ pref("security.webauth.webauthn_enable_softtoken", false);
 pref("security.xfocsp.errorReporting.enabled", true);
 pref("security.xfocsp.errorReporting.automatic", false);
 
-// Impose a maximum age on HPKP headers, to avoid sites getting permanently
-// blacking themselves out by setting a bad pin.  (60 days by default)
-// https://tools.ietf.org/html/rfc7469#section-4.1
-pref("security.cert_pinning.max_max_age_seconds", 5184000);
-
 // 0: Disable CRLite entirely
 // 1: Enable and check revocations via CRLite, but only collect telemetry
 // 2: Enable and enforce revocations via CRLite
@@ -2168,22 +2163,6 @@ pref("security.ssl.enable_ocsp_must_staple", true);
 pref("security.insecure_field_warning.contextual.enabled", false);
 pref("security.insecure_field_warning.ignore_local_ip_address", true);
 
-// Disable pinning checks by default.
-pref("security.cert_pinning.enforcement_level", 0);
-// Do not process hpkp headers rooted by not built in roots by default.
-// This is to prevent accidental pinning from MITM devices and is used
-// for tests.
-pref("security.cert_pinning.process_headers_from_non_builtin_roots", false);
-
-// Controls whether or not HPKP (the HTTP Public Key Pinning header) is enabled.
-// If true, the header is processed and collected HPKP information is consulted
-// when looking for pinning information.
-// If false, the header is not processed and collected HPKP information is not
-// consulted when looking for pinning information. Preloaded pins are not
-// affected by this preference.
-// Default: false
-pref("security.cert_pinning.hpkp.enabled", false);
-
 // Remote settings preferences
 // Note: if you change this, make sure to also review security.onecrl.maximum_staleness_in_seconds
 pref("services.settings.poll_interval", 86400); // 24H

netwerk/base/nsIOService.cpp

@@ -258,7 +258,6 @@ static const char* gCallbackSecurityPrefs[] = {
     "security.ssl.enable_ocsp_stapling",
     "security.ssl.enable_ocsp_must_staple",
     "security.pki.certificate_transparency.mode",
-    "security.cert_pinning.enforcement_level",
     "security.pki.name_matching_mode",
     nullptr,
 };
@@ -405,7 +404,6 @@ void nsIOService::OnTLSPrefChange(const char* aPref, void* aSelf) {
   } else if (pref.EqualsLiteral("security.ssl.enable_ocsp_stapling") ||
              pref.EqualsLiteral("security.ssl.enable_ocsp_must_staple") ||
              pref.EqualsLiteral("security.pki.certificate_transparency.mode") ||
-             pref.EqualsLiteral("security.cert_pinning.enforcement_level") ||
              pref.EqualsLiteral("security.pki.name_matching_mode")) {
     SetValidationOptionsCommon();
   }

security/certverifier/CertVerifier.cpp

@@ -88,8 +88,7 @@ void CertificateTransparencyInfo::Reset() {
 CertVerifier::CertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc,
                            mozilla::TimeDuration ocspTimeoutSoft,
                            mozilla::TimeDuration ocspTimeoutHard,
-                           uint32_t certShortLifetimeInDays,
-                           PinningMode pinningMode, SHA1Mode sha1Mode,
+                           uint32_t certShortLifetimeInDays, SHA1Mode sha1Mode,
                            BRNameMatchingPolicy::Mode nameMatchingMode,
                            NetscapeStepUpPolicy netscapeStepUpPolicy,
                            CertificateTransparencyMode ctMode,
@@ -101,7 +100,6 @@ CertVerifier::CertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc,
       mOCSPTimeoutSoft(ocspTimeoutSoft),
       mOCSPTimeoutHard(ocspTimeoutHard),
       mCertShortLifetimeInDays(certShortLifetimeInDays),
-      mPinningMode(pinningMode),
       mSHA1Mode(sha1Mode),
       mNameMatchingMode(nameMatchingMode),
       mNetscapeStepUpPolicy(netscapeStepUpPolicy),
@@ -564,9 +562,9 @@ Result CertVerifier::VerifyCert(
       // just use trustEmail as it is the closest alternative.
       NSSCertDBTrustDomain trustDomain(
           trustEmail, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft,
-          mOCSPTimeoutHard, mCertShortLifetimeInDays, pinningDisabled,
-          MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff,
-          SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch, mCRLiteMode,
+          mOCSPTimeoutHard, mCertShortLifetimeInDays, MIN_RSA_BITS_WEAK,
+          ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed,
+          NetscapeStepUpPolicy::NeverMatch, mCRLiteMode,
           mCRLiteCTMergeDelaySeconds, originAttributes, mThirdPartyRootInputs,
           mThirdPartyIntermediateInputs, extraCertificates, builtChain, nullptr,
           nullptr);
@@ -637,10 +635,10 @@ Result CertVerifier::VerifyCert(
 
         NSSCertDBTrustDomain trustDomain(
             trustSSL, evOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft,
-            mOCSPTimeoutHard, mCertShortLifetimeInDays, mPinningMode,
-            MIN_RSA_BITS, ValidityCheckingMode::CheckForEV,
-            sha1ModeConfigurations[i], mNetscapeStepUpPolicy, mCRLiteMode,
-            mCRLiteCTMergeDelaySeconds, originAttributes, mThirdPartyRootInputs,
+            mOCSPTimeoutHard, mCertShortLifetimeInDays, MIN_RSA_BITS,
+            ValidityCheckingMode::CheckForEV, sha1ModeConfigurations[i],
+            mNetscapeStepUpPolicy, mCRLiteMode, mCRLiteCTMergeDelaySeconds,
+            originAttributes, mThirdPartyRootInputs,
             mThirdPartyIntermediateInputs, extraCertificates, builtChain,
             pinningTelemetryInfo, hostname);
         rv = BuildCertChainForOneKeyUsage(
@@ -720,12 +718,11 @@ Result CertVerifier::VerifyCert(
           NSSCertDBTrustDomain trustDomain(
               trustSSL, defaultOCSPFetching, mOCSPCache, pinArg,
               mOCSPTimeoutSoft, mOCSPTimeoutHard, mCertShortLifetimeInDays,
-              mPinningMode, keySizeOptions[i],
-              ValidityCheckingMode::CheckingOff, sha1ModeConfigurations[j],
-              mNetscapeStepUpPolicy, mCRLiteMode, mCRLiteCTMergeDelaySeconds,
-              originAttributes, mThirdPartyRootInputs,
-              mThirdPartyIntermediateInputs, extraCertificates, builtChain,
-              pinningTelemetryInfo, hostname);
+              keySizeOptions[i], ValidityCheckingMode::CheckingOff,
+              sha1ModeConfigurations[j], mNetscapeStepUpPolicy, mCRLiteMode,
+              mCRLiteCTMergeDelaySeconds, originAttributes,
+              mThirdPartyRootInputs, mThirdPartyIntermediateInputs,
+              extraCertificates, builtChain, pinningTelemetryInfo, hostname);
           rv = BuildCertChainForOneKeyUsage(
               trustDomain, certDER, time,
               KeyUsage::digitalSignature,  //(EC)DHE
@@ -790,10 +787,10 @@ Result CertVerifier::VerifyCert(
     case certificateUsageSSLCA: {
       NSSCertDBTrustDomain trustDomain(
           trustSSL, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft,
-          mOCSPTimeoutHard, mCertShortLifetimeInDays, pinningDisabled,
-          MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff,
-          SHA1Mode::Allowed, mNetscapeStepUpPolicy, mCRLiteMode,
-          mCRLiteCTMergeDelaySeconds, originAttributes, mThirdPartyRootInputs,
+          mOCSPTimeoutHard, mCertShortLifetimeInDays, MIN_RSA_BITS_WEAK,
+          ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed,
+          mNetscapeStepUpPolicy, mCRLiteMode, mCRLiteCTMergeDelaySeconds,
+          originAttributes, mThirdPartyRootInputs,
           mThirdPartyIntermediateInputs, extraCertificates, builtChain, nullptr,
           nullptr);
       rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeCA,
@@ -805,9 +802,9 @@ Result CertVerifier::VerifyCert(
     case certificateUsageEmailSigner: {
       NSSCertDBTrustDomain trustDomain(
           trustEmail, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft,
-          mOCSPTimeoutHard, mCertShortLifetimeInDays, pinningDisabled,
-          MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff,
-          SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch, mCRLiteMode,
+          mOCSPTimeoutHard, mCertShortLifetimeInDays, MIN_RSA_BITS_WEAK,
+          ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed,
+          NetscapeStepUpPolicy::NeverMatch, mCRLiteMode,
           mCRLiteCTMergeDelaySeconds, originAttributes, mThirdPartyRootInputs,
           mThirdPartyIntermediateInputs, extraCertificates, builtChain, nullptr,
           nullptr);
@@ -830,9 +827,9 @@ Result CertVerifier::VerifyCert(
       // based on the result of the verification(s).
       NSSCertDBTrustDomain trustDomain(
           trustEmail, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft,
-          mOCSPTimeoutHard, mCertShortLifetimeInDays, pinningDisabled,
-          MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff,
-          SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch, mCRLiteMode,
+          mOCSPTimeoutHard, mCertShortLifetimeInDays, MIN_RSA_BITS_WEAK,
+          ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed,
+          NetscapeStepUpPolicy::NeverMatch, mCRLiteMode,
           mCRLiteCTMergeDelaySeconds, originAttributes, mThirdPartyRootInputs,
           mThirdPartyIntermediateInputs, extraCertificates, builtChain, nullptr,
           nullptr);

security/certverifier/CertVerifier.h

@@ -195,13 +195,6 @@ class CertVerifier {
       /*optional out*/ CertificateTransparencyInfo* ctInfo = nullptr,
       /*optional out*/ bool* isBuiltCertChainRootBuiltInRoot = nullptr);
 
-  enum PinningMode {
-    pinningDisabled = 0,
-    pinningAllowUserCAMITM = 1,
-    pinningStrict = 2,
-    pinningEnforceTestMode = 3
-  };
-
   enum class SHA1Mode {
     Allowed = 0,
     Forbidden = 1,
@@ -224,8 +217,8 @@ class CertVerifier {
   CertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc,
                mozilla::TimeDuration ocspTimeoutSoft,
                mozilla::TimeDuration ocspTimeoutHard,
-               uint32_t certShortLifetimeInDays, PinningMode pinningMode,
-               SHA1Mode sha1Mode, BRNameMatchingPolicy::Mode nameMatchingMode,
+               uint32_t certShortLifetimeInDays, SHA1Mode sha1Mode,
+               BRNameMatchingPolicy::Mode nameMatchingMode,
                NetscapeStepUpPolicy netscapeStepUpPolicy,
                CertificateTransparencyMode ctMode, CRLiteMode crliteMode,
                uint64_t crliteCTMergeDelaySeconds,
@@ -239,7 +232,6 @@ class CertVerifier {
   const mozilla::TimeDuration mOCSPTimeoutSoft;
   const mozilla::TimeDuration mOCSPTimeoutHard;
   const uint32_t mCertShortLifetimeInDays;
-  const PinningMode mPinningMode;
   const SHA1Mode mSHA1Mode;
   const BRNameMatchingPolicy::Mode mNameMatchingMode;
   const NetscapeStepUpPolicy mNetscapeStepUpPolicy;
@@ -276,9 +268,9 @@ class CertVerifier {
 };
 
 mozilla::pkix::Result IsCertBuiltInRoot(CERTCertificate* cert, bool& result);
-mozilla::pkix::Result CertListContainsExpectedKeys(
-    const CERTCertList* certList, const char* hostname,
-    mozilla::pkix::Time time, CertVerifier::PinningMode pinningMode);
+mozilla::pkix::Result CertListContainsExpectedKeys(const CERTCertList* certList,
+                                                   const char* hostname,
+                                                   mozilla::pkix::Time time);
 
 }  // namespace psm
 }  // namespace mozilla

security/certverifier/NSSCertDBTrustDomain.cpp

@@ -66,10 +66,9 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain(
     OCSPCache& ocspCache,
     /*optional but shouldn't be*/ void* pinArg, TimeDuration ocspTimeoutSoft,
     TimeDuration ocspTimeoutHard, uint32_t certShortLifetimeInDays,
-    CertVerifier::PinningMode pinningMode, unsigned int minRSABits,
-    ValidityCheckingMode validityCheckingMode, CertVerifier::SHA1Mode sha1Mode,
-    NetscapeStepUpPolicy netscapeStepUpPolicy, CRLiteMode crliteMode,
-    uint64_t crliteCTMergeDelaySeconds,
+    unsigned int minRSABits, ValidityCheckingMode validityCheckingMode,
+    CertVerifier::SHA1Mode sha1Mode, NetscapeStepUpPolicy netscapeStepUpPolicy,
+    CRLiteMode crliteMode, uint64_t crliteCTMergeDelaySeconds,
     const OriginAttributes& originAttributes,
     const Vector<Input>& thirdPartyRootInputs,
     const Vector<Input>& thirdPartyIntermediateInputs,
@@ -84,7 +83,6 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain(
       mOCSPTimeoutSoft(ocspTimeoutSoft),
       mOCSPTimeoutHard(ocspTimeoutHard),
       mCertShortLifetimeInDays(certShortLifetimeInDays),
-      mPinningMode(pinningMode),
       mMinRSABits(minRSABits),
       mValidityCheckingMode(validityCheckingMode),
       mSHA1Mode(sha1Mode),
@@ -1186,16 +1184,9 @@ Result NSSCertDBTrustDomain::IsChainValid(const DERArray& certArray, Time time,
   if (NS_FAILED(nsrv)) {
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
-  bool skipPinningChecksBecauseOfMITMMode =
-      (!isBuiltInRoot && mPinningMode == CertVerifier::pinningAllowUserCAMITM);
   // If mHostname isn't set, we're not verifying in the context of a TLS
-  // handshake, so don't verify HPKP in those cases.
-  if (mHostname && (mPinningMode != CertVerifier::pinningDisabled) &&
-      !skipPinningChecksBecauseOfMITMMode) {
-    bool enforceTestMode =
-        (mPinningMode == CertVerifier::pinningEnforceTestMode);
-    bool chainHasValidPins;
-
+  // handshake, so don't verify key pinning in those cases.
+  if (mHostname) {
     nsTArray<Span<const uint8_t>> derCertSpanList;
     size_t numCerts = certArray.GetLength();
     for (size_t i = numCerts; i > 0; --i) {
@@ -1206,9 +1197,10 @@ Result NSSCertDBTrustDomain::IsChainValid(const DERArray& certArray, Time time,
       derCertSpanList.EmplaceBack(der->UnsafeGetData(), der->GetLength());
     }
 
+    bool chainHasValidPins;
     nsrv = PublicKeyPinningService::ChainHasValidPins(
-        derCertSpanList, mHostname, time, enforceTestMode, mOriginAttributes,
-        chainHasValidPins, mPinningTelemetryInfo);
+        derCertSpanList, mHostname, time, isBuiltInRoot, chainHasValidPins,
+        mPinningTelemetryInfo);
     if (NS_FAILED(nsrv)) {
       return Result::FATAL_ERROR_LIBRARY_FAILURE;
     }

security/certverifier/NSSCertDBTrustDomain.h

@@ -128,8 +128,7 @@ class NSSCertDBTrustDomain : public mozilla::pkix::TrustDomain {
       SECTrustType certDBTrustType, OCSPFetching ocspFetching,
       OCSPCache& ocspCache, void* pinArg, mozilla::TimeDuration ocspTimeoutSoft,
       mozilla::TimeDuration ocspTimeoutHard, uint32_t certShortLifetimeInDays,
-      CertVerifier::PinningMode pinningMode, unsigned int minRSABits,
-      ValidityCheckingMode validityCheckingMode,
+      unsigned int minRSABits, ValidityCheckingMode validityCheckingMode,
       CertVerifier::SHA1Mode sha1Mode,
       NetscapeStepUpPolicy netscapeStepUpPolicy, CRLiteMode crliteMode,
       uint64_t crliteCTMergeDelaySeconds,
@@ -247,7 +246,6 @@ class NSSCertDBTrustDomain : public mozilla::pkix::TrustDomain {
   const mozilla::TimeDuration mOCSPTimeoutSoft;
   const mozilla::TimeDuration mOCSPTimeoutHard;
   const uint32_t mCertShortLifetimeInDays;
-  CertVerifier::PinningMode mPinningMode;
   const unsigned int mMinRSABits;
   ValidityCheckingMode mValidityCheckingMode;
   CertVerifier::SHA1Mode mSHA1Mode;

security/manager/ssl/CommonSocketControl.cpp

@@ -202,34 +202,26 @@ CommonSocketControl::IsAcceptableForHost(const nsACString& hostname,
     return NS_OK;
   }
 
-  mozilla::psm::CertVerifier::PinningMode pinningMode =
-      mozilla::psm::PublicSSLState()->PinningMode();
-  if (pinningMode != mozilla::psm::CertVerifier::pinningDisabled) {
-    bool chainHasValidPins;
-    bool enforceTestMode =
-        (pinningMode == mozilla::psm::CertVerifier::pinningEnforceTestMode);
-
-    nsTArray<nsTArray<uint8_t>> rawDerCertList;
-    nsTArray<Span<const uint8_t>> derCertSpanList;
-    for (const auto& cert : mSucceededCertChain) {
-      rawDerCertList.EmplaceBack();
-      nsresult nsrv = cert->GetRawDER(rawDerCertList.LastElement());
-      if (NS_FAILED(nsrv)) {
-        return nsrv;
-      }
-      derCertSpanList.EmplaceBack(rawDerCertList.LastElement());
-    }
-
-    nsresult nsrv = mozilla::psm::PublicKeyPinningService::ChainHasValidPins(
-        derCertSpanList, PromiseFlatCString(hostname).BeginReading(), Now(),
-        enforceTestMode, GetOriginAttributes(lock), chainHasValidPins, nullptr);
+  nsTArray<nsTArray<uint8_t>> rawDerCertList;
+  nsTArray<Span<const uint8_t>> derCertSpanList;
+  for (const auto& cert : mSucceededCertChain) {
+    rawDerCertList.EmplaceBack();
+    nsresult nsrv = cert->GetRawDER(rawDerCertList.LastElement());
     if (NS_FAILED(nsrv)) {
-      return NS_OK;
+      return nsrv;
     }
+    derCertSpanList.EmplaceBack(rawDerCertList.LastElement());
+  }
+  bool chainHasValidPins;
+  nsresult nsrv = mozilla::psm::PublicKeyPinningService::ChainHasValidPins(
+      derCertSpanList, PromiseFlatCString(hostname).BeginReading(), Now(),
+      mIsBuiltCertChainRootBuiltInRoot, chainHasValidPins, nullptr);
+  if (NS_FAILED(nsrv)) {
+    return NS_OK;
+  }
 
-    if (!chainHasValidPins) {
-      return NS_OK;
-    }
+  if (!chainHasValidPins) {
+    return NS_OK;
   }
 
   // All tests pass