fix(crypto): stop device verification from silently reverting

.changeset/fix-device-verification-reverting.md

@@ -0,0 +1,5 @@
+---
+default: patch
+---
+
+Fix devices repeatedly reverting to unverified.

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',

src-tauri/gen/android/app/google-services.json

@@ -23,6 +23,25 @@
           "other_platform_oauth_client": []
         }
       }
+    },
+    {
+      "client_info": {
+        "mobilesdk_app_id": "1:136631010081:android:4cfbdacbecff60c00b7460",
+        "android_client_info": {
+          "package_name": "moe.sable.client.nightly"
+        }
+      },
+      "oauth_client": [],
+      "api_key": [
+        {
+          "current_key": "AIzaSyC-0JTo2XFXGqKda6iaUyUGoCI-NIqhMFA"
+        }
+      ],
+      "services": {
+        "appinvite_service": {
+          "other_platform_oauth_client": []
+        }
+      }
     }
   ],
   "configuration_version": "1"

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<unknown>>());
+
+vi.mock('$utils/fetch', () => ({ fetch: appFetch }));
 const emitter = new TypedEventEmitter<string, Record<string, (...args: never[]) => 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<CryptoApi['loadSessionBackupPrivateKeyFromSecretStorage']>()
       .mockResolvedValue(undefined),
