diff --git a/__tests__/ducks/auth.test.ts b/__tests__/ducks/auth.test.ts index bd6585b2e..5979f3a06 100644 --- a/__tests__/ducks/auth.test.ts +++ b/__tests__/ducks/auth.test.ts @@ -4,6 +4,8 @@ import { act, renderHook } from "@testing-library/react-hooks"; import { AUTO_LOCK_TIMER, DEFAULT_AUTO_LOCK_TIMER, + HASH_KEY_EXPIRATION_MS, + HASH_KEY_REFRESH_THROTTLE_MS, NETWORKS, STORAGE_KEYS, SENSITIVE_STORAGE_KEYS, @@ -40,10 +42,13 @@ import { clearScreenshotDek } from "helpers/screenshotCrypto"; import { AppState } from "react-native"; import { getSupportedBiometryType, BIOMETRY_TYPE } from "react-native-keychain"; import { clearAuthKeypairCache } from "services/auth/authKeypairCache"; +import { resetHashKeyRefreshAttemptThrottle } from "services/autoLock"; import { clearNonSensitiveData, clearTemporaryData, getHashKey, + getHashKeyGeneration, + setHashKey, } from "services/storage/helpers"; // Import mocked modules import { rnBiometrics } from "services/storage/secureStorage"; @@ -139,11 +144,24 @@ jest.mock("services/stellar", () => ({ getAccount: jest.fn(), })); -jest.mock("services/storage/helpers", () => ({ - clearNonSensitiveData: jest.fn(), - clearTemporaryData: jest.fn(), - getHashKey: jest.fn(), -})); +jest.mock("services/storage/helpers", () => { + const { secureDataStorage: mockedSecureStorage } = jest.requireMock< + typeof import("services/storage/storageFactory") + >("services/storage/storageFactory"); + const { SENSITIVE_STORAGE_KEYS: keys } = + jest.requireActual("config/constants"); + return { + clearNonSensitiveData: jest.fn(), + clearTemporaryData: jest.fn(), + getHashKey: jest.fn(), + getHashKeyGeneration: jest.fn(() => 0), + // Delegates to the mocked secure storage so assertions on HASH_KEY + // writes see credential-verified writes and re-anchors alike. + setHashKey: jest.fn((hashKey: unknown) => + mockedSecureStorage.setItem(keys.HASH_KEY, JSON.stringify(hashKey)), + ), + }; +}); jest.mock("config/logger", () => ({ logger: { @@ -1750,6 +1768,94 @@ describe("auth duck", () => { }); describe("getAuthStatus with LOCKED state", () => { + // The hard-expiry decision reads the wall clock, which the device holder + // controls. The persisted marker is the only clock-independent lock state, + // so restoring the clock should land back on LOCKED (password required), + // not AUTHENTICATED. + it.each([ + ["moved forward past expiresAt", 73 * 3600000], + ["moved back before generatedAt", -24 * 3600000], + ])( + "should stay LOCKED when the clock is %s and then restored", + async (_label, clockOffsetMs) => { + const realNow = Date.now(); + const nowSpy = jest.spyOn(Date, "now").mockReturnValue(realNow); + const getItemMock = secureDataStorage.getItem as jest.Mock; + const setItemMock = secureDataStorage.setItem as jest.Mock; + const removeMock = secureDataStorage.remove as jest.Mock; + const savedImpls = [ + getItemMock.getMockImplementation(), + setItemMock.getMockImplementation(), + removeMock.getMockImplementation(), + ] as const; + resetHashKeyRefreshAttemptThrottle(); + + try { + const { result } = renderHook(() => useAuthenticationStore()); + + act(() => { + useAuthenticationStore.setState({ + getAuthStatus: originalStoreMethods.getAuthStatus, + }); + }); + + (dataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === STORAGE_KEYS.ACCOUNT_LIST) { + return Promise.resolve(JSON.stringify([mockAccount])); + } + return Promise.resolve(null); + }); + + // Stateful secure storage, so a removed marker stays removed. + const secureStore = new Map([ + [SENSITIVE_STORAGE_KEYS.AUTH_STATUS, AUTH_STATUS.LOCKED], + [SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE, "encrypted-temp-store"], + ]); + getItemMock.mockImplementation((key: string) => + Promise.resolve(secureStore.get(key) ?? null), + ); + setItemMock.mockImplementation((key: string, value: string) => { + secureStore.set(key, value); + return Promise.resolve(); + }); + removeMock.mockImplementation((key: string) => { + secureStore.delete(key); + return Promise.resolve(); + }); + + // Soft-locked with a key anchored an hour ago: valid on the real clock. + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: realNow - 3600000, + expiresAt: realNow + 71 * 3600000, + }); + + nowSpy.mockReturnValue(realNow + clockOffsetMs); + await act(async () => { + expect(await result.current.getAuthStatus()).toBe( + AUTH_STATUS.HASH_KEY_EXPIRED, + ); + }); + + nowSpy.mockReturnValue(realNow); + await act(async () => { + expect(await result.current.getAuthStatus()).toBe( + AUTH_STATUS.LOCKED, + ); + }); + expect(secureStore.get(SENSITIVE_STORAGE_KEYS.AUTH_STATUS)).toBe( + AUTH_STATUS.LOCKED, + ); + } finally { + nowSpy.mockRestore(); + getItemMock.mockImplementation(savedImpls[0]); + setItemMock.mockImplementation(savedImpls[1]); + removeMock.mockImplementation(savedImpls[2]); + resetHashKeyRefreshAttemptThrottle(); + } + }, + ); it("should return LOCKED when persisted status is LOCKED and temp store exists", async () => { const { result } = renderHook(() => useAuthenticationStore()); @@ -1791,6 +1897,59 @@ describe("auth duck", () => { }); }); + it("should return HASH_KEY_EXPIRED (not LOCKED) when a persisted soft lock holds an expired key (#924)", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + + act(() => { + useAuthenticationStore.setState({ + getAuthStatus: originalStoreMethods.getAuthStatus, + }); + }); + + (dataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === STORAGE_KEYS.ACCOUNT_LIST) { + return Promise.resolve(JSON.stringify([mockAccount])); + } + return Promise.resolve(null); + }); + + // Soft-locked session sitting past the whole backstop window: with + // the TTL anchored on activity, this only happens after 72h of no + // use — exactly the case that must surface as the full re-auth + // rather than the fast unlock (#924 AC2). Under sign-in anchoring + // (#905) lock-screen expiry was routine, so LOCKED deliberately won; + // activity anchoring makes it the genuine-idle signal. + (secureDataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === SENSITIVE_STORAGE_KEYS.AUTH_STATUS) { + return Promise.resolve(AUTH_STATUS.LOCKED); + } + if (key === SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE) { + return Promise.resolve("encrypted-temp-store"); + } + return Promise.resolve(null); + }); + + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() - 73 * 3600000, + expiresAt: Date.now() - 3600000, // hard-expired an hour ago + }); + (secureDataStorage.remove as jest.Mock).mockResolvedValue(undefined); + + await act(async () => { + const status = await result.current.getAuthStatus(); + expect(status).toBe(AUTH_STATUS.HASH_KEY_EXPIRED); + }); + + // The marker survives: it is the only clock-independent lock state, + // so consuming it here would let a clock change undo the lock (see + // the clock-restore cases below). signIn clears it after the password. + expect(secureDataStorage.remove).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.AUTH_STATUS, + ); + }); + it("should clear invalid LOCKED status if temp store doesn't exist", async () => { const { result } = renderHook(() => useAuthenticationStore()); @@ -1834,11 +1993,91 @@ describe("auth duck", () => { SENSITIVE_STORAGE_KEYS.AUTH_STATUS, ); }); + + // Clearing the stale marker is a cleanup, not the decision. If it were + // allowed to reach getAuthStatus's outer catch, a known-expired session + // would answer NOT_AUTHENTICATED and RootNavigator would route an + // account-bearing wallet to the onboarding auth stack instead of the + // lock screen that collects the required password re-auth. + it.each([ + [ + "a persisted soft lock with no temporary store", + { + temporaryStore: null, + hashKey: { + hashKey: "mock-hash-key", + salt: "mock-salt", + expiresAt: Date.now() + 3600000, + }, + }, + ], + ])( + "should still return HASH_KEY_EXPIRED for %s when clearing the stale marker fails", + async (_label, { temporaryStore, hashKey }) => { + const { result } = renderHook(() => useAuthenticationStore()); + + act(() => { + useAuthenticationStore.setState({ + getAuthStatus: originalStoreMethods.getAuthStatus, + }); + }); + + (dataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === STORAGE_KEYS.ACCOUNT_LIST) { + return Promise.resolve(JSON.stringify([mockAccount])); + } + return Promise.resolve(null); + }); + + (secureDataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === SENSITIVE_STORAGE_KEYS.AUTH_STATUS) { + return Promise.resolve(AUTH_STATUS.LOCKED); + } + if (key === SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE) { + return Promise.resolve(temporaryStore); + } + return Promise.resolve(null); + }); + + (getHashKey as jest.Mock).mockResolvedValue(hashKey); + + (secureDataStorage.remove as jest.Mock).mockRejectedValueOnce( + new Error("keychain unavailable"), + ); + + await act(async () => { + const status = await result.current.getAuthStatus(); + expect(status).toBe(AUTH_STATUS.HASH_KEY_EXPIRED); + }); + + expect(secureDataStorage.remove).toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.AUTH_STATUS, + ); + }, + ); }); describe("getAuthStatus with auto-lock timer", () => { const ONE_HOUR_MS = 3600000; + // These tests drive AppState.currentState and restore it inline, but a + // failing expect() aborts the test before its restore line — leaking + // "active" into the rest of the file and cascading into unrelated + // failures (e.g. the isSessionAuthValid call-count tests). Restore here + // so a failure stays attributable to the test that caused it. + const defaultAppState = AppState.currentState; + // These integration tests exercise the real refreshHashKeyExpiration, + // whose attempt throttle is module-level state: without a reset, the + // first test to attempt a write would suppress every later one. + beforeEach(() => { + resetHashKeyRefreshAttemptThrottle(); + }); + afterEach(() => { + (AppState as { currentState: typeof defaultAppState }).currentState = + defaultAppState; + resetHashKeyRefreshAttemptThrottle(); + }); + const mockAuthenticatedStorage = ({ backgroundedAt, autoLockTimer, @@ -1911,45 +2150,419 @@ describe("auth duck", () => { ); }); - it("should stay authenticated and consume the timestamp WITHOUT refreshing the hash key TTL", async () => { + // Unlike the reads above, secureDataStorage.setItem really does throw on + // a keychain failure. Letting that reach the outer catch would answer + // NOT_AUTHENTICATED for a session just decided to be LOCKED, dropping an + // account-bearing wallet onto the onboarding stack. + it("should still soft-lock when persisting the LOCKED marker fails", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + restoreGetAuthStatus(); + + mockAuthenticatedStorage({ + backgroundedAt: Date.now() - 2 * ONE_HOUR_MS, + autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, + }); + + (secureDataStorage.setItem as jest.Mock).mockRejectedValueOnce( + new Error("Failed to store item in keychain"), + ); + + await act(async () => { + const status = await result.current.getAuthStatus(); + expect(status).toBe(AUTH_STATUS.LOCKED); + }); + + // The timestamp is deliberately NOT consumed: with no persisted + // marker, the next check must re-derive the same lock from elapsed + // background time rather than resolving AUTHENTICATED. + expect(secureDataStorage.remove).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.AUTO_LOCK_BACKGROUNDED_AT, + ); + }); + + it("should consume the timestamp and re-anchor a stale hash-key TTL on active use (#924)", async () => { const { result } = renderHook(() => useAuthenticationStore()); restoreGetAuthStatus(); - // The jest AppState mock has no real currentState; the consume - // branch only runs when the app is actively foregrounded - const previousAppState = AppState.currentState; (AppState as { currentState: string }).currentState = "active"; mockAuthenticatedStorage({ backgroundedAt: Date.now() - 60000, // 1 minute ago autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, }); + // Stale anchor: beyond the refresh throttle, but far from expired. + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() - (HASH_KEY_REFRESH_THROTTLE_MS + 60000), + expiresAt: Date.now() + ONE_HOUR_MS, + }); + const before = Date.now(); await act(async () => { const status = await result.current.getAuthStatus(); expect(status).toBe(AUTH_STATUS.AUTHENTICATED); }); + // The write must have SUCCEEDED, not been swallowed by the + // re-anchor's catch — the call list below alone can't tell the two + // apart. + expect(logger.error).not.toHaveBeenCalled(); // Timestamp is consumed... expect(secureDataStorage.remove).toHaveBeenCalledWith( SENSITIVE_STORAGE_KEYS.AUTO_LOCK_BACKGROUNDED_AT, ); - // ...but the hash key expiry must NOT advance without credential - // verification (key material lifetime stays bounded) + // ...and the hard-expiry is re-anchored on activity: same key + // material, fresh expiresAt/generatedAt. + const hashKeyWrites = ( + secureDataStorage.setItem as jest.Mock + ).mock.calls.filter(([key]) => key === SENSITIVE_STORAGE_KEYS.HASH_KEY); + expect(hashKeyWrites).toHaveLength(1); + const written = JSON.parse(hashKeyWrites[0][1] as string) as { + hashKey: string; + salt: string; + expiresAt: number; + generatedAt: number; + }; + expect(written.hashKey).toBe("mock-hash-key"); + expect(written.salt).toBe("mock-salt"); + expect(written.generatedAt).toBeGreaterThanOrEqual(before); + expect(written.expiresAt).toBe( + written.generatedAt + HASH_KEY_EXPIRATION_MS, + ); + }); + + it("should NOT re-anchor a freshly-stamped hash key (write throttle)", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + restoreGetAuthStatus(); + + (AppState as { currentState: string }).currentState = "active"; + + mockAuthenticatedStorage({ + backgroundedAt: null, + autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, + }); + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() - 60000, // anchored 1 minute ago + expiresAt: Date.now() + ONE_HOUR_MS, + }); + + await act(async () => { + const status = await result.current.getAuthStatus(); + expect(status).toBe(AUTH_STATUS.AUTHENTICATED); + }); + expect(secureDataStorage.setItem).not.toHaveBeenCalledWith( SENSITIVE_STORAGE_KEYS.HASH_KEY, expect.any(String), ); + }); + + it("should NOT re-anchor from the periodic background check", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + restoreGetAuthStatus(); + + (AppState as { currentState: string }).currentState = "background"; + + mockAuthenticatedStorage({ + backgroundedAt: Date.now() - 60000, // within the timer + autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, + }); + // Stale anchor — would refresh if the app were active. + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() - (HASH_KEY_REFRESH_THROTTLE_MS + 60000), + expiresAt: Date.now() + ONE_HOUR_MS, + }); + + await act(async () => { + const status = await result.current.getAuthStatus(); + expect(status).toBe(AUTH_STATUS.AUTHENTICATED); + }); + + // A backgrounded device must not extend its own deadline (and the + // still-counting soft timer must not be consumed either). + expect(secureDataStorage.setItem).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + expect.any(String), + ); + expect(secureDataStorage.remove).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.AUTO_LOCK_BACKGROUNDED_AT, + ); + }); + + it("should stay AUTHENTICATED when the re-anchor keychain write fails", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + restoreGetAuthStatus(); + + (AppState as { currentState: string }).currentState = "active"; + + mockAuthenticatedStorage({ + backgroundedAt: null, + autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, + }); + // Stale anchor on a healthy key: the re-anchor will attempt a write. + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() - (HASH_KEY_REFRESH_THROTTLE_MS + 60000), + expiresAt: Date.now() + ONE_HOUR_MS, + }); + // The keychain rejects the re-anchor write (this is the only setItem + // getAuthStatus performs on the AUTHENTICATED path). + // `Once` (not a sticky mockRejectedValue): jest config has no + // resetMocks, so a sticky rejection would leak a broken keychain into + // every later test in this file. + (secureDataStorage.setItem as jest.Mock).mockRejectedValueOnce( + new Error("keychain unavailable"), + ); + + await act(async () => { + const status = await result.current.getAuthStatus(); + // The re-anchor is opportunistic: a failed write must not escape to + // the outer catch and demote the session to NOT_AUTHENTICATED. The + // key simply keeps its old deadline. + expect(status).toBe(AUTH_STATUS.AUTHENTICATED); + }); - (AppState as { currentState: typeof previousAppState }).currentState = - previousAppState; + expect(logger.error).toHaveBeenCalledWith( + "getAuthStatus", + "Failed to refresh hash key expiration", + expect.any(Error), + ); + // ...and specifically NOT via the outer catch-all. + expect(logger.error).not.toHaveBeenCalledWith( + "validateAuth", + "Failed to validate auth", + expect.anything(), + ); + }); + + it("should not resurrect a hash key wiped mid-check (TOCTOU)", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + restoreGetAuthStatus(); + + (AppState as { currentState: string }).currentState = "active"; + + mockAuthenticatedStorage({ + backgroundedAt: null, + autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, + }); + const staleKey = { + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() - (HASH_KEY_REFRESH_THROTTLE_MS + 60000), + expiresAt: Date.now() + ONE_HOUR_MS, + }; + // The entry-time read sees a healthy key; by the time the re-anchor + // re-reads at write time, a concurrent logout / corruption wipe + // (clearTemporaryData) has removed it. + (getHashKey as jest.Mock) + .mockResolvedValueOnce(staleKey) + .mockResolvedValue(null); + + await act(async () => { + await result.current.getAuthStatus(); + }); + + // The in-flight tick's status is not the point (it validated a key + // that was live at entry) — the point is that the write is refused, + // so wiped key material is never resurrected with a fresh deadline. + expect(secureDataStorage.setItem).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + expect.any(String), + ); + }); + + it("should not re-anchor when the temporary store is wiped mid-check (TOCTOU)", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + restoreGetAuthStatus(); + + (AppState as { currentState: string }).currentState = "active"; + + mockAuthenticatedStorage({ + backgroundedAt: null, + autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, + }); + const staleKey = { + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() - (HASH_KEY_REFRESH_THROTTLE_MS + 60000), + expiresAt: Date.now() + ONE_HOUR_MS, + }; + // The key survives, but a wipe removes the temporary store between + // getAuthStatus's entry-time read (which saw it present) and the + // helper's write-time re-read. The call-site gate passed on stale + // data, so the helper's own session check must refuse the write — + // otherwise the wipe race leaves an orphan key valid for 72h. + (getHashKey as jest.Mock).mockResolvedValue(staleKey); + let tempStoreReads = 0; + (secureDataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE) { + tempStoreReads += 1; + return Promise.resolve( + tempStoreReads === 1 ? "encrypted-temp-store" : null, + ); + } + if (key === SENSITIVE_STORAGE_KEYS.AUTO_LOCK_TIMER_SETTING) { + return Promise.resolve(AUTO_LOCK_TIMER.ONE_HOUR); + } + return Promise.resolve(null); + }); + + await act(async () => { + await result.current.getAuthStatus(); + }); + + expect(tempStoreReads).toBeGreaterThan(1); + expect(secureDataStorage.setItem).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + expect.any(String), + ); + }); + + it("should not re-anchor when a wipe or credential-verified re-key begins during the check", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + restoreGetAuthStatus(); + + (AppState as { currentState: string }).currentState = "active"; + + mockAuthenticatedStorage({ + backgroundedAt: null, + autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, + }); + // Key and temp store both read back healthy — but the hash-key + // generation moved between getAuthStatus loading the key and the + // re-anchor's write (a clearTemporaryData wipe or a signIn rebuild + // began meanwhile, with its removes/write possibly still in flight). + // The generation check must refuse the write. + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() - (HASH_KEY_REFRESH_THROTTLE_MS + 60000), + expiresAt: Date.now() + ONE_HOUR_MS, + }); + (getHashKeyGeneration as jest.Mock) + .mockReturnValueOnce(0) + .mockReturnValueOnce(1); + + await act(async () => { + await result.current.getAuthStatus(); + }); + + // Both reads consumed: the capture before the load and the helper's + // write-time check. An earlier early-return would leave the queued 1 + // for the next test's capture and pass this one vacuously. + expect(getHashKeyGeneration).toHaveBeenCalledTimes(2); + expect(secureDataStorage.setItem).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + expect.any(String), + ); + }); + + it("should NOT re-anchor an orphan hash key with no temporary store", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + restoreGetAuthStatus(); + + (AppState as { currentState: string }).currentState = "active"; + + mockAuthenticatedStorage({ + backgroundedAt: null, + autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, + }); + // Partial wipe: the hash key survived but the temporary store is gone. + // The `!hashKey && !temporaryStore` guard can't catch this (the key IS + // present), so the re-anchor gate must refuse it — otherwise every + // later tick would push the orphan's deadline out indefinitely. + (secureDataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === SENSITIVE_STORAGE_KEYS.AUTO_LOCK_TIMER_SETTING) { + return Promise.resolve(AUTO_LOCK_TIMER.ONE_HOUR); + } + // No TEMPORARY_STORE, no backgrounded-at, no persisted AUTH_STATUS. + return Promise.resolve(null); + }); + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() - (HASH_KEY_REFRESH_THROTTLE_MS + 60000), + expiresAt: Date.now() + ONE_HOUR_MS, + }); + + await act(async () => { + await result.current.getAuthStatus(); + }); + + expect(secureDataStorage.setItem).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + expect.any(String), + ); + }); + + it("should NOT re-anchor an expired hash key (idle device forces full re-auth)", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + restoreGetAuthStatus(); + + (AppState as { currentState: string }).currentState = "active"; + + mockAuthenticatedStorage({ + backgroundedAt: null, // e.g. cold start where nothing was recorded + autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, + }); + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() - 73 * ONE_HOUR_MS, + expiresAt: Date.now() - ONE_HOUR_MS, // hard-expired + }); + + await act(async () => { + const status = await result.current.getAuthStatus(); + expect(status).toBe(AUTH_STATUS.HASH_KEY_EXPIRED); + }); + + expect(secureDataStorage.setItem).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + expect.any(String), + ); + }); + + it("should NOT re-anchor a rolled-back-clock hash key (generatedAt guard, #905)", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + restoreGetAuthStatus(); + + (AppState as { currentState: string }).currentState = "active"; + + mockAuthenticatedStorage({ + backgroundedAt: null, + autoLockTimer: AUTO_LOCK_TIMER.ONE_HOUR, + }); + // Clock rolled back below the key's anchor: generatedAt is in the + // future, expiresAt still looks fine by wall clock. + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: Date.now() + 60000, + expiresAt: Date.now() + ONE_HOUR_MS, + }); + + await act(async () => { + const status = await result.current.getAuthStatus(); + expect(status).toBe(AUTH_STATUS.HASH_KEY_EXPIRED); + }); + + expect(secureDataStorage.setItem).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + expect.any(String), + ); }); it("should return HASH_KEY_EXPIRED when the hash key expired even if within the timer", async () => { const { result } = renderHook(() => useAuthenticationStore()); restoreGetAuthStatus(); - const previousAppState = AppState.currentState; (AppState as { currentState: string }).currentState = "active"; mockAuthenticatedStorage({ @@ -1974,9 +2587,6 @@ describe("auth duck", () => { // Hard-expiry must evict the derived auth keypair — retention, not just // use, ends at expiry (the eviction-coverage gap raised in review). expect(clearAuthKeypairCache).toHaveBeenCalled(); - - (AppState as { currentState: typeof previousAppState }).currentState = - previousAppState; }); it("should return HASH_KEY_EXPIRED (not LOCKED) when backgrounded beyond BOTH the timer and the hash-key TTL", async () => { @@ -2049,6 +2659,14 @@ describe("auth duck", () => { }); describe("softLock", () => { + afterEach(() => { + // jest.clearAllMocks keeps implementations; drop the sticky + + // keychain-outage rejection installed by the test below. + + (secureDataStorage.setItem as jest.Mock).mockReset(); + }); + it("should set LOCKED with isSoftLocked and persist the status without touching navigation", async () => { const { result } = renderHook(() => useAuthenticationStore()); @@ -2137,6 +2755,80 @@ describe("auth duck", () => { expect(result.current.isSoftLocked).toBe(true); }); + // Companion to the module-level "persisting the LOCKED marker fails" + // test, which starts from NOT_AUTHENTICATED (cold start) and so never + // reaches softLock. On the AUTHENTICATED -> LOCKED transition the store + // funnels through softLock, whose deliberate policy is retry-once-then- + // rethrow. The contract that matters is that the in-memory lock lands + // first, so a keychain outage surfaces the fault without ever leaving + // the wallet unlocked. + it("should still land the in-memory soft lock when the keychain write fails throughout", async () => { + const { result } = renderHook(() => useAuthenticationStore()); + act(() => { + useAuthenticationStore.setState({ + getAuthStatus: originalStoreMethods.getAuthStatus, + softLock: originalStoreMethods.softLock, + authStatus: AUTH_STATUS.AUTHENTICATED, + isSoftLocked: false, + }); + }); + + (dataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === STORAGE_KEYS.ACCOUNT_LIST) { + return Promise.resolve(JSON.stringify([mockAccount])); + } + return Promise.resolve(null); + }); + (secureDataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE) { + return Promise.resolve("encrypted-temp-store"); + } + if (key === SENSITIVE_STORAGE_KEYS.AUTO_LOCK_BACKGROUNDED_AT) { + return Promise.resolve(String(Date.now() - 7200000)); // 2h ago + } + if (key === SENSITIVE_STORAGE_KEYS.AUTO_LOCK_TIMER_SETTING) { + return Promise.resolve(AUTO_LOCK_TIMER.ONE_HOUR); + } + return Promise.resolve(null); + }); + (getHashKey as jest.Mock).mockResolvedValue({ + hashKey: "mock-hash-key", + salt: "mock-salt", + expiresAt: Date.now() + 3600000, + }); + // Sticky on purpose: getAuthStatus's own swallowed LOCKED write, + // then softLock's attempt and retry, must all fail. The describe's + // afterEach resets it so the outage can't leak into later tests. + (secureDataStorage.setItem as jest.Mock).mockRejectedValue( + new Error("Failed to store item in keychain"), + ); + + const observedInvalidStates: string[] = []; + const unsubscribe = useAuthenticationStore.subscribe((state) => { + if (state.authStatus === AUTH_STATUS.LOCKED && !state.isSoftLocked) { + observedInvalidStates.push(state.authStatus); + } + }); + + await act(async () => { + // softLock rethrows after its retry, by design — the module-level + // swallow moves the failure here rather than hiding it. + await expect(result.current.getAuthStatus()).rejects.toThrow( + "Failed to store item in keychain", + ); + }); + + unsubscribe(); + expect(observedInvalidStates).toHaveLength(0); + expect(result.current.authStatus).toBe(AUTH_STATUS.LOCKED); + expect(result.current.isSoftLocked).toBe(true); + // The backgrounded-at timestamp survives, so a cold start after the + // outage re-derives the same lock instead of resolving AUTHENTICATED. + expect(secureDataStorage.remove).not.toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.AUTO_LOCK_BACKGROUNDED_AT, + ); + }); + it("should make navigateToLockScreen a no-op while soft-locked", () => { const { result } = renderHook(() => useAuthenticationStore()); @@ -2258,7 +2950,16 @@ describe("auth duck", () => { SENSITIVE_STORAGE_KEYS.AUTH_STATUS, ); - // Verify TTL was refreshed on the existing hash key (no re-encryption) + // Verify TTL was refreshed on the existing hash key (no re-encryption), + // through the generation-bumping writer so an in-flight re-anchor yields. + expect(setHashKey).toHaveBeenCalledWith( + expect.objectContaining({ + hashKey: "old-mock-hash-key", + salt: "old-mock-salt", + expiresAt: expect.any(Number), + generatedAt: expect.any(Number), + }), + ); expect(secureDataStorage.setItem).toHaveBeenCalledWith( SENSITIVE_STORAGE_KEYS.HASH_KEY, expect.stringContaining('"expiresAt":'), @@ -2267,6 +2968,136 @@ describe("auth duck", () => { expect(decryptDataWithPassword).not.toHaveBeenCalled(); expect(encryptDataWithPassword).not.toHaveBeenCalled(); }); + + it.each([ + [ + "expired", + { + hashKey: "stale-mock-hash-key", + salt: "stale-mock-salt", + generatedAt: Date.now() - 73 * 3600000, + expiresAt: Date.now() - 3600000, // hard-expired an hour ago + }, + ], + [ + "rolled-back (future generatedAt)", + { + hashKey: "stale-mock-hash-key", + salt: "stale-mock-salt", + generatedAt: Date.now() + 3600000, // clock moved backward + expiresAt: Date.now() + 3600000, + }, + ], + ])( + "should rebuild the session instead of re-stamping a %s key on LOCKED sign-in (#924)", + async (_label, staleKey) => { + // Expiry can cross between the getAuthStatus check and the unlock + // (or a stale persisted LOCKED can carry an expired key straight + // into signIn): the fast path must refuse to re-stamp the stale + // material and fall through to the full temporary-store rebuild, + // which mints a fresh hash key. + mockKeyManager.loadKey.mockResolvedValueOnce({ + id: mockAccount.id, + publicKey: mockAccount.publicKey, + privateKey: "mock-private-key", + extra: { mnemonicPhrase: "mock mnemonic phrase" }, + }); + mockKeyManager.loadAllKeyIds.mockResolvedValueOnce([mockAccount.id]); + + (dataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === STORAGE_KEYS.ACTIVE_ACCOUNT_ID) { + return Promise.resolve(mockAccount.id); + } + if (key === STORAGE_KEYS.ACCOUNT_LIST) { + return Promise.resolve(JSON.stringify([mockAccount])); + } + return Promise.resolve(null); + }); + + // Stateful hash-key storage: reads reflect writes, so once the + // rebuild mints a fresh key the background getActiveAccount sees + // it (a static stale mock would knock the session back to LOCKED + // regardless of which branch ran). + let storedHashKey: Record = staleKey; + (getHashKey as jest.Mock).mockImplementation(() => + Promise.resolve(storedHashKey), + ); + (secureDataStorage.setItem as jest.Mock).mockImplementation( + (key, value) => { + if (key === SENSITIVE_STORAGE_KEYS.HASH_KEY) { + storedHashKey = JSON.parse(value as string) as Record< + string, + unknown + >; + } + return Promise.resolve(undefined); + }, + ); + + (secureDataStorage.getItem as jest.Mock).mockImplementation((key) => { + if (key === SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE) { + return Promise.resolve("encrypted-temp-store"); + } + if (key === SENSITIVE_STORAGE_KEYS.HASH_KEY) { + return Promise.resolve(JSON.stringify(storedHashKey)); + } + return Promise.resolve(null); + }); + + (decryptDataWithDerivedKey as jest.Mock).mockReturnValue( + JSON.stringify({ + privateKeys: { [mockAccount.id]: "mock-private-key" }, + mnemonicPhrase: "mock mnemonic phrase", + }), + ); + (deriveKeyFromPassword as jest.Mock).mockResolvedValue( + new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]), + ); + + const { result } = renderHook(() => useAuthenticationStore()); + + act(() => { + useAuthenticationStore.setState({ + authStatus: AUTH_STATUS.LOCKED, + signIn: originalStoreMethods.signIn, + }); + }); + + await act(async () => { + await result.current.signIn({ password: "test-password" }); + }); + + await act(async () => { + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + }); + + expect(result.current.authStatus).toBe(AUTH_STATUS.AUTHENTICATED); + + // The full rebuild ran: a fresh temporary store was written (the + // TTL-refresh branch never touches TEMPORARY_STORE)... + expect(secureDataStorage.setItem).toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE, + expect.any(String), + ); + // ...and no write re-stamped the stale key material. + const hashKeyWrites = ( + secureDataStorage.setItem as jest.Mock + ).mock.calls.filter( + ([key]) => key === SENSITIVE_STORAGE_KEYS.HASH_KEY, + ); + expect(hashKeyWrites.length).toBeGreaterThan(0); + hashKeyWrites.forEach(([, value]) => { + expect(value).not.toContain("stale-mock-hash-key"); + }); + // The fresh key went through the generation-bumping writer, so a + // re-anchor in flight during the rebuild yields to it. + expect(setHashKey).toHaveBeenCalledWith( + expect.not.objectContaining({ hashKey: "stale-mock-hash-key" }), + ); + }, + ); }); describe("account switching", () => { @@ -2682,6 +3513,13 @@ describe("auth duck", () => { expect(await isSessionAuthValid()).toBe(false); }); + // getHashKey call counts are used here purely as a proxy for "how many + // funnel runs happened". That 1:1 mapping only holds because the jest + // AppState mock's default state is not "active": in production the active + // path also runs the #924 re-anchor, which re-reads the key (a second + // getHashKey per funnel run). Don't tighten these assertions against a + // production-like AppState — the counts would drift for reasons that have + // nothing to do with the memo/in-flight dedup under test. it("dedupes a concurrent burst into a single funnel run (in-flight promise shared)", async () => { mockFunnelStorage(); (getHashKey as jest.Mock).mockClear(); diff --git a/__tests__/services/autoLock.test.ts b/__tests__/services/autoLock.test.ts index ba0e38b28..8a7440e27 100644 --- a/__tests__/services/autoLock.test.ts +++ b/__tests__/services/autoLock.test.ts @@ -1,15 +1,22 @@ import { AUTO_LOCK_TIMER, DEFAULT_AUTO_LOCK_TIMER, + HASH_KEY_EXPIRATION_MS, + HASH_KEY_REFRESH_THROTTLE_MS, SENSITIVE_STORAGE_KEYS, } from "config/constants"; +import { HashKey } from "config/types"; import { clearBackgroundedAt, getAutoLockTimer, getBackgroundedAt, + isHashKeyExpired, persistAutoLockTimer, recordBackgroundedAt, + refreshHashKeyExpiration, + resetHashKeyRefreshAttemptThrottle, } from "services/autoLock"; +import { getHashKey, getHashKeyGeneration } from "services/storage/helpers"; import { secureDataStorage } from "services/storage/storageFactory"; jest.mock("services/storage/storageFactory", () => ({ @@ -27,11 +34,16 @@ jest.mock("services/storage/storageFactory", () => ({ jest.mock("services/storage/helpers", () => ({ getHashKey: jest.fn(), + getHashKeyGeneration: jest.fn(() => 0), })); describe("autoLock service", () => { beforeEach(() => { jest.clearAllMocks(); + // The attempt throttle is module-level state that would otherwise leak + // across tests: the first write-expecting case would consume the window + // and every later one would skip its write. + resetHashKeyRefreshAttemptThrottle(); }); describe("getAutoLockTimer", () => { @@ -145,4 +157,276 @@ describe("autoLock service", () => { ); }); }); + + describe("isHashKeyExpired", () => { + const baseKey: HashKey = { + hashKey: "mock-hash-key", + salt: "mock-salt", + expiresAt: Date.now() + 3_600_000, + generatedAt: Date.now(), + }; + + it("returns false for a valid, unexpired key", () => { + expect(isHashKeyExpired(baseKey)).toBe(false); + }); + + it("returns true when expiresAt has passed", () => { + expect( + isHashKeyExpired({ ...baseKey, expiresAt: Date.now() - 1000 }), + ).toBe(true); + }); + + it("returns true for a future generatedAt (clock rollback)", () => { + // expiresAt is still ahead of the rolled-back clock — only the + // generatedAt guard catches this. + expect( + isHashKeyExpired({ ...baseKey, generatedAt: Date.now() + 60_000 }), + ).toBe(true); + }); + + it("falls back to the plain expiry check for legacy keys without generatedAt", () => { + const { generatedAt, ...legacyKey } = baseKey; + expect(isHashKeyExpired(legacyKey)).toBe(false); + expect( + isHashKeyExpired({ ...legacyKey, expiresAt: Date.now() - 1000 }), + ).toBe(true); + }); + }); + + describe("refreshHashKeyExpiration", () => { + const staleValidKey: HashKey = { + hashKey: "mock-hash-key", + salt: "mock-salt", + // Stale: last anchored beyond the throttle window, but not expired. + generatedAt: Date.now() - (HASH_KEY_REFRESH_THROTTLE_MS + 60_000), + expiresAt: Date.now() + 3_600_000, + }; + + // What getAuthStatus captured before loading the key; the mocked + // getHashKeyGeneration also returns 0, so storage reads as unmoved. + const GENERATION_AT_LOAD = 0; + + beforeEach(() => { + // The write-time re-read also requires a session to still exist (a + // present temporary store); arm it for the write-expecting tests — + // orphan-case tests override with null. + (secureDataStorage.getItem as jest.Mock).mockResolvedValue( + "encrypted-temp-store", + ); + }); + + it("never re-anchors when the temporary store is gone at write time (orphan key)", async () => { + // A wipe that removed the temp store but not (yet) the hash key must + // not have the key's deadline pushed out — it should hard-expire. + (getHashKey as jest.Mock).mockResolvedValue(staleValidKey); + (secureDataStorage.getItem as jest.Mock).mockResolvedValue(null); + + await refreshHashKeyExpiration(staleValidKey, GENERATION_AT_LOAD); + + expect(secureDataStorage.getItem).toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE, + ); + expect(secureDataStorage.setItem).not.toHaveBeenCalled(); + }); + + it("re-stamps expiresAt and generatedAt for a valid, stale key", async () => { + // The TOCTOU guard re-reads at write time: storage still holds the + // exact key the caller validated. + (getHashKey as jest.Mock).mockResolvedValue(staleValidKey); + + const before = Date.now(); + await refreshHashKeyExpiration(staleValidKey, GENERATION_AT_LOAD); + + expect(secureDataStorage.setItem).toHaveBeenCalledTimes(1); + const [key, value] = (secureDataStorage.setItem as jest.Mock).mock + .calls[0] as [string, string]; + expect(key).toBe(SENSITIVE_STORAGE_KEYS.HASH_KEY); + + const written = JSON.parse(value) as HashKey; + // Key material is untouched — only the timestamps move. + expect(written.hashKey).toBe(staleValidKey.hashKey); + expect(written.salt).toBe(staleValidKey.salt); + expect(written.generatedAt).toBeGreaterThanOrEqual(before); + expect(written.expiresAt).toBe( + (written.generatedAt as number) + HASH_KEY_EXPIRATION_MS, + ); + }); + + it("skips the write while generatedAt is within the throttle window", async () => { + await refreshHashKeyExpiration( + { + ...staleValidKey, + generatedAt: Date.now() - 60_000, // freshly anchored + }, + GENERATION_AT_LOAD, + ); + + expect(secureDataStorage.setItem).not.toHaveBeenCalled(); + }); + + it("never resurrects an expired key", async () => { + await refreshHashKeyExpiration( + { + ...staleValidKey, + expiresAt: Date.now() - 1000, + }, + GENERATION_AT_LOAD, + ); + + expect(secureDataStorage.setItem).not.toHaveBeenCalled(); + }); + + it("never refreshes a rolled-back-clock key (future generatedAt)", async () => { + await refreshHashKeyExpiration( + { + ...staleValidKey, + generatedAt: Date.now() + 60_000, + expiresAt: Date.now() + 3_600_000, + }, + GENERATION_AT_LOAD, + ); + + expect(secureDataStorage.setItem).not.toHaveBeenCalled(); + }); + + it("refuses to write when the stored key was wiped mid-check (TOCTOU)", async () => { + // A concurrent logout / corruption wipe (clearTemporaryData) removed the + // key between the caller's read and this write. Re-stamping the stale + // snapshot here would resurrect wiped key material with a fresh 72h + // deadline — and, since the key would then exist without a temporary + // store, the `!hashKey && !temporaryStore` guard would never fire. + (getHashKey as jest.Mock).mockResolvedValue(null); + + await refreshHashKeyExpiration(staleValidKey, GENERATION_AT_LOAD); + + expect(secureDataStorage.setItem).not.toHaveBeenCalled(); + }); + + it("refuses to write when the stored key changed mid-check (TOCTOU)", async () => { + // e.g. a concurrent signIn already re-stamped the key with a + // credential-verified anchor; our stale snapshot must not clobber it. + (getHashKey as jest.Mock).mockResolvedValue({ + ...staleValidKey, + generatedAt: Date.now(), + expiresAt: Date.now() + HASH_KEY_EXPIRATION_MS, + }); + + await refreshHashKeyExpiration(staleValidKey, GENERATION_AT_LOAD); + + expect(secureDataStorage.setItem).not.toHaveBeenCalled(); + }); + + it("writes exactly once when the stored key is identical to the snapshot", async () => { + (getHashKey as jest.Mock).mockResolvedValue({ ...staleValidKey }); + + await refreshHashKeyExpiration(staleValidKey, GENERATION_AT_LOAD); + + expect(secureDataStorage.setItem).toHaveBeenCalledTimes(1); + expect(secureDataStorage.setItem).toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + expect.any(String), + ); + }); + + it("throttles repeat ATTEMPTS, not just successful anchors", async () => { + // The generatedAt throttle only advances when the write succeeds, so a + // persistently failing keychain would be retried (and logged) on every + // 5s auth tick. The attempt throttle bounds that to one try per window. + (getHashKey as jest.Mock).mockResolvedValue(staleValidKey); + + await refreshHashKeyExpiration(staleValidKey, GENERATION_AT_LOAD); + expect(secureDataStorage.setItem).toHaveBeenCalledTimes(1); + + // Same stale snapshot (as if the write had failed and generatedAt never + // moved): the attempt throttle suppresses the retry. + await refreshHashKeyExpiration(staleValidKey, GENERATION_AT_LOAD); + expect(secureDataStorage.setItem).toHaveBeenCalledTimes(1); + + // Once the attempt window elapses, the retry is allowed again. + resetHashKeyRefreshAttemptThrottle(); + await refreshHashKeyExpiration(staleValidKey, GENERATION_AT_LOAD); + expect(secureDataStorage.setItem).toHaveBeenCalledTimes(2); + }); + + it("recovers the attempt throttle after a backward clock change", async () => { + // A refresh attempt stamps lastRefreshAttemptAt with the pre-rollback + // wall time. If the clock then rolls back, that marker sits in the + // future and `now - marker < throttle` holds until wall time catches + // up — for a >71h rollback, long enough to hard-expire even the fresh + // key a forced re-auth just minted, despite continued use. A future + // marker must be treated as invalid, like the module's other + // future-timestamp guards. + const T0 = 1_800_000_000_000; + const dateSpy = jest.spyOn(Date, "now").mockReturnValue(T0); + + try { + const preRollbackKey: HashKey = { + hashKey: "mock-hash-key", + salt: "mock-salt", + generatedAt: T0 - (HASH_KEY_REFRESH_THROTTLE_MS + 60_000), + expiresAt: T0 + 3_600_000, + }; + (getHashKey as jest.Mock).mockResolvedValue(preRollbackKey); + await refreshHashKeyExpiration(preRollbackKey, GENERATION_AT_LOAD); + // Attempt marker is now stamped at T0. + expect(secureDataStorage.setItem).toHaveBeenCalledTimes(1); + + // Clock rolls back 72h. The old key hard-expires via the future- + // generatedAt guard and a full re-auth mints a new key at the + // rolled-back time; model it an hour+ later, when it is stale + // enough that a re-anchor SHOULD fire. + const T1 = T0 - 72 * 60 * 60 * 1000; + dateSpy.mockReturnValue(T1); + const reAuthedKey: HashKey = { + hashKey: "mock-hash-key-2", + salt: "mock-salt-2", + generatedAt: T1 - (HASH_KEY_REFRESH_THROTTLE_MS + 60_000), + expiresAt: T1 + 3_600_000, + }; + (getHashKey as jest.Mock).mockResolvedValue(reAuthedKey); + + await refreshHashKeyExpiration(reAuthedKey, GENERATION_AT_LOAD); + expect(secureDataStorage.setItem).toHaveBeenCalledTimes(2); + } finally { + dateSpy.mockRestore(); + } + }); + + it("refuses the write when the key was mutated after the caller loaded it", async () => { + // Storage still reads back the exact validated key and a live temp + // store — but the hash-key generation moved since the caller captured + // it before its load. That covers a clearTemporaryData wipe or a + // credential-verified setHashKey that began anywhere between the + // caller's load and this write, including before this helper was even + // entered (its removes/write may still be in flight behind the stale + // re-read). Compared synchronously before the write, so the mutation's + // result is what ends up on disk. + (getHashKey as jest.Mock).mockResolvedValue(staleValidKey); + // Once: `beforeEach` only clears calls, so a sticky value would leak + // the refusal into the following tests. + (getHashKeyGeneration as jest.Mock).mockReturnValueOnce( + GENERATION_AT_LOAD + 1, + ); + + await refreshHashKeyExpiration(staleValidKey, GENERATION_AT_LOAD); + + expect(secureDataStorage.setItem).not.toHaveBeenCalled(); + }); + + it("re-stamps a legacy key without generatedAt (and upgrades it)", async () => { + const { generatedAt, ...legacyKey } = staleValidKey; + (getHashKey as jest.Mock).mockResolvedValue(legacyKey); + + await refreshHashKeyExpiration(legacyKey, GENERATION_AT_LOAD); + + expect(secureDataStorage.setItem).toHaveBeenCalledTimes(1); + const [, value] = (secureDataStorage.setItem as jest.Mock).mock + .calls[0] as [string, string]; + const written = JSON.parse(value) as HashKey; + expect(written.generatedAt).toBeDefined(); + expect(written.expiresAt).toBe( + (written.generatedAt as number) + HASH_KEY_EXPIRATION_MS, + ); + }); + }); }); diff --git a/__tests__/services/storage/helpers.test.ts b/__tests__/services/storage/helpers.test.ts new file mode 100644 index 000000000..c859d4138 --- /dev/null +++ b/__tests__/services/storage/helpers.test.ts @@ -0,0 +1,90 @@ +import { SENSITIVE_STORAGE_KEYS } from "config/constants"; +import { HashKey } from "config/types"; +import { + clearTemporaryData, + getHashKeyGeneration, + setHashKey, +} from "services/storage/helpers"; +import { secureDataStorage } from "services/storage/storageFactory"; + +jest.mock("services/storage/storageFactory", () => ({ + dataStorage: { + getItem: jest.fn(), + setItem: jest.fn().mockResolvedValue(undefined), + remove: jest.fn().mockResolvedValue(undefined), + }, + secureDataStorage: { + getItem: jest.fn(), + setItem: jest.fn().mockResolvedValue(undefined), + remove: jest.fn().mockResolvedValue(undefined), + }, +})); + +describe("storage helpers", () => { + beforeEach(() => { + jest.clearAllMocks(); + // clearAllMocks keeps implementations: the held-open promises the tests + // below install would otherwise hang every later setItem/remove. + (secureDataStorage.setItem as jest.Mock).mockResolvedValue(undefined); + (secureDataStorage.remove as jest.Mock).mockResolvedValue(undefined); + }); + + describe("hash-key generation", () => { + it("bumps synchronously when a wipe begins, before any remove lands", async () => { + const before = getHashKeyGeneration(); + + // Hold the removes open so the wipe is genuinely in flight. + const releaseRemoves: Array<() => void> = []; + (secureDataStorage.remove as jest.Mock).mockImplementation( + () => + new Promise((resolve) => { + releaseRemoves.push(resolve); + }), + ); + + const wipe = clearTemporaryData(); + + // An in-flight opportunistic writer (the hash-key re-anchor) checking + // the generation now — mid-wipe — must already see it moved. + expect(getHashKeyGeneration()).toBe(before + 1); + + releaseRemoves.forEach((release) => release()); + await wipe; + expect(getHashKeyGeneration()).toBe(before + 1); + }); + + it("bumps synchronously when a credential-verified key write begins, before it lands", async () => { + // A signIn rebuild mints a new key while a re-anchor's re-read is in + // flight. The re-anchor's write-time check must already see the + // generation moved, or it would write the OLD key back under a + // temporary store the rebuild re-encrypted with the new one. + const before = getHashKeyGeneration(); + + let releaseWrite: () => void = () => {}; + (secureDataStorage.setItem as jest.Mock).mockImplementation( + () => + new Promise((resolve) => { + releaseWrite = resolve; + }), + ); + + const newKey: HashKey = { + hashKey: "new-hash-key", + salt: "new-salt", + generatedAt: 1_800_000_000_000, + expiresAt: 1_800_000_000_000 + 1, + }; + const write = setHashKey(newKey); + + expect(getHashKeyGeneration()).toBe(before + 1); + expect(secureDataStorage.setItem).toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + JSON.stringify(newKey), + ); + + releaseWrite(); + await write; + expect(getHashKeyGeneration()).toBe(before + 1); + }); + }); +}); diff --git a/src/config/constants.ts b/src/config/constants.ts index 312109869..f49bf6a79 100644 --- a/src/config/constants.ts +++ b/src/config/constants.ts @@ -91,17 +91,29 @@ export const ACCOUNTS_TO_VERIFY_ON_EXISTING_MNEMONIC_PHRASE = 6; // rather than taking the fast soft-lock unlock path. It is a separate, coarser // bound than the user-configurable soft auto-lock: the soft timer governs how // soon the wallet re-locks (fast unlock), while this caps how long key material -// may live in secure storage regardless of that choice. +// may live in secure storage without the wallet being used. // -// Set above the largest AUTO_LOCK_TIMER preset (24h) so that preset's soft-lock -// fast path stays reachable — if this were <= 24h, the hard expiry (anchored at -// sign-in and checked before the soft timer) would fire first and the 24h -// preset could never fast-unlock. 72h (3x the max preset) gives enough headroom -// that even a loosely-active 24h-preset user — one whose gaps stay just under -// 24h so the soft-lock never fires and never resets this clock — is unlikely to -// hit a surprise full re-auth. (The complete fix is to re-anchor this on -// activity rather than sign-in; tracked as a follow-up.) +// Anchored on activity, not sign-in (#924): every authenticated foreground +// auth check re-stamps expiresAt (throttled via HASH_KEY_REFRESH_THROTTLE_MS), +// so the bound is "72h with no foreground use" — an actively-used wallet never +// hard-expires, matching the Freighter extension's session model. It must stay +// strictly above the largest AUTO_LOCK_TIMER preset (24h): the hard expiry is +// checked before the soft timer, so if this were <= 24h a 24h-preset user +// would hit the full re-auth instead of that preset's fast unlock. A device +// left foregrounded and untouched keeps re-anchoring until the foreground-idle +// soft lock trips (up to 24h on the max preset), so the worst case from last +// human interaction to hard expiry is ~96h. export const HASH_KEY_EXPIRATION_MS = 72 * 60 * 60 * 1000; // 72 hours + +// Minimum age of a hash key's generatedAt anchor before an authenticated +// foreground auth check re-stamps it. getAuthStatus runs as often as every 5s +// while the app is active — this gates the secure-storage (keychain) write to +// at most one per hour rather than one per tick. Granularity is negligible +// against the 72h backstop: a skipped write leaves generatedAt at most 1h +// behind the last activity, so the real idle-to-expiry window is 71-72h +// rather than exactly 72h. +export const HASH_KEY_REFRESH_THROTTLE_MS = 60 * 60 * 1000; // 1 hour + export const VISUAL_DELAY_MS = 500; const SECOND_IN_MS = 1000; diff --git a/src/ducks/auth.ts b/src/ducks/auth.ts index f31b5b61a..46ed42b20 100644 --- a/src/ducks/auth.ts +++ b/src/ducks/auth.ts @@ -64,13 +64,17 @@ import { clearBackgroundedAt, getAutoLockTimer, getBackgroundedAt, + isHashKeyExpired, persistAutoLockTimer, + refreshHashKeyExpiration, } from "services/autoLock"; import { getAccount } from "services/stellar"; import { clearNonSensitiveData, clearTemporaryData, getHashKey, + getHashKeyGeneration, + setHashKey, } from "services/storage/helpers"; import { biometricDataStorage, @@ -447,25 +451,6 @@ const loadPersistedNetwork = async ( */ const keyManager = createKeyManager(Networks.PUBLIC); -/** - * Checks if a hash key is expired. - * - * Clock-rollback backstop: a key whose generatedAt is in the future means the - * device clock was moved backward below the key's creation time — a rolled-back - * clock would otherwise keep `now <= expiresAt` true indefinitely and prevent - * the hard-expiry from ever forcing a full re-auth. Treat that as expired - * (mirrors getBackgroundedAt's future-timestamp guard for the soft timer). - * generatedAt is optional so keys persisted before this field fall back to the - * plain expiry check. - */ -const isHashKeyExpired = (hashKey: HashKey): boolean => { - const now = Date.now(); - if (hashKey.generatedAt !== undefined && hashKey.generatedAt > now) { - return true; - } - return now > hashKey.expiresAt; -}; - /** * Gets all accounts from the account list * @@ -480,6 +465,36 @@ const getAllAccounts = async (): Promise => { return JSON.parse(accountListRaw) as Account[]; }; +/** + * Clears a stale persisted soft-lock marker, best-effort. + * + * Only for `getAuthStatus`'s expiry paths, which have already decided the + * session is over and are just tidying up after themselves. That cleanup must + * not be able to change the decision: an error escaping to `getAuthStatus`'s + * outer catch downgrades a known HASH_KEY_EXPIRED session to + * NOT_AUTHENTICATED, which routes an account-bearing wallet to the onboarding + * auth stack instead of the lock screen that collects the required password + * re-auth. The marker is an optimisation; the returned status is the security + * decision. + * + * `secureDataStorage.remove` swallows its own keychain errors today, so this is + * defense in depth rather than a live fix. Deliberately NOT used for signIn's + * marker clear: there the removal is part of establishing the new session, not + * cleanup after a decision, so a failure genuinely means the persisted state + * disagrees with the session and should surface. + */ +const clearPersistedAuthStatus = async (): Promise => { + try { + await secureDataStorage.remove(SENSITIVE_STORAGE_KEYS.AUTH_STATUS); + } catch (error) { + logger.error( + "clearPersistedAuthStatus", + "Failed to clear stale persisted auth status", + error, + ); + } +}; + /** * Validates the authentication status of the user * @@ -493,6 +508,9 @@ const getAuthStatus = async (): Promise => { // Check if any accounts exist at all const hasAccount = (await getAllAccounts()).length > 0; + // Captured before the load so the re-anchor below can tell whether a + // wipe or credential-verified re-key began at any point after it. + const hashKeyGenerationAtLoad = getHashKeyGeneration(); const [hashKey, temporaryStore] = await Promise.all([ getHashKey(), secureDataStorage.getItem(SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE), @@ -510,26 +528,36 @@ const getAuthStatus = async (): Promise => { } // Read from SECURE storage (encrypted) to prevent tampering. - // LOCKED check comes before isHashKeyExpired: if the user locked the app, - // the hash key may have expired while it was sitting on the lock screen. - // We still want LOCKED (not HASH_KEY_EXPIRED) so the signIn fast path runs - // (TTL refresh + derived key cache) rather than forcing a full re-derivation. + // Hard expiry outranks a persisted soft lock (#924). Under sign-in + // anchoring (#905) the key routinely expired while sitting on the lock + // screen, so LOCKED deliberately won to keep the fast unlock reachable; + // with the TTL re-anchored on activity, an expired key here means a full + // backstop window (72h) of no use — exactly the genuine-idle case that + // must force the full password re-auth instead of the fast path. The + // marker is deliberately left in place: expiry is judged by the device + // clock, and the marker is the only clock-independent lock state, so + // clearing it would let a clock change (forward past expiresAt, or back + // before generatedAt) followed by a restore resolve AUTHENTICATED with no + // password. The store's signIn wrapper clears it once the password is + // verified (the duck-level signIn below does not touch it). const persistedAuthStatus = await secureDataStorage.getItem( SENSITIVE_STORAGE_KEYS.AUTH_STATUS, ); if (persistedAuthStatus === AUTH_STATUS.LOCKED) { + if (hashKey && isHashKeyExpired(hashKey)) { + return AUTH_STATUS.HASH_KEY_EXPIRED; + } if (temporaryStore) { return AUTH_STATUS.LOCKED; } // Temp store missing: LOCKED state is invalid, treat as expired - await secureDataStorage.remove(SENSITIVE_STORAGE_KEYS.AUTH_STATUS); + await clearPersistedAuthStatus(); return AUTH_STATUS.HASH_KEY_EXPIRED; } // Hard expiry wins over the soft timer lock: checked before the timer so a // session backgrounded past the hash-key TTL forces a full re-auth instead - // of a fast-path LOCKED that would just refresh the expired key. (The - // persisted-LOCKED branch above keeps the fast path — there it's intended.) + // of a fast-path LOCKED that would just refresh the expired key. if (hashKey && isHashKeyExpired(hashKey)) { return AUTH_STATUS.HASH_KEY_EXPIRED; } @@ -551,26 +579,81 @@ const getAuthStatus = async (): Promise => { // unlock path (all presets are positive durations, so no zero/null case // to exclude). if (elapsedInBackground >= autoLockTimerMs && temporaryStore) { - await secureDataStorage.setItem( - SENSITIVE_STORAGE_KEYS.AUTH_STATUS, - AUTH_STATUS.LOCKED, - ); - await clearBackgroundedAt(); + // Persisting the marker is best-effort here: unlike `remove`, + // `secureDataStorage.setItem` does throw on a keychain failure, and + // letting that reach the outer catch would answer NOT_AUTHENTICATED + // for a session we have just decided is LOCKED - sending an + // account-bearing wallet to the onboarding stack instead of the lock + // screen. That matters most on a cold start, where no in-memory lock + // state exists yet to fall back on. On failure the backgrounded-at + // timestamp is deliberately left intact (the clear is inside the try), + // so the next check re-derives the same lock from elapsed time and the + // wallet is never auto-unlockable. + // + // The store's softLock() takes the opposite line on the same write - + // retry once, then rethrow - and both are right: softLock also serves + // foreground-idle locks that have no backgrounded-at timestamp to + // re-derive from, so there a lost write really can leave the wallet + // unlocked on the next cold launch. It also sets the in-memory LOCKED + // state before writing, so its throw surfaces the fault without + // unlocking anything. + try { + await secureDataStorage.setItem( + SENSITIVE_STORAGE_KEYS.AUTH_STATUS, + AUTH_STATUS.LOCKED, + ); + await clearBackgroundedAt(); + } catch (error) { + logger.error( + "getAuthStatus", + "Failed to persist soft-lock auth status", + error, + ); + } return AUTH_STATUS.LOCKED; } if (AppState.currentState === "active") { // Returned within the timer: consume the timestamp so the foreground - // interval can't lock mid-use. The hash-key TTL is deliberately NOT - // refreshed here — it's only anchored at credential-verified moments - // (signIn / generateHashKey) so key material stays bounded however - // often the app is reopened. + // interval can't lock mid-use. (The hash-key TTL re-anchor happens + // below, on the shared AUTHENTICATED path — not here — so it also + // covers ticks where no backgrounded-at timestamp exists.) await clearBackgroundedAt(); } // Still backgrounded (periodic background check): leave the timestamp // intact so the timer keeps counting from the original moment. } + // Re-anchor the hash-key hard-expiry on use (#924): every authenticated + // foreground auth check pushes the deadline out, so the backstop bounds + // *inactivity* — HASH_KEY_EXPIRATION_MS with no foreground use — instead + // of time since the last credential entry, and an actively-used wallet + // never hard-expires (parity with the extension's session model). Gated + // to the active app state so the periodic background check can't extend + // the deadline of a pocketed device, and throttled inside the helper so + // the 5s foreground tick doesn't hammer the keychain. This runs strictly + // after the LOCKED / hard-expiry / clock-rollback checks above, so an + // expired or rolled-back key can never be resurrected here — those still + // require signIn's credential-verified re-stamp. An orphan key (present + // with no temporary store, e.g. a partial wipe) is never re-anchored + // either, so it still hard-expires and gets cleaned up rather than having + // its deadline pushed out forever by each tick. + if (hashKey && temporaryStore && AppState.currentState === "active") { + try { + await refreshHashKeyExpiration(hashKey, hashKeyGenerationAtLoad); + } catch (error) { + // The re-anchor is opportunistic: a failed keychain write must not + // demote an authenticated session (the outer catch returns + // NOT_AUTHENTICATED). Worst case the key keeps its old deadline and + // hard-expires as it would have before the re-anchor existed. + logger.error( + "getAuthStatus", + "Failed to refresh hash key expiration", + error, + ); + } + } + // All conditions for authentication are met return AUTH_STATUS.AUTHENTICATED; } catch (error) { @@ -962,10 +1045,7 @@ const createTemporaryStore = async (input: { if (shouldRefreshHashKey) { hashKeyObj = await generateHashKey(password); // Persist the new hash key for future decryptions - await secureDataStorage.setItem( - SENSITIVE_STORAGE_KEYS.HASH_KEY, - JSON.stringify(hashKeyObj), - ); + await setHashKey(hashKeyObj); } else { const retrievedHashKey = await getHashKey(); @@ -1517,19 +1597,24 @@ const signIn = async ({ const existingTempStore = await secureDataStorage.getItem( SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE, ); - if (existingHashKey && existingTempStore) { + if ( + existingHashKey && + existingTempStore && + !isHashKeyExpired(existingHashKey) + ) { // Fast path: temp store is intact — just refresh the hard-expiry TTL. // Re-anchor generatedAt too (this is a credential-verified moment) so the - // clock-rollback backstop stays aligned with the new expiry. + // clock-rollback backstop stays aligned with the new expiry. An expired + // or rolled-back key never takes this path (getAuthStatus reports it as + // HASH_KEY_EXPIRED, but expiry can also cross between that check and + // this call): it falls through to the full rebuild below, which mints a + // fresh key instead of re-stamping stale material. const refreshNow = Date.now(); - await secureDataStorage.setItem( - SENSITIVE_STORAGE_KEYS.HASH_KEY, - JSON.stringify({ - ...existingHashKey, - expiresAt: refreshNow + HASH_KEY_EXPIRATION_MS, - generatedAt: refreshNow, - }), - ); + await setHashKey({ + ...existingHashKey, + expiresAt: refreshNow + HASH_KEY_EXPIRATION_MS, + generatedAt: refreshNow, + }); } else { // Hash key or temp store missing — rebuild session. await createTemporaryStore({ @@ -3282,8 +3367,9 @@ export const clearSessionAuthValidMemo = (): void => { * so it evaluates account existence, persisted LOCKED, hash-key hard-expiry AND * the auto-lock timer (`backgroundedAt` + `autoLockTimer`), rather than * re-implementing a subset. Result is memoized for SESSION_AUTH_VALID_TTL_MS - * (see above). Does not decrypt the temporary store (no PBKDF2); persisted/secure - * reads only. + * (see above). Does not decrypt the temporary store (no scrypt); secure-storage + * reads plus at most one throttled hash-key re-anchor write per hour on the + * active path. */ export const isSessionAuthValid = async (): Promise => { const memo = sessionAuthValidMemo; diff --git a/src/services/autoLock.ts b/src/services/autoLock.ts index d68efb298..9665b5251 100644 --- a/src/services/autoLock.ts +++ b/src/services/autoLock.ts @@ -1,9 +1,12 @@ import { AUTO_LOCK_TIMER, DEFAULT_AUTO_LOCK_TIMER, + HASH_KEY_EXPIRATION_MS, + HASH_KEY_REFRESH_THROTTLE_MS, SENSITIVE_STORAGE_KEYS, } from "config/constants"; -import { getHashKey } from "services/storage/helpers"; +import { HashKey } from "config/types"; +import { getHashKey, getHashKeyGeneration } from "services/storage/helpers"; import { secureDataStorage } from "services/storage/storageFactory"; /** @@ -88,6 +91,152 @@ const getBackgroundedAt = async (): Promise => { return parsedBackgroundedAt; }; +/** + * Checks if a hash key is expired. + * + * Clock-rollback backstop: a key whose generatedAt is in the future means the + * device clock was moved backward below the key's creation time — a rolled-back + * clock would otherwise keep `now <= expiresAt` true indefinitely and prevent + * the hard-expiry from ever forcing a full re-auth. Treat that as expired + * (mirrors getBackgroundedAt's future-timestamp guard for the soft timer). + * generatedAt is optional so keys persisted before this field fall back to the + * plain expiry check. + */ +const isHashKeyExpired = (hashKey: HashKey): boolean => { + const now = Date.now(); + if (hashKey.generatedAt !== undefined && hashKey.generatedAt > now) { + return true; + } + return now > hashKey.expiresAt; +}; + +// Last time a re-anchor write was attempted (module-level, process-lifetime). +// The generatedAt throttle below only advances when the write SUCCEEDS; if +// the keychain write fails persistently, generatedAt never moves and every +// 5s auth tick would retry (and log) forever. Throttling attempts bounds +// that failure mode to one attempt per throttle window. In-memory on +// purpose: a process restart retrying immediately is fine. +let lastRefreshAttemptAt = 0; + +/** + * Resets the module-level attempt throttle (tests only — module state would + * otherwise leak across cases in the same file). + */ +const resetHashKeyRefreshAttemptThrottle = (): void => { + lastRefreshAttemptAt = 0; +}; + +/** + * Re-anchors the hash-key hard-expiry on use (#924): pushes expiresAt out to + * a full HASH_KEY_EXPIRATION_MS from now, so the backstop bounds *inactivity* + * rather than time since the last credential entry. + * + * Guards (defense in depth — the getAuthStatus call site is also gated): + * - An expired or rolled-back key is never refreshed; only signIn's + * credential-verified path may re-stamp those. + * - The write is throttled: a key anchored within HASH_KEY_REFRESH_THROTTLE_MS + * is left alone, so the 5s foreground auth tick doesn't hammer the keychain. + * A legacy key without generatedAt can't prove it was recently anchored, so + * it refreshes immediately (gaining generatedAt, after which the throttle + * applies). + * - Refresh *attempts* are throttled too (lastRefreshAttemptAt): the + * generatedAt throttle only advances on a successful write, so a + * persistently failing keychain would otherwise be retried (and logged) + * on every 5s auth tick. + * + * Takes the caller-validated HashKey snapshot to run the guards and throttle + * (every caller has just loaded it for the expiry checks) and the hash-key + * generation the caller captured BEFORE that load, then re-reads the stored + * key and the temporary store once at write time for the TOCTOU check below — + * the re-anchor requires the exact validated key, a live session, and no + * wipe or credential-verified write having begun since the caller's load. + */ +const refreshHashKeyExpiration = async ( + hashKey: HashKey, + hashKeyGenerationAtLoad: number, +): Promise => { + if (isHashKeyExpired(hashKey)) { + return; + } + + const now = Date.now(); + if ( + hashKey.generatedAt !== undefined && + now - hashKey.generatedAt < HASH_KEY_REFRESH_THROTTLE_MS + ) { + return; + } + + // A backward clock change strands the attempt marker in the future, and + // the check below would then suppress every re-anchor until wall time + // caught back up — for a >71h rollback, long enough to hard-expire even + // the fresh key the forced re-auth just minted, despite continued use. + // Mirror this module's other future-timestamp guards: a future marker is + // invalid, reset it. + if (lastRefreshAttemptAt > now) { + lastRefreshAttemptAt = 0; + } + if (now - lastRefreshAttemptAt < HASH_KEY_REFRESH_THROTTLE_MS) { + return; + } + // Set before the write so a throwing write still counts as an attempt. + lastRefreshAttemptAt = now; + + // TOCTOU guard: the caller validated this key several awaits ago; a logout + // or corruption wipe (clearTemporaryData) may have removed or replaced it — + // or removed just the temporary store — since. Re-stamp only the exact key + // that was validated, and only while a session still exists on disk: never + // resurrect a wiped key, never clobber a concurrent credential-verified + // re-stamp, and never push out the deadline of an orphan key (no temp + // store), which must hard-expire and get cleaned up instead. Checking the + // store here rather than only at the call site keeps the orphan invariant + // with the helper for any future caller. + const [currentHashKey, temporaryStore] = await Promise.all([ + getHashKey(), + secureDataStorage.getItem(SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE), + ]); + if ( + !currentHashKey || + !temporaryStore || + currentHashKey.hashKey !== hashKey.hashKey || + currentHashKey.salt !== hashKey.salt || + currentHashKey.expiresAt !== hashKey.expiresAt || + currentHashKey.generatedAt !== hashKey.generatedAt + ) { + return; + } + + // The re-read above only proves the key survived up to the point those + // reads resolved. A clearTemporaryData wipe or a credential-verified + // setHashKey that began anywhere after the caller loaded the key — even + // before this helper was entered — can still have its removes/write land + // after the write below, and a stale re-read would not show it. Both bump + // the generation synchronously at entry, so comparing against the value the + // caller captured before its load (no await between here and the write) + // refuses every such interleaving. A mutation that starts after the write + // is dispatched is outside this check and relies on native ordering: the + // iOS keychain bridge is a serial queue, but Android runs set in a mutex'd + // coroutine and reset synchronously, so a remove can land before a write + // dispatched earlier. That residual is accepted and shared with signIn's + // fast-path read-modify-write. + if (getHashKeyGeneration() !== hashKeyGenerationAtLoad) { + return; + } + + await secureDataStorage.setItem( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + JSON.stringify({ + // Spread the re-read key, not the caller's snapshot: the comparison + // above proves they are identical today, but if HashKey ever gains a + // field the explicit check stops covering it and spreading the stale + // snapshot would silently revert it. + ...currentHashKey, + expiresAt: now + HASH_KEY_EXPIRATION_MS, + generatedAt: now, + } satisfies HashKey), + ); +}; + /** * Whether an unlockable session is persisted on device (a hash key and a * temporary store both exist). Lets the background handler decide whether to @@ -110,5 +259,8 @@ export { recordBackgroundedAt, getBackgroundedAt, clearBackgroundedAt, + isHashKeyExpired, + refreshHashKeyExpiration, + resetHashKeyRefreshAttemptThrottle, hasPersistedSession, }; diff --git a/src/services/storage/helpers.ts b/src/services/storage/helpers.ts index ea9f938ce..130875e01 100644 --- a/src/services/storage/helpers.ts +++ b/src/services/storage/helpers.ts @@ -5,6 +5,39 @@ import { secureDataStorage, } from "services/storage/storageFactory"; +// Monotonic counter bumped synchronously at the START of every mutation of +// the stored hash key that the opportunistic re-anchor (services/autoLock) +// must yield to: a clearTemporaryData wipe or a credential-verified write via +// setHashKey. Lets a re-anchor in flight detect that storage moved after its +// caller loaded the key and refuse its write, instead of racing the mutation +// and re-creating just-wiped material or writing the old key back under a +// temporary store already re-encrypted with a new one. JS is single-threaded, +// so a bump is visible to any check that runs after the mutation starts. A +// re-anchor write dispatched before the bump is outside this check and relies +// on native keychain ordering (serial on iOS, not guaranteed on Android). +let hashKeyGeneration = 0; + +/** + * Current hash-key generation — see hashKeyGeneration above. Capture BEFORE + * loading the key for an optimistic read-validate-write sequence and compare + * (synchronously, with no await in between) right before the write. + */ +const getHashKeyGeneration = (): number => hashKeyGeneration; + +/** + * Persists a credential-verified hash key (sign-in, sign-up, import). Every + * such write must come through here so the generation bump above happens + * before the write is dispatched; the re-anchor is the only writer that may + * bypass it, because it is the one checking the generation. + */ +const setHashKey = async (hashKey: HashKey): Promise => { + hashKeyGeneration += 1; + await secureDataStorage.setItem( + SENSITIVE_STORAGE_KEYS.HASH_KEY, + JSON.stringify(hashKey), + ); +}; + /** * Clears the hash key, temporary store, and derived key cache from secure storage. * @@ -15,6 +48,7 @@ import { * the logout-wipe path alongside clearAllWebViewData. */ const clearTemporaryData = async (): Promise => { + hashKeyGeneration += 1; await Promise.all([ secureDataStorage.remove(SENSITIVE_STORAGE_KEYS.HASH_KEY), secureDataStorage.remove(SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE), @@ -53,4 +87,10 @@ const getHashKey = async (): Promise => { return hashKey ? (JSON.parse(hashKey) as HashKey) : null; }; -export { clearTemporaryData, clearNonSensitiveData, getHashKey }; +export { + clearTemporaryData, + clearNonSensitiveData, + getHashKey, + getHashKeyGeneration, + setHashKey, +};