Bug 1453690 - Fail nicely when RemoteSetting collection is not signed r=mgoodwin

MozReview-Commit-ID: ESP7GXfmYzU

--HG--
extra : rebase_source : 659f97e6d5317a5662c5d7f09fcf770d5928b444

services/common/remote-settings.js

@@ -35,6 +35,7 @@ const PREF_SETTINGS_LOAD_DUMP          = "services.settings.load_dump";
 const TELEMETRY_HISTOGRAM_KEY = "settings-changes-monitoring";
 
 const INVALID_SIGNATURE = "Invalid content/signature";
+const MISSING_SIGNATURE = "Missing signature";
 
 
 function mergeChanges(collection, localRecords, changes) {
@@ -57,15 +58,15 @@ function mergeChanges(collection, localRecords, changes) {
 }
 
 
-function fetchCollectionMetadata(remote, collection) {
+async function fetchCollectionMetadata(remote, collection) {
   const client = new KintoHttpClient(remote);
-  return client.bucket(collection.bucket).collection(collection.name).getData()
-    .then(result => {
-      return result.signature;
-    });
+  const { signature } = await client.bucket(collection.bucket)
+                                    .collection(collection.name)
+                                    .getData();
+  return signature;
 }
 
-function fetchRemoteCollection(remote, collection) {
+async function fetchRemoteCollection(remote, collection) {
   const client = new KintoHttpClient(remote);
   return client.bucket(collection.bucket)
            .collection(collection.name)
@@ -302,8 +303,11 @@ class RemoteSettingsClient {
           }
 
         } else {
-          // The sync has thrown, it can be a network or a general error.
-          if (/NetworkError/.test(e.message)) {
+          // The sync has thrown, it can be related to metadata, network or a general error.
+          if (e.message == MISSING_SIGNATURE) {
+            // Collection metadata has no signature info, no need to retry.
+            reportStatus = UptakeTelemetry.STATUS.SIGNATURE_ERROR;
+          } else if (/NetworkError/.test(e.message)) {
             reportStatus = UptakeTelemetry.STATUS.NETWORK_ERROR;
           } else if (/Backoff/.test(e.message)) {
             reportStatus = UptakeTelemetry.STATUS.BACKOFF;
@@ -367,9 +371,12 @@ class RemoteSettingsClient {
 
   async _validateCollectionSignature(remote, payload, collection, options = {}) {
     const {ignoreLocal} = options;
-
     // this is a content-signature field from an autograph response.
-    const {x5u, signature} = await fetchCollectionMetadata(remote, collection);
+    const signaturePayload = await fetchCollectionMetadata(remote, collection);
+    if (!signaturePayload) {
+      throw new Error(MISSING_SIGNATURE);
+    }
+    const {x5u, signature} = signaturePayload;
     const certChainResponse = await fetch(x5u);
     const certChain = await certChainResponse.text();
 

services/common/tests/unit/test_blocklist_signatures.js

@@ -262,6 +262,19 @@ add_task(async function test_check_signatures() {
   const RESPONSE_BODY_META_EMPTY_SIG = makeMetaResponseBody(1000,
     "vxuAg5rDCB-1pul4a91vqSBQRXJG_j7WOYUTswxRSMltdYmbhLRH8R8brQ9YKuNDF56F-w6pn4HWxb076qgKPwgcEBtUeZAO_RtaHXRkRUUgVzAr86yQL4-aJTbv3D6u");
 
+  const RESPONSE_META_NO_SIG = {
+    sampleHeaders: [
+      "Content-Type: application/json; charset=UTF-8",
+      `ETag: \"123456\"`
+    ],
+    status: {status: 200, statusText: "OK"},
+    responseBody: JSON.stringify({
+      data: {
+        last_modified: 123456
+      }
+    })
+  };
+
   // The collection metadata containing the signature for the empty
   // collection.
   const RESPONSE_META_EMPTY_SIG =
@@ -563,6 +576,31 @@ add_task(async function test_check_signatures() {
   endHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY);
   expectedIncrements = {[UptakeTelemetry.STATUS.SIGNATURE_RETRY_ERROR]: 1};
   checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
+
+
+  const missingSigResponses = {
+    // In this test, we deliberately serve metadata without the signature attribute.
+    // As if the collection was not signed.
+    "GET:/v1/buckets/blocklists/collections/certificates?":
+      [RESPONSE_META_NO_SIG],
+  };
+
+  startHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY);
+  registerHandlers(missingSigResponses);
+  try {
+    await OneCRLBlocklistClient.maybeSync(6000, startTime);
+    do_throw("Sync should fail (the signature is missing)");
+  } catch (e) {
+    await checkRecordCount(OneCRLBlocklistClient, 2);
+  }
+
+  // Ensure that the failure is reflected in the accumulated telemetry:
+  endHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY);
+  expectedIncrements = {
+    [UptakeTelemetry.STATUS.SIGNATURE_ERROR]: 1,
+    [UptakeTelemetry.STATUS.SIGNATURE_RETRY_ERROR]: 0  // Not retried since missing.
+  };
+  checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
 });
 
 function run_test() {