Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/components/ChangePasswordCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -249,6 +252,7 @@ export default function ChangePasswordCard({
setConfirmPassword('');
setSuccess(true);
} finally {
zeroizeBytes(newMaster);
setSubmitting(false);
}
},
Expand Down
22 changes: 20 additions & 2 deletions src/components/ChangePasswordCard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({}),
};
Expand Down Expand Up @@ -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));
});
});
8 changes: 6 additions & 2 deletions src/components/DeleteAccountCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -141,6 +144,7 @@ export default function DeleteAccountCard({
handleAuthLost();
history.replace('/');
} finally {
zeroizeBytes(master);
setSubmitting(false);
}
},
Expand Down
46 changes: 45 additions & 1 deletion src/components/DeleteAccountCard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,21 @@ 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
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
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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() };
Expand Down
1 change: 1 addition & 0 deletions src/components/SessionExpiredBanner.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
2 changes: 2 additions & 0 deletions src/context/AuthContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:
//
Expand Down Expand Up @@ -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;
Expand Down
50 changes: 29 additions & 21 deletions src/context/VaultContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]
);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
64 changes: 44 additions & 20 deletions src/lib/authService.js
Original file line number Diff line number Diff line change
@@ -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.
//
Expand All @@ -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) {
Expand All @@ -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 }) {
Expand Down Expand Up @@ -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 }) {
Expand All @@ -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;
}

Expand Down
Loading
Loading