Bug 1549730 - Add guardrails for Remote Settings preferences r=glasserc
authorMathieu Leplatre <mathieu@mozilla.com>
Tue, 14 May 2019 20:45:03 +0000
changeset 473838 8553fd10ee8923735468ebf54f0b9ed1c33d8c81
parent 473837 2e773f4f69c468c09e84f87c12a093c6da9d0dfd
child 473839 7b3a1ee70cd723a6370e0f080f9fa36c2947d0de
push id36017
push userrgurzau@mozilla.com
push dateWed, 15 May 2019 09:25:56 +0000
treeherdermozilla-central@76bbedc1ec1a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglasserc
bugs1549730
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 1549730 - Add guardrails for Remote Settings preferences r=glasserc Differential Revision: https://phabricator.services.mozilla.com/D31043
modules/libpref/init/all.js
services/settings/RemoteSettingsClient.jsm
services/settings/Utils.jsm
services/settings/remote-settings.js
services/settings/servicesSettings.manifest
services/settings/test/unit/test_remote_settings_jexl_filters.js
services/settings/test/unit/test_remote_settings_poll.js
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -2720,19 +2720,17 @@ pref("security.view-source.reachable-fro
 pref("security.strict_security_checks.enabled", true);
 #else
 pref("security.strict_security_checks.enabled", false);
 #endif
 
 // Remote settings preferences
 pref("services.settings.poll_interval", 86400); // 24H
 pref("services.settings.server", "https://firefox.settings.services.mozilla.com/v1");
-pref("services.settings.changes.path", "/buckets/monitor/collections/changes/records");
 pref("services.settings.default_bucket", "main");
-pref("services.settings.default_signer", "remote-settings.content-signature.mozilla.org");
 
 // The percentage of clients who will report uptake telemetry as
 // events instead of just a histogram. This only applies on Release;
 // other channels always report events.
 pref("services.common.uptake.sampleRate", 1);   // 1%
 
 // Security state OneCRL.
 pref("services.settings.security.onecrl.bucket", "security-state");
--- a/services/settings/RemoteSettingsClient.jsm
+++ b/services/settings/RemoteSettingsClient.jsm
@@ -30,18 +30,16 @@ XPCOMUtils.defineLazyGlobalGetters(this,
 
 // IndexedDB name.
 const DB_NAME = "remote-settings";
 
 const TELEMETRY_COMPONENT = "remotesettings";
 
 XPCOMUtils.defineLazyPreferenceGetter(this, "gServerURL",
                                       "services.settings.server");
-XPCOMUtils.defineLazyPreferenceGetter(this, "gChangesPath",
-                                      "services.settings.changes.path");
 
 /**
  * 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();
@@ -250,17 +248,17 @@ class RemoteSettingsClient extends Event
   /**
    * Synchronize the local database with the remote server.
    *
    * @param {Object} options See #maybeSync() options.
    */
   async sync(options) {
     // We want to know which timestamp we are expected to obtain in order to leverage
     // cache busting. We don't provide ETag because we don't want a 304.
-    const { changes } = await Utils.fetchLatestChanges(gServerURL + gChangesPath, {
+    const { changes } = await Utils.fetchLatestChanges(gServerURL, {
       filters: {
         collection: this.collectionName,
         bucket: this.bucketName,
       },
     });
     if (changes.length === 0) {
       throw new Error(`Unknown collection "${this.identifier}"`);
     }
--- a/services/settings/Utils.jsm
+++ b/services/settings/Utils.jsm
@@ -7,16 +7,17 @@
 ];
 
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyGlobalGetters(this, ["fetch"]);
 
 var Utils = {
+  CHANGES_PATH: "/buckets/monitor/collections/changes/records",
 
   /**
    * Check if local data exist for the specified client.
    *
    * @param {RemoteSettingsClient} client
    * @return {bool} Whether it exists or not.
    */
   async hasLocalData(client) {
@@ -38,36 +39,39 @@ var Utils = {
       return true;
     } catch (e) {
       return false;
     }
   },
 
   /**
    * Fetch the list of remote collections and their timestamp.
-   * @param {String} url               The poll URL (eg. `http://${server}{pollingEndpoint}`)
+   * @param {String} serverUrl         The server URL (eg. `https://server.org/v1`)
    * @param {int}    expectedTimestamp The timestamp that the server is supposed to return.
    *                                   We obtained it from the Megaphone notification payload,
    *                                   and we use it only for cache busting (Bug 1497159).
    * @param {String} lastEtag          (optional) The Etag of the latest poll to be matched
    *                                   by the server (eg. `"123456789"`).
    * @param {Object} filters
    */
-  async fetchLatestChanges(url, options = {}) {
+  async fetchLatestChanges(serverUrl, options = {}) {
     const { expectedTimestamp, lastEtag = "", filters = {} } = options;
+
     //
     // Fetch the list of changes objects from the server that looks like:
     // {"data":[
     //   {
     //     "host":"kinto-ota.dev.mozaws.net",
     //     "last_modified":1450717104423,
     //     "bucket":"blocklists",
     //     "collection":"certificates"
     //    }]}
 
+    let url = serverUrl + Utils.CHANGES_PATH;
+
     // Use ETag to obtain a `304 Not modified` when no change occurred,
     // and `?_since` parameter to only keep entries that weren't processed yet.
     const headers = {};
     const params = { ...filters };
     if (lastEtag != "") {
       headers["If-None-Match"] = lastEtag;
       params._since = lastEtag;
     }
--- a/services/settings/remote-settings.js
+++ b/services/settings/remote-settings.js
@@ -28,36 +28,36 @@ ChromeUtils.defineModuleGetter(this, "Fi
 
 XPCOMUtils.defineLazyGlobalGetters(this, ["fetch"]);
 
 const PREF_SETTINGS_DEFAULT_BUCKET     = "services.settings.default_bucket";
 const PREF_SETTINGS_BRANCH             = "services.settings.";
 const PREF_SETTINGS_SERVER             = "server";
 const PREF_SETTINGS_DEFAULT_SIGNER     = "default_signer";
 const PREF_SETTINGS_SERVER_BACKOFF     = "server.backoff";
-const PREF_SETTINGS_CHANGES_PATH       = "changes.path";
 const PREF_SETTINGS_LAST_UPDATE        = "last_update_seconds";
 const PREF_SETTINGS_LAST_ETAG          = "last_etag";
 const PREF_SETTINGS_CLOCK_SKEW_SECONDS = "clock_skew_seconds";
 const PREF_SETTINGS_LOAD_DUMP          = "load_dump";
 
-
 // Telemetry identifiers.
 const TELEMETRY_COMPONENT = "remotesettings";
 const TELEMETRY_SOURCE_POLL = "settings-changes-monitoring";
 const TELEMETRY_SOURCE_SYNC = "settings-sync";
 
 // Push broadcast id.
 const BROADCAST_ID = "remote-settings/monitor_changes";
 
+// Signer to be used when not specified (see Ci.nsIContentSignatureVerifier).
+const DEFAULT_SIGNER = "remote-settings.content-signature.mozilla.org";
+
 XPCOMUtils.defineLazyGetter(this, "gPrefs", () => {
   return Services.prefs.getBranch(PREF_SETTINGS_BRANCH);
 });
 XPCOMUtils.defineLazyPreferenceGetter(this, "gServerURL", PREF_SETTINGS_BRANCH + PREF_SETTINGS_SERVER);
-XPCOMUtils.defineLazyPreferenceGetter(this, "gChangesPath", PREF_SETTINGS_BRANCH + PREF_SETTINGS_CHANGES_PATH);
 
 /**
  * Default entry filtering function, in charge of excluding remote settings entries
  * where the JEXL expression evaluates into a falsy value.
  * @param {Object}            entry       The Remote Settings entry to be excluded or kept.
  * @param {ClientEnvironment} environment Information about version, language, platform etc.
  * @returns {?Object} the entry or null if excluded.
  */
@@ -79,20 +79,19 @@ async function jexlFilterFunc(entry, env
 }
 
 
 function remoteSettingsFunction() {
   const _clients = new Map();
   let _invalidatePolling = false;
 
   // If not explicitly specified, use the default signer.
-  const defaultSigner = gPrefs.getCharPref(PREF_SETTINGS_DEFAULT_SIGNER);
   const defaultOptions = {
     bucketNamePref: PREF_SETTINGS_DEFAULT_BUCKET,
-    signerName: defaultSigner,
+    signerName: DEFAULT_SIGNER,
     filterFunc: jexlFilterFunc,
   };
 
   /**
    * RemoteSettings constructor.
    *
    * @param {String} collectionName The remote settings identifier
    * @param {Object} options Advanced options
@@ -107,22 +106,16 @@ function remoteSettingsFunction() {
       _clients.set(collectionName, c);
       // Invalidate the polling status, since we want the new collection to
       // be taken into account.
       _invalidatePolling = true;
     }
     return _clients.get(collectionName);
   };
 
-  Object.defineProperty(remoteSettings, "pollingEndpoint", {
-    get() {
-      return gServerURL + gChangesPath;
-    },
-  });
-
   /**
    * Internal helper to retrieve existing instances of clients or new instances
    * with default options if possible, or `null` if bucket/collection are unknown.
    */
   async function _client(bucketName, collectionName) {
     // Check if a client was registered for this bucket/collection. Potentially
     // with some specific options like signer, filter function etc.
     const client = _clients.get(collectionName);
@@ -181,17 +174,17 @@ function remoteSettingsFunction() {
     Services.obs.notifyObservers(null, "remote-settings:changes-poll-start", JSON.stringify({ expectedTimestamp }));
 
     // Do we have the latest version already?
     // Every time we register a new client, we have to fetch the whole list again.
     const lastEtag = _invalidatePolling ? "" : gPrefs.getCharPref(PREF_SETTINGS_LAST_ETAG, "");
 
     let pollResult;
     try {
-      pollResult = await Utils.fetchLatestChanges(remoteSettings.pollingEndpoint, { expectedTimestamp, lastEtag });
+      pollResult = await Utils.fetchLatestChanges(gServerURL, { expectedTimestamp, lastEtag });
     } catch (e) {
       // Report polling error to Uptake Telemetry.
       let reportStatus;
       if (/JSON\.parse/.test(e.message)) {
         reportStatus = UptakeTelemetry.STATUS.PARSE_ERROR;
       } else if (/content-type/.test(e.message)) {
         reportStatus = UptakeTelemetry.STATUS.CONTENT_ERROR;
       } else if (/Server/.test(e.message)) {
@@ -286,17 +279,17 @@ function remoteSettingsFunction() {
     Services.obs.notifyObservers(null, "remote-settings:changes-poll-end");
   };
 
   /**
    * Returns an object with polling status information and the list of
    * known remote settings collections.
    */
   remoteSettings.inspect = async () => {
-    const { changes, currentEtag: serverTimestamp } = await Utils.fetchLatestChanges(remoteSettings.pollingEndpoint);
+    const { changes, currentEtag: serverTimestamp } = await Utils.fetchLatestChanges(gServerURL);
 
     const collections = await Promise.all(changes.map(async (change) => {
       const { bucket, collection, last_modified: serverTimestamp } = change;
       const client = await _client(bucket, collection);
       if (!client) {
         return null;
       }
       const kintoCol = await client.openCollection();
@@ -309,21 +302,22 @@ function remoteSettingsFunction() {
         serverTimestamp,
         lastCheck,
         signerName: client.signerName,
       };
     }));
 
     return {
       serverURL: gServerURL,
+      pollingEndpoint: gServerURL + Utils.CHANGES_PATH,
       serverTimestamp,
       localTimestamp: gPrefs.getCharPref(PREF_SETTINGS_LAST_ETAG, null),
       lastCheck: gPrefs.getIntPref(PREF_SETTINGS_LAST_UPDATE, 0),
       mainBucket: Services.prefs.getCharPref(PREF_SETTINGS_DEFAULT_BUCKET),
-      defaultSigner,
+      defaultSigner: DEFAULT_SIGNER,
       collections: collections.filter(c => !!c),
     };
   };
 
   /**
    * Startup function called from nsBrowserGlue.
    */
   remoteSettings.init = () => {
--- a/services/settings/servicesSettings.manifest
+++ b/services/settings/servicesSettings.manifest
@@ -1,5 +1,7 @@
 # Register resource aliases
 resource services-settings resource://gre/modules/services-settings/
 
 # Schedule polling of remote settings changes
-category update-timer RemoteSettingsComponents @mozilla.org/services/settings;1,getService,services-settings-poll-changes,services.settings.poll_interval,86400
+# (default 24H, max 72H)
+# see syntax https://searchfox.org/mozilla-central/rev/cc280c4be94ff8cf64a27cc9b3d6831ffa49fa45/toolkit/components/timermanager/UpdateTimerManager.jsm#155
+category update-timer RemoteSettingsComponents @mozilla.org/services/settings;1,getService,services-settings-poll-changes,services.settings.poll_interval,86400,259200
--- a/services/settings/test/unit/test_remote_settings_jexl_filters.js
+++ b/services/settings/test/unit/test_remote_settings_jexl_filters.js
@@ -99,20 +99,20 @@ add_task(async function test_support_of_
 });
 
 add_task(async function test_support_of_preferences_filters() {
   await createRecords([{
     willMatch: true,
     filter_expression: '"services.settings.last_etag"|preferenceValue == 42',
   }, {
     willMatch: true,
-    filter_expression: '"services.settings.changes.path"|preferenceExists == true',
+    filter_expression: '"services.settings.default_bucket"|preferenceExists == true',
   }, {
     willMatch: true,
-    filter_expression: '"services.settings.changes.path"|preferenceIsUserSet == false',
+    filter_expression: '"services.settings.default_bucket"|preferenceIsUserSet == false',
   }, {
     willMatch: true,
     filter_expression: '"services.settings.last_etag"|preferenceIsUserSet == true',
   }]);
 
   // Set a pref for the user.
   Services.prefs.setIntPref("services.settings.last_etag", 42);
 
--- a/services/settings/test/unit/test_remote_settings_poll.js
+++ b/services/settings/test/unit/test_remote_settings_poll.js
@@ -7,32 +7,33 @@ const { setTimeout } = ChromeUtils.impor
 const { UptakeTelemetry } = ChromeUtils.import("resource://services-common/uptake-telemetry.js");
 const { Kinto } = ChromeUtils.import("resource://services-common/kinto-offline-client.js");
 const { pushBroadcastService } = ChromeUtils.import("resource://gre/modules/PushBroadcastService.jsm");
 const {
   RemoteSettings,
   remoteSettingsBroadcastHandler,
   BROADCAST_ID,
 } = ChromeUtils.import("resource://services-settings/remote-settings.js");
+const { Utils } = ChromeUtils.import("resource://services-settings/Utils.jsm");
 const { TelemetryTestUtils } = ChromeUtils.import("resource://testing-common/TelemetryTestUtils.jsm");
 
 
 const IS_ANDROID = AppConstants.platform == "android";
 
 const PREF_SETTINGS_SERVER = "services.settings.server";
 const PREF_SETTINGS_SERVER_BACKOFF = "services.settings.server.backoff";
 const PREF_LAST_UPDATE = "services.settings.last_update_seconds";
 const PREF_LAST_ETAG = "services.settings.last_etag";
 const PREF_CLOCK_SKEW_SECONDS = "services.settings.clock_skew_seconds";
 
 const DB_NAME = "remote-settings";
 // Telemetry report result.
 const TELEMETRY_HISTOGRAM_POLL_KEY = "settings-changes-monitoring";
 const TELEMETRY_HISTOGRAM_SYNC_KEY = "settings-sync";
-const CHANGES_PATH = "/v1/buckets/monitor/collections/changes/records";
+const CHANGES_PATH = "/v1" + Utils.CHANGES_PATH;
 
 var server;
 
 async function clear_state() {
   // set up prefs so the kinto updater talks to the test server
   Services.prefs.setCharPref(PREF_SETTINGS_SERVER,
     `http://localhost:${server.identity.primaryPort}/v1`);