diff --git a/delphi-backend/Handlers/PM.Handler.Auth.pas b/delphi-backend/Handlers/PM.Handler.Auth.pas index 4074a9c..937ac24 100644 --- a/delphi-backend/Handlers/PM.Handler.Auth.pas +++ b/delphi-backend/Handlers/PM.Handler.Auth.pas @@ -35,8 +35,31 @@ const // are transparently upgraded at next login (see HandleLogin/HandleReauth). // Value picked per OWASP 2023 PBKDF2-SHA256 recommendation. 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'); +// 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); var LQ: TFDQuery; @@ -138,16 +161,16 @@ begin end; LSalt := RandomHex(32); - // New accounts use the current target iteration count — no migration - // path needed since this is a brand-new vault with zero entries. - LHash := PBKDF2_SHA256_Hex(LPwd, LSalt, PBKDF2_ITERATIONS_TARGET); + // New accounts use the current target iteration count + the SHA-256 + // wrapped auth-hash scheme. password_hash is no longer the AES key. + LHash := ComputeAuthHashCurrent(LPwd, LSalt, PBKDF2_ITERATIONS_TARGET); LQ := TFDQuery.Create(nil); try LQ.Connection := DB.Connection; LQ.SQL.Text := '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('h').AsString := LHash; LQ.ParamByName('s').AsString := LSalt; @@ -240,14 +263,21 @@ begin end; LValid := False; - if SameText(LAlgo, 'pbkdf2') then + if SameText(LAlgo, HASH_ALGO_LEGACY) then begin - // Verify with the user's own iteration count (NOT the global constant). - // Legacy users at 100k still need to log in successfully so the client - // can decrypt their entries before triggering the /migrate-kdf flow. + // Legacy scheme: stored hash is raw PBKDF2 hex (= AES key bytes). Verify + // by direct comparison. On success, login proceeds normally — the + // 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); LValid := ConstantTimeEquals(LComputed, LStoredHash); 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 begin // Not implemented in Delphi backend yet @@ -275,11 +305,14 @@ begin EnsureDefaultFolders(LUserId); CreateSession(LUserId, LToken, LCSRF); LogAudit(LUserId, 'login', LIP); - // Signal migration when the user's current iteration count is below the - // target. The client will re-encrypt all entries and call /migrate-kdf - // to commit everything atomically. - SendAuthSuccess(AResponse, LUserId, LToken, LSalt, LCSRF, - LKdfIters, LKdfIters < PBKDF2_ITERATIONS_TARGET); + // Signal migration whenever EITHER: + // - the user's iteration count is below the target (KDF bump needed), OR + // - the user's hash_algo is not the current scheme (format upgrade needed + // to remove the AES-key-in-vault.db architectural flaw). + // 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; // ===== /logout =============================================================== @@ -378,11 +411,15 @@ begin if RejectIfAccountLocked(AResponse, LUser) then Exit; LValid := False; - if SameText(LAlgo, 'pbkdf2') then + if SameText(LAlgo, HASH_ALGO_LEGACY) then begin - // Verify with the user's stored iteration count, same as HandleLogin. LComputed := PBKDF2_SHA256_Hex(LPwd, LSalt, LKdfIters); LValid := ConstantTimeEquals(LComputed, LStoredHash); + end + else if SameText(LAlgo, HASH_ALGO_CURRENT) then + begin + LComputed := ComputeAuthHashCurrent(LPwd, LSalt, LKdfIters); + LValid := ConstantTimeEquals(LComputed, LStoredHash); end; if not LValid then @@ -400,12 +437,14 @@ begin // 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 - // 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 var LObj := TJSONObject.Create; LObj.AddPair('message', 'OK'); 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 var LMig := TJSONObject.Create; LMig.AddPair('target', TJSONNumber.Create(PBKDF2_ITERATIONS_TARGET)); @@ -482,19 +521,29 @@ begin LQ.Free; end; - // Idempotency: if already at target, nothing to do. - if LOldIters >= PBKDF2_ITERATIONS_TARGET then + // Idempotency: nothing to do if BOTH iter count is at target AND + // 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 TJSONHelper.SendOK(AResponse, 'Already at target'); Exit; 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; - if SameText(LAlgo, 'pbkdf2') then + if SameText(LAlgo, HASH_ALGO_LEGACY) then begin LComputed := PBKDF2_SHA256_Hex(LPwd, LSalt, LOldIters); LValid := ConstantTimeEquals(LComputed, LStoredHash); + end + else if SameText(LAlgo, HASH_ALGO_CURRENT) then + begin + LComputed := ComputeAuthHashCurrent(LPwd, LSalt, LOldIters); + LValid := ConstantTimeEquals(LComputed, LStoredHash); end; if not LValid then begin @@ -504,8 +553,11 @@ begin Exit; end; - // Step 3: compute the new password hash with target iterations. - LNewHash := PBKDF2_SHA256_Hex(LPwd, LSalt, PBKDF2_ITERATIONS_TARGET); + // Step 3: compute the new password hash. ALWAYS uses the current + // 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 // ciphertext together. Any failure rolls back, leaving the user on @@ -515,11 +567,16 @@ begin LQ := TFDQuery.Create(nil); try 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 := - 'UPDATE users SET password_hash = :h, kdf_iterations = :it ' + + 'UPDATE users SET password_hash = :h, kdf_iterations = :it, ' + + ' hash_algo = :algo ' + 'WHERE id = :uid'; LQ.ParamByName('h').AsString := LNewHash; LQ.ParamByName('it').AsInteger := PBKDF2_ITERATIONS_TARGET; + LQ.ParamByName('algo').AsString := HASH_ALGO_CURRENT; LQ.ParamByName('uid').AsInteger := LUserId; LQ.ExecSQL; finally diff --git a/js/app.js b/js/app.js index cc9589d..cb267bf 100644 --- a/js/app.js +++ b/js/app.js @@ -377,38 +377,55 @@ async function runKdfMigration(masterPwd, fromIters, toIters) { kdfMigrationInProgress = true; try { - // Derive the new key. The old key is already in state.cryptoKey - // (used to decrypt the entries we just loaded). - const newKey = await deriveKey(masterPwd, state.salt, toIters); + // Two distinct migration scenarios: + // A. fromIters !== toIters: KDF iteration count is changing, so + // 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 - // the new key — never reuse the old IV with the new key (would be - // pointless but also a small information leak via IV reuse patterns). - const newCiphertexts = []; - for (const entry of state.entries) { - const plain = await decryptPwd(entry.encrypted_password, entry.iv); - if (plain === '[ERROR]') { - // One decrypt failure aborts the whole migration — better to - // stay on the legacy config than to commit partial state. - throw new Error('Could not decrypt entry id=' + entry.id); - } - const tmpKey = state.cryptoKey; - try { - state.cryptoKey = newKey; - const re = await encryptPwd(plain); - newCiphertexts.push({ - id: entry.id, - encrypted_password: re.encrypted, - iv: re.iv, - }); - } finally { - state.cryptoKey = tmpKey; // restore for any concurrent read + if (kdfChange) { + newKey = await deriveKey(masterPwd, state.salt, toIters); + // Re-encrypt every entry. Each entry gets a fresh random IV + // under the new key — never reuse the old IV with the new key + // (would be pointless but also a small information leak via IV + // reuse patterns). + newCiphertexts = []; + for (const entry of state.entries) { + const plain = await decryptPwd(entry.encrypted_password, entry.iv); + if (plain === '[ERROR]') { + // One decrypt failure aborts the whole migration — + // better to stay on the legacy config than commit + // partial state. + throw new Error('Could not decrypt entry id=' + entry.id); + } + const tmpKey = state.cryptoKey; + try { + state.cryptoKey = newKey; + const re = await encryptPwd(plain); + newCiphertexts.push({ + id: entry.id, + 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 - // against the OLD hash, then updates hash + iterations + every - // entry in a single transaction. + // against the OLD hash, then updates the user row (hash, iter + // count, hash_algo) AND every entry's ciphertext in a single + // transaction. await api('/migrate-kdf', { method: 'POST', 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 - // the cached ciphertexts in state.entries so subsequent reads use - // the new key transparently. - state.cryptoKey = newKey; - await persistCryptoKey(); - for (let i = 0; i < state.entries.length; i++) { - const nc = newCiphertexts[i]; - state.entries[i].encrypted_password = nc.encrypted_password; - state.entries[i].iv = nc.iv; + if (kdfChange) { + // Swap to the new AES key + update cached ciphertexts. + state.cryptoKey = newKey; + await persistCryptoKey(); + for (let i = 0; i < state.entries.length; i++) { + const nc = newCiphertexts[i]; + state.entries[i].encrypted_password = nc.encrypted_password; + 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) { // Silent retry on next login — the migration is idempotent and // safe to abandon (server rolled back).