Skip to content

Commit 2b128e2

Browse files
authored
fix(e2ee): verify own-key signature before merging synced verifications (#416)
This change threads the signature metadata through `DecryptFn` and gates the merge: `fetchVerificationsFromServer` now requires a valid signature from the account's **own primary key** before returning the map. Unsigned, present-but-unverified, and foreign-signed payloads are all discarded. The publish side already sign+encrypts, so legitimate cross-device sync is unaffected.
1 parent b5e3fa7 commit 2b128e2

4 files changed

Lines changed: 223 additions & 6 deletions

File tree

apps/fluux/src/e2ee/OpenPGPPluginBase.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -995,13 +995,15 @@ export abstract class OpenPGPPluginBase implements E2EEPlugin {
995995
this._syncingFromRemoteCount++
996996
const ctx = this.ctx
997997
const ownPublicArmored = this.ownBundle.publicArmored
998+
const ownFingerprint = this.ownBundle.fingerprint
998999
try {
9991000
const remote = await fetchVerificationsFromServer(
10001001
ctx,
10011002
(ciphertext, senderKey) =>
10021003
this.decryptWithOwnKey(ctx.account.jid, ciphertext, senderKey),
10031004
ctx.account.jid,
10041005
ownPublicArmored,
1006+
ownFingerprint,
10051007
)
10061008
if (!remote) return
10071009
const local = useVerifiedPeerKeysStore.getState().verifiedFingerprintByJid

apps/fluux/src/e2ee/SequoiaPgpPlugin.test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3411,13 +3411,16 @@ describe('SequoiaPgpPlugin', () => {
34113411

34123412
// Build a PEP item that looks exactly like one publishVerificationsToServer
34133413
// would produce: base64(OPENPGP-STUB ciphertext) inside verifications-data.
3414+
// `signerFp` defaults to `ownFp` (a legitimate self-signed item); pass a
3415+
// different value to simulate a server-forged node signed by a foreign key.
34143416
function buildVerificationsPepItem(
34153417
ownFp: string,
34163418
verifications: Record<string, string>,
3419+
signerFp: string = ownFp,
34173420
): PEPItem {
34183421
const json = JSON.stringify({ v: 1, ts: 1000, verifications })
34193422
const encoded = btoa(unescape(encodeURIComponent(json)))
3420-
const armored = `OPENPGP-STUB:${ownFp}:${ownFp}:${encoded}`
3423+
const armored = `OPENPGP-STUB:${ownFp}:${signerFp}:${encoded}`
34213424
const b64Armored = btoa(unescape(encodeURIComponent(armored)))
34223425
return {
34233426
id: 'current',
@@ -3454,6 +3457,31 @@ describe('SequoiaPgpPlugin', () => {
34543457
expect(isVerified('alice@example.com', 'ALICE_FP')).toBe(true)
34553458
})
34563459

3460+
it('ignores a server-forged verifications node signed by a foreign key', async () => {
3461+
const { ctx, peerPublish } = makeContext('me@example.com')
3462+
const fp = 'FP_FORGE_TEST'
3463+
fake.accounts.set('me@example.com', {
3464+
fingerprint: fp,
3465+
publicArmored: makeOpenPgpArmor(
3466+
'PGP PUBLIC KEY BLOCK',
3467+
`Fingerprint: ${fp}\nUID: xmpp:me@example.com\nKind: public\nRotation: 0\n`,
3468+
),
3469+
keychainBacked: true,
3470+
})
3471+
// A malicious server can encrypt an arbitrary map to our public key, but
3472+
// it cannot sign as us — so the embedded signer fingerprint is foreign.
3473+
peerPublish(
3474+
'me@example.com',
3475+
VERIFICATIONS_NODE,
3476+
buildVerificationsPepItem(fp, { 'mallory@example.com': 'MALLORY_FP' }, 'ATTACKER_FP'),
3477+
)
3478+
await plugin.init(ctx)
3479+
await new Promise((r) => setTimeout(r, 0))
3480+
3481+
const { isPeerVerified: isVerified } = await import('@/stores/verifiedPeerKeysStore')
3482+
expect(isVerified('mallory@example.com', 'MALLORY_FP')).toBe(false)
3483+
})
3484+
34573485
it('publishes the verifications PEP node after a local verification is added', async () => {
34583486
vi.useFakeTimers()
34593487
try {
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
import { describe, expect, it } from 'vitest'
2+
3+
import type { PluginContext } from '@fluux/sdk'
4+
5+
import {
6+
fetchVerificationsFromServer,
7+
VERIFICATIONS_NODE,
8+
type DecryptFn,
9+
} from './verificationSync'
10+
11+
const OWN_FP = 'AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555'
12+
const OWN_PUBLIC = 'OWN-PUBLIC-ARMOR'
13+
const OWN_JID = 'me@example.com'
14+
15+
const VALID_PAYLOAD = JSON.stringify({
16+
v: 1,
17+
ts: 1000,
18+
verifications: { 'bob@example.com': 'BOB_FP' },
19+
})
20+
21+
/** Minimal PluginContext whose queryPEP serves a single verifications item. */
22+
function makeCtx(dataChild = 'AAAA'): PluginContext {
23+
return {
24+
xmpp: {
25+
queryPEP: async () => [
26+
{
27+
id: 'current',
28+
payload: {
29+
name: 'verifications-data',
30+
attrs: { xmlns: VERIFICATIONS_NODE },
31+
children: [{ name: 'data', attrs: {}, children: [dataChild] }],
32+
},
33+
},
34+
],
35+
},
36+
} as unknown as PluginContext
37+
}
38+
39+
/** A decrypt fn that returns fixed plaintext + caller-controlled signature metadata. */
40+
function decryptReturning(meta: {
41+
plaintext?: string
42+
signatureVerified: boolean
43+
signerFingerprint: string | null
44+
signaturePresent: boolean
45+
}): DecryptFn {
46+
return async () => ({
47+
plaintext: meta.plaintext ?? VALID_PAYLOAD,
48+
signatureVerified: meta.signatureVerified,
49+
signerFingerprint: meta.signerFingerprint,
50+
signaturePresent: meta.signaturePresent,
51+
})
52+
}
53+
54+
describe('fetchVerificationsFromServer — signature enforcement', () => {
55+
it('returns the map for a payload validly signed by our own primary key', async () => {
56+
const result = await fetchVerificationsFromServer(
57+
makeCtx(),
58+
decryptReturning({
59+
signatureVerified: true,
60+
signerFingerprint: OWN_FP,
61+
signaturePresent: true,
62+
}),
63+
OWN_JID,
64+
OWN_PUBLIC,
65+
OWN_FP,
66+
)
67+
expect(result).toEqual({ 'bob@example.com': 'BOB_FP' })
68+
})
69+
70+
it('rejects an unsigned payload (server-forged ciphertext to our public key)', async () => {
71+
const result = await fetchVerificationsFromServer(
72+
makeCtx(),
73+
decryptReturning({
74+
signatureVerified: false,
75+
signerFingerprint: null,
76+
signaturePresent: false,
77+
}),
78+
OWN_JID,
79+
OWN_PUBLIC,
80+
OWN_FP,
81+
)
82+
expect(result).toBeNull()
83+
})
84+
85+
it('rejects a payload whose signature is present but does not verify', async () => {
86+
const result = await fetchVerificationsFromServer(
87+
makeCtx(),
88+
decryptReturning({
89+
signatureVerified: false,
90+
signerFingerprint: OWN_FP,
91+
signaturePresent: true,
92+
}),
93+
OWN_JID,
94+
OWN_PUBLIC,
95+
OWN_FP,
96+
)
97+
expect(result).toBeNull()
98+
})
99+
100+
it('rejects a payload validly signed by a key other than our own primary key', async () => {
101+
const result = await fetchVerificationsFromServer(
102+
makeCtx(),
103+
decryptReturning({
104+
signatureVerified: true,
105+
signerFingerprint: 'DEADBEEF0000111122223333444455556666AAAA',
106+
signaturePresent: true,
107+
}),
108+
OWN_JID,
109+
OWN_PUBLIC,
110+
OWN_FP,
111+
)
112+
expect(result).toBeNull()
113+
})
114+
115+
it('rejects a verified signature with no signer fingerprint (defensive)', async () => {
116+
const result = await fetchVerificationsFromServer(
117+
makeCtx(),
118+
decryptReturning({
119+
signatureVerified: true,
120+
signerFingerprint: null,
121+
signaturePresent: true,
122+
}),
123+
OWN_JID,
124+
OWN_PUBLIC,
125+
OWN_FP,
126+
)
127+
expect(result).toBeNull()
128+
})
129+
130+
it('matches the signer fingerprint case-insensitively', async () => {
131+
const result = await fetchVerificationsFromServer(
132+
makeCtx(),
133+
decryptReturning({
134+
signatureVerified: true,
135+
signerFingerprint: OWN_FP.toLowerCase(),
136+
signaturePresent: true,
137+
}),
138+
OWN_JID,
139+
OWN_PUBLIC,
140+
OWN_FP.toUpperCase(),
141+
)
142+
expect(result).toEqual({ 'bob@example.com': 'BOB_FP' })
143+
})
144+
})

apps/fluux/src/e2ee/verificationSync.ts

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,15 @@
22
* Cross-device synchronisation of peer verification records.
33
*
44
* Publishes the local `verifiedPeerKeysStore` map to a private PEP node
5-
* (`urn:xmpp:fluux:verifications:0`, `accessModel='whitelist'`) encrypted
5+
* (`urn:xmpp:fluux:verifications:0`, `accessModel='whitelist'`) sign+encrypted
66
* to the user's own OpenPGP key. Other devices of the same account receive
77
* a PEP headline, fetch the node, decrypt it, and merge the entries.
88
*
9+
* Trust hinges on the signature, not the access model: the fetch path discards
10+
* any payload not signed by the account's own primary key, so a tampering
11+
* server cannot inject forged "verified" entries by encrypting to the user's
12+
* (public) key.
13+
*
914
* Crypto is abstracted behind {@link EncryptFn}/{@link DecryptFn} so this
1015
* module works with both the Sequoia/Tauri backend and the openpgp.js web
1116
* backend without modification.
@@ -23,11 +28,28 @@ export type EncryptFn = (
2328
recipientPublicArmored: string,
2429
) => Promise<string>
2530

26-
/** Decrypt `ciphertext` (encrypted to us). Returns `{ plaintext }`. */
31+
/**
32+
* Decrypt `ciphertext` (encrypted to us) and report on its signature.
33+
*
34+
* The signature metadata is mandatory: cross-device sync only trusts a
35+
* payload that carries a valid signature from the account's own key, so the
36+
* caller must surface whatever the crypto backend reports. Mirrors the
37+
* backend `DecryptOutput` shape.
38+
*/
2739
export type DecryptFn = (
2840
ciphertext: string,
2941
senderPublicArmored: string,
30-
) => Promise<{ plaintext: string }>
42+
) => Promise<{
43+
plaintext: string
44+
signatureVerified: boolean
45+
signerFingerprint: string | null
46+
signaturePresent: boolean
47+
}>
48+
49+
/** Case- and whitespace-insensitive fingerprint comparison. */
50+
function fingerprintsEqual(a: string, b: string): boolean {
51+
return a.replace(/\s+/g, '').toLowerCase() === b.replace(/\s+/g, '').toLowerCase()
52+
}
3153

3254
export const VERIFICATIONS_NODE = 'urn:xmpp:fluux:verifications:0'
3355
const VERIFICATIONS_XMLNS = VERIFICATIONS_NODE
@@ -109,13 +131,23 @@ export async function publishVerificationsToServer(
109131

110132
/**
111133
* Fetch the private PEP node, decrypt it, and return the verification map.
112-
* Returns `null` when the node is absent or decryption fails.
134+
*
135+
* Downgrade protection: the decrypted payload is only trusted when it carries
136+
* a signature that verifies against the account's *own primary key*
137+
* (`ownFingerprint`). A malicious server can encrypt an arbitrary map to the
138+
* user's public key (it is, after all, public), but it cannot forge a
139+
* signature from the user's secret key — so any unsigned, wrong-signed, or
140+
* foreign-signed payload is discarded.
141+
*
142+
* Returns `null` when the node is absent, decryption fails, or the signature
143+
* check does not pass.
113144
*/
114145
export async function fetchVerificationsFromServer(
115146
ctx: PluginContext,
116147
decryptFn: DecryptFn,
117148
ownJid: string,
118149
ownPublicArmored: string,
150+
ownFingerprint: string,
119151
): Promise<Record<string, string> | null> {
120152
let items: { id: string; payload: XMLElementData }[]
121153
try {
@@ -130,13 +162,24 @@ export async function fetchVerificationsFromServer(
130162

131163
const armored = b64Decode(base64Ciphertext)
132164

133-
let decrypted: { plaintext: string }
165+
let decrypted: Awaited<ReturnType<DecryptFn>>
134166
try {
135167
decrypted = await decryptFn(armored, ownPublicArmored)
136168
} catch {
137169
return null
138170
}
139171

172+
// Downgrade protection: only a payload signed by our own primary key is
173+
// trusted. Reject unsigned, unverified, or foreign-signed maps — these can
174+
// only originate from a tampering server, not from one of our devices.
175+
if (
176+
!decrypted.signatureVerified ||
177+
!decrypted.signerFingerprint ||
178+
!fingerprintsEqual(decrypted.signerFingerprint, ownFingerprint)
179+
) {
180+
return null
181+
}
182+
140183
try {
141184
const payload = JSON.parse(decrypted.plaintext) as unknown
142185
if (

0 commit comments

Comments
 (0)