From 5bda1ea0d1827f66b609bcbe3f9bbc8ae5fccd3b Mon Sep 17 00:00:00 2001 From: Monica Chew Date: Tue, 15 Apr 2014 15:35:41 -0700 Subject: [PATCH] Bug 991177: Disallow overrides for SEC_ERROR_CA_CERT_INVALID (r=keeler) --- .../BrowserElementChildPreload.js | 1 - security/manager/ssl/src/NSSErrorsService.cpp | 3 +- .../ssl/src/SSLServerCertVerification.cpp | 2 - .../ssl/tests/unit/test_cert_overrides.js | 53 +++++++++++-------- 4 files changed, 33 insertions(+), 26 deletions(-) diff --git a/dom/browser-element/BrowserElementChildPreload.js b/dom/browser-element/BrowserElementChildPreload.js index 1d004c6e3705..e3f7bfcb384c 100644 --- a/dom/browser-element/BrowserElementChildPreload.js +++ b/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: diff --git a/security/manager/ssl/src/NSSErrorsService.cpp b/security/manager/ssl/src/NSSErrorsService.cpp index 90bd0eb16d58..11e91131b934 100644 --- a/security/manager/ssl/src/NSSErrorsService.cpp +++ b/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; diff --git a/security/manager/ssl/src/SSLServerCertVerification.cpp b/security/manager/ssl/src/SSLServerCertVerification.cpp index c42511b41d4d..4640c94bb5c4 100644 --- a/security/manager/ssl/src/SSLServerCertVerification.cpp +++ b/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: diff --git a/security/manager/ssl/tests/unit/test_cert_overrides.js b/security/manager/ssl/tests/unit/test_cert_overrides.js index b13cc83ab15e..af3b519820b2 100644 --- a/security/manager/ssl/tests/unit/test_cert_overrides.js +++ b/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", - Ci.nsICertOverrideService.ERROR_UNTRUSTED, - getXPCOMStatusFromNSS( - useMozillaPKIX ? SEC_ERROR_UNKNOWN_ISSUER - : SEC_ERROR_CA_CERT_INVALID)); + if (useMozillaPKIX) { + add_cert_override_test("selfsigned.example.com", + Ci.nsICertOverrideService.ERROR_UNTRUSTED, + getXPCOMStatusFromNSS(SEC_ERROR_UNKNOWN_ISSUER)); + } 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", - Ci.nsICertOverrideService.ERROR_UNTRUSTED, - getXPCOMStatusFromNSS( - useMozillaPKIX ? SEC_ERROR_UNKNOWN_ISSUER - : SEC_ERROR_CA_CERT_INVALID)); + if (useMozillaPKIX) { + add_cert_override_test("selfsigned-inadequateEKU.example.com", + Ci.nsICertOverrideService.ERROR_UNTRUSTED, + getXPCOMStatusFromNSS(SEC_ERROR_UNKNOWN_ISSUER)); + } 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", - getXPCOMStatusFromNSS(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); - }); + add_non_overridable_test("inadequatekeyusage.example.com", + SEC_ERROR_INADEQUATE_KEY_USAGE); } else { add_cert_override_test("inadequatekeyusage.example.com", Ci.nsICertOverrideService.ERROR_UNTRUSTED,