Skip to content

Commit e395cc7

Browse files
authored
fix(e2ee): stop revoked verifications from resurfacing on sync (#417)
## Summary Cross-device verification sync could let a revoked verification come back. `publishVerificationsToServer` skipped publishing when the map was empty (leaving a stale node on the server), and the merge was an additive union — so a removal on one device was re-added from another device's stale copy, or replayed by the server from an older signed snapshot. This replaces the union merge with a **versioned, last-writer-wins snapshot**: - Every publish carries a monotonic `version`, and the empty map is now always published, so revoking the last verification overwrites the server node instead of leaving the old one. - A fetched snapshot is applied only when its `version` is strictly newer than the highest already applied (persisted locally), then *replaces* local state — additions, fingerprint changes, and removals all converge across devices. - The monotonic gate also rejects a server replaying an older but genuinely signed snapshot, closing the trust-rollback path (a signature proves authorship, not freshness). The merge decision now lives in a pure `planVerificationUpdate`; the plugin applies it (set + clear) under the existing remote-sync guard. Legacy (`v1`) snapshots decode to version `0` and are still picked up once.
1 parent 2b128e2 commit e395cc7

4 files changed

Lines changed: 554 additions & 146 deletions

File tree

apps/fluux/src/e2ee/OpenPGPPluginBase.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,10 @@ import {
8585
import {
8686
VERIFICATIONS_NODE,
8787
fetchVerificationsFromServer,
88-
mergeVerifications,
88+
loadAppliedVerificationsVersion,
89+
planVerificationUpdate,
8990
publishVerificationsToServer,
91+
saveAppliedVerificationsVersion,
9092
} from './verificationSync'
9193
import {
9294
clearKeyChangeAlert,
@@ -1007,11 +1009,11 @@ export abstract class OpenPGPPluginBase implements E2EEPlugin {
10071009
)
10081010
if (!remote) return
10091011
const local = useVerifiedPeerKeysStore.getState().verifiedFingerprintByJid
1010-
const { hasNewEntries, merged } = mergeVerifications(remote, local)
1011-
if (!hasNewEntries) return
1012-
for (const [jid, fp] of Object.entries(merged)) {
1013-
if (!local[jid]) setPeerVerified(jid, fp)
1014-
}
1012+
const plan = planVerificationUpdate(remote, local, loadAppliedVerificationsVersion())
1013+
if (!plan.apply) return
1014+
for (const { jid, fingerprint } of plan.toSet) setPeerVerified(jid, fingerprint)
1015+
for (const jid of plan.toClear) clearPeerVerified(jid)
1016+
saveAppliedVerificationsVersion(plan.version)
10151017
} catch {
10161018
// Non-blocking — local store is always the source of truth.
10171019
} finally {
@@ -1028,13 +1030,19 @@ export abstract class OpenPGPPluginBase implements E2EEPlugin {
10281030
if (!this.ownBundle || !this.ctx) return
10291031
const ctx = this.ctx
10301032
const ownPublicArmored = this.ownBundle.publicArmored
1033+
// Reserve the next version above the highest we've applied/published.
1034+
// Real versions start at 1; 0 is reserved for legacy (v1) snapshots.
1035+
const nextVersion = Math.max(loadAppliedVerificationsVersion(), 0) + 1
10311036
void publishVerificationsToServer(
10321037
ctx,
10331038
(plaintext, recipientKey) =>
10341039
this.encryptToRecipient(ctx.account.jid, recipientKey, plaintext),
10351040
ownPublicArmored,
10361041
verifications,
1037-
).catch(() => {})
1042+
nextVersion,
1043+
)
1044+
.then(() => saveAppliedVerificationsVersion(nextVersion))
1045+
.catch(() => {})
10381046
}, 500)
10391047
}
10401048

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

