diff --git a/netwerk/base/security-prefs.js b/netwerk/base/security-prefs.js index 5550e49d518e..0153ffde2138 100644 --- a/netwerk/base/security-prefs.js +++ b/netwerk/base/security-prefs.js @@ -55,8 +55,8 @@ pref("security.OCSP.GET.enabled", false); pref("security.pki.cert_short_lifetime_in_days", 10); // NB: Changes to this pref affect CERT_CHAIN_SHA1_POLICY_STATUS telemetry. // See the comment in CertVerifier.cpp. -// 4 = allow SHA-1 for certificates issued before 2016 or by an imported root. -pref("security.pki.sha1_enforcement_level", 4); +// 3 = allow SHA-1 for certificates issued before 2016 or by an imported root. +pref("security.pki.sha1_enforcement_level", 3); // security.pki.name_matching_mode controls how the platform matches hostnames // to name information in TLS certificates. The possible values are: diff --git a/security/certverifier/CertVerifier.cpp b/security/certverifier/CertVerifier.cpp index 2565a08d3a28..fab419650820 100644 --- a/security/certverifier/CertVerifier.cpp +++ b/security/certverifier/CertVerifier.cpp @@ -152,9 +152,9 @@ CertVerifier::SHA1ModeMoreRestrictiveThanGivenMode(SHA1Mode mode) switch (mSHA1Mode) { case SHA1Mode::Forbidden: return mode != SHA1Mode::Forbidden; + case SHA1Mode::Before2016: + return mode != SHA1Mode::Forbidden && mode != SHA1Mode::Before2016; case SHA1Mode::ImportedRoot: - return mode != SHA1Mode::Forbidden && mode != SHA1Mode::ImportedRoot; - case SHA1Mode::ImportedRootOrBefore2016: return mode == SHA1Mode::Allowed; case SHA1Mode::Allowed: return false; @@ -283,15 +283,15 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, // results of setting the default policy to a particular configuration. SHA1Mode sha1ModeConfigurations[] = { SHA1Mode::Forbidden, + SHA1Mode::Before2016, SHA1Mode::ImportedRoot, - SHA1Mode::ImportedRootOrBefore2016, SHA1Mode::Allowed, }; SHA1ModeResult sha1ModeResults[] = { SHA1ModeResult::SucceededWithoutSHA1, + SHA1ModeResult::SucceededWithSHA1Before2016, SHA1ModeResult::SucceededWithImportedRoot, - SHA1ModeResult::SucceededWithImportedRootOrSHA1Before2016, SHA1ModeResult::SucceededWithSHA1, }; @@ -346,6 +346,15 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, KeyPurposeId::id_kp_serverAuth, evPolicy, stapledOCSPResponse, ocspStaplingStatus); + // If we succeeded with the SHA1Mode of only allowing imported roots to + // issue SHA1 certificates after 2015, if the chain we built doesn't + // terminate with an imported root, we must reject it. (This only works + // because we try SHA1 configurations in order of decreasing + // strictness.) + // Note that if there existed a certificate chain with a built-in root + // that had SHA1 certificates issued before 2016, it would have already + // been accepted. If such a chain had SHA1 certificates issued after + // 2015, it will only be accepted in the SHA1Mode::Allowed case. if (rv == Success && sha1ModeConfigurations[i] == SHA1Mode::ImportedRoot) { bool isBuiltInRoot = false; @@ -429,6 +438,15 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, CertPolicyId::anyPolicy, stapledOCSPResponse, ocspStaplingStatus); + // If we succeeded with the SHA1Mode of only allowing imported roots + // to issue SHA1 certificates after 2015, if the chain we built + // doesn't terminate with an imported root, we must reject it. (This + // only works because we try SHA1 configurations in order of + // decreasing strictness.) + // Note that if there existed a certificate chain with a built-in root + // that had SHA1 certificates issued before 2016, it would have + // already been accepted. If such a chain had SHA1 certificates issued + // after 2015, it will only be accepted in the SHA1Mode::Allowed case. if (rv == Success && sha1ModeConfigurations[j] == SHA1Mode::ImportedRoot) { bool isBuiltInRoot = false; @@ -458,13 +476,10 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, 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 Allowed or Forbidden, so - // we only collect for ImportedRoot or ImportedRootOrBefore2016. - if (sha1ModeResult && - (mSHA1Mode == SHA1Mode::ImportedRoot || - mSHA1Mode == SHA1Mode::ImportedRootOrBefore2016)) { + // Only collect CERT_CHAIN_SHA1_POLICY_STATUS telemetry indicating a + // failure when mSHA1Mode is the default. + // NB: When we change the default, we have to change this. + if (sha1ModeResult && mSHA1Mode == SHA1Mode::ImportedRoot) { *sha1ModeResult = SHA1ModeResult::Failed; } @@ -477,7 +492,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, mCertShortLifetimeInDays, pinningDisabled, MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff, - SHA1Mode::Allowed, mNetscapeStepUpPolicy, + mSHA1Mode, mNetscapeStepUpPolicy, builtChain, nullptr, nullptr); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeCA, KeyUsage::keyCertSign, diff --git a/security/certverifier/CertVerifier.h b/security/certverifier/CertVerifier.h index 392cf6facb56..d9cc7b949384 100644 --- a/security/certverifier/CertVerifier.h +++ b/security/certverifier/CertVerifier.h @@ -27,8 +27,8 @@ enum class KeySizeStatus { enum class SHA1ModeResult { NeverChecked = 0, SucceededWithoutSHA1 = 1, - SucceededWithImportedRoot = 2, - SucceededWithImportedRootOrSHA1Before2016 = 3, + SucceededWithSHA1Before2016 = 2, + SucceededWithImportedRoot = 3, SucceededWithSHA1 = 4, Failed = 5, }; @@ -110,12 +110,8 @@ public: 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, + Before2016 = 2, ImportedRoot = 3, - ImportedRootOrBefore2016 = 4, }; enum OcspDownloadConfig { @@ -149,8 +145,8 @@ private: // 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.) + // Forbidden. Next is Before2016, then ImportedRoot, then Allowed. + // (A mode is never more restrictive than itself.) bool SHA1ModeMoreRestrictiveThanGivenMode(SHA1Mode mode); }; diff --git a/security/certverifier/NSSCertDBTrustDomain.cpp b/security/certverifier/NSSCertDBTrustDomain.cpp index 1f11fe170db9..afea52d6528f 100644 --- a/security/certverifier/NSSCertDBTrustDomain.cpp +++ b/security/certverifier/NSSCertDBTrustDomain.cpp @@ -835,7 +835,7 @@ NSSCertDBTrustDomain::CheckSignatureDigestAlgorithm(DigestAlgorithm aAlg, case CertVerifier::SHA1Mode::Forbidden: MOZ_LOG(gCertVerifierLog, LogLevel::Debug, ("SHA-1 certificate rejected")); return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED; - case CertVerifier::SHA1Mode::ImportedRootOrBefore2016: + case CertVerifier::SHA1Mode::Before2016: if (JANUARY_FIRST_2016 <= notBefore) { MOZ_LOG(gCertVerifierLog, LogLevel::Debug, ("Post-2015 SHA-1 certificate rejected")); return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED; diff --git a/security/manager/ssl/nsNSSComponent.cpp b/security/manager/ssl/nsNSSComponent.cpp index 27aa24b63783..28256ac72312 100644 --- a/security/manager/ssl/nsNSSComponent.cpp +++ b/security/manager/ssl/nsNSSComponent.cpp @@ -1516,20 +1516,14 @@ void nsNSSComponent::setValidationOptions(bool isInitialSetting, switch (sha1Mode) { case CertVerifier::SHA1Mode::Allowed: case CertVerifier::SHA1Mode::Forbidden: - case CertVerifier::SHA1Mode::UsedToBeBefore2016ButNowIsForbidden: + case CertVerifier::SHA1Mode::Before2016: 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; - } - BRNameMatchingPolicy::Mode nameMatchingMode = static_cast (Preferences::GetInt("security.pki.name_matching_mode", diff --git a/security/manager/ssl/tests/unit/test_cert_sha1.js b/security/manager/ssl/tests/unit/test_cert_sha1.js index f0d7c570a6a6..0b8e636aa5ab 100644 --- a/security/manager/ssl/tests/unit/test_cert_sha1.js +++ b/security/manager/ssl/tests/unit/test_cert_sha1.js @@ -31,6 +31,11 @@ function checkEndEntity(cert, expectedResult) { certificateUsageSSLServer, VALIDATION_TIME); } +function checkIntermediate(cert, expectedResult) { + checkCertErrorGenericAtTime(certdb, cert, expectedResult, + certificateUsageSSLCA, VALIDATION_TIME); +} + function run_test() { loadCertWithTrust("ca", "CTu,,"); loadCertWithTrust("int-pre", ",,"); @@ -42,101 +47,81 @@ function run_test() { // =========================== // root // | - // +--- pre-2016 + // +--- pre-2016 <--- (a) // | | - // | +----- pre-2016 <--- (a) - // | +----- post-2016 <--- (b) + // | +----- pre-2016 <--- (b) + // | +----- post-2016 <--- (c) // | - // +--- post-2016 + // +--- post-2016 <--- (d) // | - // +----- post-2016 <--- (c) + // +----- post-2016 <--- (e) // // 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) + // a b c d e + // Allowed=0 Acc Acc Acc Acc Acc + // Forbidden=1 Rej Rej Rej Rej Rej + // Before2016=2 Acc Acc Rej Rej Rej // - // The pref setting of ImportedRoot accepts usage of SHA-1 only for - // certificates issued by non-built-in roots. By default, the testing + // The pref setting of ImportedRoot (3) accepts usage of SHA-1 for + // certificates valid before 2016 issued by built-in roots or SHA-1 for + // certificates issued any time 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). + // now have the ability to treat a given non-built-in root as built-in. We + // test both of these situations below. // SHA-1 allowed Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 0); + checkIntermediate(certFromFile("int-pre"), PRErrorCodeSuccess); checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); checkEndEntity(certFromFile("ee-post_int-pre"), PRErrorCodeSuccess); + checkIntermediate(certFromFile("int-post"), PRErrorCodeSuccess); checkEndEntity(certFromFile("ee-post_int-post"), PRErrorCodeSuccess); // SHA-1 forbidden Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 1); + checkIntermediate(certFromFile("int-pre"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); checkEndEntity(certFromFile("ee-pre_int-pre"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); checkEndEntity(certFromFile("ee-post_int-pre"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); + checkIntermediate(certFromFile("int-post"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); checkEndEntity(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) + // SHA-1 allowed only before 2016 Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 2); - checkEndEntity(certFromFile("ee-pre_int-pre"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); + checkIntermediate(certFromFile("int-pre"), PRErrorCodeSuccess); + checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); checkEndEntity(certFromFile("ee-post_int-pre"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); + checkIntermediate(certFromFile("int-post"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); 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). + // SHA-1 allowed only before 2016 or 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); - checkEndEntity(certFromFile("ee-pre_int-pre"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); - checkEndEntity(certFromFile("ee-post_int-pre"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); - 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. - checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); - checkEndEntity(certFromFile("ee-post_int-pre"), PRErrorCodeSuccess); - 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); + checkIntermediate(certFromFile("int-pre"), PRErrorCodeSuccess); checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); checkEndEntity(certFromFile("ee-post_int-pre"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); + // This should fail but it doesn't, because the implementation makes no + // effort to enforce that when verifying a certificate for the capability + // of issuing TLS server auth certificates (i.e. the + // "certificateUsageSSLCA" usage), if SHA-1 was necessary, then the root of + // trust is an imported certificate. We don't really care, though, because + // the platform doesn't actually make trust decisions in this way and the + // ability to even verify a certificate for this purpose is intended to go + // away in bug 1257362. + // checkIntermediate(certFromFile("int-post"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); 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. + // SHA-1 still allowed only before 2016 or when issued by an imported root. // Now test with the test root considered a non-built-in. + checkIntermediate(certFromFile("int-pre"), PRErrorCodeSuccess); checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); checkEndEntity(certFromFile("ee-post_int-pre"), PRErrorCodeSuccess); + checkIntermediate(certFromFile("int-post"), PRErrorCodeSuccess); 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 e8d8f52e6d5e..133d388138cf 100644 --- a/security/manager/ssl/tests/unit/test_ocsp_caching.js +++ b/security/manager/ssl/tests/unit/test_ocsp_caching.js @@ -62,7 +62,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); + Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 3); add_tls_server_setup("OCSPStaplingServer", "ocsp_certs"); let ocspResponder = new HttpServer(); 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 b49be85e702b..1b0f544691dc 100644 --- a/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js +++ b/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js @@ -31,7 +31,7 @@ function add_ocsp_test(aHost, aExpectedResult, aOCSPResponseToServe, 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); +Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 3); var args = [["good", "default-ee", "unused"], ["expiredresponse", "default-ee", "unused"], ["oldvalidperiod", "default-ee", "unused"], diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 3935f55249d6..866cf03e354d 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -8176,7 +8176,7 @@ "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" + "description": "1 = No SHA1 signatures, 2 = SHA1 certificates issued before 2016, 3 = SHA1 certificates issued by an imported root, 4 = SHA1 certificates issued after 2015, 5 = another error prevented successful verification" }, "WEAVE_CONFIGURED": { "expires_in_version": "default",