Zaki/Password-Manager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(crypto): SHA-256 wrap auth hash so vault.db at rest no longer = AES key

Browse Source 
THE PROBLEM
===========
Before this commit, users.password_hash stored on the server contained
PBKDF2(pw, salt, iters) in hex — the exact same 32 bytes the client
uses as the AES-GCM key to encrypt every entry. Anyone who got hold of
vault.db (filesystem access, backup leak, etc.) had the encryption key
in their hand, no brute force needed. The increased PBKDF2 iteration
count from the previous commit helped against the cipher-text path,
but the easier path was right there in the user row.

THE FIX
=======
Wrap the PBKDF2 output in SHA-256 before storing:

  password_hash = SHA256(PBKDF2(pw, salt, iters))

SHA-256 is one-way. The stored hash can still be verified at login
(server recomputes PBKDF2 from the posted master pw, then SHA-256s it,
compares to stored), but the AES key can no longer be recovered from
it. At rest, vault.db only contains an irreversible derivative.

The server still sees pw transiently during /login while computing
the comparison — eliminating that requires a redesigned auth
protocol where the client sends a pre-computed verifier (SRP, OPAQUE,
or simply SHA-256(PBKDF2(pw, salt, iters)) sent from the client).
That's a separate, larger refactor. This commit closes the at-rest
hole, which is the realistic attack surface for vault file leaks.

SCHEMA / MARKER
===============
users.hash_algo distinguishes the two schemes:
  'pbkdf2'        — LEGACY (raw hex, = AES key)
  'pbkdf2-sha256' — CURRENT (SHA-256-wrapped, one-way)

A constant HASH_ALGO_CURRENT replaces the string literal everywhere
to avoid silent drift between the writer and the reader sides.

MIGRATION
=========
Folded into the existing /migrate-kdf endpoint introduced for the
100k→600k iteration bump. Login response now signals migration on
EITHER:
  - kdf_iterations < PBKDF2_ITERATIONS_TARGET, OR
  - hash_algo != 'pbkdf2-sha256'

The endpoint handles both transitions in one atomic transaction:
  UPDATE users SET password_hash = SHA256(PBKDF2(pw, salt, 600k)),
                   kdf_iterations = 600000,
                   hash_algo = 'pbkdf2-sha256'
  UPDATE vault_entries SET encrypted_password, iv (per entry, if KDF changed)

Idempotency tightened: the "already at target" short-circuit now
requires BOTH conditions, not just the iteration count. Without this,
users who migrated KDF before this commit landed would have been
stuck on the legacy hash format.

CLIENT
======
runKdfMigration() branches on whether the KDF actually changed:
  - kdfChange (fromIters !== toIters): re-encrypt all entries with the
    new key, send them in the entries array, swap state.cryptoKey on
    success. Shows "Vault security upgraded" toast.
  - !kdfChange (hash format only): skip the entry re-encryption loop
    entirely, send entries: []. Silent — the user didn't perceive a
    weakness change worth toasting about.

LOGIN / REAUTH
==============
Both now branch on hash_algo to pick the right verifier:
  HASH_ALGO_LEGACY  → ConstantTimeEquals(stored, PBKDF2(pw, salt, iters))
  HASH_ALGO_CURRENT → ConstantTimeEquals(stored, SHA256(PBKDF2(pw, salt, iters)))

Same constant-time comparison helper as before. Same legacy bcrypt
fallback (still 501-not-implemented).

ALL THREE SCENARIOS AFTER THIS COMMIT
=====================================
1. New register: starts at HASH_ALGO_CURRENT + 600k. No migration ever.
2. Legacy 100k + 'pbkdf2': full migration on next login (hash format
   + iter count + entry re-encryption) in one transaction.
3. Mid-state (already-migrated KDF + still-'pbkdf2'): hash format
   upgrade only on next login, no entry re-encryption.
