Bug 991177: Disallow overrides for SEC_ERROR_CA_CERT_INVALID (r=keeler)

dom/browser-element/BrowserElementChildPreload.js

@@ -84,7 +84,6 @@ function getErrorClass(errorCode) {
 
   switch (NSPRCode) {
     case SEC_ERROR_UNKNOWN_ISSUER:
-    case SEC_ERROR_CA_CERT_INVALID:
     case SEC_ERROR_UNTRUSTED_ISSUER:
     case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:
     case SEC_ERROR_UNTRUSTED_CERT:

security/manager/ssl/src/NSSErrorsService.cpp

@@ -95,8 +95,8 @@ NSSErrorsService::GetErrorClass(nsresult aXPCOMErrorCode, uint32_t *aErrorClass)
 
   switch (aNSPRCode)
   {
+    // Overridable errors.
     case SEC_ERROR_UNKNOWN_ISSUER:
-    case SEC_ERROR_CA_CERT_INVALID:
     case SEC_ERROR_UNTRUSTED_ISSUER:
     case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:
     case SEC_ERROR_UNTRUSTED_CERT:
@@ -105,6 +105,7 @@ NSSErrorsService::GetErrorClass(nsresult aXPCOMErrorCode, uint32_t *aErrorClass)
     case SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED:
       *aErrorClass = ERROR_CLASS_BAD_CERT;
       break;
+    // Non-overridable errors.
     default:
       *aErrorClass = ERROR_CLASS_SSL_PROTOCOL;
       break;

security/manager/ssl/src/SSLServerCertVerification.cpp

@@ -299,7 +299,6 @@ MapCertErrorToProbeValue(PRErrorCode errorCode)
   switch (errorCode)
   {
     case SEC_ERROR_UNKNOWN_ISSUER:                     return  2;
-    case SEC_ERROR_CA_CERT_INVALID:                    return  3;
     case SEC_ERROR_UNTRUSTED_ISSUER:                   return  4;
     case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:         return  5;
     case SEC_ERROR_UNTRUSTED_CERT:                     return  6;
@@ -563,7 +562,6 @@ PRErrorCodeToOverrideType(PRErrorCode errorCode)
   switch (errorCode)
   {
     case SEC_ERROR_UNKNOWN_ISSUER:
-    case SEC_ERROR_CA_CERT_INVALID:
     case SEC_ERROR_UNTRUSTED_ISSUER:
     case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:
     case SEC_ERROR_UNTRUSTED_CERT:

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

@@ -34,6 +34,18 @@ function add_cert_override_test(aHost, aExpectedBits, aExpectedError) {
   add_connection_test(aHost, Cr.NS_OK);
 }
 
+function add_non_overridable_test(aHost, aExpectedError) {
+  add_connection_test(
+    aHost, getXPCOMStatusFromNSS(aExpectedError), null,
+    function (securityInfo) {
+      // bug 754369 - no SSLStatus probably means this is a non-overridable
+      // error, which is what we're testing (although it would be best to test
+      // this directly).
+      securityInfo.QueryInterface(Ci.nsISSLStatusProvider);
+      do_check_eq(securityInfo.SSLStatus, null);
+    });
+}
+
 function check_telemetry() {
   let histogram = Cc["@mozilla.org/base/telemetry;1"]
                     .getService(Ci.nsITelemetry)
@@ -41,7 +53,7 @@ function check_telemetry() {
                     .snapshot();
   do_check_eq(histogram.counts[ 0], 0);
   do_check_eq(histogram.counts[ 2], 8 + 1); // SEC_ERROR_UNKNOWN_ISSUER
-  do_check_eq(histogram.counts[ 3], 0 + 2); // SEC_ERROR_CA_CERT_INVALID
+  do_check_eq(histogram.counts[ 3], 0);     // SEC_ERROR_CA_CERT_INVALID
   do_check_eq(histogram.counts[ 4], 0 + 5); // SEC_ERROR_UNTRUSTED_ISSUER
   do_check_eq(histogram.counts[ 5], 0 + 1); // SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE
   do_check_eq(histogram.counts[ 6], 0 + 1); // SEC_ERROR_UNTRUSTED_CERT
@@ -49,7 +61,6 @@ function check_telemetry() {
   do_check_eq(histogram.counts[ 8], 2 + 2); // SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED
   do_check_eq(histogram.counts[ 9], 4 + 4); // SSL_ERROR_BAD_CERT_DOMAIN
   do_check_eq(histogram.counts[10], 5 + 5); // SEC_ERROR_EXPIRED_CERTIFICATE
-
   run_next_test();
 }
 
@@ -93,11 +104,14 @@ function add_simple_tests(useMozillaPKIX) {
   add_cert_override_test("expired.example.com",
                          Ci.nsICertOverrideService.ERROR_TIME,
                          getXPCOMStatusFromNSS(SEC_ERROR_EXPIRED_CERTIFICATE));
-  add_cert_override_test("selfsigned.example.com",
+  if (useMozillaPKIX) {
-                         Ci.nsICertOverrideService.ERROR_UNTRUSTED,
+    add_cert_override_test("selfsigned.example.com",
-                         getXPCOMStatusFromNSS(
+                           Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-                            useMozillaPKIX ? SEC_ERROR_UNKNOWN_ISSUER
+                           getXPCOMStatusFromNSS(SEC_ERROR_UNKNOWN_ISSUER));
-                                           : SEC_ERROR_CA_CERT_INVALID));
+  } else {
+    add_non_overridable_test("selfsigned.example.com",
+                             SEC_ERROR_CA_CERT_INVALID);
+  }
   add_cert_override_test("unknownissuer.example.com",
                          Ci.nsICertOverrideService.ERROR_UNTRUSTED,
                          getXPCOMStatusFromNSS(SEC_ERROR_UNKNOWN_ISSUER));
@@ -123,25 +137,20 @@ function add_simple_tests(useMozillaPKIX) {
   // SEC_ERROR_INADEQUATE_KEY_USAGE must be overridable (although,
   // confusingly, this isn't the main error reported).
   // mozilla::pkix just says this certificate's issuer is unknown.
-  add_cert_override_test("selfsigned-inadequateEKU.example.com",
+  if (useMozillaPKIX) {
-                         Ci.nsICertOverrideService.ERROR_UNTRUSTED,
+    add_cert_override_test("selfsigned-inadequateEKU.example.com",
-                         getXPCOMStatusFromNSS(
+                           Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-                            useMozillaPKIX ? SEC_ERROR_UNKNOWN_ISSUER
+                           getXPCOMStatusFromNSS(SEC_ERROR_UNKNOWN_ISSUER));
-                                           : SEC_ERROR_CA_CERT_INVALID));
+  } else {
+    add_non_overridable_test("selfsigned-inadequateEKU.example.com",
+                             SEC_ERROR_CA_CERT_INVALID);
+  }
 
   // SEC_ERROR_INADEQUATE_KEY_USAGE is overridable in general for
   // classic verification, but not for mozilla::pkix verification.
   if (useMozillaPKIX) {
-    add_connection_test("inadequatekeyusage.example.com",
+    add_non_overridable_test("inadequatekeyusage.example.com",
-                        getXPCOMStatusFromNSS(SEC_ERROR_INADEQUATE_KEY_USAGE),
+                             SEC_ERROR_INADEQUATE_KEY_USAGE);
-                        null,
-                        function (securityInfo) {
-                          // bug 754369 - no SSLStatus probably means this is
-                          // a non-overridable error, which is what we're testing
-                          // (although it would be best to test this directly).
-                          securityInfo.QueryInterface(Ci.nsISSLStatusProvider);
-                          do_check_eq(securityInfo.SSLStatus, null);
-                        });
   } else {
     add_cert_override_test("inadequatekeyusage.example.com",
                            Ci.nsICertOverrideService.ERROR_UNTRUSTED,