+    getDeviceVerificationStatus: vi
+      .fn<CryptoApi['getDeviceVerificationStatus']>()
+      .mockResolvedValue({ crossSigningVerified: true } as Awaited<
+        ReturnType<CryptoApi['getDeviceVerificationStatus']>
+      >),
+    getOwnDeviceKeys: vi
+      .fn<CryptoApi['getOwnDeviceKeys']>()
+      .mockResolvedValue({ ed25519: 'own-ed25519', curve25519: 'own-curve25519' }),
+    getCrossSigningKeyId: vi
+      .fn<CryptoApi['getCrossSigningKeyId']>()
+      .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 () => {

src/app/components/ManualVerification.test.tsx

@@ -13,24 +13,45 @@ const processDeviceLists = vi.hoisted(() => vi.fn<() => Promise<void>>());
 const bootstrapCrossSigning = vi.hoisted(() => vi.fn<() => Promise<void>>());
 const bootstrapSecretStorage = vi.hoisted(() => vi.fn<() => Promise<void>>());
 const loadSessionBackupPrivateKeyFromSecretStorage = vi.hoisted(() => vi.fn<() => Promise<void>>());
+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<string | null>>());
+const appFetch = vi.hoisted(() => vi.fn<() => Promise<unknown>>());
+
+vi.mock('$utils/fetch', () => ({ fetch: appFetch }));
 
 vi.mock('$types/matrix-sdk', () => ({ decodeRecoveryKey }));
 vi.mock('$client/secretStorageKeys', () => ({ storePrivateKey }));
 vi.mock('$hooks/useMatrixClient', () => ({
   useMatrixClient: () => ({
     getSafeUserId: () => '@me:example.org',
     getDeviceId: () => 'DEVICE',
+    baseUrl: 'https://example.org',
+    getAccessToken: () => 'access-token',
     secretStorage: { checkKey, get: getSecret },
     getCrypto: () =>
       ({
         processDeviceLists,
         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());

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;
     }
 

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) {
             <Dialog>
               <Box direction="Column" gap="400" style={{ padding: config.space.S400 }}>
                 {loadState.status === AsyncStatus.Error &&
-                  (nativeCryptoRecoveryRequired ? (
+                  (cryptoRecoveryRequired ? (
                     <>
-                      <Text>Sign in again to continue using encrypted chats.</Text>
                       <Text>
-                        Export your message keys first, or restore them from backup after signing
-                        in.
+                        {strandedCryptoStore?.message ??
+                          'Sign in again to continue using encrypted chats.'}
+                      </Text>
+                      <Text>
+                        {nativeCryptoError
+                          ? 'Export your message keys first, or restore them from backup after signing in.'
+                          : 'Restore your messages from key backup after signing in.'}
                       </Text>
-                      <LegacyKeyExport exporter={nativeCryptoError.exportRoomKeys} />
+                      {nativeCryptoError && (
+                        <LegacyKeyExport exporter={nativeCryptoError.exportRoomKeys} />
+                      )}
                       <AsyncError state={recoveryState} prefix="Failed to sign out" size="T300" />
                       <Button
                         variant="Critical"
@@ -521,7 +537,7 @@ export function ClientRoot({ children }: ClientRootProps) {
                 {startState.status === AsyncStatus.Error && (
                   <Text>{`Failed to start. ${errorMessage(startState.error)}`}</Text>
                 )}
-                {!nativeCryptoRecoveryRequired && (
+                {!cryptoRecoveryRequired && (
                   <Button variant="Critical" onClick={mx ? () => startMatrix(mx) : loadMatrix}>
                     <Text as="span" size="B400">
                       Retry

src/app/state/sessions.ts

@@ -147,6 +147,13 @@ export type SessionsAction =
       session: Session;
     };
 
+// A PUT carries a session rebuilt from a login response, and `fallbackSdkStores` picks the
+// IndexedDB stores, so losing it on the same device points at an empty crypto store.
+const preserveStoreLocation = (existing: Session, next: Session): Session =>
+  existing.fallbackSdkStores && existing.deviceId === next.deviceId
+    ? { ...next, fallbackSdkStores: true }
+    : next;
+
 export const sessionsAtom = atom<Sessions, [SessionsAction], void>(
   (get) => get(baseSessionsAtom),
   (get, set, action) => {
@@ -160,7 +167,11 @@ export const sessionsAtom = atom<Sessions, [SessionsAction], void>(
         sessions.push(action.session);
       } else {
         log.log('PUT update session', action.session.userId);
-        sessions.splice(sessionIndex, 1, action.session);
+        sessions.splice(
+          sessionIndex,
+          1,
+          preserveStoreLocation(sessions[sessionIndex]!, action.session)
+        );
       }
       set(baseSessionsAtom, sessions);
       return;

src/app/state/sessions.updateTokens.test.ts

@@ -83,3 +83,62 @@ describe("sessionsAtom 'UPDATE' action", () => {
     expect(session?.slidingSyncOptIn).toBe(true);
   });
 });
+
+describe("sessionsAtom 'PUT' action", () => {
+  beforeEach(() => {
+    localStorage.clear();
+  });
+
+  it('keeps the fallback store flag when the same device signs in again', () => {
+    const store = createStore();
+    store.set(sessionsAtom, {
+      type: 'PUT',
+      session: { ...baseSession, fallbackSdkStores: true },
+    });
+
+    store.set(sessionsAtom, {
+      type: 'PUT',
+      session: { ...baseSession, accessToken: 'fresh-access' },
+    });
+
+    const [alice] = store.get(sessionsAtom);
+    expect(alice!.fallbackSdkStores).toBe(true);
+    expect(alice!.accessToken).toBe('fresh-access');
+  });
+
+  it('drops the fallback store flag when the session moves to a new device', () => {
+    const store = createStore();
+    store.set(sessionsAtom, {
+      type: 'PUT',
+      session: { ...baseSession, fallbackSdkStores: true },
+    });
+
+    store.set(sessionsAtom, {
+      type: 'PUT',
+      session: { ...baseSession, deviceId: 'DEV2', accessToken: 'fresh-access' },
+    });
+
+    const [alice] = store.get(sessionsAtom);
+    expect(alice!.fallbackSdkStores).toBeUndefined();
+    expect(alice!.deviceId).toBe('DEV2');
+  });
+
+  it('does not resurrect fields a re-login cleared', () => {
+    const store = createStore();
+    store.set(sessionsAtom, { type: 'PUT', session: baseSession });
+
+    store.set(sessionsAtom, {
+      type: 'PUT',
+      session: {
+        baseUrl: baseSession.baseUrl,
+        userId: baseSession.userId,
+        deviceId: baseSession.deviceId,
+        accessToken: 'password-login-access',
+      },
+    });
+
+    const [alice] = store.get(sessionsAtom);
+    expect(alice!.refreshToken).toBeUndefined();
+    expect(alice!.oidc).toBeUndefined();
+  });
+});