Bug 1577428 - Not allow nsICertOverrideService to be implemented in js r=keeler,ato

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

--HG--
rename : security/manager/ssl/tests/unit/test_js_cert_override_service.js => security/manager/ssl/tests/unit/test_allow_all_cert_errors.js
extra : moz-landing-system : lando

security/manager/ssl/nsCertOverrideService.cpp

@@ -82,7 +82,7 @@ NS_IMPL_ISUPPORTS(nsCertOverrideService, nsICertOverrideService, nsIObserver,
                   nsISupportsWeakReference)
 
 nsCertOverrideService::nsCertOverrideService()
-    : mMutex("nsCertOverrideService.mutex") {}
+    : mDisableAllSecurityCheck(false), mMutex("nsCertOverrideService.mutex") {}
 
 nsCertOverrideService::~nsCertOverrideService() {}
 
@@ -412,6 +412,21 @@ nsCertOverrideService::HasMatchingOverride(const nsACString& aHostName,
                                            int32_t aPort, nsIX509Cert* aCert,
                                            uint32_t* aOverrideBits,
                                            bool* aIsTemporary, bool* _retval) {
+  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;
+    *_retval = true;
+    return NS_OK;
+  }
+
   if (aHostName.IsEmpty() || !IsASCII(aHostName)) {
     return NS_ERROR_INVALID_ARG;
   }
