From 9a32fe88811a839fc4f52c423b833cc9edc1c978 Mon Sep 17 00:00:00 2001 From: r-zakarya <82443831+r-zakarya@users.noreply.github.com> Date: Fri, 21 Aug 2026 14:54:05 +0100 Subject: [PATCH] perf(android): the derived key is kept in the cache Opening from the cache derives nothing, so the key learned last session was gone and the FIRST save of every session paid the 4.5 seconds again. It now travels in the sealed blob, next to the plaintext vault and to the password that opens the container - that neighbourhood is what makes it acceptable: it exposes nothing the file does not already hold. The three parts go together or not at all. A key without its salt does not say what it fits, which makes it useless, or worse, used where it does not belong. A cache written by an earlier version carries none: LoadVault returns nil, one derivation happens, and the next write stores it. A freshly registered vault still derives once - EncryptContainer does not hand back the key it just derived. The plan records what the desktop confirmed: the two entries created on the phone imported with nothing to correct. --- android-client/PLAN.md | 35 +++++++++++++++++++++++++ android-client/PMA.Store.pas | 51 +++++++++++++++++++++++++++++++++--- mainApk.pas | 29 +++++++++++++------- 3 files changed, 103 insertions(+), 12 deletions(-) diff --git a/android-client/PLAN.md b/android-client/PLAN.md index 665f824..f7e7dcf 100644 --- a/android-client/PLAN.md +++ b/android-client/PLAN.md @@ -665,6 +665,41 @@ téléphone qu'on voudra ensuite ouvrir sur le poste demandera un import. ajoute un dépôt. C'est là que la fusion revient, et elle a été évitée jusqu'ici pour de bonnes raisons. +### Vérifié : le poste reprend ce que le téléphone crée — 2026-08-21 + +Deux entrées créées sur le téléphone, puis synchronisées depuis le poste : +importées sans rien à corriger. C'est ce qui valide `AddEntry` — la forme de +l'entrée, l'uuid, les horodatages — autrement que par relecture du code. + +### Le coût de la dérivation, et où il est parti + +Ajouter une entrée prenait cinq à sept secondes. Le journal a tranché : +`key derived in 4395 ms`. Chaque enregistrement redérivait le mot de passe +maître pour retrouver la clé qui venait de servir. + +Deux temps : + +1. **La clé vit le temps de la session.** Réutilisée seulement si le conteneur + porte le même sel et le même nombre d'itérations — le poste peut avoir + rechiffré le sien avec un sel neuf, et une clé qui ne convient pas échoue + sur la marque de GCM en accusant un mot de passe qui serait bon. +2. **Elle est rangée dans le cache**, à côté du coffre en clair et du mot de + passe qui ouvre le conteneur. C'est ce voisinage qui rend l'ajout + acceptable : elle n'expose rien que ce fichier ne détienne déjà. Sans elle, + le *premier* enregistrement de chaque session repayait les 4,5 s, puisque + ouvrir depuis le cache ne dérive rien. + +Un cache écrit par une version antérieure ne porte pas de clé : `LoadVault` +rend `nil`, on redérive une fois, et l'écriture suivante la range. + +Reste dérivant une fois : un coffre **fraîchement inscrit** — `EncryptContainer` +ne rend pas la clé qu'il vient de dériver. + +**Pas mesuré sur l'appareil** : la ligne `opened with the key already in hand`. +Une lecture répond 304 et ne déchiffre rien ; un verrouillage jette la clé. +Seul un enregistrement réel la montre, et le coffre de test publie vers un +vrai dépôt. + ### Ce qui devient facultatif Sans dépôt configuré, *Sync* n'a rien à faire : les réglages du serveur diff --git a/android-client/PMA.Store.pas b/android-client/PMA.Store.pas index 174b2b4..de38dc3 100644 --- a/android-client/PMA.Store.pas +++ b/android-client/PMA.Store.pas @@ -86,9 +86,19 @@ function HasVault: Boolean; /// APin non vide : le cache est scelle sous ce code (VER_PIN), et ARequireAuth /// est alors ignore - les deux protections ne se cumulent pas, et celle du code /// est la seule qui survive a un changement de verrouillage d'ecran. +/// AKey/ASalt/AIterations : la cle DEJA DERIVEE du conteneur, rangee avec le +/// reste. Elle epargne 600 000 iterations - quatre a six secondes - au premier +/// enregistrement de chaque session, qui autrement rederivait un mot de passe +/// que ce meme fichier detient deja en clair juste a cote. C'est ce voisinage +/// qui rend l'ajout acceptable : le blob contient le coffre entier en clair et +/// le mot de passe qui ouvre le conteneur, donc la cle n'y ajoute rien. +/// +/// Elle ne vaut que pour un conteneur portant CE sel et CE nombre +/// d'iterations ; les deux voyagent avec, et le lecteur verifie. procedure SaveVault(const APlainJson, AETag, ASyncPwd: string; ARequireAuth: Boolean = False; AFreshKey: Boolean = False; - const APin: string = ''); + const APin: string = ''; const AKey: TBytes = nil; + const ASalt: TBytes = nil; AIterations: Integer = 0); /// Le fichier local est-il scelle sous un code d'application ? Lu dans /// l'en-tete, sans rien dechiffrer. @@ -103,8 +113,12 @@ function VaultNeedsAuth: Boolean; /// ASyncPwd est vide pour un fichier ecrit avant que le mot de passe y soit /// range : l'appelant redemande alors, et le prochain enregistrement le pose. function LoadVault(out APlainJson, AETag, ASyncPwd: string; + out AKey, ASalt: TBytes; out AIterations: Integer; const APin: string = ''): Boolean; +/// AKey rend nil quand le cache a ete ecrit sans cle - un fichier d'avant, ou +/// une ecriture qui n'en tenait pas. L'appelant rederive alors, comme avant. + /// Pourquoi la derniere lecture a echoue. Vide apres un succes. /// /// LoadVault N'EFFACE RIEN de lui-meme. Un cache detruit par une routine de @@ -159,7 +173,9 @@ uses // derive sa cle exactement comme le conteneur de sync, avec la meme // implementation eprouvee contre le vecteur de test. Androidapi.Jni, Androidapi.JNIBridge, Androidapi.JNI.JavaTypes, Androidapi.Helpers, - PMA.Crypto; + // System.NetEncoding : la cle derivee voyage en base64 dans le blob, comme + // tout ce que le conteneur transporte. + System.NetEncoding, PMA.Crypto; {$ENDIF} const @@ -825,7 +841,8 @@ begin end; procedure SaveVault(const APlainJson, AETag, ASyncPwd: string; - ARequireAuth, AFreshKey: Boolean; const APin: string); + ARequireAuth, AFreshKey: Boolean; const APin: string; + const AKey, ASalt: TBytes; AIterations: Integer); {$IF DEFINED(ANDROID)} var LWrap: TJSONObject; @@ -858,6 +875,14 @@ begin LWrap.AddPair('etag', AETag); LWrap.AddPair('pwd', ASyncPwd); LWrap.AddPair('vault', APlainJson); + // Les trois ensemble ou aucun : une cle sans son sel ne dit pas a quoi + // elle convient, et serait donc inutilisable - ou pire, utilisee a tort. + if (AKey <> nil) and (ASalt <> nil) and (AIterations > 0) then + begin + LWrap.AddPair('kdf_key', TNetEncoding.Base64.EncodeBytesToString(AKey)); + LWrap.AddPair('kdf_salt', TNetEncoding.Base64.EncodeBytesToString(ASalt)); + LWrap.AddPair('kdf_iterations', TJSONNumber.Create(AIterations)); + end; LPlain := TEncoding.UTF8.GetBytes(LWrap.ToJSON); finally LWrap.Free; @@ -987,6 +1012,7 @@ end; {$ENDIF} function LoadVault(out APlainJson, AETag, ASyncPwd: string; + out AKey, ASalt: TBytes; out AIterations: Integer; const APin: string = ''): Boolean; {$IF DEFINED(ANDROID)} var @@ -1000,6 +1026,9 @@ begin APlainJson := ''; AETag := ''; ASyncPwd := ''; + AKey := nil; + ASalt := nil; + AIterations := 0; FLastStoreError := ''; if not TFile.Exists(VaultPath) then Exit; @@ -1098,6 +1127,22 @@ begin AETag := LWrap.GetValue('etag', ''); ASyncPwd := LWrap.GetValue('pwd', ''); APlainJson := LWrap.GetValue('vault', ''); + // Absents d'un cache ecrit avant que la cle y soit rangee : on rend nil, + // et l'appelant rederive comme il le faisait. + AIterations := LWrap.GetValue('kdf_iterations', 0); + if AIterations > 0 then + begin + AKey := TNetEncoding.Base64.DecodeStringToBytes( + LWrap.GetValue('kdf_key', '')); + ASalt := TNetEncoding.Base64.DecodeStringToBytes( + LWrap.GetValue('kdf_salt', '')); + end; + if (AKey = nil) or (ASalt = nil) then + begin + AKey := nil; + ASalt := nil; + AIterations := 0; + end; Result := APlainJson <> ''; finally LWrap.Free; diff --git a/mainApk.pas b/mainApk.pas index 5206979..b548854 100644 --- a/mainApk.pas +++ b/mainApk.pas @@ -717,7 +717,8 @@ begin Log(Format('save: re-encrypt in %d ms', [LSw.ElapsedMilliseconds])); SaveLocalContainer(LBody); try - SaveVault(LNew, FETag, FSyncPwd, BiometricAvailable, False, FPin); + SaveVault(LNew, FETag, FSyncPwd, BiometricAvailable, False, FPin, + FKey, FKeySalt, FKeyIters); except on E: Exception do Log(' cache not written: ' + E.Message); end; @@ -768,7 +769,8 @@ begin // prochaine ouverture pour retrouver ce qu'on vient d'ecrire. FETag := LPut.ETag; try - SaveVault(LNew, FETag, FSyncPwd, BiometricAvailable, False, FPin); + SaveVault(LNew, FETag, FSyncPwd, BiometricAvailable, False, FPin, + FKey, FKeySalt, FKeyIters); except on E: Exception do Log(' cache not written: ' + E.Message); end; @@ -1147,7 +1149,8 @@ begin // Pas d'ETag : rien n'a ete publie. Le cache s'ecrit quand meme, sinon // la prochaine ouverture redemanderait six secondes de derivation pour // un coffre vide. - SaveVault(LPlain, '', APwd, BiometricAvailable, False, FPin); + SaveVault(LPlain, '', APwd, BiometricAvailable, False, FPin, + FKey, FKeySalt, FKeyIters); TThread.Queue(nil, procedure begin @@ -1217,7 +1220,8 @@ begin TThread.Queue(nil, procedure begin Log('Vault belongs to ' + LOwner + ', not ' + AUser); end); - SaveVault(LPlain, FETag, APwd, BiometricAvailable, False, FPin); + SaveVault(LPlain, FETag, APwd, BiometricAvailable, False, FPin, + FKey, FKeySalt, FKeyIters); TThread.Queue(nil, procedure begin @@ -1252,7 +1256,10 @@ begin // scelle, la mise en forme vers la page, ou le rafraichissement de fond qui // suit - sinon on cherche au hasard. LSw := TStopwatch.StartNew; - if LoadVault(LPlain, LETag, LPwd, FPin) then + // LA CLE REVIENT AVEC LE CACHE, et c'est tout l'objet de l'y avoir mise : + // ouvrir depuis le cache ne derive rien, donc sans cela le PREMIER + // enregistrement de chaque session repayait les quatre secondes et demie. + if LoadVault(LPlain, LETag, LPwd, FKey, FKeySalt, FKeyIters, FPin) then begin Log('Vault opened from cache in ' + LSw.ElapsedMilliseconds.ToString + ' ms'); FETag := LETag; @@ -1284,7 +1291,8 @@ begin if (FPin <> '') and (not VaultNeedsPin) then begin try - SaveVault(LPlain, LETag, LPwd, False, False, FPin); + SaveVault(LPlain, LETag, LPwd, False, False, FPin, + FKey, FKeySalt, FKeyIters); if VaultNeedsPin then Log('Cache re-sealed under the unlock code') else @@ -1300,7 +1308,8 @@ begin // protection a tres probablement invalide l'ancienne, et le // chiffrement ne saurait pas le voir - on scellerait un fichier // impossible a rouvrir. - SaveVault(LPlain, LETag, LPwd, True, True, FPin); + SaveVault(LPlain, LETag, LPwd, True, True, FPin, + FKey, FKeySalt, FKeyIters); if VaultNeedsAuth then Log('Cache re-sealed under authentication') else @@ -1503,7 +1512,8 @@ begin begin // Le coffre local d'abord : s'il est la, on connait deja l'ETag et on // evite les 4 secondes de derivation quand rien n'a change. - if (FETag = '') and LoadVault(LCached, LCachedETag, LCachedPwd, FPin) then + if (FETag = '') and LoadVault(LCached, LCachedETag, LCachedPwd, + FKey, FKeySalt, FKeyIters, FPin) then FETag := LCachedETag; LClient := TSyncClient.Create(AUrl, ATok); @@ -1650,7 +1660,8 @@ begin try // Le mot de passe part avec le coffre : c'est lui qui permettra au // prochain lancement de rafraichir sans rien demander. - SaveVault(LPlain, FETag, APwd, LCanAuth, False, FPin); + SaveVault(LPlain, FETag, APwd, LCanAuth, False, FPin, + FKey, FKeySalt, FKeyIters); Log(' cache now holds sha: ' + FETag); if VaultNeedsAuth then Log(' cache written - protected by biometrics/PIN')