From d339ca2730fd1f0a1dfc772bc4402fa3d0a387c2 Mon Sep 17 00:00:00 2001
From: David Keeler <dkeeler@mozilla.com>
Date: Thu, 22 Dec 2016 16:57:20 -0800
Subject: [PATCH] bug 1312827 - make the certificate blocklist only apply to
 TLS server certificates r=jcj,mgoodwin

(Note that content signature verification does not use the unified certificate
verifier and thus will still consult OneCRL.)

MozReview-Commit-ID: 6KvHOngpabT

--HG--
extra : rebase_source : 601f4d8d1c66befb77d0c07a2d84f3f04416f996
---
 .../certverifier/NSSCertDBTrustDomain.cpp     | 40 ++++++++++---------
 .../mochitest/browser/browser_certViewer.js   |  7 +++-
 .../ssl/tests/unit/test_cert_blocklist.js     | 14 +++++++
 3 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/security/certverifier/NSSCertDBTrustDomain.cpp b/security/certverifier/NSSCertDBTrustDomain.cpp
index b485928bf6b5..58557586789d 100644
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -192,25 +192,29 @@ NSSCertDBTrustDomain::GetCertTrust(EndEntityOrCA endEntityOrCA,
     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
diff --git a/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js b/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
index ec777361eb2b..d75f9f207263 100644
--- a/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
+++ b/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
@@ -106,8 +106,11 @@ add_task(function* testRevoked() {
     "VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8="); // hash of the shared key
   let cert = yield readCertificate("revoked.pem", ",,");
   let win = yield displayCertificate(cert);
-  checkError(win,
-             "Could not verify this certificate because it has been revoked.");
+  // As of bug 1312827, OneCRL only applies to TLS web server certificates, so
+  // this certificate will actually verify successfully for every end-entity
+  // usage except TLS web server.
+  checkUsages(win, ["Email Recipient Certificate", "Email Signer Certificate",
+                    "Object Signer", "SSL Client Certificate"]);
   yield BrowserTestUtils.closeWindow(win);
 });
 
diff --git a/security/manager/ssl/tests/unit/test_cert_blocklist.js b/security/manager/ssl/tests/unit/test_cert_blocklist.js
index 4a647274edb6..70edc092315f 100644
--- a/security/manager/ssl/tests/unit/test_cert_blocklist.js
+++ b/security/manager/ssl/tests/unit/test_cert_blocklist.js
@@ -139,6 +139,17 @@ function verify_cert(file, expectedError) {
   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);
@@ -294,14 +305,17 @@ function run_test() {
     // 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";