Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-09 03:15:11 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1637178 - Distinguish local DB hijacks from sign errors in Uptake Telemetry r=Gijs

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D75529
This commit is contained in:
Mathieu Leplatre 2020-05-26 11:20:03 +00:00
parent a7c1394496
commit 97875bccdd
4 changed files with 180 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
services/common/uptake-telemetry.js
View File

@ -122,6 +122,7 @@ class UptakeTelemetry {
* Only supported in Events Telemetry:
*
* - `SHUTDOWN_ERROR`: Error occuring during shutdown.
* - `CORRUPTION_ERROR`: Error related to corrupted local data.
*
* @type {Object}
*/
@ -130,6 +131,7 @@ class UptakeTelemetry {
...UptakeTelemetry.HISTOGRAM_LABELS,
// Events only.
SHUTDOWN_ERROR: "shutdown_error",
CORRUPTION_ERROR: "corruption_error",
};
}

107
services/settings/RemoteSettingsClient.jsm
View File

@ -133,13 +133,22 @@ class InvalidSignatureError extends Error {
}
}
class MissingSignatureError extends Error {
class MissingSignatureError extends InvalidSignatureError {
constructor(cid) {
super(`Missing signature (${cid})`);
super(cid);
this.message = `Missing signature (${cid})`;
this.name = "MissingSignatureError";
}
}
class CorruptedDataError extends InvalidSignatureError {
constructor(cid) {
super(cid);
this.message = `Corrupted local data (${cid})`;
this.name = "CorruptedDataError";
}
}
class UnknownCollectionError extends Error {
constructor(cid) {
super(`Unknown Collection "${cid}"`);
@ -218,6 +227,9 @@ class RemoteSettingsClient extends EventEmitter {
static get MissingSignatureError() {
return MissingSignatureError;
}
static get CorruptedDataError() {
return CorruptedDataError;
}
static get UnknownCollectionError() {
return UnknownCollectionError;
}
@ -371,7 +383,7 @@ class RemoteSettingsClient extends EventEmitter {
if (syncIfEmpty && ObjectUtils.isEmpty(metadata)) {
// No sync occured yet, may have records from dump but no metadata.
console.debug(
`Required metadata for ${this.identifier}, fetching from server.`
`${this.identifier} Required metadata, fetching from server.`
);
metadata = await this.httpClient().getData();
await this.db.saveMetadata(metadata);
@ -565,9 +577,12 @@ class RemoteSettingsClient extends EventEmitter {
);
}
} catch (e) {
if (e instanceof RemoteSettingsClient.InvalidSignatureError) {
if (e instanceof InvalidSignatureError) {
// Signature verification failed during synchronization.
reportStatus = UptakeTelemetry.STATUS.SIGNATURE_ERROR;
reportStatus =
e instanceof CorruptedDataError
? UptakeTelemetry.STATUS.CORRUPTION_ERROR
: UptakeTelemetry.STATUS.SIGNATURE_ERROR;
// If sync fails with a signature error, it's likely that our
// local data has been modified in some way.
// We will attempt to fix this by retrieving the whole
@ -679,9 +694,6 @@ class RemoteSettingsClient extends EventEmitter {
if (e instanceof RemoteSettingsClient.NetworkOfflineError) {
reportStatus = UptakeTelemetry.STATUS.NETWORK_OFFLINE_ERROR;
} else if (e instanceof RemoteSettingsClient.MissingSignatureError) {
// Collection metadata has no signature info, no need to retry.
reportStatus = UptakeTelemetry.STATUS.SIGNATURE_ERROR;
} else if (e instanceof IDBHelpers.ShutdownError) {
reportStatus = UptakeTelemetry.STATUS.SHUTDOWN_ERROR;
} else if (/unparseable/.test(e.message)) {
@ -739,8 +751,8 @@ class RemoteSettingsClient extends EventEmitter {
async _validateCollectionSignature(records, timestamp, metadata) {
const start = Cu.now() * 1000;
if (!metadata || !metadata.signature) {
throw new RemoteSettingsClient.MissingSignatureError(this.identifier);
if (!metadata?.signature) {
throw new MissingSignatureError(this.identifier);
}
if (!this._verifier) {
@ -767,7 +779,7 @@ class RemoteSettingsClient extends EventEmitter {
this.signerName
))
) {
throw new RemoteSettingsClient.InvalidSignatureError(this.identifier);
throw new InvalidSignatureError(this.identifier);
}
if (gTimingEnabled) {
const end = Cu.now() * 1000;
@ -867,33 +879,60 @@ class RemoteSettingsClient extends EventEmitter {
metadata
);
} catch (e) {
// Signature failed, clear local data.
console.error(
`${this.identifier} Signature failed ${retry ? "again" : ""} ${e}`
);
if (!(e instanceof InvalidSignatureError)) {
// If it failed for any other kind of error (eg. shutdown)
// then give up quickly.
throw e;
}
// In order to distinguish signature errors that happen
// during sync, from hijacks of local DBs, we will verify
// the signature on the data that we had before syncing.
let localTrustworthy = false;
try {
await this._validateCollectionSignature(
localRecords,
localTimestamp,
localMetadata
);
localTrustworthy = true;
} catch (sigerr) {
if (!(sigerr instanceof InvalidSignatureError)) {
// If it fails for other reason, keep original error and give up.
throw sigerr;
}
}
// Signature failed, clear local DB because it contains
// bad data (local + remote changes).
console.debug(`${this.identifier} clear local data`);
await this.db.clear();
// If signature failed again after retry, then
// we restore the local data that we had before sync.
if (retry) {
try {
// Make sure the local data before sync was not tampered.
await this._validateCollectionSignature(
localRecords,
localTimestamp,
localMetadata
);
// Signature of data before sync is good. Restore it.
await this.db.importBulk(localRecords);
await this.db.saveLastModified(localTimestamp);
await this.db.saveMetadata(localMetadata);
} catch (_) {
// Local data before sync was tampered. Restore dump if available.
if (
await Utils.hasLocalDump(this.bucketName, this.collectionName)
) {
await this._importJSONDump();
}
}
if (!localTrustworthy && !retry) {
// Local data was tampered, throw and it will retry from empty DB.
console.error(`${this.identifier} local data was corrupted`);
throw new CorruptedDataError(this.identifier);
} else if (localTrustworthy) {
// Signature of data before importing changes was good.
// If we retry, we will pull and import changes again on top of them.
// If we retried already, we will restore the local data before
// throwing.
await this.db.importBulk(localRecords);
await this.db.saveLastModified(localTimestamp);
await this.db.saveMetadata(localMetadata);
} else if (
// The local data was tampered.
// We retried and signature failed again.
// So restore the dump if available.
await Utils.hasLocalDump(this.bucketName, this.collectionName)
) {
console.info(`${this.identifier} restore dump`);
await this._importJSONDump();
}
throw e;
}
} else {

79
services/settings/test/unit/test_remote_settings.js
View File

@ -6,6 +6,9 @@ const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
const { AppConstants } = ChromeUtils.import(
"resource://gre/modules/AppConstants.jsm"
);
const { ObjectUtils } = ChromeUtils.import(
"resource://gre/modules/ObjectUtils.jsm"
);
const IS_ANDROID = AppConstants.platform == "android";
@ -68,6 +71,10 @@ function run_test() {
"/v1/buckets/monitor/collections/changes/records",
handleResponse
);
server.registerPathHandler(
"/v1/buckets/main/collections/password-fields",
handleResponse
);
server.registerPathHandler(
"/v1/buckets/main/collections/password-fields/changeset",
handleResponse
@ -304,8 +311,8 @@ add_task(async function test_get_ignores_synchronization_errors() {
});
add_task(clear_state);
add_task(async function test_get_can_verify_signature() {
// No signature in metadata.
add_task(async function test_get_verify_signature_no_sync() {
// No signature in metadata, and no sync if empty.
let error;
try {
await client.get({ verifySignature: true, syncIfEmpty: false });
@ -313,7 +320,35 @@ add_task(async function test_get_can_verify_signature() {
error = e;
}
equal(error.message, "Missing signature (main/password-fields)");
});
add_task(clear_state);
add_task(async function test_get_can_verify_signature_pulled() {
// Populate the local DB (only records, eg. loaded from dump previously)
await client._importJSONDump();
let calledSignature;
client._verifier = {
async asyncVerifyContentSignature(serialized, signature) {
calledSignature = signature;
return true;
},
};
// No metadata in local DB, but gets pulled and then verifies.
ok(ObjectUtils.isEmpty(await client.db.getMetadata()), "Metadata is empty");
await client.get({ verifySignature: true });
ok(
!ObjectUtils.isEmpty(await client.db.getMetadata()),
"Metadata was pulled"
);
ok(calledSignature.endsWith("some-sig"), "Signature was verified");
});
add_task(clear_state);
add_task(async function test_get_can_verify_signature() {
// Populate the local DB (record and metadata)
await client.maybeSync(2000);
@ -325,12 +360,14 @@ add_task(async function test_get_can_verify_signature() {
return JSON.parse(serialized).data.length == 1;
},
};
ok(await Utils.hasLocalData(client), "Local data was populated");
await client.get({ verifySignature: true });
ok(calledSignature.endsWith("abcdef"));
ok(calledSignature.endsWith("abcdef"), "Signature was verified");
// It throws when signature does not verify.
await client.db.delete("9d500963-d80e-3a91-6e74-66f3811b99cc");
error = null;
let error = null;
try {
await client.get({ verifySignature: true });
} catch (e) {
@ -967,7 +1004,9 @@ wNuvFqc=
],
status: { status: 200, statusText: "OK" },
responseBody: {
metadata: {},
metadata: {
signature: {},
},
timestamp: 4000,
changes: [
{
@ -995,7 +1034,9 @@ wNuvFqc=
],
status: { status: 200, statusText: "OK" },
responseBody: {
metadata: {},
metadata: {
signature: {},
},
timestamp: 5000,
changes: [
{
@ -1084,6 +1125,24 @@ wNuvFqc=
],
},
},
"GET:/v1/buckets/main/collections/password-fields": {
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"',
],
status: { status: 200, statusText: "OK" },
responseBody: {
data: {
signature: {
signature: "some-sig",
x5u: `http://localhost:${port}/fake-x5u`,
},
},
},
},
"GET:/v1/buckets/main/collections/password-fields/changeset?_expected=1337": {
sampleHeaders: [
"Access-Control-Allow-Origin: *",
@ -1094,7 +1153,9 @@ wNuvFqc=
],
status: { status: 200, statusText: "OK" },
responseBody: {
metadata: {},
metadata: {
signature: {},
},
timestamp: 3000,
changes: [
{
@ -1202,7 +1263,9 @@ wNuvFqc=
status: { status: 200, statusText: "OK" },
responseBody: {
timestamp: 3000,
metadata: {},
metadata: {
signature: {},
},
changes: [
{
id: "1f5c98b9-6d93-4c13-aa26-978b38695096",

91
services/settings/test/unit/test_remote_settings_signatures.js
View File

@ -9,6 +9,9 @@ const { RemoteSettings } = ChromeUtils.import(
const { UptakeTelemetry } = ChromeUtils.import(
"resource://services-common/uptake-telemetry.js"
);
const { TelemetryTestUtils } = ChromeUtils.import(
"resource://testing-common/TelemetryTestUtils.jsm"
);
const PREF_SETTINGS_SERVER = "services.settings.server";
const PREF_SIGNATURE_ROOT = "security.content.signature.root_hash";
@ -590,6 +593,7 @@ add_task(async function test_check_synchronization_with_signatures() {
// the local DB contains same id as RECORD2 and a fake record.
// the final server collection contains RECORD2 and RECORD3
await client.db.clear();
await client.db.saveMetadata({ signature: { x5u, signature: "abc" } });
await client.db.create(
{ ...RECORD2, last_modified: 1234567890, serialNumber: "abc" },
{ synced: true, useRecordId: true }
@ -602,11 +606,34 @@ add_task(async function test_check_synchronization_with_signatures() {
syncData = data;
});
await client.maybeSync(5000);
// Clear events snapshot.
TelemetryTestUtils.assertEvents([], {}, { process: "dummy" });
// Local data was replaced. But we use records IDs to determine
// what was created and deleted. So fake local data will appear
// in the sync event.
await withFakeChannel("nightly", async () => {
// Events telemetry is sampled on released, use fake channel.
await client.maybeSync(5000);
// We should report a corruption_error.
TelemetryTestUtils.assertEvents([
[
"uptake.remotecontent.result",
"uptake",
"remotesettings",
UptakeTelemetry.STATUS.CORRUPTION_ERROR,
{
source: client.identifier,
duration: v => v > 0,
trigger: "manual",
},
],
]);
});
// The local data was corrupted, and the Telemetry status reflects it.
// But the sync overwrote the bad data and was eventually a success.
// Since local data was replaced, we use records IDs to determine
// what was created and deleted. And bad local data will appear
// in the sync event as deleted.
equal(syncData.current.length, 2);
equal(syncData.created.length, 1);
equal(syncData.created[0].id, RECORD3.id);
@ -636,7 +663,7 @@ add_task(async function test_check_synchronization_with_signatures() {
metadata: {
signature: {
x5u,
signature: "wrong-sig-here-too",
signature: "aaaaaaaaaaaaaaaaaaaaaaaa", // sig verifier wants proper length or will crash.
},
},
changes: [
@ -671,13 +698,13 @@ add_task(async function test_check_synchronization_with_signatures() {
checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
// When signature fails after retry, the local data present before sync
// should be maintained.
// should be maintained (if its signature is valid).
ok(
arrayEqual(
(await client.get()).map(r => r.id),
[RECORD3.id, RECORD2.id]
),
"Remote changes were not changed"
"Local records were not changed"
);
// And local data should still be valid.
await client.get({ verifySignature: true }); // Not raising.
@ -703,54 +730,4 @@ add_task(async function test_check_synchronization_with_signatures() {
}
// Since local data was tampered, it was cleared.
equal((await client.get()).length, 0, "Local database is now empty.");
//
// 11.
// - collection: [] -> []
// - timestamp: 4000 -> 6000
//
// Check that we don't apply changes when signature is missing in remote.
const RESPONSE_NO_SIG = {
sampleHeaders: [
"Content-Type: application/json; charset=UTF-8",
`ETag: \"123456\"`,
],
status: { status: 200, statusText: "OK" },
responseBody: JSON.stringify({
metadata: {
last_modified: 123456,
},
changes: [],
timestamp: 123456,
}),
};
const missingSigResponses = {
// In this test, we deliberately serve metadata without the signature attribute.
// As if the collection was not signed.
"GET:/v1/buckets/main/collections/signed/changeset?_expected=6000": [
RESPONSE_NO_SIG,
],
};
// Local data was empty after last test.
equal((await client.get()).length, 0);
startHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY);
registerHandlers(missingSigResponses);
try {
await client.maybeSync(6000);
do_throw("Sync should fail (the signature is missing)");
} catch (e) {
equal((await client.get()).length, 0, "Local remains empty");
}
// 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);
});