fix(kdf): neutralize the /migrate-kdf endpoint server-side (6c-1 / 6c-4)

6c-1 removed the client caller, but the server handler stayed armed: a stray
authed+CSRF'd call could still re-encrypt only 1 blob of 8 (data loss) or
rewrite a -v2 account's hash_algo (permanent lockout, 6c-4). Gutted the ~165
destructive lines and replaced the body with an auth+CSRF gate then HTTP 410.
Both risks are now closed server-side, not merely unreachable by convention.
The vestigial kdfMigration login signal is left (client ignores it, endpoint
410s) — removing it would cascade through a login-path param for no benefit.
Fixed the now-false comments pointing at migrate-kdf.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
r-zakarya
2026-07-20 05:43:39 +01:00
parent 47aec30934
commit 370432f638
2 changed files with 27 additions and 188 deletions
+9 -6
View File
@@ -729,11 +729,14 @@ l'a déjà fait (et après **6c-2** les deux sont sur la même base de temps).
### 6c-4. 🟡 `/migrate-kdf` rétrograderait un compte `-v2` (verrouillage définitif) ### 6c-4. 🟡 `/migrate-kdf` rétrograderait un compte `-v2` (verrouillage définitif)
> ⛔ **SANS OBJET depuis 6c-1 (2026-07-16).** Le chemin migrate-kdf est > ✅ **RÉSOLU 2026-07-16.** Au-delà de 6c-1 (client), l'endpoint serveur
> désactivé côté client (fonction `runKdfMigration` supprimée, triggers > `HandleMigrateKdf` a été **vidé de ses ~165 lignes destructrices** et remplacé
> retirés). L'endpoint reste inerte (rien ne l'appelle) → ce scénario de brick > par un reject `410` après auth+CSRF ([Auth.pas:656](delphi-backend/Handlers/PM.Handler.Auth.pas)).
> n'est plus atteignable. Garde à ré-ajouter uniquement si migrate-kdf revenait > Un appel isolé ne peut donc plus **ni corrompre** (6c-1) **ni brick** un compte
> un jour. > -v2 (6c-4) : les deux sont fermés côté serveur, pas seulement « inatteignables
> par convention ». Le signal `kdfMigration` du login reste (vestigial, client
> l'ignore, endpoint = 410) — non retiré car câblé via un paramètre sur le
> hot-path login, churn sans bénéfice.
Le garde d'idempotence ([Auth.pas:722](delphi-backend/Handlers/PM.Handler.Auth.pas:722)) Le garde d'idempotence ([Auth.pas:722](delphi-backend/Handlers/PM.Handler.Auth.pas:722))
ne court-circuite que si l'algo vaut `pbkdf2-sha256`. Un compte `argon2id-v2` ou ne court-circuite que si l'algo vaut `pbkdf2-sha256`. Un compte `argon2id-v2` ou
@@ -950,7 +953,7 @@ Le second est correct **dans les deux cas** (no-op si c'était déjà le défaut
| **6c-2** | 🟠 | — | ✅ **Fait** — `expires_at` en UTC. Ferme §2.2. | | **6c-2** | 🟠 | — | ✅ **Fait** — `expires_at` en UTC. Ferme §2.2. |
| **6c-3** | 🟡 | — | ✅ **Fait** — CSRF lié à la session présentée (token_hash). | | **6c-3** | 🟡 | — | ✅ **Fait** — CSRF lié à la session présentée (token_hash). |
| **6c-9** | ❓ | — | ✅ **Fait** — `ForeignKeys=On` explicite. | | **6c-9** | ❓ | — | ✅ **Fait** — `ForeignKeys=On` explicite. |
| **6c-4** | 🟡 | — | ⛔ **Sans objet** — l'endpoint migrate-kdf est inatteignable depuis 6c-1. | | **6c-4** | 🟡 | — | ✅ **Fait** — endpoint migrate-kdf vidé + reject 410 (ferme 6c-1 & 6c-4 côté serveur). |
| **6c-5** | 🔵 | Trivial | À faire — 5 lignes × 2 endroits (fail-fast params Argon2). | | **6c-5** | 🔵 | Trivial | À faire — 5 lignes × 2 endroits (fail-fast params Argon2). |
| **6c-6/7/8** | 🔵 | Faible | À faire — cohérence / hygiène, aucune urgence. | | **6c-6/7/8** | 🔵 | Faible | À faire — cohérence / hygiène, aucune urgence. |
+18 -182
View File
@@ -206,8 +206,10 @@ begin
LObj.AddPair('csrfToken', ACSRFToken); LObj.AddPair('csrfToken', ACSRFToken);
// kdfIterations is the iteration count the client must use when deriving // kdfIterations is the iteration count the client must use when deriving
// the AES-GCM key for THIS session — matches the count under which the // the AES-GCM key for THIS session — matches the count under which the
// existing entries are encrypted. If the server signals migration, the // existing entries are encrypted. NOTE: the kdfMigration signal below is
// client should re-encrypt with the new target and call /migrate-kdf. // now VESTIGIAL — /migrate-kdf was disabled (§6c-1) and the client ignores
// it. Legacy accounts upgrade via /change-master-password instead. Kept to
// avoid churning the login hot path; harmless (endpoint returns 410).
LObj.AddPair('kdfIterations', TJSONNumber.Create(AKdfIterations)); LObj.AddPair('kdfIterations', TJSONNumber.Create(AKdfIterations));
if ANeedsMigration then if ANeedsMigration then
begin begin
@@ -643,26 +645,18 @@ begin
end; end;
// ===== /migrate-kdf ========================================================== // ===== /migrate-kdf ==========================================================
// Atomic transition from an old PBKDF2 iteration count to the current target. // DISABLED 2026-07-16 (§6c-1 / §6c-4). This path re-derived the AES key
// Client side: derive both old and new AES keys, decrypt each entry with old, // (100k -> 600k) but re-encrypted ONLY encrypted_password — totp_secret,
// re-encrypt with new, then POST the new ciphertext blob to this endpoint // custom_fields, the *_enc metadata + attachments stayed under the old key =
// along with the master password (so we can recompute the new server hash). // data loss — and it would rewrite a -v2 account's hash_algo, permanently
// Server side: verify the master pw with the old hash, then in a single // locking it out. The client no longer calls it; any stray call is rejected so
// transaction: update users.password_hash to the new PBKDF2 output, set // it can neither corrupt nor brick a vault. Legacy <600k accounts upgrade
// kdf_iterations to TARGET, and replace each entry's encrypted_password/iv. // safely via /change-master-password (re-encrypts everything + moves to
// All-or-nothing: if anything fails, the user stays on the old config. // Argon2id).
procedure HandleMigrateKdf(ARequest: TIdHTTPRequestInfo; procedure HandleMigrateKdf(ARequest: TIdHTTPRequestInfo;
AResponse: TIdHTTPResponseInfo; const AParams: TArray<string>); AResponse: TIdHTTPResponseInfo; const AParams: TArray<string>);
var var
LUserId, LOldIters, I: Integer; LUserId: Integer;
LBody, LEntry: TJSONObject;
LEntries: TJSONArray;
LUser, LPwd, LOldVerifier, LNewVerifier, LSalt, LStoredHash, LAlgo, LIP,
LComputed, LNewHash: string;
LQ: TFDQuery;
LValid: Boolean;
LEntryId: Integer;
LEncPwd, LIv: string;
begin begin
try try
LUserId := Authenticate(ARequest, AResponse); LUserId := Authenticate(ARequest, AResponse);
@@ -670,166 +664,8 @@ begin
except except
on ESessionRejected do Exit; on ESessionRejected do Exit;
end; end;
TJSONHelper.SendError(AResponse, 410,
LIP := GetClientIP(ARequest); 'KDF migration disabled — rotate the master password instead');
LBody := TJSONHelper.ReadBody(ARequest);
try
LPwd := LBody.GetValue<string>('masterPassword', '');
// ZK path: client provides PBKDF2 hex at the OLD iter count (oldVerifier,
// for current-hash verification) AND at the new TARGET iter count
// (newVerifier, for the post-migration stored hash).
LOldVerifier := LBody.GetValue<string>('oldVerifier', '');
LNewVerifier := LBody.GetValue<string>('newVerifier', '');
LEntries := LBody.GetValue<TJSONArray>('entries');
if LEntries = nil then
begin
TJSONHelper.SendError(AResponse, 400, 'Missing entries array');
Exit;
end;
DB.Lock;
try
// Step 1: load current user state.
LQ := TFDQuery.Create(nil);
try
LQ.Connection := DB.Connection;
LQ.SQL.Text :=
'SELECT username, password_hash, salt, hash_algo, kdf_iterations ' +
'FROM users WHERE id = :uid';
LQ.ParamByName('uid').AsInteger := LUserId;
LQ.Open;
if LQ.IsEmpty then
begin
TJSONHelper.SendError(AResponse, 401, 'User not found');
Exit;
end;
LUser := LQ.FieldByName('username').AsString;
LStoredHash := LQ.FieldByName('password_hash').AsString;
LSalt := LQ.FieldByName('salt').AsString;
LAlgo := LQ.FieldByName('hash_algo').AsString;
LOldIters := LQ.FieldByName('kdf_iterations').AsInteger;
if LAlgo = '' then LAlgo := 'pbkdf2';
if LOldIters <= 0 then LOldIters := PBKDF2_ITERATIONS;
finally
LQ.Free;
end;
// 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. Prefer
// the ZK verifier path; fall back to plaintext for legacy clients.
LValid := False;
if LOldVerifier <> '' then
LValid := CheckVerifier(LOldVerifier, LStoredHash, LAlgo)
else 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
RecordFailedAccountAttempt(LUser, LIP);
LogAudit(LUserId, 'failed_migrate_kdf', LIP);
TJSONHelper.SendError(AResponse, 401, 'Invalid password');
Exit;
end;
// Step 3: compute the new password hash under the current scheme
// (SHA-256 wrap) at the target iter count. ZK path takes the
// newVerifier (PBKDF2 at the target iters, computed client-side) and
// just wraps it; plaintext path runs PBKDF2 server-side.
if LNewVerifier <> '' then
begin
if not IsValidVerifier(LNewVerifier) then
begin
TJSONHelper.SendError(AResponse, 400, 'Malformed newVerifier');
Exit;
end;
LNewHash := VerifierToStoredHash(LNewVerifier, HASH_ALGO_CURRENT);
end
else
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
// the legacy config (safe to retry next login).
DB.Connection.StartTransaction;
try
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, ' +
' 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
LQ.Free;
end;
LQ := TFDQuery.Create(nil);
try
LQ.Connection := DB.Connection;
LQ.SQL.Text :=
'UPDATE vault_entries ' +
'SET encrypted_password = :ep, iv = :iv, updated_at = CURRENT_TIMESTAMP ' +
'WHERE id = :id AND user_id = :uid';
for I := 0 to LEntries.Count - 1 do
begin
LEntry := LEntries.Items[I] as TJSONObject;
LEntryId := LEntry.GetValue<Integer>('id', 0);
LEncPwd := LEntry.GetValue<string>('encrypted_password', '');
LIv := LEntry.GetValue<string>('iv', '');
if (LEntryId <= 0) or (LEncPwd = '') or (LIv = '') then
raise Exception.CreateFmt('Invalid entry payload at index %d', [I]);
LQ.ParamByName('id').AsInteger := LEntryId;
LQ.ParamByName('uid').AsInteger := LUserId;
LQ.ParamByName('ep').AsString := LEncPwd;
LQ.ParamByName('iv').AsString := LIv;
LQ.ExecSQL;
end;
finally
LQ.Free;
end;
DB.Connection.Commit;
except
DB.Connection.Rollback;
raise;
end;
finally
DB.Unlock;
end;
finally
LBody.Free;
end;
LogAudit(LUserId, Format('migrate_kdf %d->%d', [LOldIters, PBKDF2_ITERATIONS_TARGET]), LIP);
TJSONHelper.SendOK(AResponse, 'Migration complete');
end; end;
// ===== POST /change-master-password ========================================== // ===== POST /change-master-password ==========================================
@@ -852,9 +688,9 @@ end;
// working past the pw change. // working past the pw change.
// - Writes an audit_log entry. // - Writes an audit_log entry.
// //
// The /migrate-kdf endpoint exists for the same "re-encrypt all entries" // This is now the ONLY correct "re-encrypt all entries under a new key" path
// pattern when the master pw stays the same; this endpoint differs by // (the old /migrate-kdf was disabled in §6c-1 for re-encrypting just 1 blob
// rotating the salt + pw too. // of 8). Legacy <600k accounts reach 600k + Argon2id through here.
procedure HandleChangeMasterPassword(ARequest: TIdHTTPRequestInfo; procedure HandleChangeMasterPassword(ARequest: TIdHTTPRequestInfo;
AResponse: TIdHTTPResponseInfo; const AParams: TArray<string>); AResponse: TIdHTTPResponseInfo; const AParams: TArray<string>);
var var