bug 1312827 - make the certificate blocklist only apply to TLS server certificates r?mgoodwin,jcj
(Note that content signature verification does not use the unified certificate
verifier and thus will still consult OneCRL.)
MozReview-Commit-ID: 6KvHOngpabT
--- 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/mochitest/browser/browser_certViewer.js
+++ b/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
@@ -101,18 +101,21 @@ add_task(function* testRevoked() {
// likely) and subject (less likely).
let certBlocklist = Cc["@mozilla.org/security/certblocklist;1"]
.getService(Ci.nsICertBlocklist);
certBlocklist.revokeCertBySubjectAndPubKey(
"MBIxEDAOBgNVBAMMB3Jldm9rZWQ=", // CN=revoked
"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);
});
add_task(function* testInvalid() {
// This certificate has a keyUsage extension asserting cRLSign and
// keyCertSign, but it doesn't have a basicConstraints extension. This
// shouldn't be valid for any usage. Sadly, we give a pretty lame error
// message in this case.
--- a/security/manager/ssl/tests/unit/test_cert_blocklist.js
+++ b/security/manager/ssl/tests/unit/test_cert_blocklist.js
@@ -134,16 +134,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 || "", {});
@@ -289,24 +300,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);