Bug 1280224 - Initial values for the content signature root pref are ignored. r=keeler draft
authorMark Goodwin <mgoodwin@mozilla.com>
Tue, 21 Jun 2016 15:24:52 +0100
changeset 380299 8ef354057c1d1c085d1aa638d05d93da544714be
parent 380291 0ffa18cfc8b4e9970275ab6dd94686792a5c8ae1
child 523698 d6b52f0700f19a43e677b683d403f5610463374b
push id21191
push usermgoodwin@mozilla.com
push dateTue, 21 Jun 2016 14:26:07 +0000
reviewerskeeler
bugs1280224
milestone50.0a1
Bug 1280224 - Initial values for the content signature root pref are ignored. r=keeler MozReview-Commit-ID: 9y8wsVcz0hz
security/manager/ssl/ContentSignatureVerifier.cpp
security/manager/ssl/nsNSSComponent.cpp
security/manager/ssl/tests/unit/test_content_signing.js
--- a/security/manager/ssl/ContentSignatureVerifier.cpp
+++ b/security/manager/ssl/ContentSignatureVerifier.cpp
@@ -177,17 +177,21 @@ ContentSignatureVerifier::CreateContext(
   CSTrustDomain trustDomain(certCertList);
   result = BuildCertChain(trustDomain, certDER, Now(),
                           EndEntityOrCA::MustBeEndEntity,
                           KeyUsage::noParticularKeyUsageRequired,
                           KeyPurposeId::id_kp_codeSigning,
                           CertPolicyId::anyPolicy,
                           nullptr/*stapledOCSPResponse*/);
   if (result != Success) {
-    // the chain is bad
+    // if there was a library error, return an appropriate error
+    if (IsFatalError(result)) {
+      return NS_ERROR_FAILURE;
+    }
+    // otherwise, assume the signature was invalid
     CSVerifier_LOG(("CSVerifier: The supplied chain is bad\n"));
     return NS_ERROR_INVALID_SIGNATURE;
   }
 
   // Check the SAN
   Input hostnameInput;
 
   result = hostnameInput.Init(uint8_t_ptr_cast(aName.BeginReading()),
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -1743,16 +1743,20 @@ nsNSSComponent::InitializeNSS()
   if (nocertdb || init_rv != SECSuccess) {
     init_rv = NSS_NoDB_Init(nullptr);
   }
   if (init_rv != SECSuccess) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Error, ("could not initialize NSS - panicking\n"));
     return NS_ERROR_NOT_AVAILABLE;
   }
 
+  // ensure we have an initial value for the content signer root
+  mContentSigningRootHash =
+    Preferences::GetString("security.content.signature.root_hash");
+
   mNSSInitialized = true;
 
   PK11_SetPasswordFunc(PK11PasswordPrompt);
 
   SharedSSLState::GlobalInit();
 
   // Register an observer so we can inform NSS when these prefs change
   Preferences::AddStrongObserver(this, "security.");
@@ -2176,26 +2180,29 @@ NS_IMETHODIMP
 nsNSSComponent::IsCertContentSigningRoot(CERTCertificate* cert, bool& result)
 {
   MutexAutoLock lock(mutex);
   MOZ_ASSERT(mNSSInitialized);
 
   result = false;
 
   if (mContentSigningRootHash.IsEmpty()) {
-    return NS_OK;
+    MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("mContentSigningRootHash is empty"));
+    return NS_ERROR_FAILURE;
   }
 
   RefPtr<nsNSSCertificate> nsc = nsNSSCertificate::Create(cert);
   if (!nsc) {
+    MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("creating nsNSSCertificate failed"));
     return NS_ERROR_FAILURE;
   }
   nsAutoString certHash;
   nsresult rv = nsc->GetSha256Fingerprint(certHash);
   if (NS_FAILED(rv)) {
+    MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("getting cert fingerprint failed"));
     return rv;
   }
 
   result = mContentSigningRootHash.Equals(certHash);
   return NS_OK;
 }
 
 SharedCertVerifier::~SharedCertVerifier() { }
--- a/security/manager/ssl/tests/unit/test_content_signing.js
+++ b/security/manager/ssl/tests/unit/test_content_signing.js
@@ -39,36 +39,41 @@ function run_test() {
   const GOOD_SIGNATURE = "p384ecdsa=" +
       readFile(do_get_file(TEST_DATA_DIR + 'test.txt.signature'))
       .trim();
 
   const BAD_SIGNATURE = "p384ecdsa=WqRXFQ7tnlVufpg7A-ZavXvWd2Zln0o4woHBy26C2r" +
                         "UWM4GJke4pE8ecHiXoi-7KnZXty6Pe3s4o3yAIyKDP9jUC52Ek1G" +
                         "q25j_X703nP5rk5gM1qz5Fe-qCWakPPl6L";
 
-  setRoot(TEST_DATA_DIR + "content_signing_root.pem");
-
   let remoteNewTabChain = loadChain(TEST_DATA_DIR + "content_signing",
                                     ["remote_newtab_ee", "int", "root"]);
 
   let oneCRLChain = loadChain(TEST_DATA_DIR + "content_signing",
                               ["onecrl_ee", "int", "root"]);
 
   let oneCRLBadKeyChain = loadChain(TEST_DATA_DIR + "content_signing",
                                     ["onecrl_wrong_key_ee", "int", "root"]);
 
   let oneCRLRSAKeyChain = loadChain(TEST_DATA_DIR + "content_signing",
                                     ["onecrl_RSA_ee", "int", "root"]);
 
   let noSANChain = loadChain(TEST_DATA_DIR + "content_signing",
                              ["onecrl_no_SAN_ee", "int", "root"]);
 
-  // Check good signatures from good certificates with the correct SAN
+  // Check signature verification works without error before the root is set
   let chain1 = oneCRLChain.join("\n");
   let verifier = getSignatureVerifier();
+  ok(!verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, chain1, ONECRL_NAME),
+     "Before the root is set, signatures should fail to verify but not throw.");
+
+  setRoot(TEST_DATA_DIR + "content_signing_root.pem");
+
+  // Check good signatures from good certificates with the correct SAN
+  verifier = getSignatureVerifier();
   ok(verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, chain1, ONECRL_NAME),
      "A OneCRL signature should verify with the OneCRL chain");
   let chain2 = remoteNewTabChain.join("\n");
   verifier = getSignatureVerifier();
   ok(verifier.verifyContentSignature(DATA, GOOD_SIGNATURE, chain2,
                                      ABOUT_NEWTAB_NAME),
      "A newtab signature should verify with the newtab chain");