fix(crypto): restore device verification from an unverified session

src/app/components/DeviceVerification.tsx

@@ -92,13 +92,23 @@ type VerificationStartProps = {
   onStart: () => Promise<void>;
 };
 function AutoVerificationStart({ onStart }: VerificationStartProps) {
+  const [error, setError] = useState<Error>();
+
   useEffect(() => {
-    onStart().catch(() => undefined);
+    onStart().catch((reason: unknown) => {
+      const failure = reason instanceof Error ? reason : new Error(String(reason));
+      Sentry.captureException(failure, { tags: { flow: 'device-verification-start' } });
+      setError(failure);
+    });
   }, [onStart]);
 
   return (
     <Box direction="Column" gap="400">
-      <WaitingMessage message="Starting verification using emoji comparison..." />
+      {error ? (
+        <Text size="T200">{error.message}</Text>
+      ) : (
+        <WaitingMessage message="Asking your other devices to start emoji comparison..." />
+      )}
     </Box>
   );
 }

src/app/crypto/engineCrypto/EngineCrypto.ts

@@ -129,7 +129,7 @@
 
   const entries = Object.entries(value as Record<string, unknown>)
     .filter(([, item]) => item !== undefined)
     .sort(([a], [b]) => (a < b ? -1 : 1));
   return `{${entries.map(([key, item]) => `${JSON.stringify(key)}:${canonicalJson(item)}`).join(',')}}`;
 };
 
@@ -719,21 +719,34 @@
     });
   }
 
-  async onIncomingKeyVerificationRequest(sender: string, transactionId: string): Promise<void> {
+  async onIncomingKeyVerificationRequest(sender: string, transactionId: string): Promise<boolean> {
     const state = (await this.#call('getVerificationRequest', {
       userId: sender,
       flowId: transactionId,
     })) as EngineVerificationState | null;
-    if (!state) return;
+    if (!state) return false;
 
     const existing = this.#verificationRequests.get(transactionId);
     if (existing) {
       existing.apply(state);
-      return;
+      return true;
     }
     const request = new EngineVerificationRequest(this.#engineCall, state);
     this.#verificationRequests.set(transactionId, request);
     this.emit(CryptoEvent.VerificationRequestReceived, request);
+    return true;
+  }
+
+  async #retryVerificationRequestWithKeys(
+    sender: string,
+    transactionId: string,
+    event: IToDeviceEvent
+  ): Promise<void> {
+    await this.#trackUsers([sender]);
+    await this.#sendTracked(await this.#call('queryKeysForUsers', { users: [sender] }));
+    await this.#flushOutgoingRequests();
+    await this.#receiveSyncChanges({ toDeviceEvents: [event] });
+    await this.onIncomingKeyVerificationRequest(sender, transactionId);
   }
 
   #flushOutgoingRequests(): Promise<void> {
