Bug 1312827 - make the certificate blocklist only apply to TLS server certificates r=jcj,mgoodwin a=rkent THUNDERBIRD_45_VERBRANCH
authorDavid Keeler <dkeeler@mozilla.com>
Thu, 22 Dec 2016 16:57:20 -0800
branchTHUNDERBIRD_45_VERBRANCH
changeset 312615 5c894801eb1e1269e4e5d940402d0389c46f6daf
parent 312614 8e1a21d2b531d9734b02f7239d5c782caad14356
child 312616 83ee87b301bd952e4fef2de8189062ea0b19276a
push id397
push userkent@caspia.com
push dateMon, 23 Jan 2017 20:59:41 +0000
treeherdermozilla-esr45@5c894801eb1e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjcj, mgoodwin, rkent
bugs1312827
milestone45.7.0
Bug 1312827 - make the certificate blocklist only apply to TLS server certificates r=jcj,mgoodwin a=rkent (rkent: note browser_certViewer.js changes were not incorporated in THUNDEBIRD_45_VERBRANCH due to massive post-esr45 changes, but these tests do not run in Thunderbird anyway) (Note that content signature verification does not use the unified certificate verifier and thus will still consult OneCRL.) MozReview-Commit-ID: 6KvHOngpabT
security/certverifier/NSSCertDBTrustDomain.cpp
security/manager/ssl/tests/unit/test_cert_blocklist.js
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -187,35 +187,39 @@ NSSCertDBTrustDomain::GetCertTrust(EndEn
     return MapPRErrorCodeToResult(PR_GetError());
   }
 
   // Check the certificate against the OneCRL cert blocklist
   if (!mCertBlocklist) {
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
-  bool isCertRevoked;
-  nsresult nsrv = mCertBlocklist->IsCertRevoked(
-                    candidateCert->derIssuer.data,
-                    candidateCert->derIssuer.len,
-                    candidateCert->serialNumber.data,
-                    candidateCert->serialNumber.len,
-                    candidateCert->derSubject.data,
-                    candidateCert->derSubject.len,
-                    candidateCert->derPublicKey.data,
-                    candidateCert->derPublicKey.len,
-                    &isCertRevoked);
-  if (NS_FAILED(nsrv)) {
-    return Result::FATAL_ERROR_LIBRARY_FAILURE;
-  }
+  // The certificate blocklist currently only applies to TLS server
+  // certificates.
+  if (mCertDBTrustType == trustSSL) {
+    bool isCertRevoked;
+    nsresult nsrv = mCertBlocklist->IsCertRevoked(
+                      candidateCert->derIssuer.data,
+                      candidateCert->derIssuer.len,
+                      candidateCert->serialNumber.data,
+                      candidateCert->serialNumber.len,
+                      candidateCert->derSubject.data,
+                      candidateCert->derSubject.len,
+                      candidateCert->derPublicKey.data,
+                      candidateCert->derPublicKey.len,
+                      &isCertRevoked);
+    if (NS_FAILED(nsrv)) {
+      return Result::FATAL_ERROR_LIBRARY_FAILURE;
+    }
 
-  if (isCertRevoked) {
-    MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
-           ("NSSCertDBTrustDomain: certificate is in blocklist"));
-    return Result::ERROR_REVOKED_CERTIFICATE;
+    if (isCertRevoked) {
+      MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
+             ("NSSCertDBTrustDomain: certificate is in blocklist"));
+      return Result::ERROR_REVOKED_CERTIFICATE;
+    }
   }
 
   // XXX: CERT_GetCertTrust seems to be abusing SECStatus as a boolean, where
   // SECSuccess means that there is a trust record and SECFailure means there
   // is not a trust record. I looked at NSS's internal uses of
   // CERT_GetCertTrust, and all that code uses the result as a boolean meaning
   // "We have a trust record."
   CERTCertTrust trust;
--- a/security/manager/ssl/tests/unit/test_cert_blocklist.js
+++ b/security/manager/ssl/tests/unit/test_cert_blocklist.js
@@ -170,16 +170,27 @@ var converter = Cc["@mozilla.org/intl/sc
                   .createInstance(Ci.nsIScriptableUnicodeConverter);
 converter.charset = "UTF-8";
 
 function verify_cert(file, expectedError) {
   let ee = constructCertFromFile(file);
   checkCertErrorGeneric(certDB, ee, expectedError, certificateUsageSSLServer);
 }
 
+// The certificate blocklist currently only applies to TLS server certificates.
+function verify_non_tls_usage_succeeds(file) {
+  let ee = constructCertFromFile(file);
+  checkCertErrorGeneric(certDB, ee, PRErrorCodeSuccess,
+                        certificateUsageSSLClient);
+  checkCertErrorGeneric(certDB, ee, PRErrorCodeSuccess,
+                        certificateUsageEmailSigner);
+  checkCertErrorGeneric(certDB, ee, PRErrorCodeSuccess,
+                        certificateUsageEmailRecipient);
+}
+
 function load_cert(cert, trust) {
   let file = "bad_certs/" + cert + ".pem";
   addCertFromFile(certDB, file, trust);
 }
 
 function test_is_revoked(certList, issuerString, serialString, subjectString,
                          pubKeyString) {
   let issuer = converter.convertToByteArray(issuerString ? issuerString : '',
@@ -328,24 +339,27 @@ function run_test() {
 
     // Check the blocklist entry has been persisted properly to the backing
     // file
     check_revocations_txt_contents(expected);
 
     // Check the blocklisted intermediate now causes a failure
     let file = "test_onecrl/test-int-ee.pem";
     verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
+    verify_non_tls_usage_succeeds(file);
 
     // Check the ee with the blocklisted root also causes a failure
     file = "bad_certs/other-issuer-ee.pem";
     verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
+    verify_non_tls_usage_succeeds(file);
 
     // Check the ee blocked by subject / pubKey causes a failure
     file = "test_onecrl/same-issuer-ee.pem";
     verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
+    verify_non_tls_usage_succeeds(file);
 
     // Check a non-blocklisted chain still validates OK
     file = "bad_certs/default-ee.pem";
     verify_cert(file, PRErrorCodeSuccess);
 
     // Check a bad cert is still bad (unknown issuer)
     file = "bad_certs/unknownissuer.pem";
     verify_cert(file, SEC_ERROR_UNKNOWN_ISSUER);