diff --git a/security/certverifier/CertVerifier.cpp b/security/certverifier/CertVerifier.cpp index 6d11f9380662..671e5675e419 100644 --- a/security/certverifier/CertVerifier.cpp +++ b/security/certverifier/CertVerifier.cpp @@ -188,8 +188,8 @@ SECStatus chainValidationCallback(void* state, const CERTCertList* certList, } } - const bool enforceTestMode = (callbackState->pinningEnforcementLevel == - CertVerifier::pinningEnforceTestMode); + bool enforceTestMode = (callbackState->pinningEnforcementLevel == + CertVerifier::pinningEnforceTestMode); *chainOK = PublicKeyPinningService:: ChainHasValidPins(certList, callbackState->hostname, callbackState->time, enforceTestMode); diff --git a/security/manager/boot/src/PublicKeyPinningService.h b/security/manager/boot/src/PublicKeyPinningService.h index 4a2eb9f0a64d..569c51b93ee9 100644 --- a/security/manager/boot/src/PublicKeyPinningService.h +++ b/security/manager/boot/src/PublicKeyPinningService.h @@ -14,15 +14,15 @@ class PublicKeyPinningService { public: /** - * Returns true if the given (host, certList) passes pinning checks, - * false otherwise. If the host is pinned, return true if one of the keys in - * the given certificate chain matches the pin set specified by the - * hostname. If the hostname is null or empty evaluate against all the - * possible names for the EE cert (Common Name (CN) plus all DNS Name: - * subject Alt Name entries). The certList's head is the EE cert and the - * tail is the trust anchor. - * Note: if an alt name is a wildcard, it won't necessarily find a pinset - * that would otherwise be valid for it + * Returns true if the given (host, certList) passes pinning checks, + * false otherwise. If the host is pinned, return true if one of the keys in + * the given certificate chain matches the pin set specified by the + * hostname. If the hostname is null or empty evaluate against all the + * possible names for the EE cert (Common Name (CN) plus all DNS Name: + * subject Alt Name entries). The certList's head is the EE cert and the + * tail is the trust anchor. + * Note: if an alt name is a wildcard, it won't necessarily find a pinset + * that would otherwise be valid for it */ static bool ChainHasValidPins(const CERTCertList* certList, const char* hostname, diff --git a/security/manager/ssl/src/nsNSSComponent.cpp b/security/manager/ssl/src/nsNSSComponent.cpp index eab2ce56eb81..4bacbe603982 100644 --- a/security/manager/ssl/src/nsNSSComponent.cpp +++ b/security/manager/ssl/src/nsNSSComponent.cpp @@ -1000,6 +1000,9 @@ void nsNSSComponent::setValidationOptions(bool isInitialSetting, static_cast (Preferences::GetInt("security.cert_pinning.enforcement_level", CertVerifier::pinningDisabled)); + if (pinningEnforcementLevel > CertVerifier::pinningEnforceTestMode) { + pinningEnforcementLevel = CertVerifier::pinningDisabled; + } CertVerifier::ocsp_download_config odc; CertVerifier::ocsp_strict_config osc; diff --git a/security/manager/ssl/tests/unit/test_pinning.js b/security/manager/ssl/tests/unit/test_pinning.js index 31162e2bd1cf..4261d5c4c246 100644 --- a/security/manager/ssl/tests/unit/test_pinning.js +++ b/security/manager/ssl/tests/unit/test_pinning.js @@ -30,7 +30,8 @@ const certdb = Cc["@mozilla.org/security/x509certdb;1"] function test_strict() { // In strict mode, we always evaluate pinning data, regardless of whether the - // issuer is a built-in trust anchor. + // issuer is a built-in trust anchor. We only enforce pins that are not in + // test mode. add_test(function() { Services.prefs.setIntPref("security.cert_pinning.enforcement_level", 2); run_next_test();