Bug 1547994 - Remove preference that can disable Remote Settings signature verification r=glasserc
authorMathieu Leplatre <mathieu@mozilla.com>
Tue, 07 May 2019 14:16:22 +0000
changeset 534918 e677f0ef8e59fc3e2f36c31cc38bc2d56a03b956
parent 534917 2f69959d8a8881860661e3c7cf5ab4780aa7602f
child 534919 9fcabe77196c1e35abbe19f7e3dd37abb351d25e
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglasserc
bugs1547994
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1547994 - Remove preference that can disable Remote Settings signature verification r=glasserc Differential Revision: https://phabricator.services.mozilla.com/D29656
browser/components/newtab/docs/v2-system-addon/remote_cfr.md
security/manager/ssl/tests/unit/test_cert_storage.js
security/manager/ssl/tests/unit/test_intermediate_preloads.js
security/manager/ssl/tests/unit/xpcshell.ini
services/common/blocklist-clients.js
services/common/tests/unit/test_blocklist_onecrl.js
services/common/tests/unit/test_blocklist_pinning.js
services/common/tests/unit/test_blocklist_signatures.js
services/settings/RemoteSettingsClient.jsm
services/settings/test/unit/test_remote_settings.js
toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/test_blocklist_clients.js
--- a/browser/components/newtab/docs/v2-system-addon/remote_cfr.md
+++ b/browser/components/newtab/docs/v2-system-addon/remote_cfr.md
@@ -32,17 +32,21 @@ curl -X POST ${SERVER}/buckets/main/coll
 Now there should be a message listed: https://kinto.dev.mozaws.net/v1//buckets/main/collections/cfr/records
 
 NOTE: The collection and messages can also be created manually using the [admin interface](https://kinto.dev.mozaws.net/v1/admin/).
 
 **2. Set Remote Settings prefs to use the dev server.**
 
 ```javascript
 Services.prefs.setStringPref("services.settings.server", "https://kinto.dev.mozaws.net/v1");
-Services.prefs.setBoolPref("services.settings.verify_signature", false);
+
+// Disable signature verification
+const { RemoteSettings } = ChromeUtils.import("resource://services-settings/remote-settings.js", {});
+
+RemoteSettings("cfr").verifySignature = false;
 ```
 
 **3. Set ASRouter CFR pref to use Remote Settings provider and enable asrouter devtools.**
 
 ```javascript
 Services.prefs.setStringPref("browser.newtabpage.activity-stream.asrouter.providers.cfr", JSON.stringify({"id":"cfr-remote","enabled":true,"type":"remote-settings","bucket":"cfr","frequency":{"custom":[{"period":"daily","cap":1}]},"categories":["cfrAddons","cfrFeatures"]}));
 Services.prefs.setBoolPref("browser.newtabpage.activity-stream.asrouter.devtoolsEnabled", true);
 ```
--- a/security/manager/ssl/tests/unit/test_cert_storage.js
+++ b/security/manager/ssl/tests/unit/test_cert_storage.js
@@ -181,21 +181,20 @@ async function verify_non_tls_usage_succ
 
 function load_cert(cert, trust) {
   let file = "bad_certs/" + cert + ".pem";
   addCertFromFile(certDB, file, trust);
 }
 
 function fetch_blocklist() {
   Services.prefs.setBoolPref("services.settings.load_dump", false);
-  Services.prefs.setBoolPref("services.settings.verify_signature", false);
   Services.prefs.setCharPref("services.settings.server",
                              `http://localhost:${port}/v1`);
 
-  BlocklistClients.initialize();
+  BlocklistClients.initialize({ verifySignature: false });
 
   return RemoteSettings.pollChanges();
 }
 
 function* generate_revocations_txt_lines() {
   let profile = do_get_profile();
   let revocations = profile.clone();
   revocations.append("revocations.txt");
--- a/security/manager/ssl/tests/unit/test_intermediate_preloads.js
+++ b/security/manager/ssl/tests/unit/test_intermediate_preloads.js
@@ -10,16 +10,17 @@ do_get_profile(); // must be called befo
 const {RemoteSettings} = ChromeUtils.import("resource://services-settings/remote-settings.js");
 const {TestUtils} = ChromeUtils.import("resource://testing-common/TestUtils.jsm");
 const {TelemetryTestUtils} = ChromeUtils.import("resource://testing-common/TelemetryTestUtils.jsm");
 
 let remoteSecSetting;
 if (AppConstants.MOZ_NEW_CERT_STORAGE) {
   const {RemoteSecuritySettings} = ChromeUtils.import("resource://gre/modules/psm/RemoteSecuritySettings.jsm");
   remoteSecSetting = new RemoteSecuritySettings();
+  remoteSecSetting.client.verifySignature = false;
 }
 
 let server;
 
 let intermediate1Data;
 let intermediate2Data;
 
 const INTERMEDIATES_DL_PER_POLL_PREF     = "security.remote_settings.intermediates.downloads_per_poll";
@@ -84,17 +85,16 @@ function syncAndPromiseUpdate() {
 
 function setupKintoPreloadServer(certGenerator, options = {
   attachmentCB: null,
   hashFunc: null,
   lengthFunc: null,
 }) {
   const dummyServerURL = `http://localhost:${server.identity.primaryPort}/v1`;
   Services.prefs.setCharPref("services.settings.server", dummyServerURL);
-  Services.prefs.setBoolPref("services.settings.verify_signature", false);
 
   const configPath = "/v1/";
   const recordsPath = "/v1/buckets/security-state/collections/intermediates/records";
   const attachmentsPath = "/attachments/";
 
   if (options.hashFunc == null) {
     options.hashFunc = getHash;
   }
--- a/security/manager/ssl/tests/unit/xpcshell.ini
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
@@ -105,16 +105,17 @@ run-sequentially = hardcoded ports
 skip-if = toolkit == 'android'
 [test_hash_algorithms.js]
 [test_hash_algorithms_wrap.js]
 # bug 1124289 - run_test_in_child violates the sandbox on android
 skip-if = toolkit == 'android'
 [test_hmac.js]
 [test_intermediate_basic_usage_constraints.js]
 [test_intermediate_preloads.js]
+tags = blocklist
 # Bug 1520297 - do something to handle tighter resource constraints on Android
 skip-if = toolkit == 'android'
 [test_imminent_distrust.js]
 run-sequentially = hardcoded ports
 [test_js_cert_override_service.js]
 run-sequentially = hardcoded ports
 [test_keysize.js]
 [test_keysize_ev.js]
--- a/services/common/blocklist-clients.js
+++ b/services/common/blocklist-clients.js
@@ -246,37 +246,42 @@ async function targetAppFilter(entry, en
   // Skip this entry.
   return null;
 }
 
 var OneCRLBlocklistClient;
 var PinningBlocklistClient;
 var RemoteSecuritySettingsClient;
 
-function initialize() {
+function initialize(options = {}) {
+  const { verifySignature = true } = options;
+
   OneCRLBlocklistClient = RemoteSettings(Services.prefs.getCharPref(PREF_SECURITY_SETTINGS_ONECRL_COLLECTION), {
     bucketNamePref: PREF_SECURITY_SETTINGS_ONECRL_BUCKET,
     lastCheckTimePref: PREF_SECURITY_SETTINGS_ONECRL_CHECKED,
     signerName: Services.prefs.getCharPref(PREF_SECURITY_SETTINGS_ONECRL_SIGNER),
   });
+  OneCRLBlocklistClient.verifySignature = verifySignature;
   OneCRLBlocklistClient.on("sync", updateCertBlocklist);
 
   PinningBlocklistClient = RemoteSettings(Services.prefs.getCharPref(PREF_BLOCKLIST_PINNING_COLLECTION), {
     bucketNamePref: PREF_BLOCKLIST_PINNING_BUCKET,
     lastCheckTimePref: PREF_BLOCKLIST_PINNING_CHECKED_SECONDS,
     signerName: Services.prefs.getCharPref(PREF_BLOCKLIST_PINNING_SIGNER),
   });
+  PinningBlocklistClient.verifySignature = verifySignature;
   PinningBlocklistClient.on("sync", updatePinningList);
 
   if (AppConstants.MOZ_NEW_CERT_STORAGE) {
     const { RemoteSecuritySettings } = ChromeUtils.import("resource://gre/modules/psm/RemoteSecuritySettings.jsm");
 
     // In Bug 1526018 this will move into its own service, as it's not quite like
     // the others.
     RemoteSecuritySettingsClient = new RemoteSecuritySettings();
+    RemoteSecuritySettingsClient.verifySignature = verifySignature;
 
     return {
       OneCRLBlocklistClient,
       PinningBlocklistClient,
       RemoteSecuritySettingsClient,
     };
   }
 
--- a/services/common/tests/unit/test_blocklist_onecrl.js
+++ b/services/common/tests/unit/test_blocklist_onecrl.js
@@ -14,17 +14,17 @@ let server;
 // xpcshell tests under /services/common
 add_task(async function test_something() {
   const configPath = "/v1/";
   const recordsPath = "/v1/buckets/security-state/collections/onecrl/records";
 
   const dummyServerURL = `http://localhost:${server.identity.primaryPort}/v1`;
   Services.prefs.setCharPref("services.settings.server", dummyServerURL);
 
-  const {OneCRLBlocklistClient} = BlocklistClients.initialize();
+  const {OneCRLBlocklistClient} = BlocklistClients.initialize({verifySignature: false});
 
   // register a handler
   function handleResponse(request, response) {
     try {
       const sample = getSampleResponse(request, server.identity.primaryPort);
       if (!sample) {
         do_throw(`unexpected ${request.method} request for ${request.path}?${request.queryString}`);
       }
@@ -99,20 +99,16 @@ add_task(async function test_something()
   // Check that a sync completes even when there's bad data in the
   // collection. This will throw on fail, so just calling maybeSync is an
   // acceptible test.
   Services.prefs.setCharPref("services.settings.server", dummyServerURL);
   await OneCRLBlocklistClient.maybeSync(5000);
 });
 
 function run_test() {
-  // Ensure that signature verification is disabled to prevent interference
-  // with basic certificate sync tests
-  Services.prefs.setBoolPref("services.settings.verify_signature", false);
-
   // Set up an HTTP Server
   server = new HttpServer();
   server.start(-1);
 
   run_next_test();
 
   registerCleanupFunction(function() {
     server.stop(() => { });
--- a/services/common/tests/unit/test_blocklist_pinning.js
+++ b/services/common/tests/unit/test_blocklist_pinning.js
@@ -22,17 +22,19 @@ updateAppInfo({
   crashReporter: true,
 });
 
 let server;
 
 // Some simple tests to demonstrate that the core preload sync operations work
 // correctly and that simple kinto operations are working as expected.
 add_task(async function test_something() {
-  const PinningPreloadClient = BlocklistClients.initialize().PinningBlocklistClient;
+  const {
+    PinningBlocklistClient: PinningPreloadClient,
+  } = BlocklistClients.initialize({ verifySignature: false });
 
   const configPath = "/v1/";
   const recordsPath = "/v1/buckets/pinning/collections/pins/records";
 
   Services.prefs.setCharPref("services.settings.server",
                              `http://localhost:${server.identity.primaryPort}/v1`);
 
   // register a handler
@@ -133,20 +135,16 @@ add_task(async function test_something()
   // The STS entry for five.example.com now has includeSubdomains set;
   // ensure that the new includeSubdomains value is honored.
   ok(sss.isSecureURI(sss.HEADER_HSTS,
                      Services.io.newURI("https://subdomain.five.example.com"),
                      0));
 });
 
 function run_test() {
-  // Ensure that signature verification is disabled to prevent interference
-  // with basic certificate sync tests
-  Services.prefs.setBoolPref("services.settings.verify_signature", false);
-
   // Set up an HTTP Server
   server = new HttpServer();
   server.start(-1);
 
   run_next_test();
 
   registerCleanupFunction(function() {
     server.stop(() => { });
--- a/services/common/tests/unit/test_blocklist_signatures.js
+++ b/services/common/tests/unit/test_blocklist_signatures.js
@@ -3,17 +3,16 @@
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 const { NetUtil } = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 const { BlocklistClients } = ChromeUtils.import("resource://services-common/blocklist-clients.js");
 const { UptakeTelemetry } = ChromeUtils.import("resource://services-common/uptake-telemetry.js");
 
 let server;
 
-const PREF_SETTINGS_VERIFY_SIGNATURE   = "services.settings.verify_signature";
 const PREF_SETTINGS_SERVER             = "services.settings.server";
 const PREF_SIGNATURE_ROOT              = "security.content.signature.root_hash";
 
 const CERT_DIR = "test_blocklist_signatures/";
 const CHAIN_FILES =
     ["collection_signing_ee.pem",
      "collection_signing_int.pem",
      "collection_signing_root.pem"];
@@ -586,20 +585,18 @@ add_task(async function test_check_signa
   expectedIncrements = {
     [UptakeTelemetry.STATUS.SIGNATURE_ERROR]: 1,
     [UptakeTelemetry.STATUS.SIGNATURE_RETRY_ERROR]: 0,  // Not retried since missing.
   };
   checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
 });
 
 function run_test() {
-  OneCRLBlocklistClient = BlocklistClients.initialize().OneCRLBlocklistClient;
-
-  // ensure signatures are enforced
-  Services.prefs.setBoolPref(PREF_SETTINGS_VERIFY_SIGNATURE, true);
+  // Signature verification is evabled by default.
+  ({OneCRLBlocklistClient} = BlocklistClients.initialize());
 
   // get a signature verifier to ensure nsNSSComponent is initialized
   Cc["@mozilla.org/security/contentsignatureverifier;1"]
     .createInstance(Ci.nsIContentSignatureVerifier);
 
   // set the content signing root to our test root
   setRoot();
 
--- a/services/settings/RemoteSettingsClient.jsm
+++ b/services/settings/RemoteSettingsClient.jsm
@@ -30,18 +30,16 @@ XPCOMUtils.defineLazyGlobalGetters(this,
 const DB_NAME = "remote-settings";
 
 const TELEMETRY_COMPONENT = "remotesettings";
 
 XPCOMUtils.defineLazyPreferenceGetter(this, "gServerURL",
                                       "services.settings.server");
 XPCOMUtils.defineLazyPreferenceGetter(this, "gChangesPath",
                                       "services.settings.changes.path");
-XPCOMUtils.defineLazyPreferenceGetter(this, "gVerifySignature",
-                                      "services.settings.verify_signature", true);
 
 /**
  * cacheProxy returns an object Proxy that will memoize properties of the target.
  * @param {Object} target the object to wrap.
  * @returns {Proxy}
  */
 function cacheProxy(target) {
   const cache = new Map();
@@ -184,16 +182,20 @@ class RemoteSettingsClient extends Event
     super(["sync"]); // emitted events
 
     this.collectionName = collectionName;
     this.signerName = signerName;
     this.filterFunc = filterFunc;
     this.localFields = localFields;
     this._lastCheckTimePref = lastCheckTimePref;
 
+    // This attribute allows signature verification to be disabled, when running tests
+    // or when pulling data from a dev server.
+    this.verifySignature = true;
+
     // The bucket preference value can be changed (eg. `main` to `main-preview`) in order
     // to preview the changes to be approved in a real client.
     this.bucketNamePref = bucketNamePref;
     XPCOMUtils.defineLazyPreferenceGetter(this, "bucketName", this.bucketNamePref);
 
     XPCOMUtils.defineLazyGetter(this, "_kinto", () => new Kinto({
       bucket: this.bucketName,
       adapter: Kinto.adapters.IDB,
@@ -323,17 +325,17 @@ class RemoteSettingsClient extends Event
       // to record the fact that a check happened.
       if (expectedTimestamp <= collectionLastModified) {
         reportStatus = UptakeTelemetry.STATUS.UP_TO_DATE;
         return;
       }
 
       // If signature verification is enabled, then add a synchronization hook
       // for incoming changes that validates the signature.
-      if (this.signerName && gVerifySignature) {
+      if (this.verifySignature) {
         kintoCollection.hooks["incoming-changes"] = [async (payload, collection) => {
           await this._validateCollectionSignature(payload.changes,
                                                   payload.lastModified,
                                                   collection,
                                                   { expectedTimestamp });
           // In case the signature is valid, apply the changes locally.
           return payload;
         }];
--- a/services/settings/test/unit/test_remote_settings.js
+++ b/services/settings/test/unit/test_remote_settings.js
@@ -39,22 +39,22 @@ async function clear_state() {
 function run_test() {
   // Set up an HTTP Server
   server = new HttpServer();
   server.start(-1);
 
   // Point the blocklist clients to use this local HTTP server.
   Services.prefs.setCharPref("services.settings.server",
                              `http://localhost:${server.identity.primaryPort}/v1`);
-  // Ensure that signature verification is disabled to prevent interference
-  // with basic certificate sync tests
-  Services.prefs.setBoolPref("services.settings.verify_signature", false);
 
   client = RemoteSettings("password-fields");
+  client.verifySignature = false;
+
   clientWithDump = RemoteSettings("language-dictionaries");
+  clientWithDump.verifySignature = false;
 
   // Setup server fake responses.
   function handleResponse(request, response) {
     try {
       const sample = getSampleResponse(request, server.identity.primaryPort);
       if (!sample) {
         do_throw(`unexpected ${request.method} request for ${request.path}?${request.queryString}`);
       }
--- a/toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/test_blocklist_clients.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/test_blocklist_clients.js
@@ -22,37 +22,37 @@ function run_test() {
 
   // Set up an HTTP Server
   server = new HttpServer();
   server.start(-1);
 
   // Point the blocklist clients to use this local HTTP server.
   Services.prefs.setCharPref("services.settings.server",
                              `http://localhost:${server.identity.primaryPort}/v1`);
-  // Ensure that signature verification is disabled to prevent interference
-  // with basic certificate sync tests
-  Services.prefs.setBoolPref("services.settings.verify_signature", false);
 
   // Unfortunately security settings are coupled with blocklists clients,
   // this will be fixed in Bug 1526018
   // We disable intermediate preloading because it runs when polling ends, and
   // interferes with `clear_state()` from this test suite.
   Services.prefs.setBoolPref("security.remote_settings.intermediates.enabled", false);
 
   // This will initialize the remote settings clients for blocklists.
   BlocklistGlobal.ExtensionBlocklistRS._ensureInitialized();
   BlocklistGlobal.PluginBlocklistRS._ensureInitialized();
   BlocklistGlobal.GfxBlocklistRS._ensureInitialized();
 
-
   gBlocklistClients = [
     {client: BlocklistGlobal.ExtensionBlocklistRS._client, testData: ["i808", "i720", "i539"]},
     {client: BlocklistGlobal.PluginBlocklistRS._client, testData: ["p1044", "p32", "p28"]},
     {client: BlocklistGlobal.GfxBlocklistRS._client, testData: ["g204", "g200", "g36"]},
   ];
+  // Disable signature verification in these tests.
+  for (const c of gBlocklistClients) {
+    c.verifySignature = false;
+  }
 
   // Setup server fake responses.
   function handleResponse(request, response) {
     try {
       const sample = getSampleResponse(request, server.identity.primaryPort);
       if (!sample) {
         do_throw(`unexpected ${request.method} request for ${request.path}?${request.queryString}`);
       }