From b25d0ae2e5d75da8c5e0543bcb88c14de2b0ad87 Mon Sep 17 00:00:00 2001
From: Mathieu Leplatre <mathieu@mozilla.com>
Date: Fri, 22 May 2020 00:27:06 +0000
Subject: [PATCH] Bug 1639224 - Verify signature if local timestamp is in the
 future r=Gijs

Differential Revision: https://phabricator.services.mozilla.com/D76249
---
 services/settings/RemoteSettingsClient.jsm       | 10 ++++++----
 .../settings/test/unit/test_remote_settings.js   | 16 +++++++++++-----
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/services/settings/RemoteSettingsClient.jsm b/services/settings/RemoteSettingsClient.jsm
index 300fb048567f..36fcad1cfd54 100644
--- a/services/settings/RemoteSettingsClient.jsm
+++ b/services/settings/RemoteSettingsClient.jsm
@@ -286,6 +286,8 @@ class RemoteSettingsClient extends EventEmitter {
 
   /**
    * Retrieve the collection timestamp for the last synchronization.
+   * This is an opaque and comparable value assigned automatically by
+   * the server.
    *
    * @returns {number}
    *          The timestamp in milliseconds, returns -1 if retrieving
@@ -484,11 +486,10 @@ class RemoteSettingsClient extends EventEmitter {
           Cu.reportError(e);
         }
       }
-
       let syncResult;
       try {
         // Is local timestamp up to date with the server?
-        if (expectedTimestamp <= collectionLastModified) {
+        if (expectedTimestamp == collectionLastModified) {
           console.debug(`${this.identifier} local data is up-to-date`);
           reportStatus = UptakeTelemetry.STATUS.UP_TO_DATE;
 
@@ -526,8 +527,9 @@ class RemoteSettingsClient extends EventEmitter {
             deleted: [],
           };
         } else {
-          // Local data is outdated.
-          // 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 startSyncDB = Cu.now() * 1000;
           syncResult = await this._importChanges(
             localRecords,
diff --git a/services/settings/test/unit/test_remote_settings.js b/services/settings/test/unit/test_remote_settings.js
index 12bb6211a512..889680d0785c 100644
--- a/services/settings/test/unit/test_remote_settings.js
+++ b/services/settings/test/unit/test_remote_settings.js
@@ -178,13 +178,18 @@ add_task(async function test_sync_event_is_sent_even_if_up_to_date() {
     // Skip test: we don't ship remote settings dumps on Android (see package-manifest).
     return;
   }
+  // First, determine what is the dump timestamp. Sync will load it.
+  // Use a timestamp inferior to latest record in dump.
+  await clientWithDump._importJSONDump();
+  const uptodateTimestamp = await clientWithDump.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);
@@ -412,10 +417,11 @@ add_task(
     ok(records.length > 0, "dump is loaded");
     ok(!called, "signature is missing but not verified");
 
-    // Synchronize the collection (local data is up-to-date, collection last modified > 42)
+    // Synchronize the collection (local data is up-to-date).
     // Signature verification is disabled (see `clear_state()`), so we don't bother with
     // fetching metadata.
-    await clientWithDump.maybeSync(42);
+    const uptodateTimestamp = await clientWithDump.db.getLastModified();
+    await clientWithDump.maybeSync(uptodateTimestamp);
     let metadata = await clientWithDump.db.getMetadata();
     ok(!metadata, "metadata was not fetched");