commit 9fb40f43f57174e2d41468c5d8998ae35aa36139
parent 361b6c9ea57d0706b788998387e50c14be9eda66
Author: Mathieu Leplatre <mathieu@mozilla.com>
Date: Tue, 28 Oct 2025 17:40:17 +0000
Bug 1984079 - Iterate through list of signatures r=say-yawn
Differential Revision: https://phabricator.services.mozilla.com/D268863
Diffstat:
4 files changed, 142 insertions(+), 89 deletions(-)
diff --git a/services/settings/RemoteSettingsClient.sys.mjs b/services/settings/RemoteSettingsClient.sys.mjs
@@ -1067,7 +1067,11 @@ export class RemoteSettingsClient extends EventEmitter {
* The collection metadata, that contains the signature payload.
*/
async validateCollectionSignature(records, timestamp, metadata) {
- if (!metadata?.signature) {
+ if (
+ !metadata?.signatures ||
+ !Array.isArray(metadata.signatures) ||
+ metadata.signatures.length === 0
+ ) {
throw new MissingSignatureError(this.identifier);
}
@@ -1077,29 +1081,48 @@ export class RemoteSettingsClient extends EventEmitter {
].createInstance(Ci.nsIContentSignatureVerifier);
}
- // This is a content-signature field from an autograph response.
- const {
- signature: { x5u, signature },
- } = metadata;
- const certChain = await (await lazy.Utils.fetch(x5u)).text();
// Merge remote records with local ones and serialize as canonical JSON.
const serialized = await lazy.RemoteSettingsWorker.canonicalStringify(
records,
timestamp
);
- lazy.console.debug(`${this.identifier} verify signature using ${x5u}`);
- if (
- !(await this._verifier.asyncVerifyContentSignature(
- serialized,
- "p384ecdsa=" + signature,
- certChain,
- this.signerName,
- lazy.Utils.CERT_CHAIN_ROOT_IDENTIFIER
- ))
- ) {
- throw new InvalidSignatureError(this.identifier, x5u, this.signerName);
+ // Iterate over the list of signatures until we find a valid one.
+ const thrownErrors = [];
+ const { signatures } = metadata;
+ for (const sig of signatures) {
+ // This is a content-signature field from an autograph response.
+ const { x5u, signature, mode } = sig;
+ if (!x5u || !signature || (mode && mode !== "p384ecdsa")) {
+ lazy.console.warn(
+ `${this.identifier} ignore unsupported signature entry in metadata`
+ );
+ continue;
+ }
+ const certChain = await (await lazy.Utils.fetch(x5u)).text();
+ lazy.console.debug(`${this.identifier} verify signature using ${x5u}`);
+
+ if (
+ await this._verifier.asyncVerifyContentSignature(
+ serialized,
+ "p384ecdsa=" + signature,
+ certChain,
+ this.signerName,
+ lazy.Utils.CERT_CHAIN_ROOT_IDENTIFIER
+ )
+ ) {
+ // Signature is valid, exit!
+ return;
+ }
+ // Try next signature.
+ thrownErrors.push(
+ new InvalidSignatureError(this.identifier, x5u, this.signerName)
+ );
}
+
+ // We now that the list of signature is not empty, so if we are here
+ // it means that none was valid.
+ throw thrownErrors[0];
}
/**
diff --git a/services/settings/test/unit/test_remote_settings.js b/services/settings/test/unit/test_remote_settings.js
@@ -88,8 +88,8 @@ add_task(async function test_records_obtained_from_server_are_stored_in_db() {
const timestamp = await client.db.getLastModified();
equal(timestamp, 3000, "timestamp was stored");
- const { signature } = await client.db.getMetadata();
- equal(signature.signature, "abcdef", "metadata was stored");
+ const { signatures } = await client.db.getMetadata();
+ equal(signatures[0].signature, "abcdef", "metadata was stored");
});
add_task(clear_state);
@@ -1510,10 +1510,12 @@ wNuvFqc=
metadata: {
id: "password-fields",
last_modified: 1234,
- signature: {
- signature: "abcdef",
- x5u: `http://localhost:${port}/fake-x5u`,
- },
+ signatures: [
+ {
+ signature: "abcdef",
+ x5u: `http://localhost:${port}/fake-x5u`,
+ },
+ ],
},
changes: [
{
@@ -1537,7 +1539,7 @@ wNuvFqc=
status: { status: 200, statusText: "OK" },
responseBody: {
metadata: {
- signature: {},
+ signatures: [{}],
},
timestamp: 4000,
changes: [
@@ -1568,7 +1570,7 @@ wNuvFqc=
status: { status: 200, statusText: "OK" },
responseBody: {
metadata: {
- signature: {},
+ signatures: [{}],
},
timestamp: 5000,
changes: [
@@ -1675,10 +1677,12 @@ wNuvFqc=
status: { status: 200, statusText: "OK" },
responseBody: {
metadata: {
- signature: {
- signature: "some-sig",
- x5u: `http://localhost:${port}/fake-x5u`,
- },
+ signatures: [
+ {
+ signature: "some-sig",
+ x5u: `http://localhost:${port}/fake-x5u`,
+ },
+ ],
},
timestamp: 3000,
changes: [
@@ -1703,10 +1707,12 @@ wNuvFqc=
status: { status: 200, statusText: "OK" },
responseBody: {
metadata: {
- signature: {
- signature: "some-sig",
- x5u: `http://localhost:${port}/fake-x5u`,
- },
+ signatures: [
+ {
+ signature: "some-sig",
+ x5u: `http://localhost:${port}/fake-x5u`,
+ },
+ ],
},
timestamp: 3001,
changes: [
@@ -1733,10 +1739,12 @@ wNuvFqc=
metadata: {
id: "language-dictionaries",
last_modified: 1234,
- signature: {
- signature: "xyz",
- x5u: `http://localhost:${port}/fake-x5u`,
- },
+ signatures: [
+ {
+ signature: "xyz",
+ x5u: `http://localhost:${port}/fake-x5u`,
+ },
+ ],
},
changes: [
{
@@ -1772,10 +1780,12 @@ wNuvFqc=
metadata: {
id: "with-local-fields",
last_modified: 1234,
- signature: {
- signature: "xyz",
- x5u: `http://localhost:${port}/fake-x5u`,
- },
+ signatures: [
+ {
+ signature: "xyz",
+ x5u: `http://localhost:${port}/fake-x5u`,
+ },
+ ],
},
changes: [
{
@@ -1798,7 +1808,7 @@ wNuvFqc=
responseBody: {
timestamp: 3000,
metadata: {
- signature: {},
+ signatures: [{}],
},
changes: [
{
diff --git a/services/settings/test/unit/test_remote_settings_older_than_local.js b/services/settings/test/unit/test_remote_settings_older_than_local.js
@@ -33,7 +33,7 @@ add_setup(() => {
"/v1/buckets/main/collections/some-cid/changeset?_expected=333": {
timestamp: 333,
metadata: {
- signature: { signature: "abc", x5u: "data:text/plain;base64,pem" },
+ signatures: [{ signature: "abc", x5u: "data:text/plain;base64,pem" }],
},
changes: [
{ id: "rid2", last_modified: 22 },
@@ -45,7 +45,7 @@ add_setup(() => {
{
timestamp: 222,
metadata: {
- signature: { signature: "ghi", x5u: "data:text/plain;base64,pem" },
+ signatures: [{ signature: "ghi", x5u: "data:text/plain;base64,pem" }],
},
changes: [{ id: "rid1", last_modified: 11 }],
},
@@ -53,7 +53,7 @@ add_setup(() => {
"/v1/buckets/main/collections/some-cid/changeset?_expected=222": {
timestamp: 222,
metadata: {
- signature: { signature: "ghi", x5u: "data:text/plain;base64,pem" },
+ signatures: [{ signature: "ghi", x5u: "data:text/plain;base64,pem" }],
},
changes: [{ id: "rid1", last_modified: 11 }],
},
diff --git a/services/settings/test/unit/test_remote_settings_signatures.js b/services/settings/test/unit/test_remote_settings_signatures.js
@@ -138,10 +138,12 @@ add_task(async function test_bad_signature_does_not_lead_to_empty_list() {
timestamp: 42,
changes: [],
metadata: {
- signature: {
- signature: "bad-signature",
- x5u,
- },
+ signatures: [
+ {
+ signature: "bad-signature",
+ x5u,
+ },
+ ],
},
})
);
@@ -352,11 +354,13 @@ add_task(async function test_check_synchronization_with_signatures() {
responseBody: JSON.stringify({
timestamp: 1000,
metadata: {
- signature: {
- x5u,
- signature:
- "vxuAg5rDCB-1pul4a91vqSBQRXJG_j7WOYUTswxRSMltdYmbhLRH8R8brQ9YKuNDF56F-w6pn4HWxb076qgKPwgcEBtUeZAO_RtaHXRkRUUgVzAr86yQL4-aJTbv3D6u",
- },
+ signatures: [
+ {
+ x5u,
+ signature:
+ "vxuAg5rDCB-1pul4a91vqSBQRXJG_j7WOYUTswxRSMltdYmbhLRH8R8brQ9YKuNDF56F-w6pn4HWxb076qgKPwgcEBtUeZAO_RtaHXRkRUUgVzAr86yQL4-aJTbv3D6u",
+ },
+ ],
},
changes: [],
}),
@@ -424,11 +428,13 @@ add_task(async function test_check_synchronization_with_signatures() {
responseBody: JSON.stringify({
timestamp: 3000,
metadata: {
- signature: {
- x5u,
- signature:
- "dwhJeypadNIyzGj3QdI0KMRTPnHhFPF_j73mNrsPAHKMW46S2Ftf4BzsPMvPMB8h0TjDus13wo_R4l432DHe7tYyMIWXY0PBeMcoe5BREhFIxMxTsh9eGVXBD1e3UwRy",
- },
+ signatures: [
+ {
+ x5u,
+ signature:
+ "dwhJeypadNIyzGj3QdI0KMRTPnHhFPF_j73mNrsPAHKMW46S2Ftf4BzsPMvPMB8h0TjDus13wo_R4l432DHe7tYyMIWXY0PBeMcoe5BREhFIxMxTsh9eGVXBD1e3UwRy",
+ },
+ ],
},
changes: [RECORD2, RECORD1],
}),
@@ -464,10 +470,12 @@ add_task(async function test_check_synchronization_with_signatures() {
responseBody: JSON.stringify({
timestamp: 4000,
metadata: {
- signature: {
- x5u,
- signature: THREE_ITEMS_SIG,
- },
+ signatures: [
+ {
+ x5u,
+ signature: THREE_ITEMS_SIG,
+ },
+ ],
},
changes: [RECORD3, RECORD1_DELETION],
}),
@@ -500,10 +508,12 @@ add_task(async function test_check_synchronization_with_signatures() {
responseBody: JSON.stringify({
timestamp: 4000,
metadata: {
- signature: {
- x5u,
- signature: THREE_ITEMS_SIG,
- },
+ signatures: [
+ {
+ x5u,
+ signature: THREE_ITEMS_SIG,
+ },
+ ],
},
changes: [],
}),
@@ -540,10 +550,12 @@ add_task(async function test_check_synchronization_with_signatures() {
responseBody: JSON.stringify({
timestamp: 4000,
metadata: {
- signature: {
- x5u,
- signature: THREE_ITEMS_SIG,
- },
+ signatures: [
+ {
+ x5u,
+ signature: THREE_ITEMS_SIG,
+ },
+ ],
},
changes: [RECORD2, RECORD3],
}),
@@ -554,10 +566,12 @@ add_task(async function test_check_synchronization_with_signatures() {
responseBody: JSON.stringify({
timestamp: 4000,
metadata: {
- signature: {
- x5u,
- signature: "aW52YWxpZCBzaWduYXR1cmUK",
- },
+ signatures: [
+ {
+ x5u,
+ signature: "aW52YWxpZCBzaWduYXR1cmUK",
+ },
+ ],
},
changes: [],
}),
@@ -665,10 +679,12 @@ add_task(async function test_check_synchronization_with_signatures() {
responseBody: JSON.stringify({
timestamp: 5000,
metadata: {
- signature: {
- x5u,
- signature: "aW52YWxpZCBzaWduYXR1cmUK",
- },
+ signatures: [
+ {
+ x5u,
+ signature: "aW52YWxpZCBzaWduYXR1cmUK",
+ },
+ ],
},
changes: [RECORD2, RECORD3],
}),
@@ -690,7 +706,7 @@ add_task(async function test_check_synchronization_with_signatures() {
// the final server collection contains RECORD2 and RECORD3
const localId = "0602b1b2-12ab-4d3a-b6fb-593244e7b035";
await client.db.importChanges(
- { signature: { x5u, signature: "abc" } },
+ { signatures: [{ x5u, signature: "abc" }] },
3900,
[
{ ...RECORD2, last_modified: 1234567890, serialNumber: "abc" },
@@ -777,10 +793,12 @@ add_task(async function test_check_synchronization_with_signatures() {
responseBody: JSON.stringify({
timestamp: 6000,
metadata: {
- signature: {
- x5u,
- signature: "aaaaaaaaaaaaaaaaaaaaaaaa", // sig verifier wants proper length or will crash.
- },
+ signatures: [
+ {
+ x5u,
+ signature: "aaaaaaaaaaaaaaaaaaaaaaaa", // sig verifier wants proper length or will crash.
+ },
+ ],
},
changes: [
{
@@ -795,10 +813,12 @@ add_task(async function test_check_synchronization_with_signatures() {
responseBody: JSON.stringify({
timestamp: 6000,
metadata: {
- signature: {
- x5u,
- signature: "aW52YWxpZCBzaWduYXR1cmUK",
- },
+ signatures: [
+ {
+ x5u,
+ signature: "aW52YWxpZCBzaWduYXR1cmUK",
+ },
+ ],
},
changes: [],
}),
@@ -912,7 +932,7 @@ add_task(async function test_check_synchronization_with_signatures() {
// thanks to the verifier mock.
await client.db.importChanges(
{
- signature: { x5u, signature: "aa" },
+ signatures: [{ x5u, signature: "aa" }],
},
4000,
[