@@ -592,6 +607,19 @@ nsCertOverrideService::IsCertUsedForOverrides(nsIX509Cert* aCert,
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsCertOverrideService::
+    SetDisableAllSecurityChecksAndLetAttackersInterceptMyData(bool aDisable) {
+  if (!(PR_GetEnv("XPCSHELL_TEST_PROFILE_DIR") ||
+        PR_GetEnv("MOZ_MARIONETTE"))) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
+  MutexAutoLock lock(mMutex);
+  mDisableAllSecurityCheck = aDisable;
+  return NS_OK;
+}
+
 nsresult nsCertOverrideService::EnumerateCertOverrides(
     nsIX509Cert* aCert, CertOverrideEnumerator aEnumerator, void* aUserData) {
   MutexAutoLock lock(mMutex);

security/manager/ssl/nsCertOverrideService.h

@@ -131,6 +131,7 @@ class nsCertOverrideService final : public nsICertOverrideService,
  protected:
   ~nsCertOverrideService();
 
+  bool mDisableAllSecurityCheck;
   mozilla::Mutex mMutex;
   nsCOMPtr<nsIFile> mSettingsFile;
   nsTHashtable<nsCertOverrideEntry> mSettingsTable;

security/manager/ssl/nsICertOverrideService.idl

@@ -18,7 +18,7 @@ interface nsIX509Cert;
  *   {host:port, cert-fingerprint, allowed-overrides}
  * that the user wants to accept without further warnings.
  */
-[scriptable, uuid(be019e47-22fc-4355-9f16-9ab047d6742d)]
+[scriptable, builtinclass, uuid(be019e47-22fc-4355-9f16-9ab047d6742d)]
 interface nsICertOverrideService : nsISupports {
 
   /**
@@ -130,4 +130,12 @@ interface nsICertOverrideService : nsISupports {
   uint32_t isCertUsedForOverrides(in nsIX509Cert aCert,
                                   in boolean aCheckTemporaries,
                                   in boolean aCheckPermanents);
+
+  /**
+   *  NOTE: This function is used only for testing!
+   *
+   *  @param aDisable If true, disable all security check and make
+   *                  hasMatchingOverride always return true.
+   */
+  void setDisableAllSecurityChecksAndLetAttackersInterceptMyData(in boolean aDisable);
 };

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

@@ -0,0 +1,25 @@
+/* -*- tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+function run_test() {
+  do_get_profile();
+  let certOverrideService = Cc[
+    "@mozilla.org/security/certoverride;1"
+  ].getService(Ci.nsICertOverrideService);
+  certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
+    true
+  );
+
+  add_tls_server_setup("BadCertAndPinningServer", "bad_certs");
+  add_connection_test("expired.example.com", PRErrorCodeSuccess);
+  add_test(function() {
+    certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
+      false
+    );
+    run_next_test();
+  });
+  run_next_test();
+}

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

@@ -1,60 +0,0 @@
-/* -*- tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-"use strict";
-
-// This test ensures that nsICertOverrideService can be implemented in JS.
-// It does so by creating and registering a mock implementation that indicates
-// a specific host ("expired.example.com") has a matching override (ERROR_TIME).
-// Connections to that host should succeed.
-
-// Mock implementation of nsICertOverrideService
-const gCertOverrideService = {
-  rememberValidityOverride() {
-    throw Cr.NS_ERROR_NOT_IMPLEMENTED;
-  },
-
-  rememberTemporaryValidityOverrideUsingFingerprint() {
-    throw Cr.NS_ERROR_NOT_IMPLEMENTED;
-  },
-
-  hasMatchingOverride(hostname, port, cert, overrideBits, isTemporary) {
-    Assert.equal(
-      hostname,
-      "expired.example.com",
-      "hasMatchingOverride: hostname should be expired.example.com"
-    );
-    overrideBits.value = Ci.nsICertOverrideService.ERROR_TIME;
-    isTemporary.value = false;
-    return true;
-  },
-
-  clearValidityOverride() {
-    throw Cr.NS_ERROR_NOT_IMPLEMENTED;
-  },
-
-  clearAllOverrides() {
-    throw Cr.NS_ERROR_NOT_IMPLEMENTED;
-  },
-
-  isCertUsedForOverrides() {
-    throw Cr.NS_ERROR_NOT_IMPLEMENTED;
-  },
-
-  QueryInterface: ChromeUtils.generateQI([Ci.nsICertOverrideService]),
-};
-
-function run_test() {
-  do_get_profile();
-  let certOverrideServiceCID = MockRegistrar.register(
-    "@mozilla.org/security/certoverride;1",
-    gCertOverrideService
-  );
-  registerCleanupFunction(() => {
-    MockRegistrar.unregister(certOverrideServiceCID);
-  });
-  add_tls_server_setup("BadCertAndPinningServer", "bad_certs");
-  add_connection_test("expired.example.com", PRErrorCodeSuccess);
-  run_next_test();
-}

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

@@ -129,7 +129,7 @@ tags = blocklist psm
 skip-if = toolkit == 'android'
 [test_imminent_distrust.js]
 run-sequentially = hardcoded ports
-[test_js_cert_override_service.js]
+[test_allow_all_cert_errors.js]
 run-sequentially = hardcoded ports
 [test_keysize.js]
 [test_keysize_ev.js]

testing/marionette/cert.js

@@ -11,122 +11,49 @@ const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
 
-this.EXPORTED_SYMBOLS = [
-  "CertificateOverrideManager",
-  "InsecureSweepingOverride",
-];
+this.EXPORTED_SYMBOLS = ["allowAllCerts"];
 
-const registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
 const sss = Cc["@mozilla.org/ssservice;1"].getService(
   Ci.nsISiteSecurityService
 );
 
+const certOverrideService = Cc[
+  "@mozilla.org/security/certoverride;1"
+].getService(Ci.nsICertOverrideService);
+
 const CERT_PINNING_ENFORCEMENT_PREF = "security.cert_pinning.enforcement_level";
-const CID = Components.ID("{4b67cce0-a51c-11e6-9598-0800200c9a66}");
-const CONTRACT_ID = "@mozilla.org/security/certoverride;1";
-const DESC = "All-encompassing cert service that matches on a bitflag";
 const HSTS_PRELOAD_LIST_PREF = "network.stricttransportsecurity.preloadlist";
 
-const Error = {
-  Untrusted: 1,
-  Mismatch: 2,
-  Time: 4,
-};
-
-let currentOverride = null;
-
-/** TLS certificate service override management for Marionette. */
-class CertificateOverrideManager {
-  /**
-   * Installs a TLS certificate service override.
-   *
-   * The provided `service` must implement the `register` and `unregister`
-   * functions that causes a new `nsICertOverrideService` interface
-   * implementation to be registered with the `nsIComponentRegistrar`.
-   *
-   * After `service` is registered, `nsICertOverrideService` is
-   * reinitialised to cause all Gecko components to pick up the
-   * new service.
-   *
-   * If an override is already installed this functions acts as a no-op.
-   *
-   * @param {cert.Override} service
-   *     Service generator that registers and unregisters the XPCOM service.
-   *
-   * @throws {Components.Exception}
-   *     If unable to register or initialise `service`.
-   */
-  static install(service) {
-    if (currentOverride) {
-      return;
-    }
-
-    service.register();
-    currentOverride = service;
-  }
-
-  /**
-   * Uninstall a TLS certificate service override.
-   *
-   * If there is no current override installed this function acts
-   * as a no-op.
-   */
-  static uninstall() {
-    if (!currentOverride) {
-      return;
-    }
-    currentOverride.unregister();
-    currentOverride = null;
-  }
-}
-this.CertificateOverrideManager = CertificateOverrideManager;
+/** @namespace */
+this.allowAllCerts = {};
 
 /**
- * Certificate override service that acts in an all-inclusive manner
- * on TLS certificates.
- *
- * @throws {Components.Exception}
- *     If there are any problems registering the service.
+ * Disable all security check and allow all certs.
  */
-function InsecureSweepingOverride() {
-  // This needs to be an old-style class with a function constructor
-  // and prototype assignment because... XPCOM.  Any attempt at
-  // modernisation will be met with cryptic error messages which will
-  // make your life miserable.
-  let service = function() {};
-  service.prototype = {
-    hasMatchingOverride(aHostName, aPort, aCert, aOverrideBits, aIsTemporary) {
-      aIsTemporary.value = false;
-      aOverrideBits.value = Error.Untrusted | Error.Mismatch | Error.Time;
+allowAllCerts.enable = function() {
+  // make it possible to register certificate overrides for domains
+  // that use HSTS or HPKP
+  Preferences.set(HSTS_PRELOAD_LIST_PREF, false);
+  Preferences.set(CERT_PINNING_ENFORCEMENT_PREF, 0);
 
-      return true;
-    },
+  certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
+    true
+  );
+};
 
-    QueryInterface: ChromeUtils.generateQI([Ci.nsICertOverrideService]),
-  };
-  let factory = XPCOMUtils.generateSingletonFactory(service);
+/**
+ * Enable all security check.
+ */
+allowAllCerts.disable = function() {
+  certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
+    false
+  );
 
-  return {
-    register() {
-      // make it possible to register certificate overrides for domains
-      // that use HSTS or HPKP
-      Preferences.set(HSTS_PRELOAD_LIST_PREF, false);
-      Preferences.set(CERT_PINNING_ENFORCEMENT_PREF, 0);
+  Preferences.reset(HSTS_PRELOAD_LIST_PREF);
+  Preferences.reset(CERT_PINNING_ENFORCEMENT_PREF);
 
-      registrar.registerFactory(CID, DESC, CONTRACT_ID, factory);
-    },
-
-    unregister() {
-      registrar.unregisterFactory(CID, factory);
-
-      Preferences.reset(HSTS_PRELOAD_LIST_PREF);
-      Preferences.reset(CERT_PINNING_ENFORCEMENT_PREF);
-
-      // clear collected HSTS and HPKP state
-      // through the site security service
-      sss.clearAll();
-      sss.clearPreloads();
-    },
-  };
-}
-this.InsecureSweepingOverride = InsecureSweepingOverride;
+  // clear collected HSTS and HPKP state
+  // through the site security service
+  sss.clearAll();
+  sss.clearPreloads();
+};

