Bug 1639224 - Verify signature if local timestamp is in the future. r=Gijs, a=dveditz
☠☠ backed out by 07f7d20b2c15 ☠ ☠
authorMathieu Leplatre <mathieu@mozilla.com>
Sun, 24 May 2020 23:37:47 +0200
changeset 524830 c36bb4f2f5e5003ab4df862bd50368180aed0598
parent 524829 555ae5322bbbc633c9ce24b1b2681657675bd3d1
child 524831 07f7d20b2c15a16e40fa809c69367611218595bd
push id1038
push userarchaeopteryx@coole-files.de
push dateSun, 24 May 2020 21:44:15 +0000
treeherdermozilla-esr68@c36bb4f2f5e5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs, dveditz
bugs1639224
milestone68.9.0
Bug 1639224 - Verify signature if local timestamp is in the future. r=Gijs, a=dveditz
services/settings/RemoteSettingsClient.jsm
services/settings/test/unit/test_remote_settings.js
--- a/services/settings/RemoteSettingsClient.jsm
+++ b/services/settings/RemoteSettingsClient.jsm
@@ -394,31 +394,33 @@ class RemoteSettingsClient extends Event
           },
         ];
       }
 
       let syncResult;
       try {
         // If the data is up to date, there's no need to sync. We still need
         // to record the fact that a check happened.
-        if (expectedTimestamp <= collectionLastModified) {
+        if (expectedTimestamp == collectionLastModified) {
           reportStatus = UptakeTelemetry.STATUS.UP_TO_DATE;
           // Since the data is up-to-date, if we didn't load any dump then we're done here.
           if (importedFromDump.length == 0) {
             return;
           }
           // Otherwise we want to continue with sending the sync event to notify about the created records.
           syncResult = {
             current: importedFromDump,
             created: importedFromDump,
             updated: [],
             deleted: [],
           };
         } else {
-          // Fetch changes from server, and make sure we overwrite local data.
+          // Local data is either outdated or tampered.
+          // In both cases we will fetch changes from server,
+          // and make sure we overwrite local data.
           const strategy = Kinto.syncStrategy.SERVER_WINS;
           syncResult = await kintoCollection.sync({
             remote: gServerURL,
             strategy,
             expectedTimestamp,
           });
           if (!syncResult.ok) {
             // With SERVER_WINS, there cannot be any conflicts, but don't silent it anyway.
--- a/services/settings/test/unit/test_remote_settings.js
+++ b/services/settings/test/unit/test_remote_settings.js
@@ -7,16 +7,19 @@ const { AppConstants } = ChromeUtils.imp
   "resource://gre/modules/AppConstants.jsm"
 );
 
 const IS_ANDROID = AppConstants.platform == "android";
 
 const { RemoteSettings } = ChromeUtils.import(
   "resource://services-settings/remote-settings.js"
 );
+const { RemoteSettingsWorker } = ChromeUtils.import(
+  "resource://services-settings/RemoteSettingsWorker.jsm"
+);
 const { Utils } = ChromeUtils.import("resource://services-settings/Utils.jsm");
 const { UptakeTelemetry } = ChromeUtils.import(
   "resource://services-common/uptake-telemetry.js"
 );
 const { TelemetryTestUtils } = ChromeUtils.import(
   "resource://testing-common/TelemetryTestUtils.jsm"
 );
 
@@ -125,23 +128,31 @@ add_task(async function test_records_obt
 });
 add_task(clear_state);
 
 add_task(async function test_sync_event_is_sent_even_if_up_to_date() {
   if (IS_ANDROID) {
     // Skip test: we don't ship remote settings dumps on Android (see package-manifest).
     return;
   }
+  // First, determine what is the dump timestamp.
+  await RemoteSettingsWorker.importJSONDump(
+    clientWithDump.bucketName,
+    clientWithDump.collectionName
+  );
+  const kintoCol = await clientWithDump.openCollection();
+  const uptodateTimestamp = await kintoCol.db.getLastModified();
+  await clear_state();
+
+  // Now, simulate that server data wasn't changed since dump was released.
   const startHistogram = getUptakeTelemetrySnapshot(clientWithDump.identifier);
   let received;
   clientWithDump.on("sync", ({ data }) => (received = data));
-  // Use a timestamp inferior to latest record in dump.
-  const timestamp = 1000000000000; // Sun Sep 09 2001
 
-  await clientWithDump.maybeSync(timestamp);
+  await clientWithDump.maybeSync(uptodateTimestamp);
 
   ok(received.current.length > 0, "Dump records are listed as created");
   equal(received.current.length, received.created.length);
 
   const endHistogram = getUptakeTelemetrySnapshot(clientWithDump.identifier);
   const expectedIncrements = { [UptakeTelemetry.STATUS.UP_TO_DATE]: 1 };
   checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
 });
@@ -152,17 +163,17 @@ add_task(async function test_records_can
   await c.maybeSync(2000);
 
   const col = await c.openCollection();
   await col.update({
     id: "9d500963-d80e-3a91-6e74-66f3811b99cc",
     accepted: true,
   });
 
-  await c.maybeSync(2000); // Does not fail.
+  await c.maybeSync(3000); // Does not fail.
 });
 add_task(clear_state);
 
 add_task(
   async function test_records_changes_are_overwritten_by_server_changes() {
     // Create some local conflicting data, and make sure it syncs without error.
     const collection = await client.openCollection();
     await collection.create(