Bug 1547995 - Add option to Remote Settings get() to verify signatures on read r=glasserc
☠☠ backed out by 40377335c3b5 ☠ ☠
authorMathieu Leplatre <mathieu@mozilla.com>
Thu, 09 May 2019 16:38:57 +0000
changeset 532362 0bcfcc0bbc658890cbb6e218b2f2f06b42a56b16
parent 532361 411f5783f0466ee478325b2b32f3d13d74f077ec
child 532363 efacc855c35612cec29992fd6149e38db2fd1690
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglasserc
bugs1547995
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 1547995 - Add option to Remote Settings get() to verify signatures on read r=glasserc Differential Revision: https://phabricator.services.mozilla.com/D30357
services/settings/RemoteSettingsClient.jsm
services/settings/test/unit/test_remote_settings.js
--- a/services/settings/RemoteSettingsClient.jsm
+++ b/services/settings/RemoteSettingsClient.jsm
@@ -64,53 +64,16 @@ class ClientEnvironment extends ClientEn
 
   static get toolkitVersion() {
     Services.appinfo.QueryInterface(Ci.nsIPlatformInfo);
     return Services.appinfo.platformVersion;
   }
 }
 
 /**
- * Retrieve the Autograph signature information from the collection metadata.
- *
- * @param {String} bucket Bucket name.
- * @param {String} collection Collection name.
- * @param {int} expectedTimestamp Timestamp to be used for cache busting.
- * @returns {Promise<{String, String}>}
- */
-async function fetchCollectionSignature(bucket, collection, expectedTimestamp) {
-  const client = new KintoHttpClient(gServerURL);
-  const { signature: signaturePayload } = await client.bucket(bucket)
-    .collection(collection)
-    .getData({ query: { _expected: expectedTimestamp } });
-  if (!signaturePayload) {
-    throw new RemoteSettingsClient.MissingSignatureError(`${bucket}/${collection}`);
-  }
-  const { x5u, signature } = signaturePayload;
-  const certChainResponse = await fetch(x5u);
-  const certChain = await certChainResponse.text();
-
-  return { signature, certChain };
-}
-
-/**
- * Retrieve the current list of remote records.
- *
- * @param {String} bucket Bucket name.
- * @param {String} collection Collection name.
- * @param {int} expectedTimestamp Timestamp to be used for cache busting.
- */
-async function fetchRemoteRecords(bucket, collection, expectedTimestamp) {
-  const client = new KintoHttpClient(gServerURL);
-  return client.bucket(bucket)
-    .collection(collection)
-    .listRecords({ sort: "id", filters: { _expected: expectedTimestamp } });
-}
-
-/**
  * Minimalist event emitter.
  *
  * Note: we don't use `toolkit/modules/EventEmitter` because **we want** to throw
  * an error when a listener fails to execute.
  */
 class EventEmitter {
   constructor(events) {
     this._listeners = new Map();
@@ -183,16 +146,17 @@ class RemoteSettingsClient extends Event
   constructor(collectionName, { bucketNamePref, signerName, filterFunc, localFields = [], lastCheckTimePref }) {
     super(["sync"]); // emitted events
 
     this.collectionName = collectionName;
     this.signerName = signerName;
     this.filterFunc = filterFunc;
     this.localFields = localFields;
     this._lastCheckTimePref = lastCheckTimePref;
+    this._verifier = null;
 
     // 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;
@@ -224,50 +188,66 @@ class RemoteSettingsClient extends Event
       bucket: this.bucketName,
     };
     return this._kinto.collection(this.collectionName, options);
   }
 
   /**
    * Lists settings.
    *
-   * @param  {Object} options             The options object.
-   * @param  {Object} options.filters     Filter the results (default: `{}`).
-   * @param  {Object} options.order       The order to apply (eg. `"-last_modified"`).
-   * @param  {Object} options.syncIfEmpty Synchronize from server if local data is empty (default: `true`).
+   * @param  {Object} options                  The options object.
+   * @param  {Object} options.filters          Filter the results (default: `{}`).
+   * @param  {String} options.order            The order to apply (eg. `"-last_modified"`).
+   * @param  {boolean} options.syncIfEmpty     Synchronize from server if local data is empty (default: `true`).
+   * @param  {boolean} options.verifySignature Verify the signature of the local data (default: `false`).
    * @return {Promise}
    */
   async get(options = {}) {
     const {
       filters = {},
       order = "", // not sorted by default.
       syncIfEmpty = true,
     } = options;
+    let { verifySignature = false } = options;
 
     if (syncIfEmpty && !(await Utils.hasLocalData(this))) {
       try {
         // .get() was called before we had the chance to synchronize the local database.
         // We'll try to avoid returning an empty list.
         if (await Utils.hasLocalDump(this.bucketName, this.collectionName)) {
           // Since there is a JSON dump, load it as default data.
           await RemoteSettingsWorker.importJSONDump(this.bucketName, this.collectionName);
         } else {
           // There is no JSON dump, force a synchronization from the server.
           await this.sync({ loadDump: false });
         }
+        // Either from trusted dump, or already done during sync.
+        verifySignature = false;
       } catch (e) {
         // Report but return an empty list since there will be no data anyway.
         Cu.reportError(e);
         return [];
       }
     }
 
     // Read from the local DB.
-    const kintoCol = await this.openCollection();
-    const { data } = await kintoCol.list({ filters, order });
+    const kintoCollection = await this.openCollection();
+    const { data } = await kintoCollection.list({ filters, order });
+
+    // Verify signature of local data.
+    if (verifySignature) {
+      const localRecords = data.map(r => kintoCollection.cleanLocalFields(r));
+      const timestamp = await kintoCollection.db.getLastModified();
+      const metadata = await kintoCollection.metadata();
+      await this._validateCollectionSignature([],
+                                              timestamp,
+                                              metadata,
+                                              { localRecords });
+    }
+
     // Filter the records based on `this.filterFunc` results.
     return this._filterEntries(data);
   }
 
   /**
    * Synchronize the local database with the remote server.
    *
    * @param {Object} options See #maybeSync() options.
@@ -331,20 +311,25 @@ class RemoteSettingsClient extends Event
         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.verifySignature) {
         kintoCollection.hooks["incoming-changes"] = [async (payload, collection) => {
-          await this._validateCollectionSignature(payload.changes,
-                                                  payload.lastModified,
-                                                  collection,
-                                                  { expectedTimestamp });
+          const { changes: remoteRecords, lastModified: timestamp } = payload;
+          const { data } = await kintoCollection.list({ order: "" }); // no need to sort.
+          const metadata = await collection.metadata();
+          // Local fields are stripped to compute the collection signature (server does not have them).
+          const localRecords = data.map(r => kintoCollection.cleanLocalFields(r));
+          await this._validateCollectionSignature(remoteRecords,
+                                                  timestamp,
+                                                  metadata,
+                                                  { localRecords });
           // In case the signature is valid, apply the changes locally.
           return payload;
         }];
       }
 
       let syncResult;
       try {
         // Fetch changes from server, and make sure we overwrite local data.
@@ -428,85 +413,93 @@ class RemoteSettingsClient extends Event
   }
 
   /**
    * Fetch the signature info from the collection metadata and verifies that the
    * local set of records has the same.
    *
    * @param {Array<Object>} remoteRecords   The list of changes to apply to the local database.
    * @param {int} timestamp                 The timestamp associated with the list of remote records.
-   * @param {Collection} kintoCollection    Kinto.js Collection instance.
+   * @param {Object} metadata               The collection metadata, that contains the signature payload.
    * @param {Object} options
-   * @param {int} options.expectedTimestamp Cache busting of collection metadata
-   * @param {Boolean} options.ignoreLocal   When the signature verification is retried, since we refetch
-   *                                        the whole collection, we don't take into account the local
-   *                                        data (default: `false`)
+   * @param {Array<Object>} options.localRecords List of additional local records to take into account (default: `[]`).
    * @returns {Promise}
    */
-  async _validateCollectionSignature(remoteRecords, timestamp, kintoCollection, options = {}) {
-    const { expectedTimestamp, ignoreLocal = false } = options;
-    // this is a content-signature field from an autograph response.
-    const { name: collection, bucket } = kintoCollection;
-    const { signature, certChain } = await fetchCollectionSignature(bucket, collection, expectedTimestamp);
+  async _validateCollectionSignature(remoteRecords, timestamp, metadata, options = {}) {
+    const { localRecords = [] } = options;
 
-    let localRecords = [];
-    if (!ignoreLocal) {
-      const { data } = await kintoCollection.list({ order: "" }); // no need to sort.
-      // Local fields are stripped to compute the collection signature (server does not have them).
-      localRecords = data.map(r => kintoCollection.cleanLocalFields(r));
+    if (!metadata || !metadata.signature) {
+      throw new RemoteSettingsClient.MissingSignatureError(this.identifier);
     }
 
+    if (!this._verifier) {
+        this._verifier = Cc["@mozilla.org/security/contentsignatureverifier;1"]
+          .createInstance(Ci.nsIContentSignatureVerifier);
+    }
+
+    // This is a content-signature field from an autograph response.
+    const { signature: { x5u, signature } } = metadata;
+    const certChain = await (await fetch(x5u)).text();
+    // Merge remote records with local ones and serialize as canonical JSON.
     const serialized = await RemoteSettingsWorker.canonicalStringify(localRecords,
                                                                      remoteRecords,
                                                                      timestamp);
-    const verifier = Cc["@mozilla.org/security/contentsignatureverifier;1"]
-      .createInstance(Ci.nsIContentSignatureVerifier);
-    if (!await verifier.asyncVerifyContentSignature(serialized,
-                                                    "p384ecdsa=" + signature,
-                                                    certChain,
-                                                    this.signerName)) {
-      throw new RemoteSettingsClient.InvalidSignatureError(`${bucket}/${collection}`);
+    if (!await this._verifier.asyncVerifyContentSignature(serialized,
+                                                          "p384ecdsa=" + signature,
+                                                          certChain,
+                                                          this.signerName)) {
+      throw new RemoteSettingsClient.InvalidSignatureError(this.identifier);
     }
   }
 
   /**
    * Fetch the whole list of records from the server, verify the signature again
    * and then compute a synchronization result as if the diff-based sync happened.
    * And eventually, wipe out the local data.
    *
    * @param {Collection} kintoCollection    Kinto.js Collection instance.
    * @param {int}        expectedTimestamp  Cache busting of collection metadata
    *
    * @returns {Promise<Object>} the computed sync result.
    */
   async _retrySyncFromScratch(kintoCollection, expectedTimestamp) {
-    const payload = await fetchRemoteRecords(kintoCollection.bucket, kintoCollection.name, expectedTimestamp);
-    await this._validateCollectionSignature(payload.data,
-      payload.last_modified,
-      kintoCollection,
-      { expectedTimestamp, ignoreLocal: true });
+    // Fetch collection metadata.
+    const api = new KintoHttpClient(gServerURL);
+    const client = await api.bucket(this.bucketName).collection(this.collectionName);
+    const metadata = await client.getData({ query: { _expected: expectedTimestamp }});
+    // Fetch whole list of records.
+    const {
+      data: remoteRecords,
+      last_modified: timestamp,
+    } = await client.listRecords({ sort: "id", filters: { _expected: expectedTimestamp } });
+    // Verify signature of remote content, before importing it locally.
+    await this._validateCollectionSignature(remoteRecords,
+                                            timestamp,
+                                            metadata);
 
-    // The signature is good (we haven't thrown).
-    // Now we will Inspect what we had locally.
+    // The signature of this remote content is good (we haven't thrown).
+    // Now we will store it locally. In order to replicate what `.sync()` returns
+    // we will inspect what we had locally.
     const { data: oldData } = await kintoCollection.list({ order: "" }); // no need to sort.
 
     // We build a sync result as if a diff-based sync was performed.
     const syncResult = { created: [], updated: [], deleted: [] };
 
     // If the remote last_modified is newer than the local last_modified,
     // replace the local data
     const localLastModified = await kintoCollection.db.getLastModified();
-    if (payload.last_modified >= localLastModified) {
-      const { data: newData } = payload;
+    if (timestamp >= localLastModified) {
       await kintoCollection.clear();
-      await kintoCollection.loadDump(newData);
+      await kintoCollection.loadDump(remoteRecords);
+      await kintoCollection.db.saveLastModified(timestamp);
+      await kintoCollection.db.saveMetadata(metadata);
 
       // Compare local and remote to populate the sync result
       const oldById = new Map(oldData.map(e => [e.id, e]));
-      for (const r of newData) {
+      for (const r of remoteRecords) {
         const old = oldById.get(r.id);
         if (old) {
           if (old.last_modified != r.last_modified) {
             syncResult.updated.push({ old, new: r });
           }
           oldById.delete(r.id);
         } else {
           syncResult.created.push(r);
--- a/services/settings/test/unit/test_remote_settings.js
+++ b/services/settings/test/unit/test_remote_settings.js
@@ -180,16 +180,52 @@ add_task(async function test_get_ignores
   let data = await RemoteSettings("some-unknown-key").get();
   equal(data.length, 0);
   // The sync endpoints are not mocked, this fails internally.
   data = await RemoteSettings("no-mocked-responses").get();
   equal(data.length, 0);
 });
 add_task(clear_state);
 
+add_task(async function test_get_can_verify_signature() {
+  // No signature in metadata.
+  let error;
+  try {
+    await client.get({ verifySignature: true, syncIfEmpty: false });
+  } catch (e) {
+    error = e;
+  }
+  equal(error.message, "Missing signature (main/password-fields)");
+
+  // Populate the local DB (record and metadata)
+  await client.maybeSync(2000);
+
+  // It validates signature that was stored in local DB.
+  let calledSignature;
+  client._verifier = {
+    async asyncVerifyContentSignature(serialized, signature) {
+      calledSignature = signature;
+      return JSON.parse(serialized).data.length == 1;
+    },
+  };
+  await client.get({ verifySignature: true });
+  ok(calledSignature.endsWith("abcdef"));
+
+  // It throws when signature does not verify.
+  const col = await client.openCollection();
+  await col.delete("9d500963-d80e-3a91-6e74-66f3811b99cc");
+  try {
+    await client.get({ verifySignature: true });
+  } catch (e) {
+    error = e;
+  }
+  equal(error.message, "Invalid content signature (main/password-fields)");
+});
+add_task(clear_state);
+
 add_task(async function test_sync_event_provides_information_about_records() {
   let eventData;
   client.on("sync", ({ data }) => eventData = data);
 
   await client.maybeSync(2000);
   equal(eventData.current.length, 1);
 
   await client.maybeSync(3001);