Lines changed: 147 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3416,9 +3416,13 @@ describe('SequoiaPgpPlugin', () => {
34163416
function buildVerificationsPepItem(
34173417
ownFp: string,
34183418
verifications: Record<string, string>,
3419+
version?: number,
34193420
signerFp: string = ownFp,
34203421
): PEPItem {
3421-
const json = JSON.stringify({ v: 1, ts: 1000, verifications })
3422+
const json =
3423+
version === undefined
3424+
? JSON.stringify({ v: 1, ts: 1000, verifications })
3425+
: JSON.stringify({ v: 2, ts: 1000, version, verifications })
34223426
const encoded = btoa(unescape(encodeURIComponent(json)))
34233427
const armored = `OPENPGP-STUB:${ownFp}:${signerFp}:${encoded}`
34243428
const b64Armored = btoa(unescape(encodeURIComponent(armored)))
@@ -3432,6 +3436,23 @@ describe('SequoiaPgpPlugin', () => {
34323436
}
34333437
}
34343438

3439+
// Reverse the plugin's publish encoding: data child holds
3440+
// base64(makeOpenPgpArmor('OPENPGP-STUB:<recipient>:<sender>:<base64-json>')).
3441+
function decodePublishedVerifications(item: PEPItem): {
3442+
version?: number
3443+
verifications: Record<string, string>
3444+
} {
3445+
const dataChild = item.payload.children.find(
3446+
(c): c is XMLElementData => typeof c !== 'string' && c.name === 'data',
3447+
)
3448+
const dataText = dataChild?.children[0]
3449+
if (typeof dataText !== 'string') throw new Error('no data child in published item')
3450+
const armored = decodeURIComponent(escape(atob(dataText)))
3451+
const stub = readOpenPgpArmorPayloadForTest(armored)
3452+
const payloadB64 = stub.slice('OPENPGP-STUB:'.length).split(':')[2]
3453+
return JSON.parse(decodeURIComponent(escape(atob(payloadB64))))
3454+
}
3455+
34353456
it('seeds the local verified-peers store from the server node on init', async () => {
34363457
const { ctx, peerPublish } = makeContext('me@example.com')
34373458
// Pre-seed so we know the fingerprint before init.
@@ -3473,7 +3494,7 @@ describe('SequoiaPgpPlugin', () => {
34733494
peerPublish(
34743495
'me@example.com',
34753496
VERIFICATIONS_NODE,
3476-
buildVerificationsPepItem(fp, { 'mallory@example.com': 'MALLORY_FP' }, 'ATTACKER_FP'),
3497+
buildVerificationsPepItem(fp, { 'mallory@example.com': 'MALLORY_FP' }, undefined, 'ATTACKER_FP'),
34773498
)
34783499
await plugin.init(ctx)
34793500
await new Promise((r) => setTimeout(r, 0))
@@ -3591,5 +3612,129 @@ describe('SequoiaPgpPlugin', () => {
35913612
vi.useRealTimers()
35923613
}
35933614
})
3615+
3616+
it('publishes an empty snapshot when the last verification is revoked, and it does not resurrect on resync', async () => {
3617+
vi.useFakeTimers()
3618+
try {
3619+
let verificationsCb: ((item: PEPItem) => void) | null = null
3620+
const { ctx, published } = makeContext('me@example.com')
3621+
ctx.xmpp.subscribePEP = (_jid, node, cb) => {
3622+
if (node === VERIFICATIONS_NODE) verificationsCb = cb
3623+
return { unsubscribe: () => {} }
3624+
}
3625+
const fp = 'FP_REVOKE_TEST'
3626+
fake.accounts.set('me@example.com', {
3627+
fingerprint: fp,
3628+
publicArmored: makeOpenPgpArmor(
3629+
'PGP PUBLIC KEY BLOCK',
3630+
`Fingerprint: ${fp}\nUID: xmpp:me@example.com\nKind: public\nRotation: 0\n`,
3631+
),
3632+
keychainBacked: true,
3633+
})
3634+
await plugin.init(ctx)
3635+
3636+
const store = await import('@/stores/verifiedPeerKeysStore')
3637+
store.setPeerVerified('alice@example.com', 'ALICE_FP')
3638+
await vi.advanceTimersByTimeAsync(600)
3639+
store.clearPeerVerified('alice@example.com')
3640+
await vi.advanceTimersByTimeAsync(600)
3641+
3642+
// The empty map is published (not skipped), overwriting the server node.
3643+
const verNodes = published.filter((p) => p.node === VERIFICATIONS_NODE)
3644+
const lastPayload = decodePublishedVerifications(verNodes[verNodes.length - 1].item)
3645+
expect(lastPayload.verifications).toEqual({})
3646+
3647+
// The server now serves that empty snapshot; a resync must not resurrect alice.
3648+
verificationsCb!({ id: 'current', payload: { name: '', attrs: {}, children: [] } })
3649+
for (let i = 0; i < 10; i++) await Promise.resolve()
3650+
await vi.advanceTimersByTimeAsync(600)
3651+
expect(store.isPeerVerified('alice@example.com', 'ALICE_FP')).toBe(false)
3652+
} finally {
3653+
vi.useRealTimers()
3654+
}
3655+
})
3656+
3657+
it('ignores a replayed older snapshot from the server (no trust rollback)', async () => {
3658+
let verificationsCb: ((item: PEPItem) => void) | null = null
3659+
let currentItem: PEPItem | null = null
3660+
const { ctx } = makeContext('me@example.com')
3661+
ctx.xmpp.subscribePEP = (_jid, node, cb) => {
3662+
if (node === VERIFICATIONS_NODE) verificationsCb = cb
3663+
return { unsubscribe: () => {} }
3664+
}
3665+
ctx.xmpp.queryPEP = async (_jid, node) =>
3666+
node === VERIFICATIONS_NODE && currentItem ? [currentItem] : []
3667+
const fp = 'FP_REPLAY_TEST'
3668+
fake.accounts.set('me@example.com', {
3669+
fingerprint: fp,
3670+
publicArmored: makeOpenPgpArmor(
3671+
'PGP PUBLIC KEY BLOCK',
3672+
`Fingerprint: ${fp}\nUID: xmpp:me@example.com\nKind: public\nRotation: 0\n`,
3673+
),
3674+
keychainBacked: true,
3675+
})
3676+
await plugin.init(ctx)
3677+
const store = await import('@/stores/verifiedPeerKeysStore')
3678+
3679+
// A newer snapshot (version 5): bob is verified, alice already revoked elsewhere.
3680+
currentItem = buildVerificationsPepItem(fp, { 'bob@example.com': 'BOB_FP' }, 5)
3681+
verificationsCb!({ id: 'current', payload: { name: '', attrs: {}, children: [] } })
3682+
await new Promise((r) => setTimeout(r, 0))
3683+
expect(store.isPeerVerified('bob@example.com', 'BOB_FP')).toBe(true)
3684+
expect(store.isPeerVerified('alice@example.com', 'ALICE_FP')).toBe(false)
3685+
3686+
// The server replays an OLDER snapshot (version 1) that still trusts alice.
3687+
currentItem = buildVerificationsPepItem(
3688+
fp,
3689+
{ 'alice@example.com': 'ALICE_FP', 'bob@example.com': 'BOB_FP' },
3690+
1,
3691+
)
3692+
verificationsCb!({ id: 'current', payload: { name: '', attrs: {}, children: [] } })
3693+
await new Promise((r) => setTimeout(r, 0))
3694+
3695+
// Rollback rejected: alice is NOT resurrected, bob is untouched.
3696+
expect(store.isPeerVerified('alice@example.com', 'ALICE_FP')).toBe(false)
3697+
expect(store.isPeerVerified('bob@example.com', 'BOB_FP')).toBe(true)
3698+
})
3699+
3700+
it('clears a locally-verified peer that a newer remote snapshot drops', async () => {
3701+
let verificationsCb: ((item: PEPItem) => void) | null = null
3702+
let currentItem: PEPItem | null = null
3703+
const { ctx } = makeContext('me@example.com')
3704+
ctx.xmpp.subscribePEP = (_jid, node, cb) => {
3705+
if (node === VERIFICATIONS_NODE) verificationsCb = cb
3706+
return { unsubscribe: () => {} }
3707+
}
3708+
ctx.xmpp.queryPEP = async (_jid, node) =>
3709+
node === VERIFICATIONS_NODE && currentItem ? [currentItem] : []
3710+
const fp = 'FP_DROP_TEST'
3711+
fake.accounts.set('me@example.com', {
3712+
fingerprint: fp,
3713+
publicArmored: makeOpenPgpArmor(
3714+
'PGP PUBLIC KEY BLOCK',
3715+
`Fingerprint: ${fp}\nUID: xmpp:me@example.com\nKind: public\nRotation: 0\n`,
3716+
),
3717+
keychainBacked: true,
3718+
})
3719+
3720+
const store = await import('@/stores/verifiedPeerKeysStore')
3721+
// Seed local state BEFORE init so the store subscription (attached during
3722+
// init) does not schedule a publish from these writes.
3723+
store.useVerifiedPeerKeysStore.setState({
3724+
verifiedFingerprintByJid: {
3725+
'alice@example.com': 'ALICE_FP',
3726+
'bob@example.com': 'BOB_FP',
3727+
},
3728+
})
3729+
await plugin.init(ctx)
3730+
3731+
// Another device published a newer snapshot (version 5) that dropped alice.
3732+
currentItem = buildVerificationsPepItem(fp, { 'bob@example.com': 'BOB_FP' }, 5)
3733+
verificationsCb!({ id: 'current', payload: { name: '', attrs: {}, children: [] } })
3734+
await new Promise((r) => setTimeout(r, 0))
3735+
3736+
expect(store.isPeerVerified('alice@example.com', 'ALICE_FP')).toBe(false)
3737+
expect(store.isPeerVerified('bob@example.com', 'BOB_FP')).toBe(true)
3738+
})
35943739
})
35953740
})

0 commit comments

Comments
 (0)