Bug 1547995 - Add option to Remote Settings get() to verify signatures on read r=glasserc
authorMathieu Leplatre <mathieu@mozilla.com>
Mon, 13 May 2019 09:31:18 +0000
changeset 532571 8fb278dd620a5dab42e6ecc175ab7418ec62a72a
parent 532570 3840128adf0a6c129ef34650fdf4d5cc0a79db38
child 532572 28912333e8b808d478f432ca66bc6349c3624349
push id11270
push userrgurzau@mozilla.com
push dateWed, 15 May 2019 15:07:19 +0000
treeherdermozilla-beta@571bc76da583 [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
@@ -79,16 +79,17 @@ function run_test() {
   const configPath = "/v1/";
   const changesPath = "/v1/buckets/monitor/collections/changes/records";
   const metadataPath = "/v1/buckets/main/collections/password-fields";
   const recordsPath  = "/v1/buckets/main/collections/password-fields/records";
   server.registerPathHandler(configPath, handleResponse);
   server.registerPathHandler(changesPath, handleResponse);
   server.registerPathHandler(metadataPath, handleResponse);
   server.registerPathHandler(recordsPath, handleResponse);
+  server.registerPathHandler("/fake-x5u", handleResponse);
 
   run_next_test();
 
   registerCleanupFunction(function() {
     server.stop(() => { });
   });
 }
 
@@ -180,16 +181,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);
@@ -532,21 +569,33 @@ function getSampleResponse(req, port) {
       ],
       "status": { status: 200, statusText: "OK" },
       "responseBody": JSON.stringify({
         "data": {
           "id": "password-fields",
           "last_modified": 1234,
           "signature": {
             "signature": "abcdef",
-            "x5u": "http://localhost/dummy",
+            "x5u": `http://localhost:${port}/fake-x5u`,
           },
         },
       }),
     },
+    "GET:/fake-x5u": {
+      "sampleHeaders": [
+        "Content-Type: /octet-stream",
+      ],
+      "status": { status: 200, statusText: "OK" },
+      "responseBody": `-----BEGIN CERTIFICATE-----
+MIIGYTCCBEmgAwIBAgIBATANBgkqhkiG9w0BAQwFADB9MQswCQYDVQQGEwJVU
+ZARKjbu1TuYQHf0fs+GwID8zeLc2zJL7UzcHFwwQ6Nda9OJN4uPAuC/BKaIpxCLL
+26b24/tRam4SJjqpiq20lynhUrmTtt6hbG3E1Hpy3bmkt2DYnuMFwEx2gfXNcnbT
+wNuvFqc=
+-----END CERTIFICATE-----`,
+    },
     "GET:/v1/buckets/main/collections/password-fields/records?_expected=2000&_sort=-last_modified": {
       "sampleHeaders": [
         "Access-Control-Allow-Origin: *",
         "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff",
         "Content-Type: application/json; charset=UTF-8",
         "Server: waitress",
         "Etag: \"3000\"",
       ],