@@ -805,11 +818,20 @@
         if (transactionId && message.sender) {
           if (message.type === EventType.KeyVerificationRequest) {
             // eslint-disable-next-line no-await-in-loop
-            await this.onIncomingKeyVerificationRequest(message.sender, transactionId);
+            const handled = await this.onIncomingKeyVerificationRequest(
+              message.sender,
+              transactionId
+            );
+            if (!handled) {
+              // eslint-disable-next-line no-await-in-loop
+              await this.#retryVerificationRequestWithKeys(message.sender, transactionId, message);
+            }
           } else if (message.type === EventType.KeyVerificationDone) {
             // Rust removes completed requests while consuming the event, so no state snapshot
             // exists to refresh. Keep the JS request alive long enough to expose Done.
             this.#verificationRequests.get(transactionId)?.markDone();
+            // eslint-disable-next-line no-await-in-loop
+            await this.#queryOwnKeys();
           } else {
             // Without this the verifier never learns the SAS digits arrived.
             // eslint-disable-next-line no-await-in-loop
@@ -1562,6 +1584,22 @@
     await this.#flushOutgoingRequests();
   }
 
+  async #signOwnDeviceIfNeeded(): Promise<void> {
+    const status = await this.getDeviceVerificationStatus(
+      this.#identity.userId,
+      this.#identity.deviceId
+    );
+    if (status?.crossSigningVerified) return;
+
+    const request = (await this.#call('device.verify', {
+      userId: this.#identity.userId,
+      deviceId: this.#identity.deviceId,
+    })) as OutgoingRequest | null;
+    if (!request) return;
+    await sendOutgoingRequest(this.#mx, request);
+    await this.#queryOwnKeys();
+  }
+
   async crossSignDevice(deviceId: string): Promise<void> {
     await this.#sendTracked(
       await this.#call('device.verify', { userId: this.#identity.userId, deviceId })
@@ -1602,6 +1640,7 @@
       if (!stored && (await this.#mx.secretStorage.hasKey())) {
         await this.#exportCrossSigningKeysToStorage();
       }
+      await this.#signOwnDeviceIfNeeded();
       return;
     }
 
@@ -1756,6 +1795,7 @@
         { ...key.keyInfo, key: key.privateKey }
       );
       await this.#mx.secretStorage.setDefaultKeyId(keyId);