This commit is contained in:
Zaki
2026-05-23 05:19:39 +01:00
parent cf94f67488
commit 60aa106a30
2 changed files with 140 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files
delphi-backend/Handlers/PM.Handler.Auth.pas
+81 -24
View File
@@ -35,8 +35,31 @@ const
// are transparently upgraded at next login (see HandleLogin/HandleReauth). // are transparently upgraded at next login (see HandleLogin/HandleReauth).
// Value picked per OWASP 2023 PBKDF2-SHA256 recommendation. // Value picked per OWASP 2023 PBKDF2-SHA256 recommendation.
PBKDF2_ITERATIONS_TARGET = 600000; PBKDF2_ITERATIONS_TARGET = 600000;
// ---- Hash algorithm markers (users.hash_algo) ----
// 'pbkdf2' : LEGACY. Stored hash = PBKDF2(pw, salt, iters) raw hex.
// Catastrophic at rest: those same bytes ARE the AES
// key the client uses to encrypt entries. A stolen
// vault.db hands the attacker the key directly.
// 'pbkdf2-sha256' : CURRENT. Stored hash = SHA256(PBKDF2(pw, salt, iters)).
// One-way wrap. vault.db at rest no longer contains
// the AES key. Server still sees pw transiently
// during /login to compute the comparison.
HASH_ALGO_LEGACY = 'pbkdf2';
HASH_ALGO_CURRENT = 'pbkdf2-sha256';
DEFAULT_FOLDERS: array[0..4] of string = ('All', 'Social', 'Banking', 'Work', 'Personal'); DEFAULT_FOLDERS: array[0..4] of string = ('All', 'Social', 'Banking', 'Work', 'Personal');
// Auth-hash computation for the current scheme. Wraps PBKDF2 output in
// SHA-256 so the stored value is no longer usable as the AES decryption
// key. Use this everywhere we write or verify a hash under
// HASH_ALGO_CURRENT — register, login, reauth, and migrate-kdf all
// go through here for consistency.
function ComputeAuthHashCurrent(const APwd, ASalt: string; AIters: Integer): string;
begin
Result := SHA256Hex(PBKDF2_SHA256_Hex(APwd, ASalt, AIters));
end;
procedure EnsureDefaultFolders(AUserId: Integer); procedure EnsureDefaultFolders(AUserId: Integer);
var var
LQ: TFDQuery; LQ: TFDQuery;
@@ -138,16 +161,16 @@ begin
end; end;
LSalt := RandomHex(32); LSalt := RandomHex(32);
// New accounts use the current target iteration count — no migration // New accounts use the current target iteration count + the SHA-256
// path needed since this is a brand-new vault with zero entries. // wrapped auth-hash scheme. password_hash is no longer the AES key.
LHash := PBKDF2_SHA256_Hex(LPwd, LSalt, PBKDF2_ITERATIONS_TARGET); LHash := ComputeAuthHashCurrent(LPwd, LSalt, PBKDF2_ITERATIONS_TARGET);
LQ := TFDQuery.Create(nil); LQ := TFDQuery.Create(nil);
try try
LQ.Connection := DB.Connection; LQ.Connection := DB.Connection;
LQ.SQL.Text := LQ.SQL.Text :=
'INSERT INTO users (username, password_hash, salt, hash_algo, kdf_iterations) ' + 'INSERT INTO users (username, password_hash, salt, hash_algo, kdf_iterations) ' +
'VALUES (:u, :h, :s, ''pbkdf2'', :it)'; 'VALUES (:u, :h, :s, ''' + HASH_ALGO_CURRENT + ''', :it)';
LQ.ParamByName('u').AsString := LUser; LQ.ParamByName('u').AsString := LUser;
LQ.ParamByName('h').AsString := LHash; LQ.ParamByName('h').AsString := LHash;
LQ.ParamByName('s').AsString := LSalt; LQ.ParamByName('s').AsString := LSalt;
@@ -240,14 +263,21 @@ begin
end; end;
LValid := False; LValid := False;
if SameText(LAlgo, 'pbkdf2') then if SameText(LAlgo, HASH_ALGO_LEGACY) then
begin begin
// Verify with the user's own iteration count (NOT the global constant). // Legacy scheme: stored hash is raw PBKDF2 hex (= AES key bytes). Verify
// Legacy users at 100k still need to log in successfully so the client // by direct comparison. On success, login proceeds normally — the
// can decrypt their entries before triggering the /migrate-kdf flow. // migration to HASH_ALGO_CURRENT is signaled via kdfMigration in the
// auth response and handled by the client through /migrate-kdf.
LComputed := PBKDF2_SHA256_Hex(LPwd, LSalt, LKdfIters); LComputed := PBKDF2_SHA256_Hex(LPwd, LSalt, LKdfIters);
LValid := ConstantTimeEquals(LComputed, LStoredHash); LValid := ConstantTimeEquals(LComputed, LStoredHash);
end end
else if SameText(LAlgo, HASH_ALGO_CURRENT) then
begin
// Current scheme: stored hash is SHA-256 of the PBKDF2 output.
LComputed := ComputeAuthHashCurrent(LPwd, LSalt, LKdfIters);
LValid := ConstantTimeEquals(LComputed, LStoredHash);
end
else if SameText(LAlgo, 'bcrypt') then else if SameText(LAlgo, 'bcrypt') then
begin begin
// Not implemented in Delphi backend yet // Not implemented in Delphi backend yet
@@ -275,11 +305,14 @@ begin
EnsureDefaultFolders(LUserId); EnsureDefaultFolders(LUserId);
CreateSession(LUserId, LToken, LCSRF); CreateSession(LUserId, LToken, LCSRF);
LogAudit(LUserId, 'login', LIP); LogAudit(LUserId, 'login', LIP);
// Signal migration when the user's current iteration count is below the // Signal migration whenever EITHER:
// target. The client will re-encrypt all entries and call /migrate-kdf // - the user's iteration count is below the target (KDF bump needed), OR
// to commit everything atomically. // - the user's hash_algo is not the current scheme (format upgrade needed
SendAuthSuccess(AResponse, LUserId, LToken, LSalt, LCSRF, // to remove the AES-key-in-vault.db architectural flaw).
LKdfIters, LKdfIters < PBKDF2_ITERATIONS_TARGET); // The client then calls /migrate-kdf which fixes both in one atomic step.
SendAuthSuccess(AResponse, LUserId, LToken, LSalt, LCSRF, LKdfIters,
(LKdfIters < PBKDF2_ITERATIONS_TARGET) or
not SameText(LAlgo, HASH_ALGO_CURRENT));
end; end;
// ===== /logout =============================================================== // ===== /logout ===============================================================
@@ -378,11 +411,15 @@ begin
if RejectIfAccountLocked(AResponse, LUser) then Exit; if RejectIfAccountLocked(AResponse, LUser) then Exit;
LValid := False; LValid := False;
if SameText(LAlgo, 'pbkdf2') then if SameText(LAlgo, HASH_ALGO_LEGACY) then
begin begin
// Verify with the user's stored iteration count, same as HandleLogin.
LComputed := PBKDF2_SHA256_Hex(LPwd, LSalt, LKdfIters); LComputed := PBKDF2_SHA256_Hex(LPwd, LSalt, LKdfIters);
LValid := ConstantTimeEquals(LComputed, LStoredHash); LValid := ConstantTimeEquals(LComputed, LStoredHash);
end
else if SameText(LAlgo, HASH_ALGO_CURRENT) then
begin
LComputed := ComputeAuthHashCurrent(LPwd, LSalt, LKdfIters);
LValid := ConstantTimeEquals(LComputed, LStoredHash);
end; end;
if not LValid then if not LValid then
@@ -400,12 +437,14 @@ begin
// Return KDF state so the client can detect legacy accounts that haven't // Return KDF state so the client can detect legacy accounts that haven't
// been migrated yet — unlock from a locked state goes through reauth, not // been migrated yet — unlock from a locked state goes through reauth, not
// login, so we need the same migration signaling here. // login, so we need the same migration signaling here. Migration triggers
// on KDF iter mismatch OR hash format mismatch (same rule as HandleLogin).
begin begin
var LObj := TJSONObject.Create; var LObj := TJSONObject.Create;
LObj.AddPair('message', 'OK'); LObj.AddPair('message', 'OK');
LObj.AddPair('kdfIterations', TJSONNumber.Create(LKdfIters)); LObj.AddPair('kdfIterations', TJSONNumber.Create(LKdfIters));
if LKdfIters < PBKDF2_ITERATIONS_TARGET then if (LKdfIters < PBKDF2_ITERATIONS_TARGET) or
not SameText(LAlgo, HASH_ALGO_CURRENT) then
begin begin
var LMig := TJSONObject.Create; var LMig := TJSONObject.Create;
LMig.AddPair('target', TJSONNumber.Create(PBKDF2_ITERATIONS_TARGET)); LMig.AddPair('target', TJSONNumber.Create(PBKDF2_ITERATIONS_TARGET));
@@ -482,19 +521,29 @@ begin
LQ.Free; LQ.Free;
end; end;
// Idempotency: if already at target, nothing to do. // Idempotency: nothing to do if BOTH iter count is at target AND
if LOldIters >= PBKDF2_ITERATIONS_TARGET then // hash format is current. Previously we short-circuited on iter
// count alone, which would have skipped the hash-format upgrade for
// users who migrated KDF before this commit landed.
if (LOldIters >= PBKDF2_ITERATIONS_TARGET) and
SameText(LAlgo, HASH_ALGO_CURRENT) then
begin begin
TJSONHelper.SendOK(AResponse, 'Already at target'); TJSONHelper.SendOK(AResponse, 'Already at target');
Exit; Exit;
end; end;
// Step 2: verify the master pw against the CURRENT (old) hash. // Step 2: verify the master pw against the CURRENT (old) hash,
// using whichever scheme the user is currently on.
LValid := False; LValid := False;
if SameText(LAlgo, 'pbkdf2') then if SameText(LAlgo, HASH_ALGO_LEGACY) then
begin begin
LComputed := PBKDF2_SHA256_Hex(LPwd, LSalt, LOldIters); LComputed := PBKDF2_SHA256_Hex(LPwd, LSalt, LOldIters);
LValid := ConstantTimeEquals(LComputed, LStoredHash); LValid := ConstantTimeEquals(LComputed, LStoredHash);
end
else if SameText(LAlgo, HASH_ALGO_CURRENT) then
begin
LComputed := ComputeAuthHashCurrent(LPwd, LSalt, LOldIters);
LValid := ConstantTimeEquals(LComputed, LStoredHash);
end; end;
if not LValid then if not LValid then
begin begin
@@ -504,8 +553,11 @@ begin
Exit; Exit;
end; end;
// Step 3: compute the new password hash with target iterations. // Step 3: compute the new password hash. ALWAYS uses the current
LNewHash := PBKDF2_SHA256_Hex(LPwd, LSalt, PBKDF2_ITERATIONS_TARGET); // scheme (SHA-256 wrap) and the target iteration count, regardless
// of where the user was before — migration converges everyone to
// the same modern config.
LNewHash := ComputeAuthHashCurrent(LPwd, LSalt, PBKDF2_ITERATIONS_TARGET);
// Step 4: atomic transaction — update user hash AND every entry's // Step 4: atomic transaction — update user hash AND every entry's
// ciphertext together. Any failure rolls back, leaving the user on // ciphertext together. Any failure rolls back, leaving the user on
@@ -515,11 +567,16 @@ begin
LQ := TFDQuery.Create(nil); LQ := TFDQuery.Create(nil);
try try
LQ.Connection := DB.Connection; LQ.Connection := DB.Connection;
// Update hash, iter count, AND hash_algo all in one row update.
// hash_algo := HASH_ALGO_CURRENT is what completes the migration
// away from the "stored hash IS the AES key" architectural flaw.
LQ.SQL.Text := LQ.SQL.Text :=
'UPDATE users SET password_hash = :h, kdf_iterations = :it ' + 'UPDATE users SET password_hash = :h, kdf_iterations = :it, ' +
' hash_algo = :algo ' +
'WHERE id = :uid'; 'WHERE id = :uid';
LQ.ParamByName('h').AsString := LNewHash; LQ.ParamByName('h').AsString := LNewHash;
LQ.ParamByName('it').AsInteger := PBKDF2_ITERATIONS_TARGET; LQ.ParamByName('it').AsInteger := PBKDF2_ITERATIONS_TARGET;
LQ.ParamByName('algo').AsString := HASH_ALGO_CURRENT;
LQ.ParamByName('uid').AsInteger := LUserId; LQ.ParamByName('uid').AsInteger := LUserId;
LQ.ExecSQL; LQ.ExecSQL;
finally finally
js/app.js
+59 -39
View File
@@ -377,38 +377,55 @@ async function runKdfMigration(masterPwd, fromIters, toIters) {
kdfMigrationInProgress = true; kdfMigrationInProgress = true;
try { try {
// Derive the new key. The old key is already in state.cryptoKey // Two distinct migration scenarios:
// (used to decrypt the entries we just loaded). // A. fromIters !== toIters: KDF iteration count is changing, so
const newKey = await deriveKey(masterPwd, state.salt, toIters); // the AES key is changing. We re-encrypt every entry with the
// new key + fresh IVs, swap state.cryptoKey at the end.
// B. fromIters === toIters: same KDF, only the server-side hash
// format is being upgraded (legacy "pbkdf2" raw → "pbkdf2-sha256"
// wrapped). No entry re-encryption needed — just trigger the
// endpoint so the server rewrites the user row.
const kdfChange = fromIters !== toIters;
let newKey, newCiphertexts;
// Re-encrypt every entry. Each entry gets a fresh random IV under if (kdfChange) {
// the new key — never reuse the old IV with the new key (would be newKey = await deriveKey(masterPwd, state.salt, toIters);
// pointless but also a small information leak via IV reuse patterns). // Re-encrypt every entry. Each entry gets a fresh random IV
const newCiphertexts = []; // under the new key — never reuse the old IV with the new key
for (const entry of state.entries) { // (would be pointless but also a small information leak via IV
const plain = await decryptPwd(entry.encrypted_password, entry.iv); // reuse patterns).
if (plain === '[ERROR]') { newCiphertexts = [];
// One decrypt failure aborts the whole migration — better to for (const entry of state.entries) {
// stay on the legacy config than to commit partial state. const plain = await decryptPwd(entry.encrypted_password, entry.iv);
throw new Error('Could not decrypt entry id=' + entry.id); if (plain === '[ERROR]') {
} // One decrypt failure aborts the whole migration —
const tmpKey = state.cryptoKey; // better to stay on the legacy config than commit
try { // partial state.
state.cryptoKey = newKey; throw new Error('Could not decrypt entry id=' + entry.id);
const re = await encryptPwd(plain); }
newCiphertexts.push({ const tmpKey = state.cryptoKey;
id: entry.id, try {
encrypted_password: re.encrypted, state.cryptoKey = newKey;
iv: re.iv, const re = await encryptPwd(plain);
}); newCiphertexts.push({
} finally { id: entry.id,
state.cryptoKey = tmpKey; // restore for any concurrent read encrypted_password: re.encrypted,
iv: re.iv,
});
} finally {
state.cryptoKey = tmpKey; // restore for any concurrent read
}
} }
} else {
// Hash-format-only upgrade — server still wants an entries
// array (it's an idempotent transactional update), just empty.
newCiphertexts = [];
} }
// Send the atomic migrate request. Server verifies the master pw // Send the atomic migrate request. Server verifies the master pw
// against the OLD hash, then updates hash + iterations + every // against the OLD hash, then updates the user row (hash, iter
// entry in a single transaction. // count, hash_algo) AND every entry's ciphertext in a single
// transaction.
await api('/migrate-kdf', { await api('/migrate-kdf', {
method: 'POST', method: 'POST',
headers: authHeaders({ 'Content-Type': 'application/json' }), headers: authHeaders({ 'Content-Type': 'application/json' }),
@@ -418,19 +435,22 @@ async function runKdfMigration(masterPwd, fromIters, toIters) {
}), }),
}); });
// Server committed → switch our in-memory crypto key and update if (kdfChange) {
// the cached ciphertexts in state.entries so subsequent reads use // Swap to the new AES key + update cached ciphertexts.
// the new key transparently. state.cryptoKey = newKey;
state.cryptoKey = newKey; await persistCryptoKey();
await persistCryptoKey(); for (let i = 0; i < state.entries.length; i++) {
for (let i = 0; i < state.entries.length; i++) { const nc = newCiphertexts[i];
const nc = newCiphertexts[i]; state.entries[i].encrypted_password = nc.encrypted_password;
state.entries[i].encrypted_password = nc.encrypted_password; state.entries[i].iv = nc.iv;
state.entries[i].iv = nc.iv; }
toast('Vault security upgraded (' + fromIters.toLocaleString() +
' → ' + toIters.toLocaleString() + ' KDF iterations)');
} else {
// Format-only upgrade is silent — the user didn't perceive a
// weakness change, and nothing visible in the UI changed.
// (A subtle "Auth format upgraded" toast felt noisy.)
} }
toast('Vault security upgraded (' + fromIters.toLocaleString() +
' → ' + toIters.toLocaleString() + ' KDF iterations)');
} catch (err) { } catch (err) {
// Silent retry on next login — the migration is idempotent and // Silent retry on next login — the migration is idempotent and
// safe to abandon (server rolled back). // safe to abandon (server rolled back).