diff --git a/__tests__/ducks/auth.test.ts b/__tests__/ducks/auth.test.ts index 49ef20117..b80f512bd 100644 --- a/__tests__/ducks/auth.test.ts +++ b/__tests__/ducks/auth.test.ts @@ -3,6 +3,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, @@ -39,10 +41,12 @@ 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, + getWipeGeneration, } from "services/storage/helpers"; // Import mocked modules import { rnBiometrics } from "services/storage/secureStorage"; @@ -142,6 +146,7 @@ jest.mock("services/storage/helpers", () => ({ clearNonSensitiveData: jest.fn(), clearTemporaryData: jest.fn(), getHashKey: jest.fn(), + getWipeGeneration: jest.fn(() => 0), })); jest.mock("config/logger", () => ({ @@ -1630,6 +1635,58 @@ 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 stale soft-lock marker is consumed so the next check doesn't + // resurrect the fast path. + expect(secureDataStorage.remove).toHaveBeenCalledWith( + SENSITIVE_STORAGE_KEYS.AUTH_STATUS, + ); + }); + it("should clear invalid LOCKED status if temp store doesn't exist", async () => { const { result } = renderHook(() => useAuthenticationStore()); @@ -1678,6 +1735,24 @@ describe("auth duck", () => { 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, @@ -1750,20 +1825,25 @@ describe("auth duck", () => { ); }); - it("should stay authenticated and consume the timestamp WITHOUT refreshing the hash key TTL", async () => { + 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); @@ -1773,22 +1853,352 @@ describe("auth duck", () => { 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"; - (AppState as { currentState: typeof previousAppState }).currentState = - previousAppState; + 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); + }); + + 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 begins during the write-time re-read", 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 a + // clearTemporaryData wipe started while the re-read was in flight + // (generation moved), so its removes may land after the re-anchor's + // write. 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, + }); + (getWipeGeneration as jest.Mock) + .mockReturnValueOnce(0) + .mockReturnValue(1); + + 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 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({ @@ -1813,9 +2223,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 () => { @@ -2106,6 +2513,131 @@ 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"); + }); + }, + ); }); describe("account switching", () => { @@ -2520,6 +3052,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..cac17f663 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, getWipeGeneration } 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(), + getWipeGeneration: 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,258 @@ 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, + }; + + 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); + + 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); + + 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 + }); + + expect(secureDataStorage.setItem).not.toHaveBeenCalled(); + }); + + it("never resurrects an expired key", async () => { + await refreshHashKeyExpiration({ + ...staleValidKey, + expiresAt: Date.now() - 1000, + }); + + 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, + }); + + 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); + + 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); + + 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); + + 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); + 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); + expect(secureDataStorage.setItem).toHaveBeenCalledTimes(1); + + // Once the attempt window elapses, the retry is allowed again. + resetHashKeyRefreshAttemptThrottle(); + await refreshHashKeyExpiration(staleValidKey); + 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); + // 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); + expect(secureDataStorage.setItem).toHaveBeenCalledTimes(2); + } finally { + dateSpy.mockRestore(); + } + }); + + it("refuses the write when a wipe starts between the re-read and the write", async () => { + // Storage still holds a matching key and a live temp store, but a + // clearTemporaryData wipe BEGAN while the re-read was in flight (its + // removes may land after our write would). The generation check — + // captured before the re-read, compared synchronously before the + // write — must refuse rather than race the wipe. + (getHashKey as jest.Mock).mockResolvedValue(staleValidKey); + (getWipeGeneration as jest.Mock) + .mockReturnValueOnce(0) + .mockReturnValue(1); + + await refreshHashKeyExpiration(staleValidKey); + + 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); + + 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..a63cd43ac --- /dev/null +++ b/__tests__/services/storage/helpers.test.ts @@ -0,0 +1,49 @@ +import { + clearTemporaryData, + getWipeGeneration, +} 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(); + }); + + describe("wipe generation", () => { + it("bumps synchronously when a wipe begins, before any remove lands", async () => { + const before = getWipeGeneration(); + + // 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(getWipeGeneration()).toBe(before + 1); + + releaseRemoves.forEach((release) => release()); + await wipe; + expect(getWipeGeneration()).toBe(before + 1); + }); + }); +}); diff --git a/src/config/constants.ts b/src/config/constants.ts index 572f52373..cd8fbb904 100644 --- a/src/config/constants.ts +++ b/src/config/constants.ts @@ -94,17 +94,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 8834bf524..6abe45386 100644 --- a/src/ducks/auth.ts +++ b/src/ducks/auth.ts @@ -63,7 +63,9 @@ import { clearBackgroundedAt, getAutoLockTimer, getBackgroundedAt, + isHashKeyExpired, persistAutoLockTimer, + refreshHashKeyExpiration, } from "services/autoLock"; import { getAccount } from "services/stellar"; import { @@ -446,25 +448,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 * @@ -509,14 +492,21 @@ 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 + // stale soft-lock marker is consumed so later checks can't resurrect it. const persistedAuthStatus = await secureDataStorage.getItem( SENSITIVE_STORAGE_KEYS.AUTH_STATUS, ); if (persistedAuthStatus === AUTH_STATUS.LOCKED) { + if (hashKey && isHashKeyExpired(hashKey)) { + await secureDataStorage.remove(SENSITIVE_STORAGE_KEYS.AUTH_STATUS); + return AUTH_STATUS.HASH_KEY_EXPIRED; + } if (temporaryStore) { return AUTH_STATUS.LOCKED; } @@ -527,8 +517,7 @@ const getAuthStatus = async (): Promise => { // 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; } @@ -560,16 +549,45 @@ const getAuthStatus = async (): Promise => { 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); + } 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) { @@ -1509,10 +1527,18 @@ 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, @@ -3259,8 +3285,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..a385fa35f 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, getWipeGeneration } from "services/storage/helpers"; import { secureDataStorage } from "services/storage/storageFactory"; /** @@ -88,6 +91,144 @@ 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), 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 and a live session. + */ +const refreshHashKeyExpiration = async (hashKey: HashKey): 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. The wipe-generation check below + // covers the read-to-write gap itself, so within the JS thread no + // interleaving with clearTemporaryData can resurrect wiped material. + const wipeGenerationAtRead = getWipeGeneration(); + 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 starting while they were in + // flight could still have its removes land after the write below. The + // wipe bumps its generation synchronously at entry, so this check (no + // await between it and the write) refuses that interleaving: a wipe that + // instead starts after the write is dispatched has its removes land last + // and wins. Either ordering ends with no resurrected key. + if (getWipeGeneration() !== wipeGenerationAtRead) { + 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 +251,8 @@ export { recordBackgroundedAt, getBackgroundedAt, clearBackgroundedAt, + isHashKeyExpired, + refreshHashKeyExpiration, + resetHashKeyRefreshAttemptThrottle, hasPersistedSession, }; diff --git a/src/services/storage/helpers.ts b/src/services/storage/helpers.ts index 2cd0c1b44..b5368f33b 100644 --- a/src/services/storage/helpers.ts +++ b/src/services/storage/helpers.ts @@ -5,6 +5,22 @@ import { secureDataStorage, } from "services/storage/storageFactory"; +// Monotonic counter bumped at the START of every clearTemporaryData wipe. +// Lets an in-flight opportunistic writer (the hash-key re-anchor in +// services/autoLock) detect that a wipe began after it validated storage and +// refuse its write, instead of racing the wipe's removes and re-creating +// just-wiped key material. JS is single-threaded, so a bump is visible to any +// check that runs after the wipe starts; a write dispatched before the bump +// has its removes land after it, so either ordering ends with the key absent. +let wipeGeneration = 0; + +/** + * Current wipe generation — see wipeGeneration above. Capture before an + * optimistic read-validate-write sequence and compare (synchronously, with no + * await in between) right before the write. + */ +const getWipeGeneration = (): number => wipeGeneration; + /** * Clears the hash key, temporary store, and derived key cache from secure storage. * @@ -15,6 +31,7 @@ import { * the logout-wipe path alongside clearAllWebViewData. */ const clearTemporaryData = async (): Promise => { + wipeGeneration += 1; await Promise.all([ secureDataStorage.remove(SENSITIVE_STORAGE_KEYS.HASH_KEY), secureDataStorage.remove(SENSITIVE_STORAGE_KEYS.TEMPORARY_STORE), @@ -50,4 +67,9 @@ const getHashKey = async (): Promise => { return hashKey ? (JSON.parse(hashKey) as HashKey) : null; }; -export { clearTemporaryData, clearNonSensitiveData, getHashKey }; +export { + clearTemporaryData, + clearNonSensitiveData, + getHashKey, + getWipeGeneration, +};