testing/marionette/doc/internals/cert.rst

@@ -1,12 +1,4 @@
 cert module
 ===========
-
-CertificateOverrideManager
---------------------------
-.. js:autoclass:: CertificateOverrideManager
-  :members:
-
-InsecureSweepingOverride
-------------------------
-.. js:autoclass:: InsecureSweepingOverride
+.. js:autoclass:: allowAllCerts
   :members:

testing/marionette/driver.js

@@ -25,10 +25,9 @@ const { Capabilities, Timeouts, UnhandledPromptBehavior } = ChromeUtils.import(
 const { capture } = ChromeUtils.import(
   "chrome://marionette/content/capture.js"
 );
-const {
-  CertificateOverrideManager,
-  InsecureSweepingOverride,
-} = ChromeUtils.import("chrome://marionette/content/cert.js");
+const { allowAllCerts } = ChromeUtils.import(
+  "chrome://marionette/content/cert.js"
+);
 const { cookie } = ChromeUtils.import("chrome://marionette/content/cookie.js");
 const { WebElementEventTarget } = ChromeUtils.import(
   "chrome://marionette/content/dom.js"
@@ -733,8 +732,7 @@ GeckoDriver.prototype.newSession = async function(cmd) {
 
     if (!this.secureTLS) {
       logger.warn("TLS certificate errors will be ignored for this session");
-      let acceptAllCerts = new InsecureSweepingOverride();
-      CertificateOverrideManager.install(acceptAllCerts);
+      allowAllCerts.enable();
     }
 
     if (this.proxy.init()) {
@@ -2983,7 +2981,7 @@ GeckoDriver.prototype.deleteSession = function() {
   }
 
   this.sandboxes.clear();
-  CertificateOverrideManager.uninstall();
+  allowAllCerts.disable();
 
   this.sessionID = null;
   this.capabilities = new Capabilities();

tools/lint/eslint/modules.json

@@ -27,7 +27,7 @@
   "browser.js": ["browser", "Context", "WindowState"],
   "browserid_identity.js": ["BrowserIDManager", "AuthenticationError"],
   "capabilities.js": ["Capabilities", "PageLoadStrategy", "Proxy", "Timeouts", "UnhandledPromptBehavior"],
-  "cert.js": ["CertificateOverrideManager", "InsecureSweepingOverride"],
+  "cert.js": ["allowAllCerts"],
   "clients.js": ["ClientEngine", "ClientsRec"],
   "collection_repair.js": ["getRepairRequestor", "getAllRepairRequestors", "CollectionRepairRequestor", "getRepairResponder", "CollectionRepairResponder"],
   "collection_validator.js": ["CollectionValidator", "CollectionProblemData"],