From b3c4b21cf863c3bb5ffd6bdd5e4b1b0499b8ca13 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Fri, 18 Sep 2026 21:30:45 +0200 Subject: [PATCH 1/2] fix(crypto): stop device verification from silently reverting --- .../fix-device-verification-reverting.md | 5 ++ oxfmt.config.ts | 2 + src/app/components/BackupRestore.test.tsx | 24 ++++++ .../components/ManualVerification.test.tsx | 63 ++++++++++++++ .../pages/client/BackgroundNotifications.tsx | 6 +- src/app/pages/client/ClientRoot.tsx | 32 ++++++-- src/app/state/sessions.ts | 13 ++- src/app/state/sessions.updateTokens.test.ts | 59 +++++++++++++ src/app/utils/matrix-crypto.ts | 82 +++++++++++++++++++ src/client/initMatrix.sdk.test.ts | 55 +++++++++++-- src/client/initMatrix.ts | 47 ++++++++++- 11 files changed, 370 insertions(+), 18 deletions(-) create mode 100644 .changeset/fix-device-verification-reverting.md diff --git a/.changeset/fix-device-verification-reverting.md b/.changeset/fix-device-verification-reverting.md new file mode 100644 index 0000000000..2050576177 --- /dev/null +++ b/.changeset/fix-device-verification-reverting.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Fix devices repeatedly reverting to unverified. diff --git a/oxfmt.config.ts b/oxfmt.config.ts index 40e2525b3e..cde1d14909 100644 --- a/oxfmt.config.ts +++ b/oxfmt.config.ts @@ -12,7 +12,9 @@ export default { 'src-tauri/ios-project.yml', // Copied verbatim from the Firebase console; formatting it would drift. 'src-tauri/gen/android/app/google-services.json', + // knope rewrites these on every release; reformatting them breaks the next fmt:check. 'package.json', + 'src-tauri/tauri.conf.json', 'pnpm-lock.yaml', 'LICENSE', 'README.md', diff --git a/src/app/components/BackupRestore.test.tsx b/src/app/components/BackupRestore.test.tsx index 311df7acaa..9fffef351c 100644 --- a/src/app/components/BackupRestore.test.tsx +++ b/src/app/components/BackupRestore.test.tsx @@ -7,6 +7,9 @@ import type { SecretStorageKeyContent } from '$types/matrix/accountData'; import { BackupRestoreTile } from './BackupRestore'; const decodeRecoveryKey = vi.hoisted(() => vi.fn<(key: string) => Uint8Array>()); +const appFetch = vi.hoisted(() => vi.fn<() => Promise>()); + +vi.mock('$utils/fetch', () => ({ fetch: appFetch })); const emitter = new TypedEventEmitter void>>(); const mockClient = Object.assign(emitter, { secretStorage: { @@ -15,6 +18,8 @@ const mockClient = Object.assign(emitter, { }, getSafeUserId: () => '@me:example.org', getDeviceId: () => 'DEVICE', + baseUrl: 'https://example.org', + getAccessToken: () => 'access-token', }); vi.mock('$hooks/useMatrixClient', () => ({ @@ -61,6 +66,17 @@ const createCrypto = ({ backupInfo = BACKUP_INFO, backupKey = null }: CryptoOver loadSessionBackupPrivateKeyFromSecretStorage: vi .fn() .mockResolvedValue(undefined), + getDeviceVerificationStatus: vi + .fn() + .mockResolvedValue({ crossSigningVerified: true } as Awaited< + ReturnType + >), + getOwnDeviceKeys: vi + .fn() + .mockResolvedValue({ ed25519: 'own-ed25519', curve25519: 'own-curve25519' }), + getCrossSigningKeyId: vi + .fn() + .mockResolvedValue('own-master'), }) as unknown as CryptoBackend; const recoveryPrompt = () => screen.queryByText(/does not hold the backup decryption key/i); @@ -77,6 +93,14 @@ describe('BackupRestoreTile', () => { beforeEach(() => { vi.clearAllMocks(); decodeRecoveryKey.mockReturnValue(new Uint8Array([1, 2, 3])); + appFetch.mockResolvedValue({ + ok: true, + json: async () => ({ + device_keys: { + '@me:example.org': { DEVICE: { keys: { 'ed25519:DEVICE': 'own-ed25519' } } }, + }, + }), + }); }); it('offers recovery when a backup exists but its key is not in the crypto store', async () => { diff --git a/src/app/components/ManualVerification.test.tsx b/src/app/components/ManualVerification.test.tsx index 1f68e4cac4..879e81b4f8 100644 --- a/src/app/components/ManualVerification.test.tsx +++ b/src/app/components/ManualVerification.test.tsx @@ -13,6 +13,14 @@ const processDeviceLists = vi.hoisted(() => vi.fn<() => Promise>()); const bootstrapCrossSigning = vi.hoisted(() => vi.fn<() => Promise>()); const bootstrapSecretStorage = vi.hoisted(() => vi.fn<() => Promise>()); const loadSessionBackupPrivateKeyFromSecretStorage = vi.hoisted(() => vi.fn<() => Promise>()); +const getDeviceVerificationStatus = vi.hoisted(() => + vi.fn<() => Promise<{ crossSigningVerified: boolean } | null>>() +); +const getOwnDeviceKeys = vi.hoisted(() => vi.fn<() => Promise<{ ed25519: string }>>()); +const getCrossSigningKeyId = vi.hoisted(() => vi.fn<() => Promise>()); +const appFetch = vi.hoisted(() => vi.fn<() => Promise>()); + +vi.mock('$utils/fetch', () => ({ fetch: appFetch })); vi.mock('$types/matrix-sdk', () => ({ decodeRecoveryKey })); vi.mock('$client/secretStorageKeys', () => ({ storePrivateKey })); @@ -20,6 +28,8 @@ vi.mock('$hooks/useMatrixClient', () => ({ useMatrixClient: () => ({ getSafeUserId: () => '@me:example.org', getDeviceId: () => 'DEVICE', + baseUrl: 'https://example.org', + getAccessToken: () => 'access-token', secretStorage: { checkKey, get: getSecret }, getCrypto: () => ({ @@ -27,10 +37,21 @@ vi.mock('$hooks/useMatrixClient', () => ({ bootstrapCrossSigning, bootstrapSecretStorage, loadSessionBackupPrivateKeyFromSecretStorage, + getDeviceVerificationStatus, + getOwnDeviceKeys, + getCrossSigningKeyId, }) as unknown as CryptoApi, }), })); +const publishedKeysResponse = (ed25519: string, masterKey = 'own-master') => ({ + ok: true, + json: async () => ({ + device_keys: { '@me:example.org': { DEVICE: { keys: { 'ed25519:DEVICE': ed25519 } } } }, + master_keys: { '@me:example.org': { keys: { [`ed25519:${masterKey}`]: masterKey } } }, + }), +}); + const KEY_ID = 'key-id'; const KEY_CONTENT = { algorithm: 'm.secret_storage.v1.aes-hmac-sha2' } as SecretStorageKeyContent; const recoveryKey = new Uint8Array([1, 2, 3]); @@ -60,6 +81,10 @@ describe('ManualVerificationTile', () => { bootstrapCrossSigning.mockResolvedValue(undefined); bootstrapSecretStorage.mockResolvedValue(undefined); loadSessionBackupPrivateKeyFromSecretStorage.mockResolvedValue(undefined); + getDeviceVerificationStatus.mockResolvedValue({ crossSigningVerified: true }); + getOwnDeviceKeys.mockResolvedValue({ ed25519: 'own-ed25519' }); + getCrossSigningKeyId.mockResolvedValue('own-master'); + appFetch.mockResolvedValue(publishedKeysResponse('own-ed25519')); }); it('refreshes cross-signing public keys before importing the recovery key', async () => { @@ -87,6 +112,44 @@ describe('ManualVerificationTile', () => { expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ['device-verification'] }); }); + it('reports failure when the device is still not cross-signed after bootstrapping', async () => { + getDeviceVerificationStatus.mockResolvedValue({ crossSigningVerified: false }); + renderTile(new QueryClient()); + + submitRecoveryKey('valid-key'); + + await waitFor(() => + expect( + screen.getByText(/could not be signed by your cross-signing identity/) + ).toBeInTheDocument() + ); + expect(screen.queryByText('Device verified!')).not.toBeInTheDocument(); + }); + + it('reports failure when the server publishes different keys for this device', async () => { + appFetch.mockResolvedValue(publishedKeysResponse('stale-ed25519')); + renderTile(new QueryClient()); + + submitRecoveryKey('valid-key'); + + await waitFor(() => + expect(screen.getByText(/no longer matches the encryption keys/)).toBeInTheDocument() + ); + expect(screen.queryByText('Device verified!')).not.toBeInTheDocument(); + }); + + it('reports failure when the recovery key unlocks a superseded identity', async () => { + appFetch.mockResolvedValue(publishedKeysResponse('own-ed25519', 'rotated-master')); + renderTile(new QueryClient()); + + submitRecoveryKey('valid-key'); + + await waitFor(() => + expect(screen.getByText(/previous verification identity/)).toBeInTheDocument() + ); + expect(screen.queryByText('Device verified!')).not.toBeInTheDocument(); + }); + it('does not bootstrap when the cross-signing keys are missing from secret storage', async () => { getSecret.mockResolvedValue(undefined); renderTile(new QueryClient()); diff --git a/src/app/pages/client/BackgroundNotifications.tsx b/src/app/pages/client/BackgroundNotifications.tsx index c0abc92734..13290c4567 100644 --- a/src/app/pages/client/BackgroundNotifications.tsx +++ b/src/app/pages/client/BackgroundNotifications.tsx @@ -55,6 +55,10 @@ const debugLog = createDebugLogger('BackgroundNotifications'); const BACKGROUND_STAGGER_DELAY_MS = 5_000; +// These sync the account's own device without crypto, so they ack and discard its to-device +// traffic. Giving them a second store for the same deviceId strands the device instead. +const BACKGROUND_SYNC_CLIENTS_ENABLED = false; + let desktopNotificationSeq = 1; const nextDesktopNotificationId = (): number => { const id = desktopNotificationSeq; @@ -180,7 +184,7 @@ export function BackgroundNotifications() { } useEffect(() => { - if (!shouldRunBackgroundNotifications) { + if (!shouldRunBackgroundNotifications || !BACKGROUND_SYNC_CLIENTS_ENABLED) { return undefined; } diff --git a/src/app/pages/client/ClientRoot.tsx b/src/app/pages/client/ClientRoot.tsx index 2f2b4d569a..acdeab967e 100644 --- a/src/app/pages/client/ClientRoot.tsx +++ b/src/app/pages/client/ClientRoot.tsx @@ -13,6 +13,7 @@ import { clearLoginData, discardSessionStores, initClient, + isStrandedCryptoStoreError, logoutClient, startClient, } from '$client/initMatrix'; @@ -403,7 +404,12 @@ export function ClientRoot({ children }: ClientRootProps) { ); const isError = loadState.status === AsyncStatus.Error || startState.status === AsyncStatus.Error; - const nativeCryptoRecoveryRequired = nativeCryptoError !== undefined; + const strandedCryptoStore = + loadState.status === AsyncStatus.Error && isStrandedCryptoStoreError(loadState.error) + ? loadState.error + : undefined; + const cryptoRecoveryRequired = + nativeCryptoError !== undefined || strandedCryptoStore !== undefined; // Set matrix client context: homeserver and sync type (not PII) useEffect(() => { @@ -444,7 +450,11 @@ export function ClientRoot({ children }: ClientRootProps) { // Capture fatal client failures — useAsyncCallback swallows these into state so // they never reach the React ErrorBoundary; explicit capture is required. useEffect(() => { - if (loadState.status === AsyncStatus.Error && !isNativeCryptoStoreError(loadState.error)) { + if ( + loadState.status === AsyncStatus.Error && + !isNativeCryptoStoreError(loadState.error) && + !isStrandedCryptoStoreError(loadState.error) + ) { Sentry.captureException(loadState.error, { tags: { phase: 'load' } }); } }, [loadState]); @@ -496,14 +506,20 @@ export function ClientRoot({ children }: ClientRootProps) { {loadState.status === AsyncStatus.Error && - (nativeCryptoRecoveryRequired ? ( + (cryptoRecoveryRequired ? ( <> - Sign in again to continue using encrypted chats. - Export your message keys first, or restore them from backup after signing - in. + {strandedCryptoStore?.message ?? + 'Sign in again to continue using encrypted chats.'} + + + {nativeCryptoError + ? 'Export your message keys first, or restore them from backup after signing in.' + : 'Restore your messages from key backup after signing in.'} - + {nativeCryptoError && ( + + )}