+      this.#mx.cryptoCallbacks?.cacheSecretStorageKey?.(keyId, keyInfo, key.privateKey);
       engineCryptoLog.info('general', 'Created a new secret storage key', {
         keyId,
         algorithm: keyInfo.algorithm,

src/app/crypto/engineCrypto/bootstrapSecretStorage.test.ts

@@ -0,0 +1,58 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import type { MatrixClient } from '$types/matrix-sdk';
+import { engineInvoke } from '../olmMachine/engineInvoke';
+import { EngineCrypto } from './EngineCrypto';
+
+vi.mock('../olmMachine/engineInvoke', () => ({
+  engineInvoke: vi.fn<(...args: never[]) => Promise<unknown>>(),
+}));
+
+const mockInvoke = vi.mocked(engineInvoke);
+
+const PRIVATE_KEY = new Uint8Array([1, 2, 3]);
+
+const clientStub = () => {
+  const cacheSecretStorageKey = vi.fn<(keyId: string, keyInfo: unknown, key: Uint8Array) => void>();
+  const store = vi.fn<(name: string, value: string) => Promise<void>>(async () => undefined);
+  const mx = {
+    http: { authedRequest: vi.fn<(...args: never[]) => Promise<string>>(async () => '{}') },
+    cryptoCallbacks: { cacheSecretStorageKey },
+    secretStorage: {
+      getDefaultKeyId: async () => null,
+      addKey: async () => ({
+        keyId: 'KEYID',
+        keyInfo: { algorithm: 'm.secret_storage.v1.aes-hmac-sha2' },
+      }),
+      setDefaultKeyId: async () => undefined,
+      hasKey: async () => true,
+      get: async () => null,
+      store,
+    },
+  } as unknown as MatrixClient;
+  return { mx, cacheSecretStorageKey, store };
+};
+
+describe('bootstrapSecretStorage', () => {
+  beforeEach(() => mockInvoke.mockReset());
+
+  it('caches the key it just created so storing secrets can find it', async () => {
+    mockInvoke.mockImplementation(async (_identity, method) => {
+      if (method === 'exportCrossSigningKeys') return { masterKey: 'msk' };
+      return null;
+    });
+    const { mx, cacheSecretStorageKey } = clientStub();
+
+    await new EngineCrypto(mx, { userId: '@me:e.org', deviceId: 'D' }).bootstrapSecretStorage({
+      createSecretStorageKey: async () => ({
+        privateKey: PRIVATE_KEY,
+        encodedPrivateKey: 'encoded',
+      }),
+    });
+
+    expect(cacheSecretStorageKey).toHaveBeenCalledWith(
+      'KEYID',
+      { algorithm: 'm.secret_storage.v1.aes-hmac-sha2' },
+      PRIVATE_KEY
+    );
+  });
+});

src/app/crypto/engineCrypto/incomingVerificationRequest.test.ts

@@ -0,0 +1,85 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { CryptoEvent, EventType, type MatrixClient } from '$types/matrix-sdk';
+import { engineInvoke } from '../olmMachine/engineInvoke';
+import { EngineCrypto } from './EngineCrypto';
+
+vi.mock('../olmMachine/engineInvoke', () => ({
+  engineInvoke: vi.fn<(...args: never[]) => Promise<unknown>>(),
+}));
+
+const mockInvoke = vi.mocked(engineInvoke);
+
+const clientSpy = () => {
+  const authedRequest = vi.fn<(...args: never[]) => Promise<string>>(async () => '{}');
+  return { mx: { http: { authedRequest } } as unknown as MatrixClient, authedRequest };
+};
+
+const REQUEST_EVENT = {
+  type: EventType.KeyVerificationRequest,
+  sender: '@me:e.org',
+  content: { transaction_id: '$f', from_device: 'OTHER' },
+};
+
+const requestState = {
+  flowId: '$f',
+  otherUserId: '@me:e.org',
+  phase: 1,
+  isSelfVerification: true,
+};
+
+describe('incoming verification request', () => {
+  beforeEach(() => mockInvoke.mockReset());
+
+  it('fetches the sender keys and replays the event when the device is unknown', async () => {
+    const { mx } = clientSpy();
+    let senderKnown = false;
+    const invoked: string[] = [];
+
+    mockInvoke.mockImplementation(async (_identity, method) => {
+      invoked.push(method as string);
+      if (method === 'receiveSyncChanges') {
+        return [{ type: 3, rawEvent: JSON.stringify(REQUEST_EVENT) }];
+      }
+      if (method === 'queryKeysForUsers') {
+        senderKnown = true;
+        return { id: 'q1', type: 1, className: 'KeysQueryRequest', body: '{}' };
+      }
+      if (method === 'getVerificationRequest') return senderKnown ? requestState : null;
+      return [];
+    });
+
+    const crypto = new EngineCrypto(mx, { userId: '@me:e.org', deviceId: 'D' });
+    const received = vi.fn<(request: unknown) => void>();
+    crypto.on(CryptoEvent.VerificationRequestReceived, received);
+
+    await crypto.preprocessToDeviceMessages([REQUEST_EVENT as never]);
+
+    expect(invoked.filter((m) => m === 'queryKeysForUsers')).toHaveLength(1);
+    expect(invoked.filter((m) => m === 'receiveSyncChanges')).toHaveLength(2);
+    expect(received).toHaveBeenCalledOnce();
+  });
+
+  it('does not replay when the engine already knows the request', async () => {
+    const { mx } = clientSpy();
+    const invoked: string[] = [];
+
+    mockInvoke.mockImplementation(async (_identity, method) => {
+      invoked.push(method as string);
+      if (method === 'receiveSyncChanges') {
+        return [{ type: 3, rawEvent: JSON.stringify(REQUEST_EVENT) }];
+      }
+      if (method === 'getVerificationRequest') return requestState;
+      return [];
+    });
+
+    const crypto = new EngineCrypto(mx, { userId: '@me:e.org', deviceId: 'D' });
+    const received = vi.fn<(request: unknown) => void>();
+    crypto.on(CryptoEvent.VerificationRequestReceived, received);
+
+    await crypto.preprocessToDeviceMessages([REQUEST_EVENT as never]);
+
+    expect(invoked).not.toContain('queryKeysForUsers');
+    expect(invoked.filter((m) => m === 'receiveSyncChanges')).toHaveLength(1);
+    expect(received).toHaveBeenCalledOnce();
+  });
+});

src/app/crypto/verification/request.ts

@@ -44,6 +44,8 @@ export class EngineVerificationRequest
 
   #sasAccepted = false;
 
+  #sasWeStarted = false;
+
   constructor(call: EngineCall, state: EngineVerificationState) {
     super();
     this.#call = call;
@@ -75,8 +77,14 @@ export class EngineVerificationRequest
           : undefined;
 
     const accepted = (verification as SasState).hasBeenAccepted === true;
-    const lostTieBreak = wanted === 'Sas' && current === 'Sas' && this.#sasAccepted && !accepted;
+    const weStarted = (verification as SasState).weStarted === true;
+    const replaced =
+      wanted === 'Sas' &&
+      current === 'Sas' &&
+      ((this.#sasAccepted && !accepted) || (this.#sasWeStarted && !weStarted));
+    const lostTieBreak = replaced;
     this.#sasAccepted = wanted === 'Sas' ? accepted : false;
+    this.#sasWeStarted = wanted === 'Sas' ? weStarted : false;
 
     if (current !== wanted || lostTieBreak) {
       if (wanted === 'Sas') {
@@ -118,7 +126,15 @@ export class EngineVerificationRequest
       'verificationRequest.state',
       this.#flow
     )) as EngineVerificationState | null;
-    if (!next) return;
+    if (!next) {
+      if (this.#state.phase === EnginePhase.Done || this.#state.phase === EnginePhase.Cancelled) {
+        return;
+      }
+      this.#state = { ...this.#state, phase: EnginePhase.Cancelled, isCancelled: true };
+      this.#syncVerifier();
+      this.emit(VerificationRequestEvent.Change);
+      return;
+    }
 
     this.#state = next;
     this.#syncVerifier();

src/app/features/settings/devices/Devices.tsx

@@ -6,6 +6,7 @@ import { useDeviceIds, useDeviceList, useSplitCurrentDevice } from '$hooks/useDe
 import { useMatrixClient } from '$hooks/useMatrixClient';
 import {
   useDeviceVerificationStatus,
+  useVerifiedDeviceCount,
   useUnverifiedDeviceCount,
   VerificationStatus,
 } from '$hooks/useDeviceVerificationStatus';
@@ -49,6 +50,7 @@ export function Devices({ requestBack, requestClose }: DevicesProps) {
   );
 
   const otherDevicesId = useDeviceIds(otherDevices);
+  const verifiedDeviceCount = useVerifiedDeviceCount(crypto, mx.getSafeUserId(), otherDevicesId);
   const unverifiedDeviceCount = useUnverifiedDeviceCount(
     crypto,
     mx.getSafeUserId(),
@@ -111,15 +113,13 @@ export function Devices({ requestBack, requestClose }: DevicesProps) {
                     >
                       {crypto && <DeviceKeyDetails crypto={crypto} />}
                     </DeviceTile>
-                    {crossSigningActive &&
-                      verificationStatus === VerificationStatus.Unverified &&
-                      defaultSecretStorageKeyId &&
-                      defaultSecretStorageKeyContent && (
-                        <VerifyCurrentDeviceTile
-                          secretStorageKeyId={defaultSecretStorageKeyId}
-                          secretStorageKeyContent={defaultSecretStorageKeyContent}
-                        />
-                      )}
+                    {crossSigningActive && verificationStatus === VerificationStatus.Unverified && (
+                      <VerifyCurrentDeviceTile
+                        secretStorageKeyId={defaultSecretStorageKeyId}
+                        secretStorageKeyContent={defaultSecretStorageKeyContent}
+                        hasVerifiedOtherDevice={(verifiedDeviceCount ?? 0) > 0}
+                      />
+                    )}
                     {crypto && verificationStatus === VerificationStatus.Verified && (
                       <BackupRestoreTile
                         crypto={crypto}
@@ -137,9 +137,7 @@ export function Devices({ requestBack, requestClose }: DevicesProps) {
                 <OtherDevices
                   devices={otherDevices}
                   refreshDeviceList={refreshDeviceList}
-                  showVerification={
-                    crossSigningActive && verificationStatus === VerificationStatus.Verified
-                  }
+                  showVerification={crossSigningActive}
                 />
               )}
               <LocalBackup />