Bug 1611381 - attempt to detect more self-signed certificates r=jscha…

…nck a=RyanVM

When two self-signed certificates have the same issuer and subject but
different keys, mozilla::pkix can return a bad signature error or an inadequate
key usage error (assuming no other suitable issuer is found). The real issue is
that the certificates are self-signed, so this change attempts to detect that
and return a more appropriate error.

Differential Revision: https://phabricator.services.mozilla.com/D195176

security/certverifier/CertVerifier.cpp

@@ -854,10 +854,13 @@ Result CertVerifier::VerifySSLServerCert(
     if (peerBackCert.Init() != Success) {
       return rv;
     }
-    if (rv == Result::ERROR_UNKNOWN_ISSUER &&
+    if ((rv == Result::ERROR_UNKNOWN_ISSUER ||
+         rv == Result::ERROR_BAD_SIGNATURE ||
+         rv == Result::ERROR_INADEQUATE_KEY_USAGE) &&
         CertIsSelfSigned(peerBackCert, pinarg)) {
-      // In this case we didn't find any issuer for the certificate and the
-      // certificate is self-signed.
+      // In this case we didn't find any issuer for the certificate, or we did
+      // find other certificates with the same subject but different keys, and
+      // the certificate is self-signed.
       return Result::ERROR_SELF_SIGNED_CERT;
     }
     if (rv == Result::ERROR_UNKNOWN_ISSUER) {

security/manager/ssl/tests/unit/test_self_signed_certs.js

@@ -23,7 +23,7 @@
 //   -q secp256r1 -x -k ec -z <(date +%s) -1 -2 -n cert$num; sleep 2;
 // done
 
-add_task(async function run_test_no_overlong_path_building() {
+add_task(async function test_no_overlong_path_building() {
   let profile = do_get_profile();
   const CERT_DB_NAME = "cert9.db";
   let srcCertDBFile = do_get_file(`test_self_signed_certs/${CERT_DB_NAME}`);
@@ -67,3 +67,43 @@ add_task(async function run_test_no_overlong_path_building() {
   let secondsElapsed = (timeAfter - timeBefore) / 1000;
   ok(secondsElapsed < 120, "verifications shouldn't take too long");
 });
+
+add_task(async function test_no_bad_signature() {
+  // If there are two self-signed CA certificates with the same subject and
+  // issuer but different keys, where one is trusted, test that using the other
+  // one as a server certificate doesn't result in a non-overridable "bad
+  // signature" error but rather a "self-signed cert" error.
+  let selfSignedCert = constructCertFromFile("test_self_signed_certs/ca1.pem");
+  let certDB = Cc["@mozilla.org/security/x509certdb;1"].getService(
+    Ci.nsIX509CertDB
+  );
+  addCertFromFile(certDB, "test_self_signed_certs/ca2.pem", "CTu,,");
+  await checkCertErrorGeneric(
+    certDB,
+    selfSignedCert,
+    MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT,
+    certificateUsageSSLServer,
+    false,
+    "example.com"
+  );
+});
+
+add_task(async function test_no_inadequate_key_usage() {
+  // If there are two different non-CA, self-signed certificates with the same
+  // subject and issuer but different keys, test that using one of them as a
+  // server certificate doesn't result in a non-overridable "inadequate key
+  // usage" error but rather a "self-signed cert" error.
+  let selfSignedCert = constructCertFromFile("test_self_signed_certs/ee1.pem");
+  let certDB = Cc["@mozilla.org/security/x509certdb;1"].getService(
+    Ci.nsIX509CertDB
+  );
+  addCertFromFile(certDB, "test_self_signed_certs/ee2.pem", ",,");
+  await checkCertErrorGeneric(
+    certDB,
+    selfSignedCert,
+    MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT,
+    certificateUsageSSLServer,
+    false,
+    "example.com"
+  );
+});

security/manager/ssl/tests/unit/test_self_signed_certs/ca1.pem

@@ -0,0 +1,18 @@
+-----BEGIN CERTIFICATE-----
+MIICzjCCAbagAwIBAgIBATANBgkqhkiG9w0BAQsFADAZMRcwFQYDVQQDDA5TZWxm
+LVNpZ25lZCBDQTAiGA8yMDIxMTEyNzAwMDAwMFoYDzIwMjQwMjA1MDAwMDAwWjAZ
+MRcwFQYDVQQDDA5TZWxmLVNpZ25lZCBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEP
+ADCCAQoCggEBALqIUahEjhbWQf1utogGNhA9PBPZ6uQ1SrTs9WhXbCR7wcclqODY
+H72xnAabbhqG8mvir1p1a2pkcQh6pVqnRYf3HNUknAJ+zUP8HmnQOCApk6sgw0nk
+27lMwmtsDu0Vgg/xfq1pGrHTAjqLKkHup3DgDw2N/WYLK7AkkqR9uYhheZCxV5A9
+0jvF4LhIH6g304hD7ycW2FW3ZlqqfgKQLzp7EIAGJMwcbJetlmFbt+KWEsB1MaMM
+kd20yvf8rR0l0wnvuRcOp2jhs3svIm9p47SKlWEd7ibWJZ2rkQhONsscJAQsvxaL
+L+Xxj5kXMbiz/kkj+nJRxDHVA6zaGAo17Y0CAwEAAaMdMBswDAYDVR0TBAUwAwEB
+/zALBgNVHQ8EBAMCAQYwDQYJKoZIhvcNAQELBQADggEBAGYbweuY4SItzbgVeI2G
+EvJKGbHGlPdB/pYA8j02AIajr8zY79b/mcuPmGmBJHXmmQc9ipV3QnVO3IYjKZwo
+HEAR7duowBrB36B3MXjelaruYwno2qw0+duiz0OLpo0DOstcMDIs2UiSoNTZ1wue
++tmrNlnWWC7V7hTLZZ4Z7v7bi55G0OviCNOdW+EBi9VoQBGJLxZFcW9uE//mBCNx
+nKANNdb4ube7ulIfox/pQ1+yE+ddY/MfQHd67oX4wPNRfHAePT4vvJkUWXiXap21
+gdkAUbbwz0r1mDzvvKmVauT7N5+m2Zj6b3wCBvtITuD4zi/og61o6xyQ52DTYu4M
+hPk=
+-----END CERTIFICATE-----

security/manager/ssl/tests/unit/test_self_signed_certs/ca1.pem.certspec

@@ -0,0 +1,5 @@
+issuer:Self-Signed CA
+subject:Self-Signed CA
+serialNumber:1
+extension:basicConstraints:cA,
+extension:keyUsage:cRLSign,keyCertSign

security/manager/ssl/tests/unit/test_self_signed_certs/ca2.pem

@@ -0,0 +1,18 @@
+-----BEGIN CERTIFICATE-----
+MIICzjCCAbagAwIBAgIBAjANBgkqhkiG9w0BAQsFADAZMRcwFQYDVQQDDA5TZWxm
+LVNpZ25lZCBDQTAiGA8yMDIxMTEyNzAwMDAwMFoYDzIwMjQwMjA1MDAwMDAwWjAZ
+MRcwFQYDVQQDDA5TZWxmLVNpZ25lZCBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEP
+ADCCAQoCggEBAMF1xlJmCZ93CCpnkfG4dsN/XOU4sGxKzSKxy9RvplraKt1ByMJJ
+isSjs8H2FIf0G2mJQb2ApRw8EgJExYSkxEgzBeUTjAEGzwi+moYnYLrmoujzbyPF
+2YMTud+vN4NF2s5R1Nbc0qbLPMcG680wcOyYzOQKpZHXKVp/ccW+ZmkdKy3+yElE
+WQvFo+pJ/ZOx11NAXxdzdpmVhmYlR5ftQmkIiAgRQiBpmIpD/uSM5oeB3SK2ppzS
+g3UTH5MrEozihvp9JRwGKtJ+8Bbxh83VToMrNbiTD3S6kKqLx2FnJCqx/W1iFA0Y
+xMC4xo/DdIRXMkrX3obmVS8dHhkdcSFo07sCAwEAAaMdMBswDAYDVR0TBAUwAwEB
+/zALBgNVHQ8EBAMCAQYwDQYJKoZIhvcNAQELBQADggEBAL5sqCV7HbsLEO3Az1n4
+ulmzaJHdyrae6l46Fws849G65O2suhBS55ENGFLmpof12WOXSvYxRf6swswYnm6S
+oTcjg/A5gVfmRYl/wMR0qBZQV1stLmOsnMqiVqB5BSB1szAv/ItAo2DLtH+6FDaW
+1xl0bcJsRxhtKqK9ST9Jr+HLsGZOGlA1r5++PZXaa+OA+dARcjDmNnJUFZYR0/vq
+gMc2MHl4/4XvrPZvfbCV3Alzy7UDq05/9EodR+DlACs/LWExHi8nTW0ZtwkfsbHH
+cfRw7xa3KInaCVYVs3jXzsFNNxUYKCmvm0cojdf3S9Va9/4z555XIK/a30kCFBbf
+hJc=
+-----END CERTIFICATE-----

security/manager/ssl/tests/unit/test_self_signed_certs/ca2.pem.certspec

@@ -0,0 +1,7 @@
+issuer:Self-Signed CA
+subject:Self-Signed CA
+serialNumber:2
+issuerKey:alternate
+subjectKey:alternate
+extension:basicConstraints:cA,
+extension:keyUsage:cRLSign,keyCertSign

security/manager/ssl/tests/unit/test_self_signed_certs/ee1.pem

@@ -0,0 +1,17 @@
+-----BEGIN CERTIFICATE-----
+MIICszCCAZugAwIBAgIBATANBgkqhkiG9w0BAQsFADAbMRkwFwYDVQQDDBBTZWxm
+LVNpZ25lZCBDZXJ0MCIYDzIwMjExMTI3MDAwMDAwWhgPMjAyNDAyMDUwMDAwMDBa
+MBsxGTAXBgNVBAMMEFNlbGYtU2lnbmVkIENlcnQwggEiMA0GCSqGSIb3DQEBAQUA
+A4IBDwAwggEKAoIBAQC6iFGoRI4W1kH9braIBjYQPTwT2erkNUq07PVoV2wke8HH
+Jajg2B+9sZwGm24ahvJr4q9adWtqZHEIeqVap0WH9xzVJJwCfs1D/B5p0DggKZOr
+IMNJ5Nu5TMJrbA7tFYIP8X6taRqx0wI6iypB7qdw4A8Njf1mCyuwJJKkfbmIYXmQ
+sVeQPdI7xeC4SB+oN9OIQ+8nFthVt2Zaqn4CkC86exCABiTMHGyXrZZhW7filhLA
+dTGjDJHdtMr3/K0dJdMJ77kXDqdo4bN7LyJvaeO0ipVhHe4m1iWdq5EITjbLHCQE
+LL8Wiy/l8Y+ZFzG4s/5JI/pyUcQx1QOs2hgKNe2NAgMBAAEwDQYJKoZIhvcNAQEL
+BQADggEBADHcrQaX6Vw4BP8YDSNQ9dACjWliS6NMIDlG9ZIck2NvnmiDY6yOiENX
+G/gOAI4k+Eq6l83ZtE3EEB0ljREzhoUkxuP8w8mKZUflvAxtYh5t1sLmhNnC7YCN
+KG2SspKJC2/7nKsxG6s2jvnCqkreeNNH7BdJ2AHwRPEu2UudXrooDh9mgPac0Woa
+wx0nT9Kqun7JYMXxro+/837+qvaAhWlFYKglq3PaNMQShBkjRARXpCMV0N5TWNFp
+f7MXUoKC/+R+4rRKgln4KTXtkk7Ggn13oBrGPhF3fvVZlP2ySAkTW8Ii58MA24N9
+TNM0MP661fQXHZGVg7EAKcqa2bxKIAY=
+-----END CERTIFICATE-----

security/manager/ssl/tests/unit/test_self_signed_certs/ee1.pem.certspec

@@ -0,0 +1,3 @@
+issuer:Self-Signed Cert
+subject:Self-Signed Cert
+serialNumber:1

security/manager/ssl/tests/unit/test_self_signed_certs/ee2.pem

@@ -0,0 +1,17 @@
+-----BEGIN CERTIFICATE-----
+MIICszCCAZugAwIBAgIBAjANBgkqhkiG9w0BAQsFADAbMRkwFwYDVQQDDBBTZWxm
+LVNpZ25lZCBDZXJ0MCIYDzIwMjExMTI3MDAwMDAwWhgPMjAyNDAyMDUwMDAwMDBa
+MBsxGTAXBgNVBAMMEFNlbGYtU2lnbmVkIENlcnQwggEiMA0GCSqGSIb3DQEBAQUA
+A4IBDwAwggEKAoIBAQDBdcZSZgmfdwgqZ5HxuHbDf1zlOLBsSs0iscvUb6Za2ird
+QcjCSYrEo7PB9hSH9BtpiUG9gKUcPBICRMWEpMRIMwXlE4wBBs8IvpqGJ2C65qLo
+828jxdmDE7nfrzeDRdrOUdTW3NKmyzzHBuvNMHDsmMzkCqWR1ylaf3HFvmZpHSst
+/shJRFkLxaPqSf2TsddTQF8Xc3aZlYZmJUeX7UJpCIgIEUIgaZiKQ/7kjOaHgd0i
+tqac0oN1Ex+TKxKM4ob6fSUcBirSfvAW8YfN1U6DKzW4kw90upCqi8dhZyQqsf1t
+YhQNGMTAuMaPw3SEVzJK196G5lUvHR4ZHXEhaNO7AgMBAAEwDQYJKoZIhvcNAQEL
+BQADggEBAFqvHU0CJMIydbm8TKpLB5eI/gHE6dGxHKDyZlfTeGCGsILl/ejp46VU
+Cs+aDmLxyMuNmnsNNoHBa8oTV4jyVRR/zWkibVKM4Fzz1YJclOMVrcxJqhDk1Gcj
+og+N29pxG8BwCdNbKLFma6xjIGK7j6mc6kpb+dbURiEI7tPkEW2jGk77xkGM93co
+drRMCwsmTt2QTsQDVmpNnd5avfQgguBYy/5/kyvAo6vDQE1Sj7cBg0+YPASzjFR4
+oIfyo+neTITxDfd0QLBSSFa+dIZUGKVF+XKe4FCKLlfVmO/MI7llJilR8RiNcZCh
+eX/8OfjNpptiQBydCgKycHcXSVkvWRg=
+-----END CERTIFICATE-----

security/manager/ssl/tests/unit/test_self_signed_certs/ee2.pem.certspec

@@ -0,0 +1,5 @@
+issuer:Self-Signed Cert
+subject:Self-Signed Cert
+serialNumber:2
+issuerKey:alternate
+subjectKey:alternate