Skip to content

Commit

Permalink
Also update KDF cache in biometric storage if password changes
Browse files Browse the repository at this point in the history
This fixes various bugs including several relating to the uninstallation
 and reinstallation of the app on iOS, particuarlly affecting the
autofill feature becauase that relies on the KDF result since it does
not have the memory capacity to run an argon2 derivation on demand.

Will also fix up slower unlocking on Android if user had changed
their password since we introduced the KDF cache.

Also resets the countdown to the next biometric sign in being required
when a new password is used to unlock the vault.
  • Loading branch information
luckyrat committed Jul 6, 2023
1 parent 425c249 commit 77df3f3
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 8 deletions.
2 changes: 1 addition & 1 deletion ios/KdbxSwift/Sources/KdbxSwift/DatabaseFileManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public class DatabaseFileManager {
database = db

} catch {
fatalError("Unprocessed exception while opening database. Probably hardware failure has corrupted the data on this device.")
fatalError("Unprocessed exception while opening database. Possibly hardware failure has corrupted the data on this device.")
}
return dbFile
}
Expand Down
2 changes: 1 addition & 1 deletion ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,6 @@ SPEC CHECKSUMS:
SwiftyGif: 6c3eafd0ce693cad58bb63d2b2fb9bacb8552780
url_launcher_ios: 08a3dfac5fb39e8759aeb0abbd5d9480f30fc8b4

PODFILE CHECKSUM: 8cb4adfb927a70500d1242c9cf7d4069aa30608d
PODFILE CHECKSUM: 1061c2007eaabd205b946f1b9687990689969f14

COCOAPODS: 1.11.3
19 changes: 15 additions & 4 deletions lib/credentials/quick_unlocker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class QuickUnlocker {
// any existing user's ID and they used to default to emailHashed anyway, this will keep working.
// Maybe one day we could/should rename the user parameter to complete the tidy-up.
Future<QUStatus> initialiseForUser(String user, bool force) async {
l.v('initialiseForUser $user');
if (!force && _currentCreds != null && _currentUser != null && _currentUser == user) {
return QUStatus.credsAvailable;
}
Expand Down Expand Up @@ -134,6 +135,7 @@ class QuickUnlocker {
}

Future<String?> _read(BiometricStorageFile storage) async {
l.v('QU _read');
try {
final contents = await storage.read(
promptInfo: PromptInfo(
Expand All @@ -157,6 +159,7 @@ $stackTrace''');
}

// This is the only place we enable autofill for ios
// maybe do it in more places? one day?
Future<void> _write(BiometricStorageFile storage, String contents) async {
l.v('QU _write');
try {
Expand Down Expand Up @@ -258,6 +261,8 @@ $stackTrace''');
}

ExpiringCachedCredentials? updatedCreds = _currentCreds;
final encodedCreds = base64.encode(creds!.getHash());
final kdfResult = base64.encode(kdfCache.getItemByKey(kdfCacheKey)!);
if (updatedCreds == null) {
if (newUserPassKey?.isEmpty ?? true) {
if (_currentUser != localUserMagicString) {
Expand All @@ -266,16 +271,22 @@ $stackTrace''');
}
newUserPassKey = 'notARealPassword';
}
final encodedCreds = base64.encode(creds!.getHash());
final kdfResult = base64.encode(kdfCache.getItemByKey(kdfCacheKey)!);
updatedCreds = ExpiringCachedCredentials(encodedCreds, kdfCacheKey, kdfResult, newUserPassKey!, expiryTime);
} else {
final encodedCreds = base64.encode(creds!.getHash());
if (encodedCreds == _currentCreds!.kdbxBase64Hash) {
// Important to check more than just the encodedCreds because keychain items in iOS persist after
// reinstallation of the app and if the user picks the same password again for their new empty
// vault, autofill won't work (and the app will be slower in general because the kdf cache won't
// be correctly repopulated at startup).
if (encodedCreds == _currentCreds!.kdbxBase64Hash &&
kdfCacheKey == _currentCreds!.kdbxKdfCacheKey &&
kdfResult == _currentCreds!.kdbxKdfResultBase64) {
l.d('FileCredentials has not changed');
return;
}
updatedCreds.kdbxBase64Hash = encodedCreds;
updatedCreds.kdbxKdfCacheKey = kdfCacheKey;
updatedCreds.kdbxKdfResultBase64 = kdfResult;
updatedCreds.expiry = expiryTime;
}

final storage = await _storageFile();
Expand Down
5 changes: 3 additions & 2 deletions lib/local_vault_repository.dart
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,9 @@ class LocalVaultRepository {
final requireFullPasswordPeriod =
int.tryParse(Settings.getValue<String>('requireFullPasswordPeriod') ?? '60') ?? 60;
l.d('Will require a full password to be entered every $requireFullPasswordPeriod days');
await qu.saveQuickUnlockFileCredentials(credentials,
DateTime.now().add(Duration(days: requireFullPasswordPeriod)).millisecondsSinceEpoch, await kdbx.kdfCacheKey);
// this fails every time cos we don't know the currentuser yet. Remove in 2024 if no unexpected issues crop up.
// await qu.saveQuickUnlockFileCredentials(credentials,
// DateTime.now().add(Duration(days: requireFullPasswordPeriod)).millisecondsSinceEpoch, await kdbx.kdfCacheKey);

final lockedKdbx = LockedVaultFile(
saved,
Expand Down

0 comments on commit 77df3f3

Please sign in to comment.