Bug 1781104 - remove unnecessary bits parameter from nsICertOverrideService r=djackson,necko-reviewers,geckoview-reviewers,extension-reviewers,kershaw,calu

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

browser/base/content/test/about/browser.ini

@@ -12,7 +12,6 @@ support-files =
 [browser_aboutCertError_clockSkew.js]
 [browser_aboutCertError_exception.js]
 [browser_aboutCertError_mitm.js]
-[browser_aboutCertError_multiple_errors.js]
 [browser_aboutCertError_noSubjectAltName.js]
 [browser_aboutCertError_offlineSupport.js]
 [browser_aboutCertError_telemetry.js]

browser/base/content/test/about/browser_aboutCertError_exception.js

@@ -91,7 +91,6 @@ add_task(async function checkPermanentExceptionPref() {
       -1,
       {},
       cert,
-      {},
       isTemporary
     );
     ok(hasException, "Has stored an exception for the page.");

browser/base/content/test/about/browser_aboutCertError_multiple_errors.js

@@ -1,153 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-const EXPIRED_CERT = "https://expired.example.com/";
-const BAD_CERT = "https://mismatch.badcertdomain.example.com/";
-
-const kErrors = [
-  "MOZILLA_PKIX_ERROR_MITM_DETECTED",
-  "SEC_ERROR_UNKNOWN_ISSUER",
-  "SEC_ERROR_CA_CERT_INVALID",
-  "SEC_ERROR_UNTRUSTED_ISSUER",
-  "SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED",
-  "SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE",
-  "MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT",
-  "MOZILLA_PKIX_ERROR_ADDITIONAL_POLICY_CONSTRAINT_FAILED",
-];
-
-/**
- * This is a quasi-unit test style test to check what happens
- * when we encounter certificates with multiple problems.
- *
- * We should consistently display the most urgent message first.
- */
-add_task(async function test_expired_bad_cert() {
-  let tab = await openErrorPage(EXPIRED_CERT);
-  const kExpiryLabel = "cert-error-expired-now";
-  let browser = tab.linkedBrowser;
-  await SpecialPowers.spawn(browser, [kExpiryLabel, kErrors], async function(
-    knownExpiryLabel,
-    errorCodes
-  ) {
-    await ContentTaskUtils.waitForCondition(
-      () =>
-        !!content.document.querySelectorAll(`#badCertTechnicalInfo label`)
-          .length
-    );
-    // First check that for this real cert, which is simply expired and nothing else,
-    // we show the expiry info:
-    let rightLabel = content.document.querySelector(
-      `#badCertTechnicalInfo label[data-l10n-id="${knownExpiryLabel}"]`
-    );
-    ok(rightLabel, "Expected a label with the right contents.");
-    info(content.document.querySelector("#badCertTechnicalInfo").innerHTML);
-
-    const kExpiredErrors = errorCodes.map(err => {
-      return Cu.cloneInto(
-        {
-          errorCodeString: err,
-          isUntrusted: true,
-          isNotValidAtThisTime: true,
-          validNotBefore: 0,
-          validNotAfter: 86400000,
-        },
-        content.window
-      );
-    });
-    for (let err of kExpiredErrors) {
-      // Test hack: invoke the content-privileged helper method with the fake cert info.
-      await Cu.waiveXrays(content.window).setTechnicalDetailsOnCertError(err);
-      let allLabels = content.document.querySelectorAll(
-        "#badCertTechnicalInfo label"
-      );
-      ok(
-        allLabels.length,
-        "There should be an advanced technical description for " +
-          err.errorCodeString
-      );
-      for (let label of allLabels) {
-        let id = label.getAttribute("data-l10n-id");
-        ok(
-          id,
-          `Label for ${err.errorCodeString} should have data-l10n-id (was: ${id}).`
-        );
-        isnot(
-          id,
-          knownExpiryLabel,
-          `Label for ${err.errorCodeString} should not be about expiry.`
-        );
-      }
-    }
-  });
-  await BrowserTestUtils.removeTab(tab);
-});
-
-/**
- * The same as above, but now for alt-svc domain mismatch certs.
- */
-add_task(async function test_alt_svc_bad_cert() {
-  let tab = await openErrorPage(BAD_CERT);
-  const kErrKnownPrefix = "cert-error-domain-mismatch";
-  const kErrKnownAlt = "cert-error-domain-mismatch-single-nolink";
-  let browser = tab.linkedBrowser;
-  await SpecialPowers.spawn(
-    browser,
-    [kErrKnownAlt, kErrKnownPrefix, kErrors],
-    async function(knownAlt, knownAltPrefix, errorCodes) {
-      await ContentTaskUtils.waitForCondition(
-        () =>
-          !!content.document.querySelectorAll(`#badCertTechnicalInfo label`)
-            .length
-      );
-      // First check that for this real cert, which is simply for the wrong domain and nothing else,
-      // we show the alt-svc info:
-      let rightLabel = content.document.querySelector(
-        `#badCertTechnicalInfo label[data-l10n-id="${knownAlt}"]`
-      );
-      ok(rightLabel, "Expected a label with the right contents.");
-      info(content.document.querySelector("#badCertTechnicalInfo").innerHTML);
-
-      const kAltSvcErrors = errorCodes.map(err => {
-        return Cu.cloneInto(
-          {
-            errorCodeString: err,
-            isUntrusted: true,
-            isDomainMismatch: true,
-          },
-          content.window
-        );
-      });
-      for (let err of kAltSvcErrors) {
-        // Test hack: invoke the content-privileged helper method with the fake cert info.
-        await Cu.waiveXrays(content.window).setTechnicalDetailsOnCertError(err);
-        let allLabels = content.document.querySelectorAll(
-          "#badCertTechnicalInfo label"
-        );
-        ok(
-          allLabels.length,
-          "There should be an advanced technical description for " +
-            err.errorCodeString
-        );
-        for (let label of allLabels) {
-          let id = label.getAttribute("data-l10n-id");
-          ok(
-            id,
-            `Label for ${err.errorCodeString} should have data-l10n-id (was: ${id}).`
-          );
-          isnot(
-            id,
-            knownAlt,
-            `Label for ${err.errorCodeString} should not mention other domains.`
-          );
-          ok(
-            !id.startsWith(knownAltPrefix),
-            `Label shouldn't start with ${knownAltPrefix}`
-          );
-        }
-      }
-    }
-  );
-  await BrowserTestUtils.removeTab(tab);
-});

browser/base/content/test/siteIdentity/browser_navigation_failures.js

@@ -129,15 +129,11 @@ add_task(async function() {
   let cert = getTestServerCertificate();
   // Start a server and trust its certificate.
   let server = startServer(cert);
-  let overrideBits =
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_MISMATCH;
   certOverrideService.rememberValidityOverride(
     "localhost",
     server.port,
     {},
     cert,
-    overrideBits,
     true
   );
 

browser/base/content/test/siteIdentity/browser_secure_transport_insecure_scheme.js

@@ -134,15 +134,11 @@ add_task(async function() {
   let cert = getTestServerCertificate();
   // Start the proxy and configure Firefox to trust its certificate.
   let server = startServer(cert);
-  let overrideBits =
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_MISMATCH;
   certOverrideService.rememberValidityOverride(
     "localhost",
     server.port,
     {},
     cert,
-    overrideBits,
     true
   );
   // Configure Firefox to use the proxy.

browser/components/urlbar/tests/browser/browser_speculative_connect_not_with_client_cert.js

@@ -160,15 +160,11 @@ add_setup(async function() {
     },
   ]);
 
-  let overrideBits =
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_MISMATCH;
   certOverrideService.rememberValidityOverride(
     "localhost",
     server.port,
     {},
     cert,
-    overrideBits,
     true
   );
 

dom/base/Document.cpp

@@ -1572,27 +1572,6 @@ already_AddRefed<mozilla::dom::Promise> Document::AddCertException(
     return promise.forget();
   }
 
-  bool isUntrusted = true;
-  rv = tsi->GetIsUntrusted(&isUntrusted);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    promise->MaybeReject(rv);
-    return promise.forget();
-  }
-
-  bool isDomainMismatch = true;
-  rv = tsi->GetIsDomainMismatch(&isDomainMismatch);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    promise->MaybeReject(rv);
-    return promise.forget();
-  }
-
-  bool isNotValidAtThisTime = true;
-  rv = tsi->GetIsNotValidAtThisTime(&isNotValidAtThisTime);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    promise->MaybeReject(rv);
-    return promise.forget();
-  }
-
   nsCOMPtr<nsIX509Cert> cert;
   rv = tsi->GetServerCert(getter_AddRefs(cert));
   if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -1604,17 +1583,6 @@ already_AddRefed<mozilla::dom::Promise> Document::AddCertException(
     return promise.forget();
   }
 
-  uint32_t flags = 0;
-  if (isUntrusted) {
-    flags |= nsICertOverrideService::ERROR_UNTRUSTED;
-  }
-  if (isDomainMismatch) {
-    flags |= nsICertOverrideService::ERROR_MISMATCH;
-  }
-  if (isNotValidAtThisTime) {
-    flags |= nsICertOverrideService::ERROR_TIME;
-  }
-
   if (XRE_IsContentProcess()) {
     nsCOMPtr<nsISerializable> certSer = do_QueryInterface(cert);
     nsCString certSerialized;
@@ -1623,8 +1591,7 @@ already_AddRefed<mozilla::dom::Promise> Document::AddCertException(
     ContentChild* cc = ContentChild::GetSingleton();
     MOZ_ASSERT(cc);
     OriginAttributes const& attrs = NodePrincipal()->OriginAttributesRef();
-    cc->SendAddCertException(certSerialized, flags, host, port, attrs,
+    cc->SendAddCertException(certSerialized, host, port, attrs, aIsTemporary)
-                             aIsTemporary)
         ->Then(GetCurrentSerialEventTarget(), __func__,
                [promise](const mozilla::MozPromise<
                          nsresult, mozilla::ipc::ResponseRejectReason,
@@ -1648,7 +1615,7 @@ already_AddRefed<mozilla::dom::Promise> Document::AddCertException(
 
     OriginAttributes const& attrs = NodePrincipal()->OriginAttributesRef();
     rv = overrideService->RememberValidityOverride(host, port, attrs, cert,
-                                                   flags, aIsTemporary);
+                                                   aIsTemporary);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       promise->MaybeReject(rv);
       return promise.forget();

dom/ipc/ContentParent.cpp

@@ -6547,9 +6547,8 @@ mozilla::ipc::IPCResult ContentParent::RecvBHRThreadHang(
 }
 
 mozilla::ipc::IPCResult ContentParent::RecvAddCertException(
-    const nsACString& aSerializedCert, uint32_t aFlags,
+    const nsACString& aSerializedCert, const nsACString& aHostName,
-    const nsACString& aHostName, int32_t aPort,
+    int32_t aPort, const OriginAttributes& aOriginAttributes, bool aIsTemporary,
-    const OriginAttributes& aOriginAttributes, bool aIsTemporary,
     AddCertExceptionResolver&& aResolver) {
   nsCOMPtr<nsISupports> certObj;
   nsresult rv = NS_DeserializeObject(aSerializedCert, getter_AddRefs(certObj));
@@ -6564,7 +6563,7 @@ mozilla::ipc::IPCResult ContentParent::RecvAddCertException(
         rv = NS_ERROR_FAILURE;
       } else {
         rv = overrideService->RememberValidityOverride(
-            aHostName, aPort, aOriginAttributes, cert, aFlags, aIsTemporary);
+            aHostName, aPort, aOriginAttributes, cert, aIsTemporary);
       }
     }
   }

dom/ipc/ContentParent.h

@@ -1236,10 +1236,9 @@ class ContentParent final : public PContentParent,
   mozilla::ipc::IPCResult RecvBHRThreadHang(const HangDetails& aHangDetails);
 
   mozilla::ipc::IPCResult RecvAddCertException(
-      const nsACString& aSerializedCert, uint32_t aFlags,
+      const nsACString& aSerializedCert, const nsACString& aHostName,
-      const nsACString& aHostName, int32_t aPort,
+      int32_t aPort, const OriginAttributes& aOriginAttributes,
-      const OriginAttributes& aOriginAttributes, bool aIsTemporary,
+      bool aIsTemporary, AddCertExceptionResolver&& aResolver);
-      AddCertExceptionResolver&& aResolver);
 
   mozilla::ipc::IPCResult RecvAutomaticStorageAccessPermissionCanBeGranted(
       nsIPrincipal* aPrincipal,

dom/ipc/PContent.ipdl

@@ -1626,8 +1626,8 @@ parent:
     /*
      * Adds a certificate exception for the given hostname and port.
      */
-    async AddCertException(nsCString aSerializedCert, uint32_t aFlags,
+    async AddCertException(nsCString aSerializedCert, nsCString aHostName,
-                           nsCString aHostName, int32_t aPort, OriginAttributes aOriginAttributes,
+                           int32_t aPort, OriginAttributes aOriginAttributes,
                            bool aIsTemporary)
           returns (nsresult success);
 

dom/media/webrtc/tests/mochitests/addTurnsSelfsignedCert.js

@@ -25,7 +25,6 @@ addMessageListener("add-turns-certs", certs => {
       port,
       {},
       cert,
-      Ci.nsICertOverrideService.ERROR_UNTRUSTED,
       false
     );
   });

mobile/android/modules/geckoview/GeckoViewProgress.jsm

@@ -156,7 +156,6 @@ var IdentityHandler = {
         uri.port,
         {},
         cert,
-        {},
         {}
       );
 

netwerk/test/unit/test_be_conservative.js

@@ -150,18 +150,7 @@ function storeCertOverride(port, cert) {
   let certOverrideService = Cc[
     "@mozilla.org/security/certoverride;1"
   ].getService(Ci.nsICertOverrideService);
-  let overrideBits =
+  certOverrideService.rememberValidityOverride(hostname, port, {}, cert, true);
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_TIME |
-    Ci.nsICertOverrideService.ERROR_MISMATCH;
-  certOverrideService.rememberValidityOverride(
-    hostname,
-    port,
-    {},
-    cert,
-    overrideBits,
-    true
-  );
 }
 
 function startClient(port, beConservative, expectSuccess) {

netwerk/test/unit/test_be_conservative_error_handling.js

@@ -143,18 +143,7 @@ function storeCertOverride(port, cert) {
   let certOverrideService = Cc[
     "@mozilla.org/security/certoverride;1"
   ].getService(Ci.nsICertOverrideService);
-  let overrideBits =
+  certOverrideService.rememberValidityOverride(hostname, port, {}, cert, true);
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_TIME |
-    Ci.nsICertOverrideService.ERROR_MISMATCH;
-  certOverrideService.rememberValidityOverride(
-    hostname,
-    port,
-    {},
-    cert,
-    overrideBits,
-    true
-  );
 }
 
 function startClient(port, beConservative, expectSuccess) {

netwerk/test/unit/test_tls_flags.js

@@ -158,18 +158,7 @@ function storeCertOverride(port, cert) {
   let certOverrideService = Cc[
     "@mozilla.org/security/certoverride;1"
   ].getService(Ci.nsICertOverrideService);
-  let overrideBits =
+  certOverrideService.rememberValidityOverride(hostname, port, {}, cert, true);
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_TIME |
-    Ci.nsICertOverrideService.ERROR_MISMATCH;
-  certOverrideService.rememberValidityOverride(
-    hostname,
-    port,
-    {},
-    cert,
-    overrideBits,
-    true
-  );
 }
 
 function startClient(port, tlsFlags, expectSuccess) {

netwerk/test/unit/test_tls_server.js

@@ -116,16 +116,11 @@ function startServer(
 }
 
 function storeCertOverride(port, cert) {
-  let overrideBits =
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_TIME |
-    Ci.nsICertOverrideService.ERROR_MISMATCH;
   certOverrideService.rememberValidityOverride(
     "127.0.0.1",
     port,
     {},
     cert,
-    overrideBits,
     true
   );
 }

netwerk/test/unit/test_tls_server_multiple_clients.js

@@ -62,16 +62,11 @@ function startServer(cert) {
 }
 
 function storeCertOverride(port, cert) {
-  let overrideBits =
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_TIME |
-    Ci.nsICertOverrideService.ERROR_MISMATCH;
   certOverrideService.rememberValidityOverride(
     "127.0.0.1",
     port,
     {},
     cert,
-    overrideBits,
     true
   );
 }

security/certverifier/CertVerifier.cpp

@@ -752,6 +752,24 @@ static bool CertIsSelfSigned(const BackCert& backCert, void* pinarg) {
   return rv == Success;
 }
 
+static Result CheckCertHostnameHelper(Input peerCertInput,
+                                      const nsACString& hostname) {
+  Input hostnameInput;
+  Result rv = hostnameInput.Init(
+      BitwiseCast<const uint8_t*, const char*>(hostname.BeginReading()),
+      hostname.Length());
+  if (rv != Success) {
+    return Result::FATAL_ERROR_INVALID_ARGS;
+  }
+
+  rv = CheckCertHostname(peerCertInput, hostnameInput);
+  // Treat malformed name information as a domain mismatch.
+  if (rv == Result::ERROR_BAD_DER) {
+    return Result::ERROR_BAD_CERT_DOMAIN;
+  }
+  return rv;
+}
+
 Result CertVerifier::VerifySSLServerCert(
     const nsTArray<uint8_t>& peerCertBytes, Time time,
     /*optional*/ void* pinarg, const nsACString& hostname,
@@ -836,6 +854,17 @@ Result CertVerifier::VerifySSLServerCert(
         return Result::ERROR_MITM_DETECTED;
       }
     }
+    // If the certificate is expired or not yet valid, first check whether or
+    // not it is valid for the indicated hostname, because that would be a more
+    // serious error.
+    if (rv == Result::ERROR_EXPIRED_CERTIFICATE ||
+        rv == Result::ERROR_NOT_YET_VALID_CERTIFICATE ||
+        rv == Result::ERROR_INVALID_DER_TIME) {
+      Result hostnameResult = CheckCertHostnameHelper(peerCertInput, hostname);
+      if (hostnameResult != Success) {
+        return hostnameResult;
+      }
+    }
     return rv;
   }
 
@@ -865,21 +894,8 @@ Result CertVerifier::VerifySSLServerCert(
     }
   }
 
-  Input hostnameInput;
+  rv = CheckCertHostnameHelper(peerCertInput, hostname);
-  rv = hostnameInput.Init(
-      BitwiseCast<const uint8_t*, const char*>(hostname.BeginReading()),
-      hostname.Length());
   if (rv != Success) {
-    return Result::FATAL_ERROR_INVALID_ARGS;
-  }
-
-  rv = CheckCertHostname(peerCertInput, hostnameInput);
-  if (rv != Success) {
-    // Treat malformed name information as a domain mismatch.
-    if (rv == Result::ERROR_BAD_DER) {
-      return Result::ERROR_BAD_CERT_DOMAIN;
-    }
-
     return rv;
   }
 

security/manager/ssl/PVerifySSLServerCert.ipdl

@@ -21,8 +21,8 @@ child:
                                        uint8_t aEVStatus,
                                        bool isBuiltCertChainRootBuiltInRoot);
 
-  async OnVerifiedSSLServerCertFailure(uint32_t aFinalError,
+  async OnVerifiedSSLServerCertFailure(int32_t aFinalError,
-                                       uint32_t aCollectedErrors);
+                                       uint32_t aOverridableErrorCategory);
 
   async __delete__();
 };

security/manager/ssl/SSLServerCertVerification.cpp

@@ -145,13 +145,9 @@ using namespace mozilla::pkix;
 namespace mozilla {
 namespace psm {
 
-namespace {
-
 // do not use a nsCOMPtr to avoid static initializer/destructor
 nsIThreadPool* gCertVerificationThreadPool = nullptr;
 
-}  // unnamed namespace
-
 // Called when the socket transport thread starts, to initialize the SSL cert
 // verification thread pool. By tying the thread pool startup/shutdown directly
 // to the STS thread's lifetime, we ensure that they are *always* available for
@@ -190,8 +186,6 @@ void StopSSLServerCertVerificationThreads() {
   }
 }
 
-namespace {
-
 // A probe value of 1 means "no error".
 uint32_t MapOverridableErrorToProbeValue(PRErrorCode errorCode) {
   switch (errorCode) {
@@ -236,7 +230,7 @@ uint32_t MapOverridableErrorToProbeValue(PRErrorCode errorCode) {
   }
   NS_WARNING(
       "Unknown certificate error code. Does MapOverridableErrorToProbeValue "
-      "handle everything in DetermineCertOverrideErrors?");
+      "handle everything in CategorizeCertificateError?");
   return 0;
 }
 
@@ -270,35 +264,19 @@ static uint32_t MapCertErrorToProbeValue(PRErrorCode errorCode) {
   return probeValue;
 }
 
-SECStatus DetermineCertOverrideErrors(const nsCOMPtr<nsIX509Cert>& cert,
+enum class OverridableErrorCategory : uint32_t {
-                                      const nsACString& hostName,
+  Unset = 0,
-                                      mozilla::pkix::Time now,
+  Trust = 1,
-                                      PRErrorCode defaultErrorCodeToReport,
+  Domain = 2,
-                                      /*out*/ uint32_t& collectedErrors,
+  Time = 3,
-                                      /*out*/ PRErrorCode& errorCodeTrust,
+};
-                                      /*out*/ PRErrorCode& errorCodeMismatch,
-                                      /*out*/ PRErrorCode& errorCodeTime) {
-  MOZ_ASSERT(cert);
-  MOZ_ASSERT(collectedErrors == 0);
-  MOZ_ASSERT(errorCodeTrust == 0);
-  MOZ_ASSERT(errorCodeMismatch == 0);
-  MOZ_ASSERT(errorCodeTime == 0);
 
-  nsTArray<uint8_t> certDER;
+// If the given PRErrorCode is an overridable certificate error, return which
-  if (NS_FAILED(cert->GetRawDER(certDER))) {
+// category (trust, time, domain mismatch) it falls in. If it is not
-    PR_SetError(SEC_ERROR_LIBRARY_FAILURE, 0);
+// overridable, return Nothing.
-    return SECFailure;
+Maybe<OverridableErrorCategory> CategorizeCertificateError(
-  }
+    PRErrorCode certificateError) {
-  mozilla::pkix::Input certInput;
+  switch (certificateError) {
-  if (certInput.Init(certDER.Elements(), certDER.Length()) != Success) {
-    PR_SetError(SEC_ERROR_BAD_DER, 0);
-    return SECFailure;
-  }
-
-  // Assumes the error prioritization described in mozilla::pkix's
-  // BuildForward function. Also assumes that CheckCertHostname was only
-  // called if CertVerifier::VerifyCert succeeded.
-  switch (defaultErrorCodeToReport) {
     case SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED:
     case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:
     case SEC_ERROR_UNKNOWN_ISSUER:
@@ -310,95 +288,21 @@ SECStatus DetermineCertOverrideErrors(const nsCOMPtr<nsIX509Cert>& cert,
     case mozilla::pkix::MOZILLA_PKIX_ERROR_MITM_DETECTED:
     case mozilla::pkix::MOZILLA_PKIX_ERROR_NOT_YET_VALID_ISSUER_CERTIFICATE:
     case mozilla::pkix::MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT:
-    case mozilla::pkix::MOZILLA_PKIX_ERROR_V1_CERT_USED_AS_CA: {
+    case mozilla::pkix::MOZILLA_PKIX_ERROR_V1_CERT_USED_AS_CA:
-      collectedErrors = nsICertOverrideService::ERROR_UNTRUSTED;
+      return Some(OverridableErrorCategory::Trust);
-      errorCodeTrust = defaultErrorCodeToReport;
-
-      mozilla::pkix::BackCert backCert(
-          certInput, mozilla::pkix::EndEntityOrCA::MustBeEndEntity, nullptr);
-      Result rv = backCert.Init();
-      if (rv != Success) {
-        PR_SetError(MapResultToPRErrorCode(rv), 0);
-        return SECFailure;
-      }
-      mozilla::pkix::Time notBefore(mozilla::pkix::Time::uninitialized);
-      mozilla::pkix::Time notAfter(mozilla::pkix::Time::uninitialized);
-      // If the validity can't be parsed, ParseValidity will return
-      // Result::ERROR_INVALID_DER_TIME.
-      rv = mozilla::pkix::ParseValidity(backCert.GetValidity(), &notBefore,
-                                        &notAfter);
-      if (rv != Success) {
-        collectedErrors |= nsICertOverrideService::ERROR_TIME;
-        errorCodeTime = MapResultToPRErrorCode(rv);
-        break;
-      }
-      // If `now` is outside of the certificate's validity period,
-      // CheckValidity will return Result::ERROR_NOT_YET_VALID_CERTIFICATE or
-      // Result::ERROR_EXPIRED_CERTIFICATE, as appropriate, and Success
-      // otherwise.
-      rv = mozilla::pkix::CheckValidity(now, notBefore, notAfter);
-      if (rv != Success) {
-        collectedErrors |= nsICertOverrideService::ERROR_TIME;
-        errorCodeTime = MapResultToPRErrorCode(rv);
-      }
-      break;
-    }
 
     case SEC_ERROR_INVALID_TIME:
     case SEC_ERROR_EXPIRED_CERTIFICATE:
     case mozilla::pkix::MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE:
-      collectedErrors = nsICertOverrideService::ERROR_TIME;
+      return Some(OverridableErrorCategory::Time);
-      errorCodeTime = defaultErrorCodeToReport;
-      break;
 
     case SSL_ERROR_BAD_CERT_DOMAIN:
-      collectedErrors = nsICertOverrideService::ERROR_MISMATCH;
+      return Some(OverridableErrorCategory::Domain);
-      errorCodeMismatch = SSL_ERROR_BAD_CERT_DOMAIN;
-      break;
-
-    case 0:
-      NS_ERROR("No error code set during certificate validation failure.");
-      PR_SetError(PR_INVALID_STATE_ERROR, 0);
-      return SECFailure;
 
     default:
-      PR_SetError(defaultErrorCodeToReport, 0);
+      break;
-      return SECFailure;
   }
-
+  return Nothing();
-  if (defaultErrorCodeToReport != SSL_ERROR_BAD_CERT_DOMAIN) {
-    Input hostnameInput;
-    Result result = hostnameInput.Init(
-        BitwiseCast<const uint8_t*, const char*>(hostName.BeginReading()),
-        hostName.Length());
-    if (result != Success) {
-      PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
-      return SECFailure;
-    }
-    // CheckCertHostname expects that its input represents a certificate that
-    // has already been successfully validated by BuildCertChain. This is
-    // obviously not the case, however, because we're in the error path of
-    // certificate verification. Thus, this is problematic. In the future, it
-    // would be nice to remove this optimistic additional error checking and
-    // simply punt to the front-end, which can more easily (and safely) perform
-    // extra checks to give the user hints as to why verification failed.
-    result = CheckCertHostname(certInput, hostnameInput);
-    // Treat malformed name information as a domain mismatch.
-    if (result == Result::ERROR_BAD_DER ||
-        result == Result::ERROR_BAD_CERT_DOMAIN) {
-      collectedErrors |= nsICertOverrideService::ERROR_MISMATCH;
-      errorCodeMismatch = SSL_ERROR_BAD_CERT_DOMAIN;
-    } else if (IsFatalError(result)) {
-      // Because its input has not been validated by BuildCertChain,
-      // CheckCertHostname can return an error that is less important than the
-      // original certificate verification error. Only return an error result
-      // from this function if we've encountered a fatal error.
-      PR_SetError(MapResultToPRErrorCode(result), 0);
-      return SECFailure;
-    }
-  }
-
-  return SECSuccess;
 }
 
 // Helper function to determine if overrides are allowed for this host.
@@ -727,112 +631,60 @@ PRErrorCode AuthCertificateParseResults(
     uint64_t aPtrForLog, const nsACString& aHostName, int32_t aPort,
     const OriginAttributes& aOriginAttributes,
     const nsCOMPtr<nsIX509Cert>& aCert, mozilla::pkix::Time aTime,
-    PRErrorCode aDefaultErrorCodeToReport,
+    PRErrorCode aCertVerificationError,
-    /* out */ uint32_t& aCollectedErrors) {
+    /* out */ OverridableErrorCategory& aOverridableErrorCategory) {
-  if (aDefaultErrorCodeToReport == 0) {
+  uint32_t probeValue = MapCertErrorToProbeValue(aCertVerificationError);
-    MOZ_ASSERT_UNREACHABLE(
-        "No error set during certificate validation failure");
-    return SEC_ERROR_LIBRARY_FAILURE;
-  }
-
-  uint32_t probeValue = MapCertErrorToProbeValue(aDefaultErrorCodeToReport);
   Telemetry::Accumulate(Telemetry::SSL_CERT_VERIFICATION_ERRORS, probeValue);
 
-  aCollectedErrors = 0;
+  Maybe<OverridableErrorCategory> maybeOverridableErrorCategory =
-  PRErrorCode errorCodeTrust = 0;
+      CategorizeCertificateError(aCertVerificationError);
-  PRErrorCode errorCodeMismatch = 0;
+  // If this isn't an overridable error, return it now. This will stop the
-  PRErrorCode errorCodeTime = 0;
+  // connection and report the given error.
-  if (DetermineCertOverrideErrors(
+  if (!maybeOverridableErrorCategory.isSome()) {
-          aCert, aHostName, aTime, aDefaultErrorCodeToReport, aCollectedErrors,
+    return aCertVerificationError;
-          errorCodeTrust, errorCodeMismatch, errorCodeTime) != SECSuccess) {
-    PRErrorCode errorCode = PR_GetError();
-    MOZ_ASSERT(!ErrorIsOverridable(errorCode));
-    if (errorCode == 0) {
-      MOZ_ASSERT_UNREACHABLE(
-          "No error set during DetermineCertOverrideErrors failure");
-      return SEC_ERROR_LIBRARY_FAILURE;
-    }
-    return errorCode;
-  }
-
-  if (!aCollectedErrors) {
-    MOZ_ASSERT_UNREACHABLE("aCollectedErrors should not be 0");
-    return SEC_ERROR_LIBRARY_FAILURE;
   }
+  aOverridableErrorCategory = *maybeOverridableErrorCategory;
 
   bool overrideAllowed = false;
-  if (NS_FAILED(OverrideAllowedForHost(aPtrForLog, aHostName, aOriginAttributes,
+  nsresult rv = OverrideAllowedForHost(aPtrForLog, aHostName, aOriginAttributes,
-                                       overrideAllowed))) {
+                                       overrideAllowed);
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+  if (NS_FAILED(rv)) {
-            ("[0x%" PRIx64 "] AuthCertificateParseResults - "
+    return aCertVerificationError;
-             "OverrideAllowedForHost failed\n",
-             aPtrForLog));
-    return aDefaultErrorCodeToReport;
   }
 
-  if (overrideAllowed) {
+  if (!overrideAllowed) {
-    nsCOMPtr<nsICertOverrideService> overrideService =
-        do_GetService(NS_CERTOVERRIDE_CONTRACTID);
-
-    uint32_t overrideBits = 0;
-    uint32_t remainingDisplayErrors = aCollectedErrors;
-
-    // it is fine to continue without the nsICertOverrideService
-    if (overrideService) {
-      bool haveOverride;
-      bool isTemporaryOverride;  // we don't care
-      nsresult rv = overrideService->HasMatchingOverride(
-          aHostName, aPort, aOriginAttributes, aCert, &overrideBits,
-          &isTemporaryOverride, &haveOverride);
-      if (NS_SUCCEEDED(rv) && haveOverride) {
-        // remove the errors that are already overriden
-        remainingDisplayErrors &= ~overrideBits;
-      }
-    }
-
-    if (!remainingDisplayErrors) {
-      // This can double- or triple-count one certificate with multiple
-      // different types of errors. Since this is telemetry and we just
-      // want a ballpark answer, we don't care.
-      if (errorCodeTrust != 0) {
-        uint32_t probeValue = MapOverridableErrorToProbeValue(errorCodeTrust);
-        Telemetry::Accumulate(Telemetry::SSL_CERT_ERROR_OVERRIDES, probeValue);
-      }
-      if (errorCodeMismatch != 0) {
-        uint32_t probeValue =
-            MapOverridableErrorToProbeValue(errorCodeMismatch);
-        Telemetry::Accumulate(Telemetry::SSL_CERT_ERROR_OVERRIDES, probeValue);
-      }
-      if (errorCodeTime != 0) {
-        uint32_t probeValue = MapOverridableErrorToProbeValue(errorCodeTime);
-        Telemetry::Accumulate(Telemetry::SSL_CERT_ERROR_OVERRIDES, probeValue);
-      }
-
-      // all errors are covered by override rules, so let's accept the cert
-      MOZ_LOG(
-          gPIPNSSLog, LogLevel::Debug,
-          ("[0x%" PRIx64 "] All errors covered by override rules", aPtrForLog));
-      return 0;
-    }
-  } else {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-            ("[0x%" PRIx64 "] HSTS or pinned host - no overrides allowed\n",
+            ("[0x%" PRIx64 "] HSTS or pinned host - no overrides allowed",
              aPtrForLog));
+    return aCertVerificationError;
   }
 
-  MOZ_LOG(
+  nsCOMPtr<nsICertOverrideService> overrideService =
-      gPIPNSSLog, LogLevel::Debug,
+      do_GetService(NS_CERTOVERRIDE_CONTRACTID);
-      ("[0x%" PRIx64 "] Certificate error was not overridden\n", aPtrForLog));
+  if (!overrideService) {
+    return aCertVerificationError;
+  }
+  bool haveOverride;
+  bool isTemporaryOverride;
+  rv = overrideService->HasMatchingOverride(aHostName, aPort, aOriginAttributes,
+                                            aCert, &isTemporaryOverride,
+                                            &haveOverride);
+  if (NS_FAILED(rv)) {
+    return aCertVerificationError;
+  }
+  Unused << isTemporaryOverride;
+  if (haveOverride) {
+    uint32_t probeValue =
+        MapOverridableErrorToProbeValue(aCertVerificationError);
+    Telemetry::Accumulate(Telemetry::SSL_CERT_ERROR_OVERRIDES, probeValue);
+    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+            ("[0x%" PRIx64 "] certificate error overridden", aPtrForLog));
+    return 0;
+  }
 
-  // pick the error code to report by priority
+  return aCertVerificationError;
-  return errorCodeTrust      ? errorCodeTrust
-         : errorCodeMismatch ? errorCodeMismatch
-         : errorCodeTime     ? errorCodeTime
-                             : aDefaultErrorCodeToReport;
 }
 
-}  // unnamed namespace
-
 static nsTArray<nsTArray<uint8_t>> CreateCertBytesArray(
     const UniqueCERTCertList& aCertChain) {
   nsTArray<nsTArray<uint8_t>> certsBytes;
@@ -928,7 +780,8 @@ SSLServerCertVerificationJob::Run() {
         std::move(builtChainBytesArray), std::move(mPeerCertChain),
         TransportSecurityInfo::ConvertCertificateTransparencyInfoToStatus(
             certificateTransparencyInfo),
-        evStatus, true, 0, 0, isCertChainRootBuiltInRoot, mProviderFlags);
+        evStatus, true, 0, OverridableErrorCategory::Unset,
+        isCertChainRootBuiltInRoot, mProviderFlags);
     return NS_OK;
   }
 
@@ -937,17 +790,18 @@ SSLServerCertVerificationJob::Run() {
       jobStartTime, TimeStamp::Now());
 
   PRErrorCode error = MapResultToPRErrorCode(rv);
-  uint32_t collectedErrors = 0;
+  OverridableErrorCategory overridableErrorCategory =
+      OverridableErrorCategory::Unset;
   nsCOMPtr<nsIX509Cert> cert(new nsNSSCertificate(std::move(certBytes)));
   PRErrorCode finalError = AuthCertificateParseResults(
       mAddrForLogging, mHostName, mPort, mOriginAttributes, cert, mTime, error,
-      collectedErrors);
+      overridableErrorCategory);
 
   // NB: finalError may be 0 here, in which the connection will continue.
   mResultTask->Dispatch(
       std::move(builtChainBytesArray), std::move(mPeerCertChain),
       nsITransportSecurityInfo::CERTIFICATE_TRANSPARENCY_NOT_APPLICABLE,
-      EVStatus::NotEV, false, finalError, collectedErrors, false,
+      EVStatus::NotEV, false, finalError, overridableErrorCategory, false,
       mProviderFlags);
   return NS_OK;
 }
@@ -1168,14 +1022,15 @@ SSLServerCertVerificationResult::SSLServerCertVerificationResult(
       mEVStatus(EVStatus::NotEV),
       mSucceeded(false),
       mFinalError(0),
-      mCollectedErrors(0),
+      mOverridableErrorCategory(OverridableErrorCategory::Unset),
       mProviderFlags(0) {}
 
 void SSLServerCertVerificationResult::Dispatch(
     nsTArray<nsTArray<uint8_t>>&& aBuiltChain,
     nsTArray<nsTArray<uint8_t>>&& aPeerCertChain,
     uint16_t aCertificateTransparencyStatus, EVStatus aEVStatus,
-    bool aSucceeded, PRErrorCode aFinalError, uint32_t aCollectedErrors,
+    bool aSucceeded, PRErrorCode aFinalError,
+    OverridableErrorCategory aOverridableErrorCategory,
     bool aIsBuiltCertChainRootBuiltInRoot, uint32_t aProviderFlags) {
   mBuiltChain = std::move(aBuiltChain);
   mPeerCertChain = std::move(aPeerCertChain);
@@ -1183,7 +1038,7 @@ void SSLServerCertVerificationResult::Dispatch(
   mEVStatus = aEVStatus;
   mSucceeded = aSucceeded;
   mFinalError = aFinalError;
-  mCollectedErrors = aCollectedErrors;
+  mOverridableErrorCategory = aOverridableErrorCategory;
   mIsBuiltCertChainRootBuiltInRoot = aIsBuiltCertChainRootBuiltInRoot;
   mProviderFlags = aProviderFlags;
 
@@ -1251,8 +1106,8 @@ SSLServerCertVerificationResult::Run() {
     // Certificate validation failed; store the peer certificate chain on
     // infoObject so it can be used for error reporting.
     mInfoObject->SetFailedCertChain(std::move(mPeerCertChain));
-    if (mCollectedErrors != 0) {
+    if (mOverridableErrorCategory != OverridableErrorCategory::Unset) {
-      mInfoObject->SetStatusErrorBits(cert, mCollectedErrors);
+      mInfoObject->SetStatusErrorBits(cert, mOverridableErrorCategory);
     }
   }
 

security/manager/ssl/SSLServerCertVerification.h

@@ -38,6 +38,8 @@ SECStatus AuthCertificateHookWithInfo(
     Maybe<nsTArray<nsTArray<uint8_t>>>& stapledOCSPResponses,
     Maybe<nsTArray<uint8_t>>& sctsFromTLSExtension, uint32_t providerFlags);
 
+enum class OverridableErrorCategory : uint32_t;
+
 // Base class for dispatching the certificate verification result.
 class BaseSSLServerCertVerificationResult {
  public:
@@ -47,7 +49,8 @@ class BaseSSLServerCertVerificationResult {
                         nsTArray<nsTArray<uint8_t>>&& aPeerCertChain,
                         uint16_t aCertificateTransparencyStatus,
                         EVStatus aEVStatus, bool aSucceeded,
-                        PRErrorCode aFinalError, uint32_t aCollectedErrors,
+                        PRErrorCode aFinalError,
+                        OverridableErrorCategory aOverridableErrorCategory,
                         bool aIsBuiltCertChainRootBuiltInRoot,
                         uint32_t aProviderFlags) = 0;
 };
@@ -71,7 +74,7 @@ class SSLServerCertVerificationResult final
                 nsTArray<nsTArray<uint8_t>>&& aPeerCertChain,
                 uint16_t aCertificateTransparencyStatus, EVStatus aEVStatus,
                 bool aSucceeded, PRErrorCode aFinalError,
-                uint32_t aCollectedErrors,
+                OverridableErrorCategory aOverridableErrorCategory,
                 bool aIsBuiltCertChainRootBuiltInRoot,
                 uint32_t aProviderFlags) override;
 
@@ -85,7 +88,7 @@ class SSLServerCertVerificationResult final
   EVStatus mEVStatus;
   bool mSucceeded;
   PRErrorCode mFinalError;
-  uint32_t mCollectedErrors;
+  OverridableErrorCategory mOverridableErrorCategory;
   bool mIsBuiltCertChainRootBuiltInRoot;
   uint32_t mProviderFlags;
 };

security/manager/ssl/TransportSecurityInfo.cpp

@@ -955,13 +955,24 @@ void RememberCertErrorsTable::LookupCertErrorBits(
 }
 
 void TransportSecurityInfo::SetStatusErrorBits(
-    const nsCOMPtr<nsIX509Cert>& cert, uint32_t collected_errors) {
+    const nsCOMPtr<nsIX509Cert>& cert,
+    OverridableErrorCategory overridableErrorCategory) {
   SetServerCert(cert, EVStatus::NotEV);
 
   mHaveCertErrorBits = true;
-  mIsDomainMismatch = collected_errors & nsICertOverrideService::ERROR_MISMATCH;
+  switch (overridableErrorCategory) {
-  mIsNotValidAtThisTime = collected_errors & nsICertOverrideService::ERROR_TIME;
+    case OverridableErrorCategory::Trust:
-  mIsUntrusted = collected_errors & nsICertOverrideService::ERROR_UNTRUSTED;
+      mIsUntrusted = true;
+      break;
+    case OverridableErrorCategory::Time:
+      mIsNotValidAtThisTime = true;
+      break;
+    case OverridableErrorCategory::Domain:
+      mIsDomainMismatch = true;
+      break;
+    default:
+      break;
+  }
 
   RememberCertErrorsTable::GetInstance().RememberCertHasError(this, SECFailure);
 }

security/manager/ssl/TransportSecurityInfo.h

@@ -27,6 +27,8 @@
 namespace mozilla {
 namespace psm {
 
+enum class OverridableErrorCategory : uint32_t;
+
 class TransportSecurityInfo : public nsITransportSecurityInfo,
                               public nsIInterfaceRequestor,
                               public nsISerializable,
@@ -79,7 +81,7 @@ class TransportSecurityInfo : public nsITransportSecurityInfo,
   bool IsCanceled();
 
   void SetStatusErrorBits(const nsCOMPtr<nsIX509Cert>& cert,
-                          uint32_t collected_errors);
+                          OverridableErrorCategory overridableErrorCategory);
 
   nsresult SetFailedCertChain(nsTArray<nsTArray<uint8_t>>&& certList);
 

security/manager/ssl/VerifySSLServerCertChild.cpp

@@ -40,23 +40,19 @@ ipc::IPCResult VerifySSLServerCertChild::RecvOnVerifiedSSLServerCertSuccess(
 
   mResultTask->Dispatch(std::move(certBytesArray), std::move(mPeerCertChain),
                         aCertTransparencyStatus,
-                        static_cast<EVStatus>(aEVStatus), true, 0, 0,
+                        static_cast<EVStatus>(aEVStatus), true, 0,
+                        OverridableErrorCategory::Unset,
                         aIsBuiltCertChainRootBuiltInRoot, mProviderFlags);
   return IPC_OK();
 }
 
 ipc::IPCResult VerifySSLServerCertChild::RecvOnVerifiedSSLServerCertFailure(
-    const uint32_t& aFinalError, const uint32_t& aCollectedErrors) {
+    const int32_t& aFinalError, const uint32_t& aOverridableErrorCategory) {
-  MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-          ("[%p]VerifySSLServerCertChild::"
-           "RecvOnVerifiedSSLServerCertFailure - aFinalError=%u, "
-           "aCollectedErrors=%u",
-           this, aFinalError, aCollectedErrors));
-
   mResultTask->Dispatch(
       nsTArray<nsTArray<uint8_t>>(), std::move(mPeerCertChain),
       nsITransportSecurityInfo::CERTIFICATE_TRANSPARENCY_NOT_APPLICABLE,
-      EVStatus::NotEV, false, aFinalError, aCollectedErrors, false,
+      EVStatus::NotEV, false, aFinalError,
+      static_cast<OverridableErrorCategory>(aOverridableErrorCategory), false,
       mProviderFlags);
   return IPC_OK();
 }

security/manager/ssl/VerifySSLServerCertChild.h

@@ -36,7 +36,7 @@ class VerifySSLServerCertChild : public PVerifySSLServerCertChild {
       const bool& aIsBuiltCertChainRootBuiltInRoot);
 
   ipc::IPCResult RecvOnVerifiedSSLServerCertFailure(
-      const uint32_t& aFinalError, const uint32_t& aCollectedErrors);
+      const int32_t& aFinalError, const uint32_t& aOverridableErrorCategory);
 
  private:
   ~VerifySSLServerCertChild() = default;

security/manager/ssl/VerifySSLServerCertParent.cpp

@@ -31,7 +31,7 @@ VerifySSLServerCertParent::VerifySSLServerCertParent() {}
 void VerifySSLServerCertParent::OnVerifiedSSLServerCert(
     const nsTArray<ByteArray>& aBuiltCertChain,
     uint16_t aCertificateTransparencyStatus, uint8_t aEVStatus, bool aSucceeded,
-    PRErrorCode aFinalError, uint32_t aCollectedErrors,
+    PRErrorCode aFinalError, uint32_t aOverridableErrorCategory,
     bool aIsBuiltCertChainRootBuiltInRoot) {
   AssertIsOnBackgroundThread();
 
@@ -44,7 +44,8 @@ void VerifySSLServerCertParent::OnVerifiedSSLServerCert(
         aBuiltCertChain, aCertificateTransparencyStatus, aEVStatus,
         aIsBuiltCertChainRootBuiltInRoot);
   } else {
-    Unused << SendOnVerifiedSSLServerCertFailure(aFinalError, aCollectedErrors);
+    Unused << SendOnVerifiedSSLServerCertFailure(aFinalError,
+                                                 aOverridableErrorCategory);
   }
   Unused << Send__delete__(this);
 }
@@ -65,7 +66,7 @@ class IPCServerCertVerificationResult final
                 nsTArray<nsTArray<uint8_t>>&& aPeerCertChain,
                 uint16_t aCertificateTransparencyStatus, EVStatus aEVStatus,
                 bool aSucceeded, PRErrorCode aFinalError,
-                uint32_t aCollectedErrors,
+                OverridableErrorCategory aOverridableErrorCategory,
                 bool aIsBuiltCertChainRootBuiltInRoot,
                 uint32_t aProviderFlags) override;
 
@@ -80,7 +81,8 @@ void IPCServerCertVerificationResult::Dispatch(
     nsTArray<nsTArray<uint8_t>>&& aBuiltChain,
     nsTArray<nsTArray<uint8_t>>&& aPeerCertChain,
     uint16_t aCertificateTransparencyStatus, EVStatus aEVStatus,
-    bool aSucceeded, PRErrorCode aFinalError, uint32_t aCollectedErrors,
+    bool aSucceeded, PRErrorCode aFinalError,
+    OverridableErrorCategory aOverridableErrorCategory,
     bool aIsBuiltCertChainRootBuiltInRoot, uint32_t aProviderFlags) {
   nsTArray<ByteArray> builtCertChain;
   if (aSucceeded) {
@@ -94,7 +96,7 @@ void IPCServerCertVerificationResult::Dispatch(
           "psm::VerifySSLServerCertParent::OnVerifiedSSLServerCert",
           [parent(mParent), builtCertChain{std::move(builtCertChain)},
            aCertificateTransparencyStatus, aEVStatus, aSucceeded, aFinalError,
-           aCollectedErrors, aIsBuiltCertChainRootBuiltInRoot,
+           aOverridableErrorCategory, aIsBuiltCertChainRootBuiltInRoot,
            aProviderFlags]() {
             if (aSucceeded &&
                 !(aProviderFlags & nsISocketProvider::NO_PERMANENT_STORAGE)) {
@@ -109,7 +111,8 @@ void IPCServerCertVerificationResult::Dispatch(
             parent->OnVerifiedSSLServerCert(
                 builtCertChain, aCertificateTransparencyStatus,
                 static_cast<uint8_t>(aEVStatus), aSucceeded, aFinalError,
-                aCollectedErrors, aIsBuiltCertChainRootBuiltInRoot);
+                static_cast<uint32_t>(aOverridableErrorCategory),
+                aIsBuiltCertChainRootBuiltInRoot);
           }),
       NS_DISPATCH_NORMAL);
   MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(nrv));

security/manager/ssl/VerifySSLServerCertParent.h

@@ -43,7 +43,7 @@ class VerifySSLServerCertParent : public PVerifySSLServerCertParent {
                                uint16_t aCertificateTransparencyStatus,
                                uint8_t aEVStatus, bool aSucceeded,
                                PRErrorCode aFinalError,
-                               uint32_t aCollectedErrors,
+                               uint32_t aOverridableErrorCategory,
                                bool aIsBuiltCertChainRootBuiltInRoot);
 
  private:

security/manager/ssl/nsCertOverrideService.cpp

@@ -137,50 +137,6 @@ nsCertOverride::GetOriginAttributes(
   return NS_ERROR_FAILURE;
 }
 
-void nsCertOverride::convertBitsToString(OverrideBits ob,
-                                         /*out*/ nsACString& str) {
-  str.Truncate();
-
-  if (ob & OverrideBits::Mismatch) {
-    str.Append('M');
-  }
-
-  if (ob & OverrideBits::Untrusted) {
-    str.Append('U');
-  }
-
-  if (ob & OverrideBits::Time) {
-    str.Append('T');
-  }
-}
-
-void nsCertOverride::convertStringToBits(const nsACString& str,
-                                         /*out*/ OverrideBits& ob) {
-  ob = OverrideBits::None;
-
-  for (uint32_t i = 0; i < str.Length(); i++) {
-    switch (str.CharAt(i)) {
-      case 'm':
-      case 'M':
-        ob |= OverrideBits::Mismatch;
-        break;
-
-      case 'u':
-      case 'U':
-        ob |= OverrideBits::Untrusted;
-        break;
-
-      case 't':
-      case 'T':
-        ob |= OverrideBits::Time;
-        break;
-
-      default:
-        break;
-    }
-  }
-}
-
 NS_IMPL_ISUPPORTS(nsCertOverrideService, nsICertOverrideService, nsIObserver,
                   nsISupportsWeakReference, nsIAsyncShutdownBlocker)
 
@@ -304,18 +260,11 @@ nsresult nsCertOverrideService::Read(const MutexAutoLock& aProofOfLock) {
   nsAutoCString buffer;
   bool isMore = true;
 
-  /* file format is:
+  // Each line is of the form:
-   *
+  // host:port:originAttributes \t sSHA256OIDString \t fingerprint \t \t dbKey
-   * host:port:originattributes \t fingerprint-algorithm \t fingerprint \t
+  // There may be some "bits" identifiers between the `fingerprint` and `dbKey`
-   * override-mask \t dbKey
+  // fields, but these are now ignored.
-   *
+  // Lines that don't match this form are silently dropped.
-   *   where override-mask is a sequence of characters,
-   *     M meaning hostname-Mismatch-override
-   *     U meaning Untrusted-override
-   *     T meaning Time-error-override (expired/not yet valid)
-   *
-   * if this format isn't respected we move onto the next line in the file.
-   */
 
   while (isMore && NS_SUCCEEDED(lineInputStream->ReadLine(buffer, &isMore))) {
     if (buffer.IsEmpty() || buffer.First() == '#') {
@@ -358,22 +307,20 @@ nsresult nsCertOverrideService::Read(const MutexAutoLock& aProofOfLock) {
       continue;
     }
     nsDependentCSubstring bitsString;
-    if (!parser.ReadUntil(Tokenizer::Token::Whitespace(), bitsString) ||
+    if (!parser.ReadUntil(Tokenizer::Token::Whitespace(), bitsString)) {
-        bitsString.Length() == 0) {
       continue;
     }
+    Unused << bitsString;
     nsDependentCSubstring dbKey;
     if (!parser.ReadUntil(Tokenizer::Token::EndOfFile(), dbKey) ||
         dbKey.Length() == 0) {
       continue;
     }
-    nsCertOverride::OverrideBits bits;
-    nsCertOverride::convertStringToBits(bitsString, bits);
 
     AddEntryToList(host, port, attributes,
                    nullptr,  // don't have the cert
                    false,    // not temporary
-                   fingerprint, bits, dbKey, aProofOfLock);
+                   fingerprint, dbKey, aProofOfLock);
   }
 
   return NS_OK;
@@ -410,16 +357,13 @@ nsresult nsCertOverrideService::Write(const MutexAutoLock& aProofOfLock) {
       continue;
     }
 
-    nsAutoCString bitsString;
-    nsCertOverride::convertBitsToString(settings->mOverrideBits, bitsString);
-
     output.Append(entry->mKeyString);
     output.Append(kTab);
     output.Append(sSHA256OIDString);
     output.Append(kTab);
     output.Append(settings->mFingerprint);
     output.Append(kTab);
-    output.Append(bitsString);
+    // the "bits" string used to go here, but it no longer exists
     output.Append(kTab);
     output.Append(settings->mDBKey);
     output.Append(NS_LINEBREAK);
@@ -452,7 +396,7 @@ NS_IMETHODIMP
 nsCertOverrideService::RememberValidityOverride(
     const nsACString& aHostName, int32_t aPort,
     const OriginAttributes& aOriginAttributes, nsIX509Cert* aCert,
-    uint32_t aOverrideBits, bool aTemporary) {
+    bool aTemporary) {
   NS_ENSURE_ARG_POINTER(aCert);
   if (aHostName.IsEmpty() || !IsAscii(aHostName)) {
     return NS_ERROR_INVALID_ARG;
@@ -503,8 +447,7 @@ nsCertOverrideService::RememberValidityOverride(
     AddEntryToList(aHostName, aPort, aOriginAttributes,
                    aTemporary ? aCert : nullptr,
                    // keep a reference to the cert for temporary overrides
-                   aTemporary, fpStr,
+                   aTemporary, fpStr, dbkey, lock);
-                   (nsCertOverride::OverrideBits)aOverrideBits, dbkey, lock);
     if (!aTemporary) {
       Write(lock);
     }
@@ -517,31 +460,26 @@ NS_IMETHODIMP
 nsCertOverrideService::RememberValidityOverrideScriptable(
     const nsACString& aHostName, int32_t aPort,
     JS::Handle<JS::Value> aOriginAttributes, nsIX509Cert* aCert,
-    uint32_t aOverrideBits, bool aTemporary, JSContext* aCx) {
+    bool aTemporary, JSContext* aCx) {
   OriginAttributes attrs;
   if (!aOriginAttributes.isObject() || !attrs.Init(aCx, aOriginAttributes)) {
     return NS_ERROR_INVALID_ARG;
   }
 
-  return RememberValidityOverride(aHostName, aPort, attrs, aCert, aOverrideBits,
+  return RememberValidityOverride(aHostName, aPort, attrs, aCert, aTemporary);
-                                  aTemporary);
 }
 
 NS_IMETHODIMP
 nsCertOverrideService::HasMatchingOverride(
     const nsACString& aHostName, int32_t aPort,
     const OriginAttributes& aOriginAttributes, nsIX509Cert* aCert,
-    uint32_t* aOverrideBits, bool* aIsTemporary, bool* aRetval) {
+    bool* aIsTemporary, bool* aRetval) {
   bool disableAllSecurityCheck = false;
   {
     MutexAutoLock lock(mMutex);
     disableAllSecurityCheck = mDisableAllSecurityCheck;
   }
   if (disableAllSecurityCheck) {
-    nsCertOverride::OverrideBits all = nsCertOverride::OverrideBits::Untrusted |
-                                       nsCertOverride::OverrideBits::Mismatch |
-                                       nsCertOverride::OverrideBits::Time;
-    *aOverrideBits = static_cast<uint32_t>(all);
     *aIsTemporary = false;
     *aRetval = true;
     return NS_OK;
@@ -553,11 +491,9 @@ nsCertOverrideService::HasMatchingOverride(
   if (aPort < -1) return NS_ERROR_INVALID_ARG;
 
   NS_ENSURE_ARG_POINTER(aCert);
-  NS_ENSURE_ARG_POINTER(aOverrideBits);
   NS_ENSURE_ARG_POINTER(aIsTemporary);
   NS_ENSURE_ARG_POINTER(aRetval);
   *aRetval = false;
-  *aOverrideBits = static_cast<uint32_t>(nsCertOverride::OverrideBits::None);
 
   RefPtr<nsCertOverride> settings(
       GetOverrideFor(aHostName, aPort, aOriginAttributes));
@@ -570,7 +506,6 @@ nsCertOverrideService::HasMatchingOverride(
     return NS_OK;
   }
 
-  *aOverrideBits = static_cast<uint32_t>(settings->mOverrideBits);
   *aIsTemporary = settings->mIsTemporary;
 
   nsAutoCString fpStr;
@@ -600,23 +535,21 @@ NS_IMETHODIMP
 nsCertOverrideService::HasMatchingOverrideScriptable(
     const nsACString& aHostName, int32_t aPort,
     JS::Handle<JS::Value> aOriginAttributes, nsIX509Cert* aCert,
-    uint32_t* aOverrideBits, bool* aIsTemporary, JSContext* aCx,
+    bool* aIsTemporary, JSContext* aCx, bool* aRetval) {
-    bool* aRetval) {
   OriginAttributes attrs;
   if (!aOriginAttributes.isObject() || !attrs.Init(aCx, aOriginAttributes)) {
     return NS_ERROR_INVALID_ARG;
   }
 
-  return HasMatchingOverride(aHostName, aPort, attrs, aCert, aOverrideBits,
+  return HasMatchingOverride(aHostName, aPort, attrs, aCert, aIsTemporary,
-                             aIsTemporary, aRetval);
+                             aRetval);
 }
 
 nsresult nsCertOverrideService::AddEntryToList(
     const nsACString& aHostName, int32_t aPort,
     const OriginAttributes& aOriginAttributes, nsIX509Cert* aCert,
     const bool aIsTemporary, const nsACString& fingerprint,
-    nsCertOverride::OverrideBits ob, const nsACString& dbKey,
+    const nsACString& dbKey, const MutexAutoLock& aProofOfLock) {
-    const MutexAutoLock& aProofOfLock) {
   mMutex.AssertCurrentThreadOwns();
   nsAutoCString keyString;
   GetKeyString(aHostName, aPort, aOriginAttributes, keyString);
@@ -637,7 +570,6 @@ nsresult nsCertOverrideService::AddEntryToList(
   settings->mOriginAttributes = aOriginAttributes;
   settings->mIsTemporary = aIsTemporary;
   settings->mFingerprint = fingerprint;
-  settings->mOverrideBits = ob;
   settings->mDBKey = dbKey;
   // remove whitespace from stored dbKey for backwards compatibility
   settings->mDBKey.StripWhitespace();

security/manager/ssl/nsCertOverrideService.h

@@ -12,7 +12,6 @@
 #include "mozilla/HashFunctions.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/TaskQueue.h"
-#include "mozilla/TypedEnumBits.h"
 #include "nsIAsyncShutdown.h"
 #include "nsICertOverrideService.h"
 #include "nsIFile.h"
@@ -27,34 +26,20 @@ class nsCertOverride final : public nsICertOverride {
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSICERTOVERRIDE
 
-  enum class OverrideBits {
+  nsCertOverride() : mPort(-1), mIsTemporary(false) {}
-    None = 0,
-    Untrusted = nsICertOverrideService::ERROR_UNTRUSTED,
-    Mismatch = nsICertOverrideService::ERROR_MISMATCH,
-    Time = nsICertOverrideService::ERROR_TIME,
-  };
-
-  nsCertOverride()
-      : mPort(-1), mIsTemporary(false), mOverrideBits(OverrideBits::None) {}
 
   nsCString mAsciiHost;
   int32_t mPort;
   OriginAttributes mOriginAttributes;
   bool mIsTemporary;  // true: session only, false: stored on disk
   nsCString mFingerprint;
-  OverrideBits mOverrideBits;
   nsCString mDBKey;
   nsCOMPtr<nsIX509Cert> mCert;
 
-  static void convertBitsToString(OverrideBits ob, nsACString& str);
-  static void convertStringToBits(const nsACString& str, OverrideBits& ob);
-
  private:
   ~nsCertOverride() = default;
 };
 
-MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(nsCertOverride::OverrideBits)
-
 // hash entry class
 class nsCertOverrideEntry final : public PLDHashEntryHdr {
  public:
@@ -147,7 +132,6 @@ class nsCertOverrideService final : public nsICertOverrideService,
                           const OriginAttributes& aOriginAttributes,
                           nsIX509Cert* aCert, const bool aIsTemporary,
                           const nsACString& fingerprint,
-                          nsCertOverride::OverrideBits ob,
                           const nsACString& dbKey,
                           const mozilla::MutexAutoLock& aProofOfLock);
   already_AddRefed<nsCertOverride> GetOverrideFor(

security/manager/ssl/nsICertOverrideService.idl

@@ -57,59 +57,32 @@ interface nsICertOverride : nsISupports {
   readonly attribute jsval originAttributes;
 };
 
-/**
- * This represents the global list of triples
- *   {host:port, cert-fingerprint, allowed-overrides}
- * that the user wants to accept without further warnings.
- */
 [scriptable, builtinclass, uuid(be019e47-22fc-4355-9f16-9ab047d6742d)]
 interface nsICertOverrideService : nsISupports {
-
   /**
-   *  Override Untrusted
+   * When making a TLS connection to the given hostname and port (in the
-   */
+   * context of the given origin attributes), if the certificate verifier
-  const short ERROR_UNTRUSTED = 1;
+   * encounters an overridable error when verifying the given certificate, the
-
+   * connection will continue (provided overrides are allowed for that host).
-  /**
-   *  Override hostname Mismatch
-   */
-  const short ERROR_MISMATCH = 2;
-
-  /**
-   *  Override Time error
-   */
-  const short ERROR_TIME = 4;
-
-  /**
-   *  The given cert should always be accepted for the given hostname:port,
-   *  regardless of errors verifying the cert.
-   *  Host:Port is a primary key, only one entry per host:port can exist.
-   *  The implementation will store a fingerprint of the cert.
-   *  The implementation will decide which fingerprint alg is used.
    *
-   *  Each override is specific to exactly the errors overridden, so
+   * @param aHostName The host (punycode) this mapping belongs to
-   *  overriding everything won't match certs at the given host:port
+   * @param aPort The port this mapping belongs to. If it is -1 then it
-   *  which only exhibit some subset of errors.
+   *              is internaly treated as 443.
-   *
+   * @param aOriginAttributes the origin attributes of the mapping
-   *  @param aHostName The host (punycode) this mapping belongs to
+   * @param aCert The certificate used by the server
-   *  @param aPort The port this mapping belongs to, if it is -1 then it
+   * @param aTemporary Whether or not to only store the mapping for the session
-   *          is internaly treated as 443
-   *  @param aCert The cert that should always be accepted
-   *  @param aOverrideBits The precise set of errors we want to be overriden
    */
   [binaryname(RememberValidityOverride), noscript, must_use]
   void rememberValidityOverrideNative(in AUTF8String aHostName,
                                 in int32_t aPort,
                                 in const_OriginAttributesRef aOriginAttributes,
                                 in nsIX509Cert aCert,
-                                in uint32_t aOverrideBits,
                                 in boolean aTemporary);
   [binaryname(RememberValidityOverrideScriptable), implicit_jscontext, must_use]
   void rememberValidityOverride(in AUTF8String aHostName,
                                 in int32_t aPort,
                                 in jsval aOriginAttributes,
                                 in nsIX509Cert aCert,
-                                in uint32_t aOverrideBits,
                                 in boolean aTemporary);
 
   /**
@@ -122,7 +95,6 @@ interface nsICertOverrideService : nsISupports {
    *  @param aPort The port this mapping belongs to, if it is -1 then it
    *         is internally treated as 443
    *  @param aCert The certificate this mapping belongs to
-   *  @param aOverrideBits The errors that are currently overridden
    *  @param aIsTemporary Whether the stored override is session-only,
    *         or permanent
    *  @return Whether an override has been stored for this host+port+cert
@@ -132,14 +104,12 @@ interface nsICertOverrideService : nsISupports {
                               in int32_t aPort,
                               in const_OriginAttributesRef aOriginAttributes,
                               in nsIX509Cert aCert,
-                              out uint32_t aOverrideBits,
                               out boolean aIsTemporary);
   [binaryname(HasMatchingOverrideScriptable), implicit_jscontext, must_use]
   boolean hasMatchingOverride(in AUTF8String aHostName,
                               in int32_t aPort,
                               in jsval aOriginAttributes,
                               in nsIX509Cert aCert,
-                              out uint32_t aOverrideBits,
                               out boolean aIsTemporary);
 
   /**

security/manager/ssl/tests/mochitest/browser/browser_certificateManager.js

@@ -118,7 +118,6 @@ add_task(async function test_cert_manager_server_tab() {
     443,
     {},
     cert,
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     false
   );
 

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

@@ -909,47 +909,23 @@ function stopOCSPResponder(responder) {
 
 // Helper function for add_cert_override_test. Probably doesn't need to be
 // called directly.
-function add_cert_override(aHost, aExpectedBits, aSecurityInfo) {
+function add_cert_override(aHost, aSecurityInfo) {
-  let bits =
-    (aSecurityInfo.isUntrusted
-      ? Ci.nsICertOverrideService.ERROR_UNTRUSTED
-      : 0) |
-    (aSecurityInfo.isDomainMismatch
-      ? Ci.nsICertOverrideService.ERROR_MISMATCH
-      : 0) |
-    (aSecurityInfo.isNotValidAtThisTime
-      ? Ci.nsICertOverrideService.ERROR_TIME
-      : 0);
-
-  Assert.equal(
-    bits,
-    aExpectedBits,
-    "Actual and expected override bits should match"
-  );
   let cert = aSecurityInfo.serverCert;
   let certOverrideService = Cc[
     "@mozilla.org/security/certoverride;1"
   ].getService(Ci.nsICertOverrideService);
-  certOverrideService.rememberValidityOverride(
+  certOverrideService.rememberValidityOverride(aHost, 8443, {}, cert, true);
-    aHost,
-    8443,
-    {},
-    cert,
-    aExpectedBits,
-    true
-  );
 }
 
-// Given a host, expected error bits (see nsICertOverrideService.idl), and an
+// Given a host and an expected error code, tests that an initial connection to
-// expected error code, tests that an initial connection to the host fails
+// the host fails with the expected error and that adding an override results
-// with the expected errors and that adding an override results in a subsequent
+// in a subsequent connection succeeding.
-// connection succeeding.
+function add_cert_override_test(aHost, aExpectedError) {
-function add_cert_override_test(aHost, aExpectedBits, aExpectedError) {
   add_connection_test(
     aHost,
     aExpectedError,
     null,
-    add_cert_override.bind(this, aHost, aExpectedBits)
+    add_cert_override.bind(this, aHost)
   );
   add_connection_test(aHost, PRErrorCodeSuccess, null, aSecurityInfo => {
     Assert.ok(
@@ -964,54 +940,28 @@ function add_cert_override_test(aHost, aExpectedBits, aExpectedError) {
 // add_cert_override except it may not be the case that the connection has an
 // SecInfo set on it. In this case, the error was not overridable anyway, so
 // we consider it a success.
-function attempt_adding_cert_override(aHost, aExpectedBits, aSecurityInfo) {
+function attempt_adding_cert_override(aHost, aSecurityInfo) {
   if (aSecurityInfo.serverCert) {
-    let bits =
-      (aSecurityInfo.isUntrusted
-        ? Ci.nsICertOverrideService.ERROR_UNTRUSTED
-        : 0) |
-      (aSecurityInfo.isDomainMismatch
-        ? Ci.nsICertOverrideService.ERROR_MISMATCH
-        : 0) |
-      (aSecurityInfo.isNotValidAtThisTime
-        ? Ci.nsICertOverrideService.ERROR_TIME
-        : 0);
-    Assert.equal(
-      bits,
-      aExpectedBits,
-      "Actual and expected override bits should match"
-    );
     let cert = aSecurityInfo.serverCert;
     let certOverrideService = Cc[
       "@mozilla.org/security/certoverride;1"
     ].getService(Ci.nsICertOverrideService);
-    certOverrideService.rememberValidityOverride(
+    certOverrideService.rememberValidityOverride(aHost, 8443, {}, cert, true);
-      aHost,
-      8443,
-      {},
-      cert,
-      aExpectedBits,
-      true
-    );
   }
 }
 
-// Given a host, expected error bits (see nsICertOverrideService.idl), and
+// Given a host and an expected error code, tests that an initial connection to
-// an expected error code, tests that an initial connection to the host fails
+// the host fails with the expected error and that adding an override does not
-// with the expected errors and that adding an override does not result in a
+// result in a subsequent connection succeeding (i.e. the same error code is
-// subsequent connection succeeding (i.e. the same error code is encountered).
+// encountered).
 // The idea here is that for HSTS hosts or hosts with key pins, no error is
 // overridable, even if an entry is added to the override service.
-function add_prevented_cert_override_test(
+function add_prevented_cert_override_test(aHost, aExpectedError) {
-  aHost,
-  aExpectedBits,
-  aExpectedError
-) {
   add_connection_test(
     aHost,
     aExpectedError,
     null,
-    attempt_adding_cert_override.bind(this, aHost, aExpectedBits)
+    attempt_adding_cert_override.bind(this, aHost)
   );
   add_connection_test(aHost, aExpectedError);
 }

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

@@ -1,117 +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";
-
-// Test that when an override exists for a (host, certificate) pair,
-// connections will succeed only if the set of error bits in the override
-// contains each bit in the set of encountered error bits.
-// Strangely, it is possible to store an override with an empty set of error
-// bits, so this tests that too.
-
-do_get_profile();
-
-function add_override_bits_mismatch_test(
-  host,
-  certPath,
-  expectedBits,
-  expectedError
-) {
-  const MAX_BITS =
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_MISMATCH |
-    Ci.nsICertOverrideService.ERROR_TIME;
-  let cert = constructCertFromFile(certPath);
-  let certOverrideService = Cc[
-    "@mozilla.org/security/certoverride;1"
-  ].getService(Ci.nsICertOverrideService);
-  for (let overrideBits = 0; overrideBits <= MAX_BITS; overrideBits++) {
-    add_test(function() {
-      certOverrideService.clearValidityOverride(host, 8443, {});
-      certOverrideService.rememberValidityOverride(
-        host,
-        8443,
-        {},
-        cert,
-        overrideBits,
-        true
-      );
-      run_next_test();
-    });
-    // The override will succeed only if the set of error bits in the override
-    // contains each bit in the set of expected error bits.
-    let successExpected = (overrideBits | expectedBits) == overrideBits;
-    add_connection_test(
-      host,
-      successExpected ? PRErrorCodeSuccess : expectedError
-    );
-  }
-}
-
-function run_test() {
-  Services.prefs.setIntPref("security.OCSP.enabled", 1);
-  add_tls_server_setup("BadCertAndPinningServer", "bad_certs");
-
-  let fakeOCSPResponder = new HttpServer();
-  fakeOCSPResponder.registerPrefixHandler("/", function(request, response) {
-    response.setStatusLine(request.httpVersion, 500, "Internal Server Error");
-  });
-  fakeOCSPResponder.start(8888);
-
-  add_override_bits_mismatch_test(
-    "unknownissuer.example.com",
-    "bad_certs/unknownissuer.pem",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-    SEC_ERROR_UNKNOWN_ISSUER
-  );
-  add_override_bits_mismatch_test(
-    "mismatch.example.com",
-    "bad_certs/mismatch.pem",
-    Ci.nsICertOverrideService.ERROR_MISMATCH,
-    SSL_ERROR_BAD_CERT_DOMAIN
-  );
-  add_override_bits_mismatch_test(
-    "expired.example.com",
-    "bad_certs/expired-ee.pem",
-    Ci.nsICertOverrideService.ERROR_TIME,
-    SEC_ERROR_EXPIRED_CERTIFICATE
-  );
-
-  add_override_bits_mismatch_test(
-    "mismatch-untrusted.example.com",
-    "bad_certs/mismatch-untrusted.pem",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-      Ci.nsICertOverrideService.ERROR_MISMATCH,
-    SEC_ERROR_UNKNOWN_ISSUER
-  );
-  add_override_bits_mismatch_test(
-    "untrusted-expired.example.com",
-    "bad_certs/untrusted-expired.pem",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-      Ci.nsICertOverrideService.ERROR_TIME,
-    SEC_ERROR_UNKNOWN_ISSUER
-  );
-  add_override_bits_mismatch_test(
-    "mismatch-expired.example.com",
-    "bad_certs/mismatch-expired.pem",
-    Ci.nsICertOverrideService.ERROR_MISMATCH |
-      Ci.nsICertOverrideService.ERROR_TIME,
-    SSL_ERROR_BAD_CERT_DOMAIN
-  );
-
-  add_override_bits_mismatch_test(
-    "mismatch-untrusted-expired.example.com",
-    "bad_certs/mismatch-untrusted-expired.pem",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-      Ci.nsICertOverrideService.ERROR_MISMATCH |
-      Ci.nsICertOverrideService.ERROR_TIME,
-    SEC_ERROR_UNKNOWN_ISSUER
-  );
-
-  add_test(function() {
-    fakeOCSPResponder.stop(run_next_test);
-  });
-
-  run_next_test();
-}

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

@@ -38,58 +38,58 @@ function run_test() {
     "# This is a generated file!  Do not edit.",
     "test.example.com:443:^privateBrowsingId=1\tOID.2.16.840.1.101.3.4.2.1\t" +
       cert1.sha256Fingerprint +
-      "\tM\t" +
+      "\t\t" +
       cert1.dbKey,
     "test.example.com:443:^privateBrowsingId=2\tOID.2.16.840.1.101.3.4.2.1\t" +
+      cert1.sha256Fingerprint +
+      "\t\t" +
+      cert1.dbKey,
+    "test.example.com:443:^privateBrowsingId=3\tOID.2.16.840.1.101.3.4.2.1\t" + // includes bits (now obsolete)
       cert1.sha256Fingerprint +
       "\tM\t" +
       cert1.dbKey,
     "example.com:443:\tOID.2.16.840.1.101.3.4.2.1\t" +
       cert2.sha256Fingerprint +
-      "\tU\t" +
+      "\t\t" +
       cert2.dbKey,
     "[::1]:443:\tOID.2.16.840.1.101.3.4.2.1\t" + // IPv6
       cert2.sha256Fingerprint +
-      "\tM\t" +
+      "\t\t" +
       cert2.dbKey,
     "old.example.com:443\tOID.2.16.840.1.101.3.4.2.1\t" + // missing attributes (defaulted)
       cert1.sha256Fingerprint +
-      "\tM\t" +
+      "\t\t" +
       cert1.dbKey,
     ":443:\tOID.2.16.840.1.101.3.4.2.1\t" + // missing host name
       cert3.sha256Fingerprint +
-      "\tU\t" +
+      "\t\t" +
       cert3.dbKey,
     "example.com::\tOID.2.16.840.1.101.3.4.2.1\t" + // missing port
       cert3.sha256Fingerprint +
-      "\tU\t" +
+      "\t\t" +
       cert3.dbKey,
     "example.com:443:\tOID.2.16.840.1.101.3.4.2.1\t" + // wrong fingerprint/dbkey
       cert2.sha256Fingerprint +
-      "\tU\t" +
+      "\t\t" +
       cert3.dbKey,
     "example.com:443:\tOID.0.00.000.0.000.0.0.0.0\t" + // bad OID
       cert3.sha256Fingerprint +
-      "\tU\t" +
+      "\t\t" +
       cert3.dbKey,
     "example.com:443:\t.0.0.0.0\t" + // malformed OID
       cert3.sha256Fingerprint +
-      "\tU\t" +
+      "\t\t" +
       cert3.dbKey,
     "example.com:443:\t\t" + // missing OID
       cert3.sha256Fingerprint +
-      "\tU\t" +
+      "\t\t" +
       cert3.dbKey,
     "example.com:443:\tOID.2.16.840.1.101.3.4.2.1\t" + // missing fingerprint
-      "\tU\t" +
-      cert3.dbKey,
-    "example.com:443:\tOID.2.16.840.1.101.3.4.2.1\t" + // missing override bits
-      cert3.sha256Fingerprint +
       "\t\t" +
       cert3.dbKey,
     "example.com:443:\tOID.2.16.840.1.101.3.4.2.1\t" + // missing dbkey
       cert3.sha256Fingerprint +
-      "\tU\t",
+      "\t\t",
   ];
   writeLinesAndClose(lines, outputStream);
   let overrideService = Cc["@mozilla.org/security/certoverride;1"].getService(
@@ -116,42 +116,42 @@ function run_test() {
       host: "test.example.com",
       port: 443,
       cert: cert1,
-      bits: Ci.nsICertOverrideService.ERROR_MISMATCH,
       attributes: { privateBrowsingId: 1 },
     },
     {
       host: "test.example.com",
       port: 443,
       cert: cert1,
-      bits: Ci.nsICertOverrideService.ERROR_MISMATCH,
       attributes: { privateBrowsingId: 2 },
     },
+    {
+      host: "test.example.com",
+      port: 443,
+      cert: cert1,
+      attributes: { privateBrowsingId: 3 },
+    },
     {
       host: "example.com",
       port: 443,
       cert: cert2,
-      bits: Ci.nsICertOverrideService.ERROR_UNTRUSTED,
       attributes: {},
     },
     {
       host: "::1",
       port: 443,
       cert: cert2,
-      bits: Ci.nsICertOverrideService.ERROR_MISMATCH,
       attributes: {},
     },
     {
       host: "example.com",
       port: 443,
       cert: cert2,
-      bits: Ci.nsICertOverrideService.ERROR_UNTRUSTED,
       attributes: { userContextId: 1 }, // only privateBrowsingId is used
     },
     {
       host: "old.example.com",
       port: 443,
       cert: cert1,
-      bits: Ci.nsICertOverrideService.ERROR_MISMATCH,
       attributes: {},
     },
   ];
@@ -160,36 +160,23 @@ function run_test() {
       host: "test.example.com",
       port: 443,
       cert: cert1,
-      bits: Ci.nsICertOverrideService.ERROR_MISMATCH,
+      attributes: { privateBrowsingId: 4 }, // wrong attributes
-      attributes: { privateBrowsingId: 3 }, // wrong attributes
     },
     {
       host: "test.example.com",
       port: 443,
       cert: cert3, // wrong certificate
-      bits: Ci.nsICertOverrideService.ERROR_UNTRUSTED,
       attributes: { privateBrowsingId: 1 },
     },
     {
       host: "example.com",
       port: 443,
       cert: cert3,
-      bits: Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-      attributes: {},
-    },
-  ];
-  const BAD_BIT_OVERRIDES = [
-    {
-      host: "example.com",
-      port: 443,
-      cert: cert2,
-      bits: Ci.nsICertOverrideService.ERROR_MISMATCH, // wrong bits
       attributes: {},
     },
   ];
 
   for (let override of OVERRIDES) {
-    let actualBits = {};
     let temp = {};
     ok(
       overrideService.hasMatchingOverride(
@@ -197,17 +184,14 @@ function run_test() {
         override.port,
         override.attributes,
         override.cert,
-        actualBits,
         temp
       ),
       `${JSON.stringify(override)} should have an override`
     );
-    equal(actualBits.value, override.bits);
     equal(temp.value, false);
   }
 
   for (let override of BAD_OVERRIDES) {
-    let actualBits = {};
     let temp = {};
     ok(
       !overrideService.hasMatchingOverride(
@@ -215,27 +199,9 @@ function run_test() {
         override.port,
         override.attributes,
         override.cert,
-        actualBits,
         temp
       ),
       `${override} should not have an override`
     );
   }
-
-  for (let override of BAD_BIT_OVERRIDES) {
-    let actualBits = {};
-    let temp = {};
-    ok(
-      overrideService.hasMatchingOverride(
-        override.host,
-        override.port,
-        override.attributes,
-        override.cert,
-        actualBits,
-        temp
-      ),
-      `${override} should have an override`
-    );
-    notEqual(actualBits.value, override.bits);
-  }
 }

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

@@ -7,8 +7,8 @@
 // Tests the certificate overrides we allow.
 // add_cert_override_test will queue a test that does the following:
 // 1. Attempt to connect to the given host. This should fail with the
-//    given error and override bits.
+//    given error.
-// 2. Add an override for that host/port/certificate/override bits.
+// 2. Add an override for that host/port/certificate.
 // 3. Connect again. This should succeed.
 
 do_get_profile();
@@ -55,12 +55,12 @@ function check_telemetry() {
   );
   equal(
     histogram.values[9],
-    13,
+    9,
     "Actual and expected SSL_ERROR_BAD_CERT_DOMAIN values should match"
   );
   equal(
     histogram.values[10],
-    5,
+    1,
     "Actual and expected SEC_ERROR_EXPIRED_CERTIFICATE values should match"
   );
   equal(
@@ -80,7 +80,7 @@ function check_telemetry() {
   );
   equal(
     histogram.values[14],
-    2,
+    1,
     "Actual and expected MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE values should match"
   );
   equal(
@@ -90,7 +90,7 @@ function check_telemetry() {
   );
   equal(
     histogram.values[16],
-    3,
+    2,
     "Actual and expected SEC_ERROR_INVALID_TIME values should match"
   );
   equal(
@@ -146,17 +146,14 @@ function run_port_equivalency_test(inPort, outPort) {
     "@mozilla.org/security/certoverride;1"
   ].getService(Ci.nsICertOverrideService);
   let cert = constructCertFromFile("bad_certs/default-ee.pem");
-  let expectedBits = Ci.nsICertOverrideService.ERROR_UNTRUSTED;
   let expectedTemporary = true;
   certOverrideService.rememberValidityOverride(
     "example.com",
     inPort,
     {},
     cert,
-    expectedBits,
     expectedTemporary
   );
-  let actualBits = {};
   let actualTemporary = {};
   Assert.ok(
     certOverrideService.hasMatchingOverride(
@@ -164,30 +161,17 @@ function run_port_equivalency_test(inPort, outPort) {
       outPort,
       {},
       cert,
-      actualBits,
       actualTemporary
     ),
     `override set on port ${inPort} should match port ${outPort}`
   );
-  equal(
-    actualBits.value,
-    expectedBits,
-    "input override bits should match output bits"
-  );
   equal(
     actualTemporary.value,
     expectedTemporary,
     "input override temporary value should match output temporary value"
   );
   Assert.ok(
-    !certOverrideService.hasMatchingOverride(
+    !certOverrideService.hasMatchingOverride("example.com", 563, {}, cert, {}),
-      "example.com",
-      563,
-      {},
-      cert,
-      {},
-      {}
-    ),
     `override set on port ${inPort} should not match port 563`
   );
   certOverrideService.clearValidityOverride("example.com", inPort, {});
@@ -197,12 +181,10 @@ function run_port_equivalency_test(inPort, outPort) {
       outPort,
       {},
       cert,
-      actualBits,
       {}
     ),
     `override cleared on port ${inPort} should match port ${outPort}`
   );
-  equal(actualBits.value, 0, "should have no bits set if there is no override");
 }
 
 function run_test() {
@@ -231,98 +213,63 @@ function run_test() {
 }
 
 function add_simple_tests() {
-  add_cert_override_test(
+  add_cert_override_test("expired.example.com", SEC_ERROR_EXPIRED_CERTIFICATE);
-    "expired.example.com",
-    Ci.nsICertOverrideService.ERROR_TIME,
-    SEC_ERROR_EXPIRED_CERTIFICATE
-  );
   add_cert_override_test(
     "notyetvalid.example.com",
-    Ci.nsICertOverrideService.ERROR_TIME,
     MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE
   );
-  add_cert_override_test(
+  add_cert_override_test("before-epoch.example.com", SEC_ERROR_INVALID_TIME);
-    "before-epoch.example.com",
-    Ci.nsICertOverrideService.ERROR_TIME,
-    SEC_ERROR_INVALID_TIME
-  );
   add_cert_override_test(
     "before-epoch-self-signed.example.com",
-    Ci.nsICertOverrideService.ERROR_TIME |
-      Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT
   );
   add_cert_override_test(
     "selfsigned.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT
   );
-  add_cert_override_test(
+  add_cert_override_test("unknownissuer.example.com", SEC_ERROR_UNKNOWN_ISSUER);
-    "unknownissuer.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-    SEC_ERROR_UNKNOWN_ISSUER
-  );
   add_cert_override_test(
     "expiredissuer.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE
   );
   add_cert_override_test(
     "notyetvalidissuer.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_NOT_YET_VALID_ISSUER_CERTIFICATE
   );
   add_cert_override_test(
     "before-epoch-issuer.example.com",
-    Ci.nsICertOverrideService.ERROR_TIME,
     SEC_ERROR_INVALID_TIME
   );
   add_cert_override_test(
     "md5signature.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED
   );
   add_cert_override_test(
     "emptyissuername.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_EMPTY_ISSUER_NAME
   );
   // This has name information in the subject alternative names extension,
   // but not the subject common name.
-  add_cert_override_test(
+  add_cert_override_test("mismatch.example.com", SSL_ERROR_BAD_CERT_DOMAIN);
-    "mismatch.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH,
-    SSL_ERROR_BAD_CERT_DOMAIN
-  );
   // This has name information in the subject common name but not the subject
   // alternative names extension.
-  add_cert_override_test(
+  add_cert_override_test("mismatch-CN.example.com", SSL_ERROR_BAD_CERT_DOMAIN);
-    "mismatch-CN.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH,
-    SSL_ERROR_BAD_CERT_DOMAIN
-  );
 
   // A Microsoft IIS utility generates self-signed certificates with
   // properties similar to the one this "host" will present.
   add_cert_override_test(
     "selfsigned-inadequateEKU.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT
   );
 
   add_prevented_cert_override_test(
     "inadequatekeyusage.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_INADEQUATE_KEY_USAGE
   );
 
   // Test triggering the MitM detection. We don't set-up a proxy here. Just
   // set the pref. Without the pref set we expect an unkown issuer error.
-  add_cert_override_test(
+  add_cert_override_test("mitm.example.com", SEC_ERROR_UNKNOWN_ISSUER);
-    "mitm.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-    SEC_ERROR_UNKNOWN_ISSUER
-  );
   add_test(function() {
     Services.prefs.setStringPref(
       "security.pki.mitm_canary_issuer",
@@ -334,11 +281,7 @@ function add_simple_tests() {
     certOverrideService.clearValidityOverride("mitm.example.com", 8443, {});
     run_next_test();
   });
-  add_cert_override_test(
+  add_cert_override_test("mitm.example.com", MOZILLA_PKIX_ERROR_MITM_DETECTED);
-    "mitm.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-    MOZILLA_PKIX_ERROR_MITM_DETECTED
-  );
   add_test(function() {
     Services.prefs.setStringPref(
       "security.pki.mitm_canary_issuer",
@@ -352,11 +295,7 @@ function add_simple_tests() {
   });
   // If the canary issuer doesn't match the one we see, we exepct and unknown
   // issuer error.
-  add_cert_override_test(
+  add_cert_override_test("mitm.example.com", SEC_ERROR_UNKNOWN_ISSUER);
-    "mitm.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-    SEC_ERROR_UNKNOWN_ISSUER
-  );
   // If security.pki.mitm_canary_issuer.enabled is false, there should always
   // be an unknown issuer error.
   add_test(function() {
@@ -370,11 +309,7 @@ function add_simple_tests() {
     certOverrideService.clearValidityOverride("mitm.example.com", 8443, {});
     run_next_test();
   });
-  add_cert_override_test(
+  add_cert_override_test("mitm.example.com", SEC_ERROR_UNKNOWN_ISSUER);
-    "mitm.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-    SEC_ERROR_UNKNOWN_ISSUER
-  );
   add_test(function() {
     Services.prefs.clearUserPref("security.pki.mitm_canary_issuer");
     run_next_test();
@@ -391,7 +326,6 @@ function add_simple_tests() {
   });
   add_prevented_cert_override_test(
     "nsCertTypeCritical.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_CRITICAL_EXTENSION
   );
   add_test(function() {
@@ -405,13 +339,11 @@ function add_simple_tests() {
   // is a scenario in which an override is allowed.
   add_cert_override_test(
     "self-signed-end-entity-with-cA-true.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT
   );
 
   add_cert_override_test(
     "ca-used-as-end-entity.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_CA_CERT_USED_AS_END_ENTITY
   );
 
@@ -419,7 +351,6 @@ function add_simple_tests() {
   // encounter an overridable error.
   add_cert_override_test(
     "end-entity-issued-by-v1-cert.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_V1_CERT_USED_AS_CA
   );
   // If we make that certificate a trust anchor, the connection will succeed.
@@ -453,36 +384,27 @@ function add_simple_tests() {
   // certificates that are not valid CAs.
   add_cert_override_test(
     "end-entity-issued-by-non-CA.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_CA_CERT_INVALID
   );
 
   // This host presents a 1016-bit RSA key.
   add_cert_override_test(
     "inadequate-key-size-ee.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_INADEQUATE_KEY_SIZE
   );
 
   add_cert_override_test(
     "ipAddressAsDNSNameInSAN.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH,
-    SSL_ERROR_BAD_CERT_DOMAIN
-  );
-  add_cert_override_test(
-    "noValidNames.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH,
     SSL_ERROR_BAD_CERT_DOMAIN
   );
+  add_cert_override_test("noValidNames.example.com", SSL_ERROR_BAD_CERT_DOMAIN);
   add_cert_override_test(
     "badSubjectAltNames.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH,
     SSL_ERROR_BAD_CERT_DOMAIN
   );
 
   add_cert_override_test(
     "bug413909.xn--hxajbheg2az3al.xn--jxalpdlp",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_test(function() {
@@ -501,7 +423,6 @@ function add_simple_tests() {
         8443,
         {},
         cert,
-        {},
         {}
       ),
       "IDN certificate should have matching override using ascii host"
@@ -513,7 +434,6 @@ function add_simple_tests() {
           8443,
           {},
           cert,
-          {},
           {}
         ),
       /NS_ERROR_ILLEGAL_VALUE/,
@@ -529,7 +449,6 @@ function add_simple_tests() {
           8443,
           {},
           cert,
-          {},
           {}
         ),
       /NS_ERROR_ILLEGAL_VALUE/,
@@ -544,24 +463,15 @@ function add_simple_tests() {
       "@mozilla.org/security/certoverride;1"
     ].getService(Ci.nsICertOverrideService);
     let cert = constructCertFromFile("bad_certs/default-ee.pem");
-    let expectedBits = Ci.nsICertOverrideService.ERROR_UNTRUSTED;
     certOverrideService.rememberValidityOverride(
       "example.com",
       443,
       {},
       cert,
-      expectedBits,
       false
     );
     Assert.ok(
-      certOverrideService.hasMatchingOverride(
+      certOverrideService.hasMatchingOverride("example.com", 443, {}, cert, {}),
-        "example.com",
-        443,
-        {},
-        cert,
-        {},
-        {}
-      ),
       "Should have added override for example.com:443"
     );
     certOverrideService.rememberValidityOverride(
@@ -569,26 +479,11 @@ function add_simple_tests() {
       80,
       {},
       cert,
-      expectedBits,
-      false
-    );
-    certOverrideService.rememberValidityOverride(
-      "::1",
-      80,
-      {},
-      cert,
-      expectedBits,
       false
     );
+    certOverrideService.rememberValidityOverride("::1", 80, {}, cert, false);
     Assert.ok(
-      certOverrideService.hasMatchingOverride(
+      certOverrideService.hasMatchingOverride("example.com", 80, {}, cert, {}),
-        "example.com",
-        80,
-        {},
-        cert,
-        {},
-        {}
-      ),
       "Should have added override for example.com:80"
     );
     certOverrideService.rememberValidityOverride(
@@ -596,22 +491,14 @@ function add_simple_tests() {
       443,
       {},
       cert,
-      expectedBits,
       false
     );
     Assert.ok(
-      certOverrideService.hasMatchingOverride(
+      certOverrideService.hasMatchingOverride("example.org", 443, {}, cert, {}),
-        "example.org",
-        443,
-        {},
-        cert,
-        {},
-        {}
-      ),
       "Should have added override for example.org:443"
     );
     Assert.ok(
-      certOverrideService.hasMatchingOverride("::1", 80, {}, cert, {}, {}),
+      certOverrideService.hasMatchingOverride("::1", 80, {}, cert, {}),
       "Should have added override for [::1]:80"
     );
     // When in a private browsing context, overrides added in non-private
@@ -622,7 +509,6 @@ function add_simple_tests() {
         443,
         { privateBrowsingId: 1 },
         cert,
-        {},
         {}
       ),
       "Should have override for example.org:443 with privateBrowsingId 1"
@@ -633,7 +519,6 @@ function add_simple_tests() {
         443,
         { privateBrowsingId: 2 },
         cert,
-        {},
         {}
       ),
       "Should have override for example.org:443 with privateBrowsingId 2"
@@ -644,7 +529,6 @@ function add_simple_tests() {
         443,
         { firstPartyDomain: "example.org", userContextId: 1 },
         cert,
-        {},
         {}
       ),
       "Should ignore firstPartyDomain and userContextId when checking overrides"
@@ -654,18 +538,10 @@ function add_simple_tests() {
       80,
       {},
       cert,
-      expectedBits,
       true
     );
     Assert.ok(
-      certOverrideService.hasMatchingOverride(
+      certOverrideService.hasMatchingOverride("example.org", 80, {}, cert, {}),
-        "example.org",
-        80,
-        {},
-        cert,
-        {},
-        {}
-      ),
       "Should have added override for example.org:80"
     );
     certOverrideService.rememberValidityOverride(
@@ -673,7 +549,6 @@ function add_simple_tests() {
       443,
       { firstPartyDomain: "example.org", userContextId: 1 },
       cert,
-      expectedBits,
       false
     );
     Assert.ok(
@@ -682,7 +557,6 @@ function add_simple_tests() {
         443,
         {},
         cert,
-        {},
         {}
       ),
       "Should ignore firstPartyDomain and userContextId when adding overrides"
@@ -693,7 +567,6 @@ function add_simple_tests() {
         443,
         { firstPartyDomain: "example.com", userContextId: 2 },
         cert,
-        {},
         {}
       ),
       "Should ignore firstPartyDomain and userContextId when checking overrides"
@@ -703,7 +576,6 @@ function add_simple_tests() {
       443,
       { privateBrowsingId: 1 },
       cert,
-      expectedBits,
       false
     );
     Assert.ok(
@@ -712,7 +584,6 @@ function add_simple_tests() {
         443,
         { privateBrowsingId: 1 },
         cert,
-        {},
         {}
       ),
       "Should have added override for example.test:443 with privateBrowsingId 1"
@@ -723,7 +594,6 @@ function add_simple_tests() {
         443,
         { privateBrowsingId: 2 },
         cert,
-        {},
         {}
       ),
       "Should not have override for example.test:443 with privateBrowsingId 2"
@@ -734,7 +604,6 @@ function add_simple_tests() {
         443,
         {},
         cert,
-        {},
         {}
       ),
       "Should not have override for example.test:443 with non-private OriginAttributes"
@@ -749,20 +618,12 @@ function add_simple_tests() {
         443,
         {},
         cert,
-        {},
         {}
       ),
       "Should have removed override for example.com:443"
     );
     Assert.ok(
-      !certOverrideService.hasMatchingOverride(
+      !certOverrideService.hasMatchingOverride("example.com", 80, {}, cert, {}),
-        "example.com",
-        80,
-        {},
-        cert,
-        {},
-        {}
-      ),
       "Should have removed override for example.com:80"
     );
     Assert.ok(
@@ -771,20 +632,12 @@ function add_simple_tests() {
         443,
         {},
         cert,
-        {},
         {}
       ),
       "Should have removed override for example.org:443"
     );
     Assert.ok(
-      !certOverrideService.hasMatchingOverride(
+      !certOverrideService.hasMatchingOverride("example.org", 80, {}, cert, {}),
-        "example.org",
-        80,
-        {},
-        cert,
-        {},
-        {}
-      ),
       "Should have removed override for example.org:80"
     );
     Assert.ok(
@@ -793,7 +646,6 @@ function add_simple_tests() {
         443,
         { privateBrowsingId: 1 },
         cert,
-        {},
         {}
       ),
       "Should have removed override for example.org:443 with privateBrowsingId 1"
@@ -804,68 +656,40 @@ function add_simple_tests() {
 }
 
 function add_localhost_tests() {
-  add_cert_override_test(
+  add_cert_override_test("localhost", SEC_ERROR_UNKNOWN_ISSUER);
-    "localhost",
+  add_cert_override_test("127.0.0.1", SSL_ERROR_BAD_CERT_DOMAIN);
-    Ci.nsICertOverrideService.ERROR_MISMATCH |
+  add_cert_override_test("::1", SSL_ERROR_BAD_CERT_DOMAIN);
-      Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-    SEC_ERROR_UNKNOWN_ISSUER
-  );
-  add_cert_override_test(
-    "127.0.0.1",
-    Ci.nsICertOverrideService.ERROR_MISMATCH,
-    SSL_ERROR_BAD_CERT_DOMAIN
-  );
-  add_cert_override_test(
-    "::1",
-    Ci.nsICertOverrideService.ERROR_MISMATCH,
-    SSL_ERROR_BAD_CERT_DOMAIN
-  );
 }
 
 function add_combo_tests() {
   add_cert_override_test(
     "mismatch-expired.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH |
-      Ci.nsICertOverrideService.ERROR_TIME,
     SSL_ERROR_BAD_CERT_DOMAIN
   );
   add_cert_override_test(
     "mismatch-notYetValid.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH |
-      Ci.nsICertOverrideService.ERROR_TIME,
     SSL_ERROR_BAD_CERT_DOMAIN
   );
   add_cert_override_test(
     "mismatch-untrusted.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH |
-      Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_cert_override_test(
     "untrusted-expired.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-      Ci.nsICertOverrideService.ERROR_TIME,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_cert_override_test(
     "mismatch-untrusted-expired.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH |
-      Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-      Ci.nsICertOverrideService.ERROR_TIME,
     SEC_ERROR_UNKNOWN_ISSUER
   );
 
   add_cert_override_test(
     "md5signature-expired.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-      Ci.nsICertOverrideService.ERROR_TIME,
     SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED
   );
 
   add_cert_override_test(
     "ca-used-as-end-entity-name-mismatch.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH |
-      Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_CA_CERT_USED_AS_END_ENTITY
   );
 }
@@ -902,11 +726,7 @@ function add_distrust_test(certFileName, hostName, expectedResult) {
     clearSessionCache();
     run_next_test();
   });
-  add_prevented_cert_override_test(
+  add_prevented_cert_override_test(hostName, expectedResult);
-    hostName,
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
-    expectedResult
-  );
   add_test(function() {
     setCertTrust(certToDistrust, "u,,");
     run_next_test();

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

@@ -9,53 +9,25 @@
 
 // Helper function for add_read_only_cert_override_test. Probably doesn't need
 // to be called directly.
-function add_read_only_cert_override(aHost, aExpectedBits, aSecurityInfo) {
+function add_read_only_cert_override(aHost, aSecurityInfo) {
-  let bits =
-    (aSecurityInfo.isUntrusted
-      ? Ci.nsICertOverrideService.ERROR_UNTRUSTED
-      : 0) |
-    (aSecurityInfo.isDomainMismatch
-      ? Ci.nsICertOverrideService.ERROR_MISMATCH
-      : 0) |
-    (aSecurityInfo.isNotValidAtThisTime
-      ? Ci.nsICertOverrideService.ERROR_TIME
-      : 0);
-
-  Assert.equal(
-    bits,
-    aExpectedBits,
-    "Actual and expected override bits should match"
-  );
   let cert = aSecurityInfo.serverCert;
   let certOverrideService = Cc[
     "@mozilla.org/security/certoverride;1"
   ].getService(Ci.nsICertOverrideService);
   // Setting the last argument to false here ensures that we attempt to store a
   // permanent override (which is what was failing in bug 1427273).
-  certOverrideService.rememberValidityOverride(
+  certOverrideService.rememberValidityOverride(aHost, 8443, {}, cert, false);
-    aHost,
-    8443,
-    {},
-    cert,
-    aExpectedBits,
-    false
-  );
 }
 
-// Given a host, expected error bits (see nsICertOverrideService.idl), and an
+// Given a host and an expected error code, tests that an initial connection to
-// expected error code, tests that an initial connection to the host fails with
+// the host fails with the expected errors and that adding an override results
-// the expected errors and that adding an override results in a subsequent
+// in a subsequent connection succeeding.
-// connection succeeding.
+function add_read_only_cert_override_test(aHost, aExpectedError) {
-function add_read_only_cert_override_test(
-  aHost,
-  aExpectedBits,
-  aExpectedError
-) {
   add_connection_test(
     aHost,
     aExpectedError,
     null,
-    add_read_only_cert_override.bind(this, aHost, aExpectedBits)
+    add_read_only_cert_override.bind(this, aHost)
   );
   add_connection_test(aHost, PRErrorCodeSuccess, null, aSecurityInfo => {
     Assert.ok(
@@ -103,19 +75,14 @@ function run_test() {
   // set in addition to whatever other specific error bits are necessary.
   add_read_only_cert_override_test(
     "expired.example.com",
-    Ci.nsICertOverrideService.ERROR_TIME |
-      Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_read_only_cert_override_test(
     "selfsigned.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT
   );
   add_read_only_cert_override_test(
     "mismatch.example.com",
-    Ci.nsICertOverrideService.ERROR_MISMATCH |
-      Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
 

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

@@ -160,17 +160,7 @@ function storeCertOverride(port, cert) {
   let certOverrideService = Cc[
     "@mozilla.org/security/certoverride;1"
   ].getService(Ci.nsICertOverrideService);
-  let overrideBits =
+  certOverrideService.rememberValidityOverride(hostname, port, {}, cert, true);
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_MISMATCH;
-  certOverrideService.rememberValidityOverride(
-    hostname,
-    port,
-    {},
-    cert,
-    overrideBits,
-    true
-  );
 }
 
 function startClient(port) {

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

@@ -52,7 +52,6 @@ function test_strict() {
   // this host, we don't allow overrides.
   add_prevented_cert_override_test(
     "unknownissuer.include-subdomains.pinning.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_clear_override("unknownissuer.include-subdomains.pinning.example.com");
@@ -103,7 +102,6 @@ function test_strict() {
   // Similarly, this pin is in test-mode, so it should be overridable.
   add_cert_override_test(
     "unknownissuer.test-mode.pinning.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_clear_override("unknownissuer.test-mode.pinning.example.com");
@@ -132,7 +130,6 @@ function test_mitm() {
   // anchor, so we can't allow overrides for it).
   add_prevented_cert_override_test(
     "unknownissuer.include-subdomains.pinning.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_clear_override("unknownissuer.include-subdomains.pinning.example.com");
@@ -155,7 +152,6 @@ function test_mitm() {
   add_connection_test("test-mode.pinning.example.com", PRErrorCodeSuccess);
   add_cert_override_test(
     "unknownissuer.test-mode.pinning.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_clear_override("unknownissuer.test-mode.pinning.example.com");
@@ -192,13 +188,11 @@ function test_disabled() {
 
   add_cert_override_test(
     "unknownissuer.include-subdomains.pinning.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_clear_override("unknownissuer.include-subdomains.pinning.example.com");
   add_cert_override_test(
     "unknownissuer.test-mode.pinning.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_clear_override("unknownissuer.test-mode.pinning.example.com");
@@ -215,7 +209,6 @@ function test_enforce_test_mode() {
   // this host, we don't allow overrides.
   add_prevented_cert_override_test(
     "unknownissuer.include-subdomains.pinning.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_clear_override("unknownissuer.include-subdomains.pinning.example.com");
@@ -259,7 +252,6 @@ function test_enforce_test_mode() {
   // this host (and since we're enforcing test mode), we don't allow overrides.
   add_prevented_cert_override_test(
     "unknownissuer.test-mode.pinning.example.com",
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED,
     SEC_ERROR_UNKNOWN_ISSUER
   );
   add_clear_override("unknownissuer.test-mode.pinning.example.com");

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

@@ -31,11 +31,7 @@ addCertFromFile(certdb, "bad_certs/ev-test-intermediate.pem", ",,");
 // information object.
 function add_resume_non_ev_with_override_test() {
   // This adds the override and makes one successful connection.
-  add_cert_override_test(
+  add_cert_override_test("expired.example.com", SEC_ERROR_EXPIRED_CERTIFICATE);
-    "expired.example.com",
-    Ci.nsICertOverrideService.ERROR_TIME,
-    SEC_ERROR_EXPIRED_CERTIFICATE
-  );
 
   // This connects again, using session resumption. Note that we don't clear
   // the TLS session cache between these operations (that would defeat the

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

@@ -66,7 +66,6 @@ function run_test() {
   };
   add_cert_override_test(
     "expired.example.com",
-    Ci.nsICertOverrideService.ERROR_TIME,
     SEC_ERROR_EXPIRED_CERTIFICATE,
     undefined,
     overrideStatus

security/manager/ssl/tests/unit/xpcshell.ini

@@ -81,8 +81,6 @@ run-if = nightly_build
 run-sequentially = hardcoded ports
 [test_cert_overrides_read_only.js]
 run-sequentially = hardcoded ports
-[test_cert_override_bits_mismatches.js]
-run-sequentially = hardcoded ports
 [test_cert_override_read.js]
 [test_cert_sha1.js]
 [test_cert_signatures.js]

toolkit/components/cleardata/tests/unit/test_certs.js

@@ -30,7 +30,6 @@ add_task(async function() {
       TEST_URI.port,
       {},
       cert,
-      {},
       {}
     ),
     `Should not have override for ${TEST_URI.asciiHost}:${TEST_URI.port} yet`
@@ -51,7 +50,6 @@ add_task(async function() {
       TEST_URI.port,
       {},
       cert,
-      {},
       {}
     ),
     `Should have override for ${TEST_URI.asciiHost}:${TEST_URI.port} now`
@@ -75,7 +73,6 @@ add_task(async function() {
       TEST_URI.port,
       {},
       cert,
-      {},
       {}
     ),
     `Should not have override for ${TEST_URI.asciiHost}:${TEST_URI.port} now`
@@ -96,7 +93,6 @@ add_task(async function() {
         uri.port,
         { privateBrowsingId: 1 },
         cert,
-        {},
         {}
       ),
       `Should have added override for ${uri.asciiHost}:${uri.port} with private browsing ID`
@@ -107,7 +103,6 @@ add_task(async function() {
         uri.port,
         { privateBrowsingId: 2 },
         cert,
-        {},
         {}
       ),
       `Should not have added override for ${uri.asciiHost}:${uri.port} with private browsing ID 2`
@@ -118,7 +113,6 @@ add_task(async function() {
         uri.port,
         {},
         cert,
-        {},
         {}
       ),
       `Should not have added override for ${uri.asciiHost}:${uri.port}`
@@ -137,7 +131,6 @@ add_task(async function() {
         uri.port,
         {},
         cert,
-        {},
         {}
       ),
       `Should have added override for ${uri.asciiHost}:${uri.port}`
@@ -158,7 +151,6 @@ add_task(async function() {
         uri.port,
         {},
         cert,
-        {},
         {}
       ),
       `Should have removed override for ${uri.asciiHost}:${uri.port}`
@@ -169,7 +161,6 @@ add_task(async function() {
         uri.port,
         { privateBrowsingId: 1 },
         cert,
-        {},
         {}
       ),
       `Should have removed override for ${uri.asciiHost}:${uri.port} with private browsing attribute`
@@ -197,27 +188,16 @@ add_task(async function test_deleteByBaseDomain() {
   let cert = certDB.constructX509FromBase64(CERT_TEST);
   ok(cert, "Cert was created");
 
-  let overrideBits =
-    Ci.nsICertOverrideService.ERROR_UNTRUSTED |
-    Ci.nsICertOverrideService.ERROR_MISMATCH;
-
   all.forEach(({ asciiHost, port }) => {
     Assert.ok(
-      !overrideService.hasMatchingOverride(asciiHost, port, {}, cert, {}, {}),
+      !overrideService.hasMatchingOverride(asciiHost, port, {}, cert, {}),
       `Should not have override for ${asciiHost}:${port} yet`
     );
 
-    overrideService.rememberValidityOverride(
+    overrideService.rememberValidityOverride(asciiHost, port, {}, cert, false);
-      asciiHost,
-      port,
-      {},
-      cert,
-      overrideBits,
-      false
-    );
 
     Assert.ok(
-      overrideService.hasMatchingOverride(asciiHost, port, {}, cert, {}, {}),
+      overrideService.hasMatchingOverride(asciiHost, port, {}, cert, {}),
       `Should have override for ${asciiHost}:${port} now`
     );
   });
@@ -236,14 +216,14 @@ add_task(async function test_deleteByBaseDomain() {
 
   toClear.forEach(({ asciiHost, port }) =>
     Assert.ok(
-      !overrideService.hasMatchingOverride(asciiHost, port, {}, cert, {}, {}),
+      !overrideService.hasMatchingOverride(asciiHost, port, {}, cert, {}),
       `Should have cleared override for ${asciiHost}:${port}`
     )
   );
 
   toKeep.forEach(({ asciiHost, port }) =>
     Assert.ok(
-      overrideService.hasMatchingOverride(asciiHost, port, {}, cert, {}, {}),
+      overrideService.hasMatchingOverride(asciiHost, port, {}, cert, {}),
       `Should have kept override for ${asciiHost}:${port}`
     )
   );

toolkit/mozapps/extensions/test/browser/head.js

@@ -589,7 +589,7 @@ CategoryUtilities.prototype = {
 
 // Returns a promise that will resolve when the certificate error override has been added, or reject
 // if there is some failure.
-function addCertOverride(host, bits) {
+function addCertOverride(host) {
   return new Promise((resolve, reject) => {
     let req = new XMLHttpRequest();
     req.open("GET", "https://" + host + "/");
@@ -608,7 +608,6 @@ function addCertOverride(host, bits) {
             -1,
             {},
             securityInfo.serverCert,
-            bits,
             false
           );
           resolve();
@@ -624,22 +623,10 @@ function addCertOverride(host, bits) {
 // Returns a promise that will resolve when the necessary certificate overrides have been added.
 function addCertOverrides() {
   return Promise.all([
-    addCertOverride(
+    addCertOverride("nocert.example.com"),
-      "nocert.example.com",
+    addCertOverride("self-signed.example.com"),
-      Ci.nsICertOverrideService.ERROR_MISMATCH
+    addCertOverride("untrusted.example.com"),
-    ),
+    addCertOverride("expired.example.com"),
-    addCertOverride(
-      "self-signed.example.com",
-      Ci.nsICertOverrideService.ERROR_UNTRUSTED
-    ),
-    addCertOverride(
-      "untrusted.example.com",
-      Ci.nsICertOverrideService.ERROR_UNTRUSTED
-    ),
-    addCertOverride(
-      "expired.example.com",
-      Ci.nsICertOverrideService.ERROR_TIME
-    ),
   ]);
 }