diff --git a/src/components/ChangePasswordCard.js b/src/components/ChangePasswordCard.js index a121211..2b27309 100644 --- a/src/components/ChangePasswordCard.js +++ b/src/components/ChangePasswordCard.js @@ -5,6 +5,7 @@ import PasswordStrengthMeter from './PasswordStrengthMeter'; import PasswordInput from './PasswordInput'; import { useAuth } from '../context/AuthContext'; import { useVault } from '../context/VaultContext'; +import { zeroizeBytes } from '../lib/crypto/kdf'; import { MIN_VAULT_PASSWORD_LENGTH, validateVaultPassword, @@ -170,17 +171,19 @@ export default function ChangePasswordCard({ } setSubmitting(true); + let newMaster; try { // 1. Derive keys up-front. This runs PBKDF2(600k) twice // (once for the old authHash, once for the new master) // so it's the most expensive part of the flow — do it // before touching any state. - const { oldAuthHash, newAuthHash, newMaster } = + const { oldAuthHash, newAuthHash, newMaster: derivedNewMaster } = await authService.deriveChangePasswordKeys( oldPassword, newPassword, user.email ); + newMaster = derivedNewMaster; // 2. Ask the vault to rewrap. `rewrap` is null iff the user // has no vault row at all — the POST will then run as a @@ -249,6 +252,7 @@ export default function ChangePasswordCard({ setConfirmPassword(''); setSuccess(true); } finally { + zeroizeBytes(newMaster); setSubmitting(false); } }, diff --git a/src/components/ChangePasswordCard.test.js b/src/components/ChangePasswordCard.test.js index 10fdcc6..20cd96c 100644 --- a/src/components/ChangePasswordCard.test.js +++ b/src/components/ChangePasswordCard.test.js @@ -67,14 +67,14 @@ function makeAuthService() { // A minimal authService for the card itself — overrides only what // the card calls, with fast stubs. -function makeCardAuthService({ changePasswordImpl } = {}) { +function makeCardAuthService({ changePasswordImpl, newMaster } = {}) { return { deriveChangePasswordKeys: jest.fn().mockResolvedValue({ oldAuthHash: 'a'.repeat(64), newAuthHash: 'b'.repeat(64), // `master` is only used to call vault.rewrapForPasswordChange, // which our mocked useVault treats as a no-op (returns null). - newMaster: new Uint8Array(32), + newMaster: newMaster || new Uint8Array(32), }), changePassword: changePasswordImpl || jest.fn().mockResolvedValue({}), }; @@ -235,4 +235,22 @@ describe('ChangePasswordCard', () => { 'authenticated' ); }); + + test('zeroes the derived new master after password-change orchestration', async () => { + const authService = makeAuthService(); + const newMaster = new Uint8Array(32).fill(0x42); + const cardAuthService = makeCardAuthService({ newMaster }); + + renderCard({ authService, cardAuthService }); + await waitFor(() => + expect(screen.getByTestId('auth-probe')).toHaveTextContent('authenticated') + ); + + await fillAndSubmit(); + + await waitFor(() => + expect(cardAuthService.changePassword).toHaveBeenCalledTimes(1) + ); + expect(Array.from(newMaster)).toEqual(new Array(32).fill(0)); + }); }); diff --git a/src/components/DeleteAccountCard.js b/src/components/DeleteAccountCard.js index f8d04df..9ded587 100644 --- a/src/components/DeleteAccountCard.js +++ b/src/components/DeleteAccountCard.js @@ -2,7 +2,7 @@ import React, { useCallback, useState } from 'react'; import { useHistory } from 'react-router-dom'; import { authService as defaultAuthService } from '../lib/authService'; -import { deriveLoginKeys } from '../lib/crypto/kdf'; +import { deriveLoginKeys, zeroizeBytes } from '../lib/crypto/kdf'; import { useAuth } from '../context/AuthContext'; import PasswordInput from './PasswordInput'; @@ -104,13 +104,16 @@ export default function DeleteAccountCard({ } setSubmitting(true); + let master; try { // Re-derive authHash the same way /login does — the server // treats this as proof that the caller knows the password. // We do NOT store or surface `master`; account deletion // has no use for the master key (there's no vault to // unlock, and we're about to erase any vault state). - const { authHash } = await deriveLoginKeys(password, user.email); + const keys = await deriveLoginKeys(password, user.email); + const { authHash } = keys; + master = keys.master; try { await authService.deleteAccount({ oldAuthHash: authHash }); @@ -141,6 +144,7 @@ export default function DeleteAccountCard({ handleAuthLost(); history.replace('/'); } finally { + zeroizeBytes(master); setSubmitting(false); } }, diff --git a/src/components/DeleteAccountCard.test.js b/src/components/DeleteAccountCard.test.js index dc198c1..509275b 100644 --- a/src/components/DeleteAccountCard.test.js +++ b/src/components/DeleteAccountCard.test.js @@ -26,6 +26,13 @@ jest.mock('../lib/crypto/kdf', () => ({ deriveMaster: jest.fn(), deriveAuthHash: jest.fn(), deriveVaultKey: jest.fn(), + zeroizeBytes: jest.fn((value) => { + if (value instanceof Uint8Array) { + value.fill(0); + return true; + } + return false; + }), })); // eslint-disable-next-line import/first @@ -33,7 +40,7 @@ import DeleteAccountCard from './DeleteAccountCard'; // eslint-disable-next-line import/first import { AuthProvider } from '../context/AuthContext'; // eslint-disable-next-line import/first -import { deriveLoginKeys } from '../lib/crypto/kdf'; +import { deriveLoginKeys, zeroizeBytes } from '../lib/crypto/kdf'; // --------------------------------------------------------------------------- // Test helpers @@ -118,6 +125,13 @@ describe('DeleteAccountCard', () => { authHash: 'a'.repeat(64), }) ); + zeroizeBytes.mockImplementation((value) => { + if (value instanceof Uint8Array) { + value.fill(0); + return true; + } + return false; + }); }); test('renders the collapsed danger entry by default', async () => { @@ -217,6 +231,36 @@ describe('DeleteAccountCard', () => { expect(body.oldAuthHash).toMatch(/^[0-9a-f]{64}$/); }); + test('zeroes the derived master after account deletion proof', async () => { + const authService = makeAuthService(); + const deletionMaster = new Uint8Array(32).fill(0x42); + deriveLoginKeys.mockResolvedValueOnce({ + master: deletionMaster, + authHash: 'b'.repeat(64), + }); + const cardAuthService = { + deleteAccount: jest.fn().mockResolvedValue(true), + }; + + renderCard({ authService, cardAuthService }); + await waitForUserHydrated(); + await expandForm(); + fireEvent.change(screen.getByTestId('delete-account-email'), { + target: { value: 'alice@example.com' }, + }); + fireEvent.change(screen.getByTestId('delete-account-password'), { + target: { value: 'hunter22a' }, + }); + await act(async () => { + fireEvent.submit(screen.getByTestId('delete-account-card')); + }); + + await waitFor(() => + expect(cardAuthService.deleteAccount).toHaveBeenCalledTimes(1) + ); + expect(Array.from(deletionMaster)).toEqual(new Array(32).fill(0)); + }); + test('refuses to submit when the password field is empty', async () => { const authService = makeAuthService(); const cardAuthService = { deleteAccount: jest.fn() }; diff --git a/src/components/SessionExpiredBanner.test.js b/src/components/SessionExpiredBanner.test.js index 49cd7dd..a33e299 100644 --- a/src/components/SessionExpiredBanner.test.js +++ b/src/components/SessionExpiredBanner.test.js @@ -11,6 +11,7 @@ jest.mock('../lib/crypto/kdf', () => ({ deriveMaster: jest.fn(), deriveAuthHash: jest.fn(), deriveVaultKey: jest.fn(), + zeroizeBytes: jest.fn(), })); /* eslint-disable import/first */ diff --git a/src/context/AuthContext.js b/src/context/AuthContext.js index 9c34881..c880482 100644 --- a/src/context/AuthContext.js +++ b/src/context/AuthContext.js @@ -10,6 +10,7 @@ import React, { import { authService as defaultAuthService } from '../lib/authService'; import { setAuthLostHandler } from '../lib/apiClient'; +import { zeroizeBytes } from '../lib/crypto/kdf'; // AuthContext shape: // @@ -216,6 +217,7 @@ export function AuthProvider({ children, authService = defaultAuthService }) { setUser(null); commitStatus(ANONYMOUS); }, myGen); + zeroizeBytes(master); const wrapped = new Error('session_not_established'); wrapped.code = 'session_not_established'; wrapped.status = 401; diff --git a/src/context/VaultContext.js b/src/context/VaultContext.js index 3c1d30d..6b26e35 100644 --- a/src/context/VaultContext.js +++ b/src/context/VaultContext.js @@ -460,7 +460,11 @@ export function VaultProvider({ throw e; } const master = await deriveMaster(password, email); - return unlockWithMaster(master); + try { + return await unlockWithMaster(master); + } finally { + zeroizeDataKey(master); + } }, [unlockWithMaster] ); @@ -574,26 +578,30 @@ export function VaultProvider({ throw e; } const master = await deriveMaster(opts.password, email); - // Derive the authHash and verify it against the server's - // stored credential BEFORE we derive vaultKey or generate a - // DK. If it doesn't match we have nothing to roll back — - // no PUT has happened, no keys are installed, no state is - // mutated. Re-tag the server's `invalid_credentials` as - // `password_mismatch` so the import modal can render copy - // tailored to the "this isn't your account password" - // case rather than a generic auth error. - const authHash = await deriveAuthHash(master); try { - await opts.verifyAuthHash(authHash); - } catch (err) { - if (err && err.code === 'invalid_credentials') { - const e = new Error('password_mismatch'); - e.code = 'password_mismatch'; - throw e; + // Derive the authHash and verify it against the server's + // stored credential BEFORE we derive vaultKey or generate a + // DK. If it doesn't match we have nothing to roll back — + // no PUT has happened, no keys are installed, no state is + // mutated. Re-tag the server's `invalid_credentials` as + // `password_mismatch` so the import modal can render copy + // tailored to the "this isn't your account password" + // case rather than a generic auth error. + const authHash = await deriveAuthHash(master); + try { + await opts.verifyAuthHash(authHash); + } catch (err) { + if (err && err.code === 'invalid_credentials') { + const e = new Error('password_mismatch'); + e.code = 'password_mismatch'; + throw e; + } + throw err; } - throw err; + vaultKey = await deriveVaultKey(master, saltV); + } finally { + zeroizeDataKey(master); } - vaultKey = await deriveVaultKey(master, saltV); dk = generateDataKey(); ifMatch = '*'; } else { @@ -845,9 +853,9 @@ export function VaultProvider({ err.code = 'vault_missing_saltv'; throw err; } - // Derive the NEW vaultKey from the new master. We do this here - // (not in the caller) to keep all raw key material inside the - // vault module — callers only ever see the envelope output. + // Derive the NEW vaultKey from the new master. The caller owns + // and wipes the raw PBKDF2 bytes after this returns; VaultContext + // only retains the non-extractable CryptoKey on successful commit. const newVaultKey = await deriveVaultKey(newMaster, saltV); // rewrapEnvelope preserves the payload bytes, so `data` is diff --git a/src/lib/authService.js b/src/lib/authService.js index 1b19b42..144a7f4 100644 --- a/src/lib/authService.js +++ b/src/lib/authService.js @@ -1,5 +1,10 @@ import { apiClient as defaultClient } from './apiClient'; -import { deriveLoginKeys, deriveMaster, deriveAuthHash } from './crypto/kdf'; +import { + deriveLoginKeys, + deriveMaster, + deriveAuthHash, + zeroizeBytes, +} from './crypto/kdf'; // High-level façade for the auth surface. // @@ -14,12 +19,16 @@ import { deriveLoginKeys, deriveMaster, deriveAuthHash } from './crypto/kdf'; export function createAuthService(client = defaultClient) { async function register(email, password) { - const { authHash } = await deriveLoginKeys(password, email); - const res = await client.post('/auth/register', { - email: email.trim(), - authHash, - }); - return res.data; + const { authHash, master } = await deriveLoginKeys(password, email); + try { + const res = await client.post('/auth/register', { + email: email.trim(), + authHash, + }); + return res.data; + } finally { + zeroizeBytes(master); + } } async function verifyEmail(token) { @@ -39,11 +48,18 @@ export function createAuthService(client = defaultClient) { // NEVER log or JSON-stringify the returned object into anything // persistent — it contains raw key material until it's been consumed. const { authHash, master } = await deriveLoginKeys(password, email); - const res = await client.post('/auth/login', { - email: email.trim(), - authHash, - }); - return { ...res.data, master }; + let handoffMaster = false; + try { + const res = await client.post('/auth/login', { + email: email.trim(), + authHash, + }); + const out = { ...res.data, master }; + handoffMaster = true; + return out; + } finally { + if (!handoffMaster) zeroizeBytes(master); + } } async function completeTotpLogin({ challengeToken, code, recoveryCode }) { @@ -91,13 +107,21 @@ export function createAuthService(client = defaultClient) { // is documented in sysnode-backend/routes/auth.js:ChangePasswordSchema. async function deriveChangePasswordKeys(oldPassword, newPassword, email) { const oldKeys = await deriveLoginKeys(oldPassword, email); - const newMaster = await deriveMaster(newPassword, email); - const newAuthHash = await deriveAuthHash(newMaster); - return { - oldAuthHash: oldKeys.authHash, - newAuthHash, - newMaster, - }; + let newMaster; + let handoffNewMaster = false; + try { + newMaster = await deriveMaster(newPassword, email); + const newAuthHash = await deriveAuthHash(newMaster); + handoffNewMaster = true; + return { + oldAuthHash: oldKeys.authHash, + newAuthHash, + newMaster, + }; + } finally { + zeroizeBytes(oldKeys.master); + if (!handoffNewMaster) zeroizeBytes(newMaster); + } } async function changePassword({ oldAuthHash, newAuthHash, vault }) { @@ -123,7 +147,7 @@ export function createAuthService(client = defaultClient) { async function deriveStepUpAuthHash(password, email) { const { master, authHash } = await deriveLoginKeys(password, email); - if (master instanceof Uint8Array) master.fill(0); + zeroizeBytes(master); return authHash; } diff --git a/src/lib/crypto/kdf.js b/src/lib/crypto/kdf.js index 68de876..727f968 100644 --- a/src/lib/crypto/kdf.js +++ b/src/lib/crypto/kdf.js @@ -55,7 +55,7 @@ function encodeUtf8(text) { } function toHex(buffer) { - const bytes = new Uint8Array(buffer); + const bytes = buffer instanceof Uint8Array ? buffer : new Uint8Array(buffer); let out = ''; for (let i = 0; i < bytes.length; i += 1) { out += bytes[i].toString(16).padStart(2, '0'); @@ -63,6 +63,14 @@ function toHex(buffer) { return out; } +export function zeroizeBytes(value) { + if (value instanceof Uint8Array) { + value.fill(0); + return true; + } + return false; +} + // Strict hex-to-bytes. `parseInt(x, 16)` tolerates partial parses — e.g. // `parseInt('Ax', 16) === 10` — which would silently coerce a malformed // saltV into a valid byte and produce a subtly wrong vault key. We guard @@ -143,7 +151,11 @@ async function hkdfBytes(master, info, salt = new Uint8Array(0), length = SUBKEY export async function deriveAuthHash(master) { const bytes = await hkdfBytes(master, AUTH_INFO); - return toHex(bytes); + try { + return toHex(bytes); + } finally { + zeroizeBytes(bytes); + } } // Returns a raw AES-GCM CryptoKey ready for encrypt/decrypt. The raw bytes @@ -153,13 +165,17 @@ export async function deriveVaultKey(master, saltV) { const salt = typeof saltV === 'string' ? fromHex(saltV) : saltV; const rawKey = await hkdfBytes(master, VAULT_INFO, salt, 32); const subtle = subtleCrypto(); - return subtle.importKey( - 'raw', - rawKey, - { name: 'AES-GCM' }, - false, // non-extractable - ['encrypt', 'decrypt'] - ); + try { + return await subtle.importKey( + 'raw', + rawKey, + { name: 'AES-GCM' }, + false, // non-extractable + ['encrypt', 'decrypt'] + ); + } finally { + zeroizeBytes(rawKey); + } } // Convenience: register/login flow. Returns both the hex authHash and the @@ -167,8 +183,14 @@ export async function deriveVaultKey(master, saltV) { // without re-running the expensive PBKDF2). export async function deriveLoginKeys(password, email) { const master = await deriveMaster(password, email); - const authHash = await deriveAuthHash(master); - return { master, authHash }; + let handoffMaster = false; + try { + const authHash = await deriveAuthHash(master); + handoffMaster = true; + return { master, authHash }; + } finally { + if (!handoffMaster) zeroizeBytes(master); + } } export const __internals = { diff --git a/src/lib/crypto/kdf.test.js b/src/lib/crypto/kdf.test.js index 34db777..0b5d271 100644 --- a/src/lib/crypto/kdf.test.js +++ b/src/lib/crypto/kdf.test.js @@ -3,6 +3,7 @@ import { deriveAuthHash, deriveVaultKey, deriveLoginKeys, + zeroizeBytes, __internals, } from './kdf'; @@ -75,6 +76,13 @@ describe('kdf.deriveMaster / deriveAuthHash — cross-check vs Node crypto', () expect(authHash).toBe(direct); }, 20000); + test('zeroizeBytes overwrites Uint8Array key material in place', () => { + const bytes = new Uint8Array([1, 2, 3, 4]); + expect(zeroizeBytes(bytes)).toBe(true); + expect(Array.from(bytes)).toEqual([0, 0, 0, 0]); + expect(zeroizeBytes(null)).toBe(false); + }); + test('email case and padding do not affect the derived master (normalization)', async () => { const a = await nodePbkdf2(password, ' USER@example.COM '); const b = await nodePbkdf2(password, 'user@example.com'); diff --git a/src/pages/NewProposal.test.js b/src/pages/NewProposal.test.js index fd27213..357e494 100644 --- a/src/pages/NewProposal.test.js +++ b/src/pages/NewProposal.test.js @@ -16,6 +16,7 @@ jest.mock('../lib/crypto/kdf', () => ({ deriveMaster: jest.fn(), deriveAuthHash: jest.fn(), deriveVaultKey: jest.fn(), + zeroizeBytes: jest.fn(), })); // Mock the network-stats fetcher that the wizard uses to anchor its diff --git a/src/pages/ProposalStatus.test.js b/src/pages/ProposalStatus.test.js index cb30566..ea29d15 100644 --- a/src/pages/ProposalStatus.test.js +++ b/src/pages/ProposalStatus.test.js @@ -8,6 +8,7 @@ jest.mock('../lib/crypto/kdf', () => ({ deriveMaster: jest.fn(), deriveAuthHash: jest.fn(), deriveVaultKey: jest.fn(), + zeroizeBytes: jest.fn(), })); jest.mock('../lib/proposalService', () => {