bug 867473 - (4/4) remove nsIX509Cert.issuer and getChain r?jcj,fkiefer
These functions cause main-thread certificate verifications, which is bad for
performance. In general, nsIX509CertDB.asyncVerifyCertAtTime should be used
instead.
MozReview-Commit-ID: 9nkUDmyFY0k
--- a/security/manager/ssl/nsIX509Cert.idl
+++ b/security/manager/ssl/nsIX509Cert.idl
@@ -132,22 +132,16 @@ interface nsIX509Cert : nsISupports {
/**
* The issuer subject's organizational unit.
*/
[must_use]
readonly attribute AString issuerOrganizationUnit;
/**
- * The certificate used by the issuer to sign this certificate.
- */
- [must_use]
- readonly attribute nsIX509Cert issuer;
-
- /**
* This certificate's validity period.
*/
readonly attribute nsIX509CertValidity validity;
/**
* A unique identifier of this certificate within the local storage.
*/
[must_use]
@@ -184,26 +178,16 @@ interface nsIX509Cert : nsISupports {
/**
* Constants for specifying the chain mode when exporting a certificate
*/
const unsigned long CMS_CHAIN_MODE_CertOnly = 1;
const unsigned long CMS_CHAIN_MODE_CertChain = 2;
const unsigned long CMS_CHAIN_MODE_CertChainWithRoot = 3;
/**
- * Obtain a list of certificates that contains this certificate
- * and the issuing certificates of all involved issuers,
- * up to the root issuer.
- *
- * @return The chain of certifficates including the issuers.
- */
- [must_use]
- nsIArray getChain();
-
- /**
* A comma separated list of localized strings representing the contents of
* the certificate's key usage extension, if present. The empty string if the
* certificate doesn't have the key usage extension, or has an empty extension.
*/
[must_use]
readonly attribute AString keyUsages;
/**
--- a/security/manager/ssl/nsNSSCertificate.cpp
+++ b/security/manager/ssl/nsNSSCertificate.cpp
@@ -517,123 +517,28 @@ nsNSSCertificate::GetIssuerOrganizationU
if (organizationUnit) {
aOrganizationUnit = NS_ConvertUTF8toUTF16(organizationUnit.get());
}
}
return NS_OK;
}
NS_IMETHODIMP
-nsNSSCertificate::GetIssuer(nsIX509Cert** aIssuer)
-{
- NS_ENSURE_ARG(aIssuer);
- *aIssuer = nullptr;
-
- nsCOMPtr<nsIArray> chain;
- nsresult rv;
- rv = GetChain(getter_AddRefs(chain));
- NS_ENSURE_SUCCESS(rv, rv);
- uint32_t length;
- if (!chain || NS_FAILED(chain->GetLength(&length)) || length == 0) {
- return NS_ERROR_UNEXPECTED;
- }
- if (length == 1) { // No known issuer
- return NS_OK;
- }
- nsCOMPtr<nsIX509Cert> cert;
- chain->QueryElementAt(1, NS_GET_IID(nsIX509Cert), getter_AddRefs(cert));
- if (!cert) {
- return NS_ERROR_UNEXPECTED;
- }
- cert.forget(aIssuer);
- return NS_OK;
-}
-
-NS_IMETHODIMP
nsNSSCertificate::GetOrganizationalUnit(nsAString& aOrganizationalUnit)
{
aOrganizationalUnit.Truncate();
if (mCert) {
UniquePORTString orgunit(CERT_GetOrgUnitName(&mCert->subject));
if (orgunit) {
aOrganizationalUnit = NS_ConvertUTF8toUTF16(orgunit.get());
}
}
return NS_OK;
}
-static nsresult
-UniqueCERTCertListToMutableArray(/*in*/ UniqueCERTCertList& nssChain,
- /*out*/ nsIArray** x509CertArray)
-{
- if (!x509CertArray) {
- return NS_ERROR_INVALID_ARG;
- }
-
- nsCOMPtr<nsIMutableArray> array = nsArrayBase::Create();
- if (!array) {
- return NS_ERROR_FAILURE;
- }
-
- CERTCertListNode* node;
- for (node = CERT_LIST_HEAD(nssChain.get());
- !CERT_LIST_END(node, nssChain.get());
- node = CERT_LIST_NEXT(node)) {
- nsCOMPtr<nsIX509Cert> cert = nsNSSCertificate::Create(node->cert);
- nsresult rv = array->AppendElement(cert);
- if (NS_FAILED(rv)) {
- return rv;
- }
- }
-
- array.forget(x509CertArray);
- return NS_OK;
-}
-
-NS_IMETHODIMP
-nsNSSCertificate::GetChain(nsIArray** _rvChain)
-{
- NS_ENSURE_ARG(_rvChain);
-
- mozilla::pkix::Time now(mozilla::pkix::Now());
-
- RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
- NS_ENSURE_TRUE(certVerifier, NS_ERROR_UNEXPECTED);
-
- UniqueCERTCertList nssChain;
- // We want to test all usages supported by the certificate verifier, but we
- // start with TLS server because most of the time Firefox users care about
- // server certs.
- const int usagesToTest[] = { certificateUsageSSLServer,
- certificateUsageSSLClient,
- certificateUsageSSLCA,
- certificateUsageEmailSigner,
- certificateUsageEmailRecipient };
- for (auto usage : usagesToTest) {
- if (certVerifier->VerifyCert(mCert.get(), usage, now,
- nullptr, /*XXX fixme*/
- nullptr, /*hostname*/
- nssChain,
- CertVerifier::FLAG_LOCAL_ONLY)
- == mozilla::pkix::Success) {
- return UniqueCERTCertListToMutableArray(nssChain, _rvChain);
- }
- }
-
- // There is no verified path for the chain, however we still want to
- // present to the user as much of a possible chain as possible, in the case
- // where there was a problem with the cert or the issuers.
- nssChain = UniqueCERTCertList(
- CERT_GetCertChainFromCert(mCert.get(), PR_Now(), certUsageSSLClient));
- if (!nssChain) {
- return NS_ERROR_FAILURE;
- }
- return UniqueCERTCertListToMutableArray(nssChain, _rvChain);
-}
-
NS_IMETHODIMP
nsNSSCertificate::GetSubjectName(nsAString& _subjectName)
{
_subjectName.Truncate();
if (mCert->subjectName) {
_subjectName = NS_ConvertUTF8toUTF16(mCert->subjectName);
}
return NS_OK;
--- a/security/manager/ssl/tests/unit/moz.build
+++ b/security/manager/ssl/tests/unit/moz.build
@@ -19,17 +19,16 @@ TEST_DIRS += [
'test_cert_sha1',
'test_cert_signatures',
'test_cert_trust',
'test_cert_version',
'test_certDB_import',
'test_content_signing',
'test_ct',
'test_ev_certs',
- 'test_getchain',
'test_intermediate_basic_usage_constraints',
'test_keysize',
'test_keysize_ev',
'test_missing_intermediate',
'test_name_constraints',
'test_ocsp_fetch_method',
'test_ocsp_url',
'test_onecrl',
deleted file mode 100644
--- a/security/manager/ssl/tests/unit/test_getchain.js
+++ /dev/null
@@ -1,85 +0,0 @@
-// -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
-// This Source Code Form is subject to the terms of the Mozilla Public
-// License, v. 2.0. If a copy of the MPL was not distributed with this
-// file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-"use strict";
-
-do_get_profile(); // must be called before getting nsIX509CertDB
-const certdb = Cc["@mozilla.org/security/x509certdb;1"]
- .getService(Ci.nsIX509CertDB);
-// This is the list of certificates needed for the test.
-var certList = [
- "ee",
- "ca-1",
- "ca-2",
-];
-
-// Since all the ca's are identical expect for the serial number
-// I have to grab them by enumerating all the certs and then finding
-// the ones that I am interested in.
-function get_ca_array() {
- let ret_array = [];
- let allCerts = certdb.getCerts();
- let enumerator = allCerts.getEnumerator();
- while (enumerator.hasMoreElements()) {
- let cert = enumerator.getNext().QueryInterface(Ci.nsIX509Cert);
- if (cert.commonName == "ca") {
- ret_array[parseInt(cert.serialNumber, 16)] = cert;
- }
- }
- return ret_array;
-}
-
-
-function check_matching_issuer_and_getchain(expected_issuer_serial, cert) {
- const nsIX509Cert = Ci.nsIX509Cert;
-
- equal(expected_issuer_serial, cert.issuer.serialNumber,
- "Expected and actual issuer serial numbers should match");
- let chain = cert.getChain();
- let issuer_via_getchain = chain.queryElementAt(1, nsIX509Cert);
- // The issuer returned by cert.issuer or cert.getchain should be consistent.
- equal(cert.issuer.serialNumber, issuer_via_getchain.serialNumber,
- "Serial numbers via cert.issuer and via getChain() should match");
-}
-
-function check_getchain(ee_cert, ssl_ca, email_ca) {
- // A certificate should first build a chain/issuer to
- // a SSL trust domain, then an EMAIL trust domain and then
- // an object signer trust domain.
-
- const nsIX509Cert = Ci.nsIX509Cert;
- certdb.setCertTrust(ssl_ca, nsIX509Cert.CA_CERT,
- Ci.nsIX509CertDB.TRUSTED_SSL);
- certdb.setCertTrust(email_ca, nsIX509Cert.CA_CERT,
- Ci.nsIX509CertDB.TRUSTED_EMAIL);
- check_matching_issuer_and_getchain(ssl_ca.serialNumber, ee_cert);
- certdb.setCertTrust(ssl_ca, nsIX509Cert.CA_CERT, 0);
- check_matching_issuer_and_getchain(email_ca.serialNumber, ee_cert);
- certdb.setCertTrust(email_ca, nsIX509Cert.CA_CERT, 0);
- // Do a final test on the case of no trust. The results must
- // be consistent (the actual value is non-deterministic).
- check_matching_issuer_and_getchain(ee_cert.issuer.serialNumber, ee_cert);
-}
-
-function run_test() {
- clearOCSPCache();
- clearSessionCache();
-
- let ee_cert = null;
- for (let cert of certList) {
- let result = addCertFromFile(certdb, `test_getchain/${cert}.pem`, ",,");
- if (cert == "ee") {
- ee_cert = result;
- }
- }
-
- notEqual(ee_cert, null, "EE cert should have successfully loaded");
-
- let ca = get_ca_array();
-
- check_getchain(ee_cert, ca[1], ca[2]);
- // Swap ca certs to deal alternate trust settings.
- check_getchain(ee_cert, ca[2], ca[1]);
-}
deleted file mode 100644
--- a/security/manager/ssl/tests/unit/test_getchain/ca-1.pem
+++ /dev/null
@@ -1,17 +0,0 @@
------BEGIN CERTIFICATE-----
-MIICtjCCAZ6gAwIBAgIBATANBgkqhkiG9w0BAQsFADANMQswCQYDVQQDDAJjYTAi
-GA8yMDE2MTEyNzAwMDAwMFoYDzIwMTkwMjA1MDAwMDAwWjANMQswCQYDVQQDDAJj
-YTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALqIUahEjhbWQf1utogG
-NhA9PBPZ6uQ1SrTs9WhXbCR7wcclqODYH72xnAabbhqG8mvir1p1a2pkcQh6pVqn
-RYf3HNUknAJ+zUP8HmnQOCApk6sgw0nk27lMwmtsDu0Vgg/xfq1pGrHTAjqLKkHu
-p3DgDw2N/WYLK7AkkqR9uYhheZCxV5A90jvF4LhIH6g304hD7ycW2FW3ZlqqfgKQ
-Lzp7EIAGJMwcbJetlmFbt+KWEsB1MaMMkd20yvf8rR0l0wnvuRcOp2jhs3svIm9p
-47SKlWEd7ibWJZ2rkQhONsscJAQsvxaLL+Xxj5kXMbiz/kkj+nJRxDHVA6zaGAo1
-7Y0CAwEAAaMdMBswDAYDVR0TBAUwAwEB/zALBgNVHQ8EBAMCAQYwDQYJKoZIhvcN
-AQELBQADggEBAFKB/sAo+kzOcOJD28nchLpfgq9pzqZoKPoGZWmYiQSQVWwlLk3z
-ZbIm8Qwioz7fCbjuH/Ilo4RDwt1t96q2tvi34rTs0DgkZSKX20JxQpUVzudUHYwc
-0SsZxwYU8/DySqOl/c0YyLXPdPr10H4fKSu7rdbTet8xPL5nD5JibwGi7LVtFLjv
-KDm/7fNg1kb5ntwPdVotDssBMvGgDzn9PPFScdL7ulTAzMlUFFPu/EfADGCx8XcY
-+1UJkDSOwmyOjLbjiN4fZdAhJ1M26O/3zjdJt4hq951Uncho2OmmaXPnfLqA9mbM
-ElgU2kCE9whztnNcNrxhvduj2wMTdSaAl0k=
------END CERTIFICATE-----
deleted file mode 100644
--- a/security/manager/ssl/tests/unit/test_getchain/ca-1.pem.certspec
+++ /dev/null
@@ -1,5 +0,0 @@
-issuer:ca
-subject:ca
-extension:basicConstraints:cA,
-extension:keyUsage:cRLSign,keyCertSign
-serialNumber:1
deleted file mode 100644
--- a/security/manager/ssl/tests/unit/test_getchain/ca-2.pem
+++ /dev/null
@@ -1,17 +0,0 @@
------BEGIN CERTIFICATE-----
-MIICtjCCAZ6gAwIBAgIBAjANBgkqhkiG9w0BAQsFADANMQswCQYDVQQDDAJjYTAi
-GA8yMDE2MTEyNzAwMDAwMFoYDzIwMTkwMjA1MDAwMDAwWjANMQswCQYDVQQDDAJj
-YTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALqIUahEjhbWQf1utogG
-NhA9PBPZ6uQ1SrTs9WhXbCR7wcclqODYH72xnAabbhqG8mvir1p1a2pkcQh6pVqn
-RYf3HNUknAJ+zUP8HmnQOCApk6sgw0nk27lMwmtsDu0Vgg/xfq1pGrHTAjqLKkHu
-p3DgDw2N/WYLK7AkkqR9uYhheZCxV5A90jvF4LhIH6g304hD7ycW2FW3ZlqqfgKQ
-Lzp7EIAGJMwcbJetlmFbt+KWEsB1MaMMkd20yvf8rR0l0wnvuRcOp2jhs3svIm9p
-47SKlWEd7ibWJZ2rkQhONsscJAQsvxaLL+Xxj5kXMbiz/kkj+nJRxDHVA6zaGAo1
-7Y0CAwEAAaMdMBswDAYDVR0TBAUwAwEB/zALBgNVHQ8EBAMCAQYwDQYJKoZIhvcN
-AQELBQADggEBAA5GrT0S6pZYYLJv05JDzNy4ax/0aBrkm6DQlwadbD1ZjJbivT7S
-fyDiIxNub0eMsp5QRKI36PV3KToN26khNoIgRualq9oSoZ5kbH1477nZXhCVIZk2
-sfC/M1uR8t3ziHPXGDpKMszmrCqLJPaEMpWDWklHMz99SCldi41sz+dg6BcdTRys
-ayWSZDGyUOM1EgS2MN+VZ6ucuXqvSnpn9l1e97XaSs/z3kPF8CpVFmL50xbBIHYl
-4fa9uiswzL0quczA3vNMkihHF4/pPG2Vc4chdNtTigrLbcjA0X42/vomLzNlkcQC
-7GKaYdFiclzUzLKwux8cTa1dJtHDyFkVLqg=
------END CERTIFICATE-----
deleted file mode 100644
--- a/security/manager/ssl/tests/unit/test_getchain/ca-2.pem.certspec
+++ /dev/null
@@ -1,5 +0,0 @@
-issuer:ca
-subject:ca
-extension:basicConstraints:cA,
-extension:keyUsage:cRLSign,keyCertSign
-serialNumber:2
deleted file mode 100644
--- a/security/manager/ssl/tests/unit/test_getchain/ee.pem
+++ /dev/null
@@ -1,17 +0,0 @@
------BEGIN CERTIFICATE-----
-MIICqjCCAZKgAwIBAgIUKQ7jtHr5Bp6UboOGF1ZvwHp9v3AwDQYJKoZIhvcNAQEL
-BQAwDTELMAkGA1UEAwwCY2EwIhgPMjAxNjExMjcwMDAwMDBaGA8yMDE5MDIwNTAw
-MDAwMFowDTELMAkGA1UEAwwCZWUwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK
-AoIBAQC6iFGoRI4W1kH9braIBjYQPTwT2erkNUq07PVoV2wke8HHJajg2B+9sZwG
-m24ahvJr4q9adWtqZHEIeqVap0WH9xzVJJwCfs1D/B5p0DggKZOrIMNJ5Nu5TMJr
-bA7tFYIP8X6taRqx0wI6iypB7qdw4A8Njf1mCyuwJJKkfbmIYXmQsVeQPdI7xeC4
-SB+oN9OIQ+8nFthVt2Zaqn4CkC86exCABiTMHGyXrZZhW7filhLAdTGjDJHdtMr3
-/K0dJdMJ77kXDqdo4bN7LyJvaeO0ipVhHe4m1iWdq5EITjbLHCQELL8Wiy/l8Y+Z
-FzG4s/5JI/pyUcQx1QOs2hgKNe2NAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAF+s
-c8Q9YfGiE9uAc4+OArgF5b9Li9euJIBIdyicdhk2FIpGSUX/bds2QXNjokvkG/2M
-i0TW2jlPkFb4GRXznqo/zFzHNpVArYjbNlfqBBy5EUmbjtzJJhqn/z6Je2Gl92PU
-yWKPSHmK72jZx1GdbIk8fAp/mRwKqjVZ7tbDR7uXAXZppw7L9XMPTMycqSQ3v1ou
-Kj0lopCPA80XbFkOdhKNx0oNapZCswohzXr46AnfBKZUTkHskhv/LS3BkLR+CFQN
-qm6wiEhzeZwqLlpAJHlcPpDaB5L8zcgFv48fVw+XMX3kPVbrPpRsx9mLJT8kWP29
-cDQySy9cWzC85GvHia0=
------END CERTIFICATE-----
deleted file mode 100644
--- a/security/manager/ssl/tests/unit/test_getchain/ee.pem.certspec
+++ /dev/null
@@ -1,2 +0,0 @@
-issuer:ca
-subject:ee
deleted file mode 100644
--- a/security/manager/ssl/tests/unit/test_getchain/moz.build
+++ /dev/null
@@ -1,15 +0,0 @@
-# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
-# vim: set filetype=python:
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-# Temporarily disabled. See bug 1256495.
-#test_certificates = (
-# 'ca-1.pem',
-# 'ca-2.pem',
-# 'ee.pem',
-#)
-#
-#for test_certificate in test_certificates:
-# GeneratedTestCertificate(test_certificate)
--- a/security/manager/ssl/tests/unit/xpcshell.ini
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
@@ -15,17 +15,16 @@ support-files =
test_cert_signatures/**
test_cert_trust/**
test_cert_version/**
test_certDB_import/**
test_certviewer_invalid_oids/**
test_content_signing/**
test_ct/**
test_ev_certs/**
- test_getchain/**
test_intermediate_basic_usage_constraints/**
test_keysize/**
test_keysize_ev/**
test_missing_intermediate/**
test_name_constraints/**
test_ocsp_fetch_method/**
test_ocsp_url/**
test_onecrl/**
@@ -84,17 +83,16 @@ skip-if = toolkit == 'android'
[test_der.js]
[test_enterprise_roots.js]
skip-if = os != 'win' # tests a Windows-specific feature
[test_ev_certs.js]
tags = blocklist
run-sequentially = hardcoded ports
[test_forget_about_site_security_headers.js]
skip-if = toolkit == 'android'
-[test_getchain.js]
[test_hash_algorithms.js]
[test_hash_algorithms_wrap.js]
# bug 1124289 - run_test_in_child violates the sandbox on android
skip-if = toolkit == 'android'
[test_hmac.js]
[test_intermediate_basic_usage_constraints.js]
[test_imminent_distrust.js]
run-sequentially = hardcoded ports