From 88108c8e9ff2aa10c7200a7df61b525ead123b12 Mon Sep 17 00:00:00 2001
From: Monica Chew <mmc@mozilla.com>
Date: Mon, 19 May 2014 13:24:41 -0700
Subject: [PATCH] Bug 1011269: Forgot to qref to pick up keeler's changes
 (r=keeler)

---
 security/certverifier/CertVerifier.cpp         |  4 ++--
 .../manager/boot/src/PublicKeyPinningService.h | 18 +++++++++---------
 security/manager/ssl/src/nsNSSComponent.cpp    |  3 +++
 .../manager/ssl/tests/unit/test_pinning.js     |  3 ++-
 4 files changed, 16 insertions(+), 12 deletions(-)

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<CertVerifier::pinning_enforcement_config>
         (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();