From aca984c8a8e899f526c4ad6aa7bbcbfafb2abc25 Mon Sep 17 00:00:00 2001 From: Noemi Erli Date: Wed, 1 Jun 2022 02:35:17 +0300 Subject: [PATCH] Backed out changeset 8ef044a6a1fe (bug 1766687) for causing bustage in NSSCertDBTrustDomain.cpp --- modules/libpref/init/StaticPrefList.yaml | 8 + security/apps/AppTrustDomain.cpp | 12 +- security/certverifier/CertVerifier.cpp | 253 +++++++++++++----- security/certverifier/CertVerifier.h | 32 ++- .../certverifier/NSSCertDBTrustDomain.cpp | 61 ++++- security/certverifier/NSSCertDBTrustDomain.h | 2 + .../OCSPVerificationTrustDomain.cpp | 114 ++++++++ .../OCSPVerificationTrustDomain.h | 97 +++++++ security/certverifier/moz.build | 1 + security/manager/ssl/CSTrustDomain.cpp | 10 +- .../manager/ssl/SSLServerCertVerification.cpp | 15 +- security/manager/ssl/SharedCertVerifier.h | 9 +- security/manager/ssl/nsNSSCallbacks.cpp | 1 + security/manager/ssl/nsNSSComponent.cpp | 24 +- .../manager/ssl/tests/unit/test_cert_sha1.js | 136 +++++++++- .../ssl/tests/unit/test_ocsp_caching.js | 14 +- .../tests/unit/test_ocsp_stapling_expired.js | 6 +- toolkit/components/telemetry/Histograms.json | 8 + .../telemetry/histogram-allowlists.json | 3 + 19 files changed, 693 insertions(+), 113 deletions(-) create mode 100644 security/certverifier/OCSPVerificationTrustDomain.cpp create mode 100644 security/certverifier/OCSPVerificationTrustDomain.h diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 76a050fb81ce..e1180f7bd9d8 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -12245,6 +12245,14 @@ value: 10 mirror: always +# NB: Changes to this pref affect CERT_CHAIN_SHA1_POLICY_STATUS telemetry. +# See the comment in CertVerifier.cpp. +# 1 = forbid sha1 in certificate signatures, even for imported roots +- name: security.pki.sha1_enforcement_level + type: RelaxedAtomicUint32 + value: 1 + mirror: always + # security.pki.netscape_step_up_policy controls how the platform handles the # id-Netscape-stepUp OID in extended key usage extensions of CA certificates. # 0: id-Netscape-stepUp is always considered equivalent to id-kp-serverAuth diff --git a/security/apps/AppTrustDomain.cpp b/security/apps/AppTrustDomain.cpp index d4e398b92afd..0f04bd6a89af 100644 --- a/security/apps/AppTrustDomain.cpp +++ b/security/apps/AppTrustDomain.cpp @@ -214,16 +214,10 @@ Result AppTrustDomain::IsChainValid(const DERArray& certChain, Time time, return Success; } -Result AppTrustDomain::CheckSignatureDigestAlgorithm(DigestAlgorithm digestAlg, +Result AppTrustDomain::CheckSignatureDigestAlgorithm(DigestAlgorithm, EndEntityOrCA, Time) { - switch (digestAlg) { - case DigestAlgorithm::sha256: // fall through - case DigestAlgorithm::sha384: // fall through - case DigestAlgorithm::sha512: - return Success; - case DigestAlgorithm::sha1: - return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED; - } + // TODO: We should restrict signatures to SHA-256 or better. + return Success; } Result AppTrustDomain::CheckRSAPublicKeyModulusSizeInBits( diff --git a/security/certverifier/CertVerifier.cpp b/security/certverifier/CertVerifier.cpp index df9ebda8aea3..445f8de626e1 100644 --- a/security/certverifier/CertVerifier.cpp +++ b/security/certverifier/CertVerifier.cpp @@ -104,7 +104,7 @@ void CertificateTransparencyInfo::Reset() { CertVerifier::CertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc, mozilla::TimeDuration ocspTimeoutSoft, mozilla::TimeDuration ocspTimeoutHard, - uint32_t certShortLifetimeInDays, + uint32_t certShortLifetimeInDays, SHA1Mode sha1Mode, NetscapeStepUpPolicy netscapeStepUpPolicy, CertificateTransparencyMode ctMode, CRLiteMode crliteMode, @@ -114,6 +114,7 @@ CertVerifier::CertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc, mOCSPTimeoutSoft(ocspTimeoutSoft), mOCSPTimeoutHard(ocspTimeoutHard), mCertShortLifetimeInDays(certShortLifetimeInDays), + mSHA1Mode(sha1Mode), mNetscapeStepUpPolicy(netscapeStepUpPolicy), mCTMode(ctMode), mCRLiteMode(crliteMode) { @@ -445,6 +446,24 @@ Result CertVerifier::VerifyCertificateTransparencyPolicy( return Success; } +bool CertVerifier::SHA1ModeMoreRestrictiveThanGivenMode(SHA1Mode mode) { + switch (mSHA1Mode) { + case SHA1Mode::Forbidden: + return mode != SHA1Mode::Forbidden; + case SHA1Mode::ImportedRoot: + return mode != SHA1Mode::Forbidden && mode != SHA1Mode::ImportedRoot; + case SHA1Mode::ImportedRootOrBefore2016: + return mode == SHA1Mode::Allowed; + case SHA1Mode::Allowed: + return false; + // MSVC warns unless we explicitly handle this now-unused option. + case SHA1Mode::UsedToBeBefore2016ButNowIsForbidden: + default: + MOZ_ASSERT(false, "unexpected SHA1Mode type"); + return true; + } +} + Result CertVerifier::VerifyCert( const nsTArray& certBytes, SECCertificateUsage usage, Time time, void* pinArg, const char* hostname, @@ -457,6 +476,7 @@ Result CertVerifier::VerifyCert( /*optional out*/ EVStatus* evStatus, /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus, /*optional out*/ KeySizeStatus* keySizeStatus, + /*optional out*/ SHA1ModeResult* sha1ModeResult, /*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo, /*optional out*/ CertificateTransparencyInfo* ctInfo, /*optional out*/ bool* isBuiltChainRootBuiltInRoot) { @@ -464,6 +484,7 @@ Result CertVerifier::VerifyCert( MOZ_ASSERT(usage == certificateUsageSSLServer || !(flags & FLAG_MUST_BE_EV)); MOZ_ASSERT(usage == certificateUsageSSLServer || !keySizeStatus); + MOZ_ASSERT(usage == certificateUsageSSLServer || !sha1ModeResult); if (NS_FAILED(BlockUntilLoadableCertsLoaded())) { return Result::FATAL_ERROR_LIBRARY_FAILURE; @@ -489,6 +510,13 @@ Result CertVerifier::VerifyCert( *keySizeStatus = KeySizeStatus::NeverChecked; } + if (sha1ModeResult) { + if (usage != certificateUsageSSLServer) { + return Result::FATAL_ERROR_INVALID_ARGS; + } + *sha1ModeResult = SHA1ModeResult::NeverChecked; + } + if (usage != certificateUsageSSLServer && (flags & FLAG_MUST_BE_EV)) { return Result::FATAL_ERROR_INVALID_ARGS; } @@ -539,10 +567,10 @@ Result CertVerifier::VerifyCert( NSSCertDBTrustDomain trustDomain( trustEmail, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft, mOCSPTimeoutHard, mCertShortLifetimeInDays, MIN_RSA_BITS_WEAK, - ValidityCheckingMode::CheckingOff, NetscapeStepUpPolicy::NeverMatch, - mCRLiteMode, originAttributes, mThirdPartyRootInputs, - mThirdPartyIntermediateInputs, extraCertificates, builtChain, nullptr, - nullptr); + ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, + NetscapeStepUpPolicy::NeverMatch, mCRLiteMode, originAttributes, + mThirdPartyRootInputs, mThirdPartyIntermediateInputs, + extraCertificates, builtChain, nullptr, nullptr); rv = BuildCertChain( trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::digitalSignature, KeyPurposeId::id_kp_clientAuth, @@ -555,6 +583,30 @@ Result CertVerifier::VerifyCert( // restrict the acceptable key usage based on the key exchange method // chosen by the server. + // These configurations are in order of most restrictive to least + // restrictive. This enables us to gather telemetry on the expected + // results of setting the default policy to a particular configuration. + SHA1Mode sha1ModeConfigurations[] = { + SHA1Mode::Forbidden, + SHA1Mode::ImportedRoot, + SHA1Mode::ImportedRootOrBefore2016, + SHA1Mode::Allowed, + }; + + SHA1ModeResult sha1ModeResults[] = { + SHA1ModeResult::SucceededWithoutSHA1, + SHA1ModeResult::SucceededWithImportedRoot, + SHA1ModeResult::SucceededWithImportedRootOrSHA1Before2016, + SHA1ModeResult::SucceededWithSHA1, + }; + + size_t sha1ModeConfigurationsCount = + MOZ_ARRAY_LENGTH(sha1ModeConfigurations); + + static_assert(MOZ_ARRAY_LENGTH(sha1ModeConfigurations) == + MOZ_ARRAY_LENGTH(sha1ModeResults), + "digestAlgorithm array lengths differ"); + // Try to validate for EV first. NSSCertDBTrustDomain::OCSPFetching evOCSPFetching = (mOCSPDownloadConfig == ocspOff) || (flags & FLAG_LOCAL_ONLY) @@ -564,14 +616,32 @@ Result CertVerifier::VerifyCert( CertPolicyId evPolicy; bool foundEVPolicy = GetFirstEVPolicy(certBytes, evPolicy); rv = Result::ERROR_UNKNOWN_ERROR; - if (foundEVPolicy) { + for (size_t i = 0; + i < sha1ModeConfigurationsCount && rv != Success && foundEVPolicy; + i++) { + // Don't attempt verification if the SHA1 mode set by preferences + // (mSHA1Mode) is more restrictive than the SHA1 mode option we're on. + // (To put it another way, only attempt verification if the SHA1 mode + // option we're on is as restrictive or more restrictive than + // mSHA1Mode.) This allows us to gather telemetry information while + // still enforcing the mode set by preferences. + if (SHA1ModeMoreRestrictiveThanGivenMode(sha1ModeConfigurations[i])) { + continue; + } + + // Because of the try-strict and fallback approach, we have to clear any + // previously noted telemetry information. + if (pinningTelemetryInfo) { + pinningTelemetryInfo->Reset(); + } + NSSCertDBTrustDomain trustDomain( trustSSL, evOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft, mOCSPTimeoutHard, mCertShortLifetimeInDays, MIN_RSA_BITS, - ValidityCheckingMode::CheckForEV, mNetscapeStepUpPolicy, - mCRLiteMode, originAttributes, mThirdPartyRootInputs, - mThirdPartyIntermediateInputs, extraCertificates, builtChain, - pinningTelemetryInfo, hostname); + ValidityCheckingMode::CheckForEV, sha1ModeConfigurations[i], + mNetscapeStepUpPolicy, mCRLiteMode, originAttributes, + mThirdPartyRootInputs, mThirdPartyIntermediateInputs, + extraCertificates, builtChain, pinningTelemetryInfo, hostname); rv = BuildCertChainForOneKeyUsage( trustDomain, certDER, time, KeyUsage::digitalSignature, // (EC)DHE @@ -579,21 +649,36 @@ Result CertVerifier::VerifyCert( KeyUsage::keyAgreement, // (EC)DH KeyPurposeId::id_kp_serverAuth, evPolicy, stapledOCSPResponse, ocspStaplingStatus); - if (rv == Success) { - rv = VerifyCertificateTransparencyPolicy( - trustDomain, builtChain, sctsFromTLSInput, time, ctInfo); + if (rv == Success && + sha1ModeConfigurations[i] == SHA1Mode::ImportedRoot && + trustDomain.GetIsBuiltChainRootBuiltInRoot()) { + rv = Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED; } if (rv == Success) { + MOZ_LOG(gCertVerifierLog, LogLevel::Debug, + ("cert is EV with status %i\n", + static_cast(sha1ModeResults[i]))); if (evStatus) { - *evStatus = EVStatus::EV; + *evStatus = foundEVPolicy ? EVStatus::EV : EVStatus::NotEV; + } + if (sha1ModeResult) { + *sha1ModeResult = sha1ModeResults[i]; + } + rv = VerifyCertificateTransparencyPolicy( + trustDomain, builtChain, sctsFromTLSInput, time, ctInfo); + if (rv != Success) { + break; } if (isBuiltChainRootBuiltInRoot) { *isBuiltChainRootBuiltInRoot = trustDomain.GetIsBuiltChainRootBuiltInRoot(); } - break; } } + if (rv == Success) { + break; + } + if (flags & FLAG_MUST_BE_EV) { rv = Result::ERROR_POLICY_VALIDATION_FAILED; break; @@ -612,53 +697,86 @@ Result CertVerifier::VerifyCert( size_t keySizeOptionsCount = MOZ_ARRAY_LENGTH(keySizeStatuses); for (size_t i = 0; i < keySizeOptionsCount && rv != Success; i++) { - // invalidate any telemetry info relating to failed chains - if (pinningTelemetryInfo) { - pinningTelemetryInfo->Reset(); - } + for (size_t j = 0; j < sha1ModeConfigurationsCount && rv != Success; + j++) { + // Don't attempt verification if the SHA1 mode set by preferences + // (mSHA1Mode) is more restrictive than the SHA1 mode option we're on. + // (To put it another way, only attempt verification if the SHA1 mode + // option we're on is as restrictive or more restrictive than + // mSHA1Mode.) This allows us to gather telemetry information while + // still enforcing the mode set by preferences. + if (SHA1ModeMoreRestrictiveThanGivenMode(sha1ModeConfigurations[j])) { + continue; + } - NSSCertDBTrustDomain trustDomain( - trustSSL, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft, - mOCSPTimeoutHard, mCertShortLifetimeInDays, keySizeOptions[i], - ValidityCheckingMode::CheckingOff, mNetscapeStepUpPolicy, - mCRLiteMode, originAttributes, mThirdPartyRootInputs, - mThirdPartyIntermediateInputs, extraCertificates, builtChain, - pinningTelemetryInfo, hostname); - rv = BuildCertChainForOneKeyUsage( - trustDomain, certDER, time, - KeyUsage::digitalSignature, //(EC)DHE - KeyUsage::keyEncipherment, // RSA - KeyUsage::keyAgreement, //(EC)DH - KeyPurposeId::id_kp_serverAuth, CertPolicyId::anyPolicy, - stapledOCSPResponse, ocspStaplingStatus); - if (rv != Success && !IsFatalError(rv) && - rv != Result::ERROR_REVOKED_CERTIFICATE && - trustDomain.GetIsErrorDueToDistrustedCAPolicy()) { - // Bug 1444440 - If there are multiple paths, at least one to a CA - // distrusted-by-policy, and none of them ending in a trusted root, - // then we might show a different error (UNKNOWN_ISSUER) than we - // intend, confusing users. - rv = Result::ERROR_ADDITIONAL_POLICY_CONSTRAINT_FAILED; - } - if (rv == Success) { - rv = VerifyCertificateTransparencyPolicy( - trustDomain, builtChain, sctsFromTLSInput, time, ctInfo); - } - if (rv == Success) { - if (keySizeStatus) { - *keySizeStatus = keySizeStatuses[i]; + // invalidate any telemetry info relating to failed chains + if (pinningTelemetryInfo) { + pinningTelemetryInfo->Reset(); } - if (isBuiltChainRootBuiltInRoot) { - *isBuiltChainRootBuiltInRoot = - trustDomain.GetIsBuiltChainRootBuiltInRoot(); + + NSSCertDBTrustDomain trustDomain( + trustSSL, defaultOCSPFetching, mOCSPCache, pinArg, + mOCSPTimeoutSoft, mOCSPTimeoutHard, mCertShortLifetimeInDays, + keySizeOptions[i], ValidityCheckingMode::CheckingOff, + sha1ModeConfigurations[j], mNetscapeStepUpPolicy, mCRLiteMode, + originAttributes, mThirdPartyRootInputs, + mThirdPartyIntermediateInputs, extraCertificates, builtChain, + pinningTelemetryInfo, hostname); + rv = BuildCertChainForOneKeyUsage( + trustDomain, certDER, time, + KeyUsage::digitalSignature, //(EC)DHE + KeyUsage::keyEncipherment, // RSA + KeyUsage::keyAgreement, //(EC)DH + KeyPurposeId::id_kp_serverAuth, CertPolicyId::anyPolicy, + stapledOCSPResponse, ocspStaplingStatus); + if (rv != Success && !IsFatalError(rv) && + rv != Result::ERROR_REVOKED_CERTIFICATE && + trustDomain.GetIsErrorDueToDistrustedCAPolicy()) { + // Bug 1444440 - If there are multiple paths, at least one to a CA + // distrusted-by-policy, and none of them ending in a trusted root, + // then we might show a different error (UNKNOWN_ISSUER) than we + // intend, confusing users. + rv = Result::ERROR_ADDITIONAL_POLICY_CONSTRAINT_FAILED; + } + if (rv == Success && + sha1ModeConfigurations[j] == SHA1Mode::ImportedRoot && + trustDomain.GetIsBuiltChainRootBuiltInRoot()) { + rv = Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED; + } + if (rv == Success) { + if (keySizeStatus) { + *keySizeStatus = keySizeStatuses[i]; + } + if (sha1ModeResult) { + *sha1ModeResult = sha1ModeResults[j]; + } + rv = VerifyCertificateTransparencyPolicy( + trustDomain, builtChain, sctsFromTLSInput, time, ctInfo); + if (rv != Success) { + break; + } + if (isBuiltChainRootBuiltInRoot) { + *isBuiltChainRootBuiltInRoot = + trustDomain.GetIsBuiltChainRootBuiltInRoot(); + } } - break; } } - if (rv != Success && keySizeStatus) { + if (rv == Success) { + break; + } + + if (keySizeStatus) { *keySizeStatus = KeySizeStatus::AlreadyBad; } + // The telemetry probe CERT_CHAIN_SHA1_POLICY_STATUS gives us feedback on + // the result of setting a specific policy. However, we don't want noise + // from users who have manually set the policy to something other than the + // default, so we only collect for Forbidden (which is the default). + if (sha1ModeResult && mSHA1Mode == SHA1Mode::Forbidden) { + *sha1ModeResult = SHA1ModeResult::Failed; + } break; } @@ -667,10 +785,10 @@ Result CertVerifier::VerifyCert( NSSCertDBTrustDomain trustDomain( trustSSL, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft, mOCSPTimeoutHard, mCertShortLifetimeInDays, MIN_RSA_BITS_WEAK, - ValidityCheckingMode::CheckingOff, mNetscapeStepUpPolicy, mCRLiteMode, - originAttributes, mThirdPartyRootInputs, - mThirdPartyIntermediateInputs, extraCertificates, builtChain, nullptr, - nullptr); + ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, + mNetscapeStepUpPolicy, mCRLiteMode, originAttributes, + mThirdPartyRootInputs, mThirdPartyIntermediateInputs, + extraCertificates, builtChain, nullptr, nullptr); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeCA, KeyUsage::keyCertSign, KeyPurposeId::id_kp_serverAuth, CertPolicyId::anyPolicy, stapledOCSPResponse); @@ -681,10 +799,10 @@ Result CertVerifier::VerifyCert( NSSCertDBTrustDomain trustDomain( trustEmail, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft, mOCSPTimeoutHard, mCertShortLifetimeInDays, MIN_RSA_BITS_WEAK, - ValidityCheckingMode::CheckingOff, NetscapeStepUpPolicy::NeverMatch, - mCRLiteMode, originAttributes, mThirdPartyRootInputs, - mThirdPartyIntermediateInputs, extraCertificates, builtChain, nullptr, - nullptr); + ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, + NetscapeStepUpPolicy::NeverMatch, mCRLiteMode, originAttributes, + mThirdPartyRootInputs, mThirdPartyIntermediateInputs, + extraCertificates, builtChain, nullptr, nullptr); rv = BuildCertChain( trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::digitalSignature, KeyPurposeId::id_kp_emailProtection, @@ -705,10 +823,10 @@ Result CertVerifier::VerifyCert( NSSCertDBTrustDomain trustDomain( trustEmail, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft, mOCSPTimeoutHard, mCertShortLifetimeInDays, MIN_RSA_BITS_WEAK, - ValidityCheckingMode::CheckingOff, NetscapeStepUpPolicy::NeverMatch, - mCRLiteMode, originAttributes, mThirdPartyRootInputs, - mThirdPartyIntermediateInputs, extraCertificates, builtChain, nullptr, - nullptr); + ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, + NetscapeStepUpPolicy::NeverMatch, mCRLiteMode, originAttributes, + mThirdPartyRootInputs, mThirdPartyIntermediateInputs, + extraCertificates, builtChain, nullptr, nullptr); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::keyEncipherment, // RSA @@ -761,6 +879,7 @@ Result CertVerifier::VerifySSLServerCert( /*optional out*/ EVStatus* evStatus, /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus, /*optional out*/ KeySizeStatus* keySizeStatus, + /*optional out*/ SHA1ModeResult* sha1ModeResult, /*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo, /*optional out*/ CertificateTransparencyInfo* ctInfo, /*optional out*/ bool* isBuiltChainRootBuiltInRoot) { @@ -792,7 +911,7 @@ Result CertVerifier::VerifySSLServerCert( PromiseFlatCString(hostname).get(), builtChain, flags, extraCertificates, stapledOCSPResponse, sctsFromTLS, originAttributes, evStatus, ocspStaplingStatus, keySizeStatus, - pinningTelemetryInfo, ctInfo, + sha1ModeResult, pinningTelemetryInfo, ctInfo, &isBuiltChainRootBuiltInRootLocal); if (rv != Success) { // we don't use the certificate for path building, so this parameter doesn't diff --git a/security/certverifier/CertVerifier.h b/security/certverifier/CertVerifier.h index 76d6bf3ff84b..b69a4b209490 100644 --- a/security/certverifier/CertVerifier.h +++ b/security/certverifier/CertVerifier.h @@ -61,6 +61,16 @@ enum class KeySizeStatus { AlreadyBad = 3, }; +// These values correspond to the CERT_CHAIN_SHA1_POLICY_STATUS telemetry. +enum class SHA1ModeResult { + NeverChecked = 0, + SucceededWithoutSHA1 = 1, + SucceededWithImportedRoot = 2, + SucceededWithImportedRootOrSHA1Before2016 = 3, + SucceededWithSHA1 = 4, + Failed = 5, +}; + enum class CRLiteMode { Disabled = 0, TelemetryOnly = 1, @@ -160,6 +170,7 @@ class CertVerifier { /*optional out*/ EVStatus* evStatus = nullptr, /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus = nullptr, /*optional out*/ KeySizeStatus* keySizeStatus = nullptr, + /*optional out*/ SHA1ModeResult* sha1ModeResult = nullptr, /*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo = nullptr, /*optional out*/ CertificateTransparencyInfo* ctInfo = nullptr, /*optional out*/ bool* isBuiltChainRootBuiltInRoot = nullptr); @@ -180,10 +191,22 @@ class CertVerifier { /*optional out*/ EVStatus* evStatus = nullptr, /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus = nullptr, /*optional out*/ KeySizeStatus* keySizeStatus = nullptr, + /*optional out*/ SHA1ModeResult* sha1ModeResult = nullptr, /*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo = nullptr, /*optional out*/ CertificateTransparencyInfo* ctInfo = nullptr, /*optional out*/ bool* isBuiltChainRootBuiltInRoot = nullptr); + enum class SHA1Mode { + Allowed = 0, + Forbidden = 1, + // There used to be a policy that only allowed SHA1 for certificates issued + // before 2016. This is no longer available. If a user has selected this + // policy in about:config, it now maps to Forbidden. + UsedToBeBefore2016ButNowIsForbidden = 2, + ImportedRoot = 3, + ImportedRootOrBefore2016 = 4, + }; + enum OcspDownloadConfig { ocspOff = 0, ocspOn = 1, ocspEVOnly = 2 }; enum OcspStrictConfig { ocspRelaxed = 0, ocspStrict }; @@ -195,7 +218,7 @@ class CertVerifier { CertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc, mozilla::TimeDuration ocspTimeoutSoft, mozilla::TimeDuration ocspTimeoutHard, - uint32_t certShortLifetimeInDays, + uint32_t certShortLifetimeInDays, SHA1Mode sha1Mode, NetscapeStepUpPolicy netscapeStepUpPolicy, CertificateTransparencyMode ctMode, CRLiteMode crliteMode, const Vector& thirdPartyCerts); @@ -208,6 +231,7 @@ class CertVerifier { const mozilla::TimeDuration mOCSPTimeoutSoft; const mozilla::TimeDuration mOCSPTimeoutHard; const uint32_t mCertShortLifetimeInDays; + const SHA1Mode mSHA1Mode; const NetscapeStepUpPolicy mNetscapeStepUpPolicy; const CertificateTransparencyMode mCTMode; const CRLiteMode mCRLiteMode; @@ -233,6 +257,12 @@ class CertVerifier { const nsTArray>& builtChain, mozilla::pkix::Input sctsFromTLS, mozilla::pkix::Time time, /*optional out*/ CertificateTransparencyInfo* ctInfo); + + // Returns true if the configured SHA1 mode is more restrictive than the given + // mode. SHA1Mode::Forbidden is more restrictive than any other mode except + // Forbidden. Next is ImportedRoot, then ImportedRootOrBefore2016, then + // Allowed. (A mode is never more restrictive than itself.) + bool SHA1ModeMoreRestrictiveThanGivenMode(SHA1Mode mode); }; mozilla::pkix::Result IsCertBuiltInRoot(pkix::Input certInput, bool& result); diff --git a/security/certverifier/NSSCertDBTrustDomain.cpp b/security/certverifier/NSSCertDBTrustDomain.cpp index af953286ef23..85af694004f9 100644 --- a/security/certverifier/NSSCertDBTrustDomain.cpp +++ b/security/certverifier/NSSCertDBTrustDomain.cpp @@ -13,6 +13,7 @@ #include "ExtendedValidation.h" #include "MultiLogCTVerifier.h" #include "NSSErrorsService.h" +#include "OCSPVerificationTrustDomain.h" #include "PublicKeyPinningService.h" #include "cert.h" #include "cert_storage/src/cert_storage.h" @@ -71,8 +72,8 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain( /*optional but shouldn't be*/ void* pinArg, TimeDuration ocspTimeoutSoft, TimeDuration ocspTimeoutHard, uint32_t certShortLifetimeInDays, unsigned int minRSABits, ValidityCheckingMode validityCheckingMode, - NetscapeStepUpPolicy netscapeStepUpPolicy, CRLiteMode crliteMode, - const OriginAttributes& originAttributes, + CertVerifier::SHA1Mode sha1Mode, NetscapeStepUpPolicy netscapeStepUpPolicy, + CRLiteMode crliteMode, const OriginAttributes& originAttributes, const Vector& thirdPartyRootInputs, const Vector& thirdPartyIntermediateInputs, const Maybe>>& extraCertificates, @@ -88,6 +89,7 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain( mCertShortLifetimeInDays(certShortLifetimeInDays), mMinRSABits(minRSABits), mValidityCheckingMode(validityCheckingMode), + mSHA1Mode(sha1Mode), mNetscapeStepUpPolicy(netscapeStepUpPolicy), mCRLiteMode(crliteMode), mSawDistrustedCAByPolicyError(false), @@ -1120,9 +1122,18 @@ Result NSSCertDBTrustDomain::VerifyAndMaybeCacheEncodedOCSPResponse( Time thisUpdate(Time::uninitialized); Time validThrough(Time::uninitialized); - Result rv = VerifyEncodedOCSPResponse(*this, certID, time, maxLifetimeInDays, - encodedResponse, expired, &thisUpdate, - &validThrough); + // We use a try and fallback approach which first mandates good signature + // digest algorithms, then falls back to SHA-1 if this fails. If a delegated + // OCSP response signing certificate was issued with a SHA-1 signature, + // verification initially fails. We cache the failure and then re-use that + // result even when doing fallback (i.e. when weak signature digest algorithms + // should succeed). To address this we use an OCSPVerificationTrustDomain + // here, rather than using *this, to ensure verification succeeds for all + // allowed signature digest algorithms. + OCSPVerificationTrustDomain trustDomain(*this); + Result rv = VerifyEncodedOCSPResponse(trustDomain, certID, time, + maxLifetimeInDays, encodedResponse, + expired, &thisUpdate, &validThrough); // If a response was stapled and expired, we don't want to cache it. Return // early to simplify the logic here. if (responseSource == ResponseWasStapled && expired) { @@ -1370,15 +1381,39 @@ Result NSSCertDBTrustDomain::IsChainValid(const DERArray& reversedDERArray, } Result NSSCertDBTrustDomain::CheckSignatureDigestAlgorithm( - DigestAlgorithm aAlg, EndEntityOrCA /*endEntityOrCA*/, Time /*notBefore*/) { - switch (aAlg) { - case DigestAlgorithm::sha256: // fall through - case DigestAlgorithm::sha384: // fall through - case DigestAlgorithm::sha512: - return Success; - case DigestAlgorithm::sha1: - return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED; + DigestAlgorithm aAlg, EndEntityOrCA endEntityOrCA, Time notBefore) { + // (new Date("2016-01-01T00:00:00Z")).getTime() / 1000 + static const Time JANUARY_FIRST_2016 = TimeFromEpochInSeconds(1451606400); + + MOZ_LOG(gCertVerifierLog, LogLevel::Debug, + ("NSSCertDBTrustDomain: CheckSignatureDigestAlgorithm")); + if (aAlg == DigestAlgorithm::sha1) { + switch (mSHA1Mode) { + case CertVerifier::SHA1Mode::Forbidden: + MOZ_LOG(gCertVerifierLog, LogLevel::Debug, + ("SHA-1 certificate rejected")); + return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED; + case CertVerifier::SHA1Mode::ImportedRootOrBefore2016: + if (JANUARY_FIRST_2016 <= notBefore) { + MOZ_LOG(gCertVerifierLog, LogLevel::Debug, + ("Post-2015 SHA-1 certificate rejected")); + return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED; + } + break; + case CertVerifier::SHA1Mode::Allowed: + // Enforcing that the resulting chain uses an imported root is only + // possible at a higher level. This is done in CertVerifier::VerifyCert. + case CertVerifier::SHA1Mode::ImportedRoot: + default: + break; + // MSVC warns unless we explicitly handle this now-unused option. + case CertVerifier::SHA1Mode::UsedToBeBefore2016ButNowIsForbidden: + MOZ_ASSERT_UNREACHABLE("unexpected SHA1Mode type"); + return Result::FATAL_ERROR_LIBRARY_FAILURE; + } } + + return Success; } Result NSSCertDBTrustDomain::CheckRSAPublicKeyModulusSizeInBits( diff --git a/security/certverifier/NSSCertDBTrustDomain.h b/security/certverifier/NSSCertDBTrustDomain.h index e4d6970314f2..98171d8cc02d 100644 --- a/security/certverifier/NSSCertDBTrustDomain.h +++ b/security/certverifier/NSSCertDBTrustDomain.h @@ -142,6 +142,7 @@ class NSSCertDBTrustDomain : public mozilla::pkix::TrustDomain { OCSPCache& ocspCache, void* pinArg, mozilla::TimeDuration ocspTimeoutSoft, mozilla::TimeDuration ocspTimeoutHard, uint32_t certShortLifetimeInDays, unsigned int minRSABits, ValidityCheckingMode validityCheckingMode, + CertVerifier::SHA1Mode sha1Mode, NetscapeStepUpPolicy netscapeStepUpPolicy, CRLiteMode crliteMode, const OriginAttributes& originAttributes, const Vector& thirdPartyRootInputs, @@ -278,6 +279,7 @@ class NSSCertDBTrustDomain : public mozilla::pkix::TrustDomain { const uint32_t mCertShortLifetimeInDays; const unsigned int mMinRSABits; ValidityCheckingMode mValidityCheckingMode; + CertVerifier::SHA1Mode mSHA1Mode; NetscapeStepUpPolicy mNetscapeStepUpPolicy; CRLiteMode mCRLiteMode; bool mSawDistrustedCAByPolicyError; diff --git a/security/certverifier/OCSPVerificationTrustDomain.cpp b/security/certverifier/OCSPVerificationTrustDomain.cpp new file mode 100644 index 000000000000..413d458e3fff --- /dev/null +++ b/security/certverifier/OCSPVerificationTrustDomain.cpp @@ -0,0 +1,114 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* 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 "OCSPVerificationTrustDomain.h" + +using namespace mozilla; +using namespace mozilla::pkix; + +namespace mozilla { +namespace psm { + +OCSPVerificationTrustDomain::OCSPVerificationTrustDomain( + NSSCertDBTrustDomain& certDBTrustDomain) + : mCertDBTrustDomain(certDBTrustDomain) {} + +Result OCSPVerificationTrustDomain::GetCertTrust( + EndEntityOrCA endEntityOrCA, const CertPolicyId& policy, + Input candidateCertDER, + /*out*/ TrustLevel& trustLevel) { + return mCertDBTrustDomain.GetCertTrust(endEntityOrCA, policy, + candidateCertDER, trustLevel); +} + +Result OCSPVerificationTrustDomain::FindIssuer(Input, IssuerChecker&, Time) { + // We do not expect this to be called for OCSP signers + return Result::FATAL_ERROR_LIBRARY_FAILURE; +} + +Result OCSPVerificationTrustDomain::IsChainValid(const DERArray&, Time, + const CertPolicyId&) { + // We do not expect this to be called for OCSP signers + return Result::FATAL_ERROR_LIBRARY_FAILURE; +} + +Result OCSPVerificationTrustDomain::CheckRevocation(EndEntityOrCA, + const CertID&, Time, + Duration, const Input*, + const Input*, + const Input*) { + // We do not expect this to be called for OCSP signers + return Result::FATAL_ERROR_LIBRARY_FAILURE; +} + +Result OCSPVerificationTrustDomain::CheckSignatureDigestAlgorithm( + DigestAlgorithm aAlg, EndEntityOrCA aEEOrCA, Time notBefore) { + // The reason for wrapping the NSSCertDBTrustDomain in an + // OCSPVerificationTrustDomain is to allow us to bypass the weaker signature + // algorithm check - thus all allowable signature digest algorithms should + // always be accepted. This is only needed while we gather telemetry on SHA-1. + return Success; +} + +Result OCSPVerificationTrustDomain::CheckRSAPublicKeyModulusSizeInBits( + EndEntityOrCA aEEOrCA, unsigned int aModulusSizeInBits) { + return mCertDBTrustDomain.CheckRSAPublicKeyModulusSizeInBits( + aEEOrCA, aModulusSizeInBits); +} + +Result OCSPVerificationTrustDomain::VerifyRSAPKCS1SignedData( + Input data, DigestAlgorithm digestAlgorithm, Input signature, + Input subjectPublicKeyInfo) { + return mCertDBTrustDomain.VerifyRSAPKCS1SignedData( + data, digestAlgorithm, signature, subjectPublicKeyInfo); +} + +Result OCSPVerificationTrustDomain::VerifyRSAPSSSignedData( + Input data, DigestAlgorithm digestAlgorithm, Input signature, + Input subjectPublicKeyInfo) { + return mCertDBTrustDomain.VerifyRSAPSSSignedData( + data, digestAlgorithm, signature, subjectPublicKeyInfo); +} + +Result OCSPVerificationTrustDomain::CheckECDSACurveIsAcceptable( + EndEntityOrCA aEEOrCA, NamedCurve aCurve) { + return mCertDBTrustDomain.CheckECDSACurveIsAcceptable(aEEOrCA, aCurve); +} + +Result OCSPVerificationTrustDomain::VerifyECDSASignedData( + Input data, DigestAlgorithm digestAlgorithm, Input signature, + Input subjectPublicKeyInfo) { + return mCertDBTrustDomain.VerifyECDSASignedData( + data, digestAlgorithm, signature, subjectPublicKeyInfo); +} + +Result OCSPVerificationTrustDomain::CheckValidityIsAcceptable( + Time notBefore, Time notAfter, EndEntityOrCA endEntityOrCA, + KeyPurposeId keyPurpose) { + return mCertDBTrustDomain.CheckValidityIsAcceptable( + notBefore, notAfter, endEntityOrCA, keyPurpose); +} + +Result OCSPVerificationTrustDomain::NetscapeStepUpMatchesServerAuth( + Time notBefore, + /*out*/ bool& matches) { + return mCertDBTrustDomain.NetscapeStepUpMatchesServerAuth(notBefore, matches); +} + +void OCSPVerificationTrustDomain::NoteAuxiliaryExtension( + AuxiliaryExtension extension, Input extensionData) { + mCertDBTrustDomain.NoteAuxiliaryExtension(extension, extensionData); +} + +Result OCSPVerificationTrustDomain::DigestBuf(Input item, + DigestAlgorithm digestAlg, + /*out*/ uint8_t* digestBuf, + size_t digestBufLen) { + return mCertDBTrustDomain.DigestBuf(item, digestAlg, digestBuf, digestBufLen); +} + +} // namespace psm +} // namespace mozilla diff --git a/security/certverifier/OCSPVerificationTrustDomain.h b/security/certverifier/OCSPVerificationTrustDomain.h new file mode 100644 index 000000000000..ddebdf288cc1 --- /dev/null +++ b/security/certverifier/OCSPVerificationTrustDomain.h @@ -0,0 +1,97 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* 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_psm__OCSPVerificationTrustDomain_h +#define mozilla_psm__OCSPVerificationTrustDomain_h + +#include "mozpkix/pkixtypes.h" +#include "NSSCertDBTrustDomain.h" + +namespace mozilla { +namespace psm { + +typedef mozilla::pkix::Result Result; + +class OCSPVerificationTrustDomain : public mozilla::pkix::TrustDomain { + public: + explicit OCSPVerificationTrustDomain(NSSCertDBTrustDomain& certDBTrustDomain); + + virtual Result FindIssuer(mozilla::pkix::Input encodedIssuerName, + IssuerChecker& checker, + mozilla::pkix::Time time) override; + + virtual Result GetCertTrust( + mozilla::pkix::EndEntityOrCA endEntityOrCA, + const mozilla::pkix::CertPolicyId& policy, + mozilla::pkix::Input candidateCertDER, + /*out*/ mozilla::pkix::TrustLevel& trustLevel) override; + + virtual Result CheckSignatureDigestAlgorithm( + mozilla::pkix::DigestAlgorithm digestAlg, + mozilla::pkix::EndEntityOrCA endEntityOrCA, + mozilla::pkix::Time notBefore) override; + + virtual Result CheckRSAPublicKeyModulusSizeInBits( + mozilla::pkix::EndEntityOrCA endEntityOrCA, + unsigned int modulusSizeInBits) override; + + virtual Result VerifyRSAPKCS1SignedData( + mozilla::pkix::Input data, mozilla::pkix::DigestAlgorithm digestAlgorithm, + mozilla::pkix::Input signature, + mozilla::pkix::Input subjectPublicKeyInfo) override; + + virtual Result VerifyRSAPSSSignedData( + mozilla::pkix::Input data, mozilla::pkix::DigestAlgorithm digestAlgorithm, + mozilla::pkix::Input signature, + mozilla::pkix::Input subjectPublicKeyInfo) override; + + virtual Result CheckECDSACurveIsAcceptable( + mozilla::pkix::EndEntityOrCA endEntityOrCA, + mozilla::pkix::NamedCurve curve) override; + + virtual Result VerifyECDSASignedData( + mozilla::pkix::Input data, mozilla::pkix::DigestAlgorithm digestAlgorithm, + mozilla::pkix::Input signature, + mozilla::pkix::Input subjectPublicKeyInfo) override; + + virtual Result DigestBuf(mozilla::pkix::Input item, + mozilla::pkix::DigestAlgorithm digestAlg, + /*out*/ uint8_t* digestBuf, + size_t digestBufLen) override; + + virtual Result CheckValidityIsAcceptable( + mozilla::pkix::Time notBefore, mozilla::pkix::Time notAfter, + mozilla::pkix::EndEntityOrCA endEntityOrCA, + mozilla::pkix::KeyPurposeId keyPurpose) override; + + virtual Result NetscapeStepUpMatchesServerAuth( + mozilla::pkix::Time notBefore, + /*out*/ bool& matches) override; + + virtual Result CheckRevocation( + mozilla::pkix::EndEntityOrCA endEntityOrCA, + const mozilla::pkix::CertID& certID, mozilla::pkix::Time time, + mozilla::pkix::Duration validityDuration, + /*optional*/ const mozilla::pkix::Input* stapledOCSPResponse, + /*optional*/ const mozilla::pkix::Input* aiaExtension, + /*optional*/ const mozilla::pkix::Input* sctExtension) override; + + virtual Result IsChainValid( + const mozilla::pkix::DERArray& certChain, mozilla::pkix::Time time, + const mozilla::pkix::CertPolicyId& requiredPolicy) override; + + virtual void NoteAuxiliaryExtension( + mozilla::pkix::AuxiliaryExtension extension, + mozilla::pkix::Input extensionData) override; + + private: + NSSCertDBTrustDomain& mCertDBTrustDomain; +}; + +} // namespace psm +} // namespace mozilla + +#endif // mozilla_psm__OCSPVerificationTrustDomain_h diff --git a/security/certverifier/moz.build b/security/certverifier/moz.build index ce992d1c5afb..bf9ac0a38cf5 100644 --- a/security/certverifier/moz.build +++ b/security/certverifier/moz.build @@ -16,6 +16,7 @@ UNIFIED_SOURCES += [ "CertVerifier.cpp", "NSSCertDBTrustDomain.cpp", "OCSPCache.cpp", + "OCSPVerificationTrustDomain.cpp", ] if not CONFIG["NSS_NO_EV_CERTS"]: diff --git a/security/manager/ssl/CSTrustDomain.cpp b/security/manager/ssl/CSTrustDomain.cpp index 99c412d60b3f..a2eccbe05891 100644 --- a/security/manager/ssl/CSTrustDomain.cpp +++ b/security/manager/ssl/CSTrustDomain.cpp @@ -131,14 +131,10 @@ Result CSTrustDomain::IsChainValid(const DERArray& certChain, Time time, Result CSTrustDomain::CheckSignatureDigestAlgorithm(DigestAlgorithm digestAlg, EndEntityOrCA endEntityOrCA, Time notBefore) { - switch (digestAlg) { - case DigestAlgorithm::sha256: // fall through - case DigestAlgorithm::sha384: // fall through - case DigestAlgorithm::sha512: - return Success; - case DigestAlgorithm::sha1: - return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED; + if (digestAlg == DigestAlgorithm::sha1) { + return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED; } + return Success; } Result CSTrustDomain::CheckRSAPublicKeyModulusSizeInBits( diff --git a/security/manager/ssl/SSLServerCertVerification.cpp b/security/manager/ssl/SSLServerCertVerification.cpp index c2218c423c1b..e6db9703a336 100644 --- a/security/manager/ssl/SSLServerCertVerification.cpp +++ b/security/manager/ssl/SSLServerCertVerification.cpp @@ -638,7 +638,7 @@ void GatherCertificateTransparencyTelemetry( static void CollectCertTelemetry( mozilla::pkix::Result aCertVerificationResult, EVStatus aEVStatus, CertVerifier::OCSPStaplingStatus aOcspStaplingStatus, - KeySizeStatus aKeySizeStatus, + KeySizeStatus aKeySizeStatus, SHA1ModeResult aSha1ModeResult, const PinningTelemetryInfo& aPinningTelemetryInfo, const nsTArray>& aBuiltCertChain, const CertificateTransparencyInfo& aCertificateTransparencyInfo) { @@ -656,6 +656,11 @@ static void CollectCertTelemetry( static_cast(aKeySizeStatus)); } + if (aSha1ModeResult != SHA1ModeResult::NeverChecked) { + Telemetry::Accumulate(Telemetry::CERT_CHAIN_SHA1_POLICY_STATUS, + static_cast(aSha1ModeResult)); + } + if (aPinningTelemetryInfo.accumulateForRoot) { Telemetry::Accumulate(Telemetry::CERT_PINNING_FAILURES_BY_CA, aPinningTelemetryInfo.rootBucket); @@ -694,6 +699,7 @@ Result AuthCertificate( CertVerifier::OCSPStaplingStatus ocspStaplingStatus = CertVerifier::OCSP_STAPLING_NEVER_CHECKED; KeySizeStatus keySizeStatus = KeySizeStatus::NeverChecked; + SHA1ModeResult sha1ModeResult = SHA1ModeResult::NeverChecked; PinningTelemetryInfo pinningTelemetryInfo; nsTArray> peerCertsBytes; @@ -709,11 +715,12 @@ Result AuthCertificate( certBytes, time, aPinArg, aHostName, builtCertChain, certVerifierFlags, Some(std::move(peerCertsBytes)), stapledOCSPResponse, sctsFromTLSExtension, dcInfo, aOriginAttributes, &evStatus, - &ocspStaplingStatus, &keySizeStatus, &pinningTelemetryInfo, - &certificateTransparencyInfo, &aIsBuiltCertChainRootBuiltInRoot); + &ocspStaplingStatus, &keySizeStatus, &sha1ModeResult, + &pinningTelemetryInfo, &certificateTransparencyInfo, + &aIsBuiltCertChainRootBuiltInRoot); CollectCertTelemetry(rv, evStatus, ocspStaplingStatus, keySizeStatus, - pinningTelemetryInfo, builtCertChain, + sha1ModeResult, pinningTelemetryInfo, builtCertChain, certificateTransparencyInfo); return rv; diff --git a/security/manager/ssl/SharedCertVerifier.h b/security/manager/ssl/SharedCertVerifier.h index a0acceb33dcb..a48936a89395 100644 --- a/security/manager/ssl/SharedCertVerifier.h +++ b/security/manager/ssl/SharedCertVerifier.h @@ -23,13 +23,14 @@ class SharedCertVerifier : public mozilla::psm::CertVerifier { SharedCertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc, mozilla::TimeDuration ocspSoftTimeout, mozilla::TimeDuration ocspHardTimeout, - uint32_t certShortLifetimeInDays, + uint32_t certShortLifetimeInDays, SHA1Mode sha1Mode, NetscapeStepUpPolicy netscapeStepUpPolicy, CertificateTransparencyMode ctMode, CRLiteMode crliteMode, const Vector& thirdPartyCerts) - : mozilla::psm::CertVerifier( - odc, osc, ocspSoftTimeout, ocspHardTimeout, certShortLifetimeInDays, - netscapeStepUpPolicy, ctMode, crliteMode, thirdPartyCerts) {} + : mozilla::psm::CertVerifier(odc, osc, ocspSoftTimeout, ocspHardTimeout, + certShortLifetimeInDays, sha1Mode, + netscapeStepUpPolicy, ctMode, crliteMode, + thirdPartyCerts) {} }; } // namespace psm diff --git a/security/manager/ssl/nsNSSCallbacks.cpp b/security/manager/ssl/nsNSSCallbacks.cpp index 11302fa83aed..6a103f057122 100644 --- a/security/manager/ssl/nsNSSCallbacks.cpp +++ b/security/manager/ssl/nsNSSCallbacks.cpp @@ -1098,6 +1098,7 @@ static void RebuildVerifiedCertificateInformation(PRFileDesc* fd, &evStatus, nullptr, // OCSP stapling telemetry nullptr, // key size telemetry + nullptr, // SHA-1 telemetry nullptr, // pinning telemetry &certificateTransparencyInfo, &isBuiltCertChainRootBuiltInRoot); diff --git a/security/manager/ssl/nsNSSComponent.cpp b/security/manager/ssl/nsNSSComponent.cpp index c055a79bf531..6b3e96b01540 100644 --- a/security/manager/ssl/nsNSSComponent.cpp +++ b/security/manager/ssl/nsNSSComponent.cpp @@ -1418,6 +1418,25 @@ void nsNSSComponent::setValidationOptions( Telemetry::Accumulate(Telemetry::CERT_OCSP_REQUIRED, ocspRequired); } + CertVerifier::SHA1Mode sha1Mode = static_cast( + StaticPrefs::security_pki_sha1_enforcement_level()); + switch (sha1Mode) { + case CertVerifier::SHA1Mode::Allowed: + case CertVerifier::SHA1Mode::Forbidden: + case CertVerifier::SHA1Mode::UsedToBeBefore2016ButNowIsForbidden: + case CertVerifier::SHA1Mode::ImportedRoot: + case CertVerifier::SHA1Mode::ImportedRootOrBefore2016: + break; + default: + sha1Mode = CertVerifier::SHA1Mode::Allowed; + break; + } + + // Convert a previously-available setting to a safe one. + if (sha1Mode == CertVerifier::SHA1Mode::UsedToBeBefore2016ButNowIsForbidden) { + sha1Mode = CertVerifier::SHA1Mode::Forbidden; + } + NetscapeStepUpPolicy netscapeStepUpPolicy = static_cast( StaticPrefs::security_pki_netscape_step_up_policy()); switch (netscapeStepUpPolicy) { @@ -1455,7 +1474,7 @@ void nsNSSComponent::setValidationOptions( softTimeout, hardTimeout); mDefaultCertVerifier = new SharedCertVerifier( - odc, osc, softTimeout, hardTimeout, certShortLifetimeInDays, + odc, osc, softTimeout, hardTimeout, certShortLifetimeInDays, sha1Mode, netscapeStepUpPolicy, ctMode, crliteMode, mEnterpriseCerts); } @@ -1472,7 +1491,7 @@ void nsNSSComponent::UpdateCertVerifierWithEnterpriseRoots() { oldCertVerifier->mOCSPStrict ? CertVerifier::ocspStrict : CertVerifier::ocspRelaxed, oldCertVerifier->mOCSPTimeoutSoft, oldCertVerifier->mOCSPTimeoutHard, - oldCertVerifier->mCertShortLifetimeInDays, + oldCertVerifier->mCertShortLifetimeInDays, oldCertVerifier->mSHA1Mode, oldCertVerifier->mNetscapeStepUpPolicy, oldCertVerifier->mCTMode, oldCertVerifier->mCRLiteMode, mEnterpriseCerts); } @@ -2273,6 +2292,7 @@ nsNSSComponent::Observe(nsISupports* aSubject, const char* aTopic, prefName.EqualsLiteral("security.ssl.enable_ocsp_must_staple") || prefName.EqualsLiteral( "security.pki.certificate_transparency.mode") || + prefName.EqualsLiteral("security.pki.sha1_enforcement_level") || prefName.EqualsLiteral("security.pki.netscape_step_up_policy") || prefName.EqualsLiteral( "security.OCSP.timeoutMilliseconds.soft") || diff --git a/security/manager/ssl/tests/unit/test_cert_sha1.js b/security/manager/ssl/tests/unit/test_cert_sha1.js index 9513102b5626..371945bf853f 100644 --- a/security/manager/ssl/tests/unit/test_cert_sha1.js +++ b/security/manager/ssl/tests/unit/test_cert_sha1.js @@ -3,7 +3,8 @@ // 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/. -// Tests the rejection of SHA-1 certificates. +// Tests the rejection of SHA-1 certificates based on the preference +// security.pki.sha1_enforcement_level. "use strict"; @@ -12,6 +13,9 @@ const certdb = Cc["@mozilla.org/security/x509certdb;1"].getService( Ci.nsIX509CertDB ); +// We need to test as if we are at a fixed time, so that we are testing the +// 2016 checks on SHA-1, not the notBefore checks. +// // (new Date("2016-03-01")).getTime() / 1000 const VALIDATION_TIME = 1456790400; @@ -38,6 +42,63 @@ add_task(async function() { loadCertWithTrust("int-pre", ",,"); loadCertWithTrust("int-post", ",,"); + // Test cases per pref setting + // + // root intermed. end entity + // =========================== + // root + // | + // +--- pre-2016 + // | | + // | +----- pre-2016 <--- (a) + // | +----- post-2016 <--- (b) + // | + // +--- post-2016 + // | + // +----- post-2016 <--- (c) + // + // Expected outcomes (accept / reject): + // + // a b c + // Allowed (0) Accept Accept Accept + // Forbidden (1) Reject Reject Reject + // (2) is no longer available and is treated as Forbidden (1) internally. + // ImportedRoot (3) Reject Reject Reject (for built-in roots) + // ImportedRoot Accept Accept Accept (for non-built-in roots) + // ImportedRootOrBefore2016 (4) Accept Reject Reject (for built-in roots) + // ImportedRootOrBefore2016 Accept Accept Accept (for non-built-in roots) + // + // The pref setting of ImportedRoot accepts usage of SHA-1 only for + // certificates issued by non-built-in roots. By default, the testing + // certificates are all considered issued by a non-built-in root. However, we + // have the ability to treat a given non-built-in root as built-in. We test + // both of these situations below. + // + // As a historical note, a policy option (Before2016) was previously available + // that only allowed SHA-1 for certificates with a notBefore before 2016. + // However, to enable the policy of only allowing SHA-1 from non-built-in + // roots in the most straightforward way (while still having a time-based + // policy that users could enable if this new policy were problematic), + // Before2016 was shifted to also allow SHA-1 from non-built-in roots, hence + // ImportedRootOrBefore2016. + // + // A note about intermediate certificates: the certificate verifier has the + // ability to directly verify a given certificate for the purpose of issuing + // TLS web server certificates. However, when asked to do so, the certificate + // verifier does not take into account the currently configured SHA-1 policy. + // This is in part due to implementation complexity and because this isn't + // actually how TLS web server certificates are verified in the TLS handshake + // (which makes a full implementation that supports heeding the SHA-1 policy + // unnecessary). + + // SHA-1 allowed + Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 0); + await checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); + await checkEndEntity(certFromFile("ee-post_int-pre"), PRErrorCodeSuccess); + await checkEndEntity(certFromFile("ee-post_int-post"), PRErrorCodeSuccess); + + // SHA-1 forbidden + Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 1); await checkEndEntity( certFromFile("ee-pre_int-pre"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED @@ -50,4 +111,77 @@ add_task(async function() { certFromFile("ee-post_int-post"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED ); + + // SHA-1 forbidden (test the case where the pref has been set to 2) + Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 2); + await checkEndEntity( + certFromFile("ee-pre_int-pre"), + SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED + ); + await checkEndEntity( + certFromFile("ee-post_int-pre"), + SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED + ); + await checkEndEntity( + certFromFile("ee-post_int-post"), + SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED + ); + + // SHA-1 allowed only when issued by an imported root. First test with the + // test root considered a built-in (on debug only - this functionality is + // disabled on non-debug builds). + Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 3); + if (isDebugBuild) { + let root = certFromFile("ca"); + Services.prefs.setCharPref( + "security.test.built_in_root_hash", + root.sha256Fingerprint + ); + await checkEndEntity( + certFromFile("ee-pre_int-pre"), + SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED + ); + await checkEndEntity( + certFromFile("ee-post_int-pre"), + SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED + ); + await checkEndEntity( + certFromFile("ee-post_int-post"), + SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED + ); + Services.prefs.clearUserPref("security.test.built_in_root_hash"); + } + + // SHA-1 still allowed only when issued by an imported root. + // Now test with the test root considered a non-built-in. + await checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); + await checkEndEntity(certFromFile("ee-post_int-pre"), PRErrorCodeSuccess); + await checkEndEntity(certFromFile("ee-post_int-post"), PRErrorCodeSuccess); + + // SHA-1 allowed before 2016 or when issued by an imported root. First test + // with the test root considered a built-in. + Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 4); + if (isDebugBuild) { + let root = certFromFile("ca"); + Services.prefs.setCharPref( + "security.test.built_in_root_hash", + root.sha256Fingerprint + ); + await checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); + await checkEndEntity( + certFromFile("ee-post_int-pre"), + SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED + ); + await checkEndEntity( + certFromFile("ee-post_int-post"), + SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED + ); + Services.prefs.clearUserPref("security.test.built_in_root_hash"); + } + + // SHA-1 still only allowed before 2016 or when issued by an imported root. + // Now test with the test root considered a non-built-in. + await checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); + await checkEndEntity(certFromFile("ee-post_int-pre"), PRErrorCodeSuccess); + await checkEndEntity(certFromFile("ee-post_int-post"), PRErrorCodeSuccess); }); diff --git a/security/manager/ssl/tests/unit/test_ocsp_caching.js b/security/manager/ssl/tests/unit/test_ocsp_caching.js index be20a72c41a7..ca0f7c92f8ce 100644 --- a/security/manager/ssl/tests/unit/test_ocsp_caching.js +++ b/security/manager/ssl/tests/unit/test_ocsp_caching.js @@ -78,6 +78,7 @@ function run_test() { do_get_profile(); Services.prefs.setBoolPref("security.ssl.enable_ocsp_stapling", true); Services.prefs.setIntPref("security.OCSP.enabled", 1); + Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 4); add_tls_server_setup("OCSPStaplingServer", "ocsp_certs"); let ocspResponder = new HttpServer(); @@ -162,7 +163,14 @@ function add_tests() { add_ocsp_test( "ocsp-stapling-none.example.com", SEC_ERROR_OCSP_UNKNOWN_CERT, - [respondWithError, respondWithError], + [ + respondWithError, + respondWithError, + respondWithError, + respondWithError, + respondWithError, + respondWithError, + ], "No stapled response -> a fetch should have been attempted" ); @@ -243,9 +251,9 @@ function add_tests() { add_ocsp_test( "ocsp-stapling-none.example.com", - SEC_ERROR_OCSP_INVALID_SIGNING_CERT, + PRErrorCodeSuccess, [respondWithSHA1OCSP], - "OCSP signing cert was issued with sha1 - should fail" + "signing cert is good (though sha1) - should succeed" ); add_test(function() { diff --git a/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js b/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js index d7ee169011ee..4e65baeb0144 100644 --- a/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js +++ b/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js @@ -54,6 +54,7 @@ Services.prefs.setIntPref("security.OCSP.enabled", 1); // That aspect of OCSP requests is not what we're testing here, so we can just // bump the timeout and hopefully avoid these failures. Services.prefs.setIntPref("security.OCSP.timeoutMilliseconds.soft", 5000); +Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 4); var args = [ ["good", "default-ee", "unused", 0], ["expiredresponse", "default-ee", "unused", 0], @@ -79,8 +80,9 @@ var ocspResponseGoodMustStaple = ocspResponses[5]; var willNotRetry = 1; // but sometimes, since a bad response is in the cache, OCSP fetch will be // attempted for each validation - in practice, for these test certs, this -// means 2 requests because various key sizes are tried. -var willRetry = 2; +// means 6 requests because various hash algorithm and key size combinations +// are tried. +var willRetry = 6; function run_test() { let ocspResponder = new HttpServer(); diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 10c3bf1326c3..918f6548caaf 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -12881,6 +12881,14 @@ "n_values": 4, "description": "Does enforcing a larger minimum RSA key size cause verification failures? 1 = no, 2 = yes, 3 = another error prevented finding a verified chain" }, + "CERT_CHAIN_SHA1_POLICY_STATUS": { + "record_in_processes": ["main", "content"], + "products": ["firefox", "fennec"], + "expires_in_version": "default", + "kind": "enumerated", + "n_values": 6, + "description": "1 = No SHA1 signatures, 2 = SHA1 certificates issued by an imported root, 3 = SHA1 certificates issued before 2016, 4 = SHA1 certificates issued after 2015, 5 = another error prevented successful verification" + }, "WEAVE_START_COUNT": { "record_in_processes": ["main", "content"], "products": ["firefox", "fennec"], diff --git a/toolkit/components/telemetry/histogram-allowlists.json b/toolkit/components/telemetry/histogram-allowlists.json index d732b5d97592..1912e980de89 100644 --- a/toolkit/components/telemetry/histogram-allowlists.json +++ b/toolkit/components/telemetry/histogram-allowlists.json @@ -73,6 +73,7 @@ "CANVAS_2D_USED", "CANVAS_WEBGL_USED", "CERT_CHAIN_KEY_SIZE_STATUS", + "CERT_CHAIN_SHA1_POLICY_STATUS", "CHANGES_OF_DETECTED_LANGUAGE", "CHANGES_OF_TARGET_LANGUAGE", "CHECK_ADDONS_MODIFIED_MS", @@ -414,6 +415,7 @@ "CANVAS_2D_USED", "CANVAS_WEBGL_USED", "CERT_CHAIN_KEY_SIZE_STATUS", + "CERT_CHAIN_SHA1_POLICY_STATUS", "CERT_OCSP_ENABLED", "CERT_OCSP_REQUIRED", "CERT_PINNING_FAILURES_BY_CA", @@ -1058,6 +1060,7 @@ "FX_THUMBNAILS_BG_CAPTURE_PAGE_LOAD_TIME_MS", "REQUESTS_OF_ORIGINAL_CONTENT", "NEWTAB_PAGE_ENHANCED", + "CERT_CHAIN_SHA1_POLICY_STATUS", "FX_THUMBNAILS_BG_CAPTURE_SERVICE_TIME_MS", "PLACES_BACKUPS_TOJSON_MS", "A11Y_ISIMPLEDOM_USAGE_FLAG",