diff --git a/netwerk/base/security-prefs.js b/netwerk/base/security-prefs.js index 0153ffde2138..5550e49d518e 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. -// 3 = allow SHA-1 for certificates issued before 2016 or by an imported root. -pref("security.pki.sha1_enforcement_level", 3); +// 4 = allow SHA-1 for certificates issued before 2016 or by an imported root. +pref("security.pki.sha1_enforcement_level", 4); // 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 fab419650820..2565a08d3a28 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,15 +346,6 @@ 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; @@ -438,15 +429,6 @@ 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; @@ -476,10 +458,13 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, if (keySizeStatus) { *keySizeStatus = KeySizeStatus::AlreadyBad; } - // 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) { + // 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)) { *sha1ModeResult = SHA1ModeResult::Failed; } @@ -492,7 +477,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, mCertShortLifetimeInDays, pinningDisabled, MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff, - mSHA1Mode, mNetscapeStepUpPolicy, + SHA1Mode::Allowed, 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 d9cc7b949384..392cf6facb56 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, - SucceededWithSHA1Before2016 = 2, - SucceededWithImportedRoot = 3, + SucceededWithImportedRoot = 2, + SucceededWithImportedRootOrSHA1Before2016 = 3, SucceededWithSHA1 = 4, Failed = 5, }; @@ -110,8 +110,12 @@ public: enum class SHA1Mode { Allowed = 0, Forbidden = 1, - Before2016 = 2, + // 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 { @@ -145,8 +149,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 Before2016, then ImportedRoot, then Allowed. - // (A mode is never more restrictive than itself.) + // Forbidden. Next is ImportedRoot, then ImportedRootOrBefore2016, 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 afea52d6528f..1f11fe170db9 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::Before2016: + 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; diff --git a/security/manager/ssl/nsNSSComponent.cpp b/security/manager/ssl/nsNSSComponent.cpp index 28256ac72312..27aa24b63783 100644 --- a/security/manager/ssl/nsNSSComponent.cpp +++ b/security/manager/ssl/nsNSSComponent.cpp @@ -1516,14 +1516,20 @@ void nsNSSComponent::setValidationOptions(bool isInitialSetting, switch (sha1Mode) { case CertVerifier::SHA1Mode::Allowed: case CertVerifier::SHA1Mode::Forbidden: - case CertVerifier::SHA1Mode::Before2016: + 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; + } + 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 0b8e636aa5ab..f0d7c570a6a6 100644 --- a/security/manager/ssl/tests/unit/test_cert_sha1.js +++ b/security/manager/ssl/tests/unit/test_cert_sha1.js @@ -31,11 +31,6 @@ 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", ",,"); @@ -47,81 +42,101 @@ function run_test() { // =========================== // root // | - // +--- pre-2016 <--- (a) + // +--- pre-2016 // | | - // | +----- pre-2016 <--- (b) - // | +----- post-2016 <--- (c) + // | +----- pre-2016 <--- (a) + // | +----- post-2016 <--- (b) // | - // +--- post-2016 <--- (d) + // +--- post-2016 // | - // +----- post-2016 <--- (e) + // +----- post-2016 <--- (c) // // Expected outcomes (accept / reject): // - // 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 + // 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 (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 + // 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 - // now have the ability to treat a given non-built-in root as built-in. We - // test both of these situations below. + // 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); - 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 allowed only before 2016 + // SHA-1 forbidden (test the case where the pref has been set to 2) Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 2); - checkIntermediate(certFromFile("int-pre"), PRErrorCodeSuccess); - checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); + 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 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). + // 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); - checkIntermediate(certFromFile("int-pre"), PRErrorCodeSuccess); - checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); + checkEndEntity(certFromFile("ee-pre_int-pre"), SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED); 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 allowed only before 2016 or when issued by an imported root. + // 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); + checkEndEntity(certFromFile("ee-pre_int-pre"), PRErrorCodeSuccess); + 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 only allowed 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 133d388138cf..e8d8f52e6d5e 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", 3); + Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 4); 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 1b0f544691dc..b49be85e702b 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", 3); +Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 4); 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 866cf03e354d..3935f55249d6 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 before 2016, 3 = SHA1 certificates issued by an imported root, 4 = SHA1 certificates issued after 2015, 5 = another error prevented successful verification" + "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_CONFIGURED": { "expires_in_version": "default",