Skip to content

Commit

Permalink
chore: Not storing the common salt using Storage.
Browse files Browse the repository at this point in the history
All TOTPs are storing their salt, so we just have to use it. This allows to directly clean empty accounts in Firebase.
  • Loading branch information
Skyost committed Jan 3, 2025
1 parent 1e82b95 commit f23577a
Show file tree
Hide file tree
Showing 14 changed files with 107 additions and 178 deletions.
2 changes: 2 additions & 0 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
include: package:flutter_lints/flutter.yaml

analyzer:
errors:
unreachable_switch_default: ignore
plugins:
- custom_lint
exclude:
Expand Down
1 change: 0 additions & 1 deletion lib/i18n/en/error.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
},
"storageMigration(map)": {
"backupError": "An error occurred while doing the backup. Please try again.",
"saltError": "\"Salt\" error while migrating your data. Please try again later.",
"currentStoragePasswordMismatch": "Invalid master password entered.",
"encryptionKeyChangeFailed": "An error occurred while encrypting your data. Please try again later.",
"genericError": "An error occurred while migrating your data. Please try again later."
Expand Down
1 change: 0 additions & 1 deletion lib/i18n/fr/error.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
},
"storageMigration(map)": {
"backupError": "Une erreur est survenue durant la sauvegarde. Veuillez réessayer.",
"saltError": "Erreur de \"Sel\" pendant la migration de vos données. Veuillez réessayer plus tard.",
"currentStoragePasswordMismatch": "Mot de passe maître invalide.",
"encryptionKeyChangeFailed": "Une erreur est survenue pendant le chiffrement de vos données. Veuillez réessayer plus tard.",
"genericError": "Une erreur est survenue pendant la migration de vos données. Veuillez réessayer plus tard."
Expand Down
14 changes: 10 additions & 4 deletions lib/model/backup.dart
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class Backup implements Comparable<Backup> {

CryptoStore cryptoStore = await CryptoStore.fromPassword(password, Salt.fromRawValue(value: base64.decode(jsonData[kSaltKey])));
HmacSecretKey hmacSecretKey = await HmacSecretKey.importRawKey(await cryptoStore.key.exportRawKey(), Hash.sha256);
if(!(await hmacSecretKey.verifyBytes(base64.decode(jsonData[kPasswordSignatureKey]), utf8.encode(password)))) {
if (!(await hmacSecretKey.verifyBytes(base64.decode(jsonData[kPasswordSignatureKey]), utf8.encode(password)))) {
throw _InvalidPasswordException();
}

Expand Down Expand Up @@ -150,7 +150,9 @@ class Backup implements Comparable<Backup> {
file.writeAsString(jsonEncode({
kPasswordSignatureKey: base64.encode(await hmacSecretKey.signBytes(utf8.encode(password))),
kSaltKey: base64.encode(newStore.salt.value),
kTotpsKey: toBackup.map((totp) => totp.toJson()).toList(),
kTotpsKey: [
for (Totp totp in toBackup) totp.toJson(),
],
}));
return const ResultSuccess();
} catch (ex, stacktrace) {
Expand Down Expand Up @@ -194,7 +196,9 @@ class _BackupFileDoesNotExistException implements Exception {
final String path;

/// Creates a new backup file doesn't exist exception instance.
_BackupFileDoesNotExistException({required this.path,});
_BackupFileDoesNotExistException({
required this.path,
});

@override
String toString() => 'Backup file does not exist : "$path"';
Expand All @@ -218,7 +222,9 @@ class _EncryptionError implements Exception {
final String operationName;

/// Creates a new encryption error instance.
_EncryptionError({required this.operationName,});
_EncryptionError({
required this.operationName,
});

@override
String toString() => 'Error while doing $operationName.';
Expand Down
36 changes: 20 additions & 16 deletions lib/model/storage/local.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class Totps extends Table {
final localStorageProvider = Provider.autoDispose<LocalStorage>((ref) {
LocalStorage storage = LocalStorage();
ref.onDispose(storage.close);
ref.cacheFor(const Duration(seconds: 3));
ref.cacheFor(const Duration(seconds: 1));
return storage;
});

Expand Down Expand Up @@ -116,15 +116,28 @@ class LocalStorage extends _$LocalStorage with Storage {
}

@override
Future<List<Totp>> listTotps() async {
List<_DriftTotp> list = await (select(totps)..orderBy([(table) => OrderingTerm(expression: table.issuer)])).get();
return list.map((totp) => totp.asTotp).toList();
Future<List<Totp>> listTotps({int? limit}) async {
List<_DriftTotp> list = await _listDriftTotps(limit: limit);
return [
for (_DriftTotp driftTotp in list) driftTotp.asTotp,
];
}

@override
Future<List<String>> listUuids() async {
List<_DriftTotp> list = await (select(totps)..orderBy([(table) => OrderingTerm(expression: table.issuer)])).get();
return list.map((totp) => totp.uuid).toList();
Future<List<String>> listUuids({int? limit}) async {
List<_DriftTotp> list = await _listDriftTotps(limit: limit);
return [
for (_DriftTotp driftTotp in list) driftTotp.uuid,
];
}

/// List the Drift TOTPs.
Future<List<_DriftTotp>> _listDriftTotps({int? limit}) async {
SimpleSelectStatement<$TotpsTable, _DriftTotp> query = select(totps)..orderBy([(table) => OrderingTerm(expression: table.issuer)]);
if (limit != null) {
query = query..limit(limit);
}
return await query.get();
}

@override
Expand All @@ -133,15 +146,6 @@ class LocalStorage extends _$LocalStorage with Storage {
batch.insertAll(totps, newTotps.map((totp) => totp.asDriftTotp));
});

@override
Future<Salt?> readSecretsSalt() async => await Salt.readFromLocalStorage();

@override
Future<void> saveSecretsSalt(Salt salt) => salt.saveToLocalStorage();

@override
Future<void> deleteSecretsSalt() => Salt.deleteFromLocalStorage();

@override
Future<void> onStorageTypeChanged({bool close = true}) async {
await clearTotps();
Expand Down
65 changes: 16 additions & 49 deletions lib/model/storage/online.dart
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import 'dart:async';
import 'dart:typed_data';

import 'package:cloud_firestore/cloud_firestore.dart';
import 'package:firebase_core/firebase_core.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:open_authenticator/app.dart';
import 'package:open_authenticator/model/authentication/firebase_authentication.dart';
import 'package:open_authenticator/model/authentication/state.dart';
import 'package:open_authenticator/model/crypto.dart';
import 'package:open_authenticator/model/storage/storage.dart';
import 'package:open_authenticator/model/storage/type.dart';
import 'package:open_authenticator/model/totp/json.dart';
Expand All @@ -33,9 +30,6 @@ class OnlineStorage with Storage {
/// The last updated key.
static const String _kUpdatedKey = 'updated';

/// The salt key.
static const String _kSaltKey = 'salt';

/// The user id.
final String? _userId;

Expand Down Expand Up @@ -108,10 +102,10 @@ class OnlineStorage with Storage {
}

@override
Future<List<Totp>> listTotps({GetOptions? getOptions}) async {
QuerySnapshot result = await _totpsCollection.orderBy(Totp.kIssuerKey).get(getOptions);
Future<List<Totp>> listTotps({int? limit, GetOptions? getOptions}) async {
List<QueryDocumentSnapshot> docs = await _listTotpDocs(limit: limit, getOptions: getOptions);
List<Totp> totps = [];
for (QueryDocumentSnapshot doc in result.docs) {
for (QueryDocumentSnapshot doc in docs) {
Totp? totp = _FirestoreTotp.fromFirestore(doc);
if (totp != null) {
totps.add(totp);
Expand All @@ -121,10 +115,10 @@ class OnlineStorage with Storage {
}

@override
Future<List<String>> listUuids() async {
QuerySnapshot result = await _totpsCollection.orderBy(Totp.kIssuerKey).get();
Future<List<String>> listUuids({int? limit}) async {
List<QueryDocumentSnapshot> docs = await _listTotpDocs(limit: limit);
List<String> uuids = [];
for (QueryDocumentSnapshot snapshot in result.docs) {
for (QueryDocumentSnapshot snapshot in docs) {
Object? data = snapshot.data();
if (data is Map<String, Object?> && data.containsKey(Totp.kUuidKey)) {
uuids.add(data[Totp.kUuidKey]!.toString());
Expand All @@ -133,6 +127,16 @@ class OnlineStorage with Storage {
return uuids;
}

/// List the TOTPs documents.
Future<List<QueryDocumentSnapshot>> _listTotpDocs({int? limit, GetOptions? getOptions}) async {
Query query = _totpsCollection.orderBy(Totp.kIssuerKey);
if (limit != null) {
query = query.limit(limit);
}
QuerySnapshot result = await query.get(getOptions);
return result.docs;
}

@override
Future<void> replaceTotps(List<Totp> newTotps) async {
CollectionReference? collection = _totpsCollection;
Expand All @@ -147,43 +151,6 @@ class OnlineStorage with Storage {
await batch.commit();
}

@override
Future<Salt?> readSecretsSalt() async {
DocumentSnapshot<Map<String, dynamic>> userDoc = await _userDocument.get();
if (!userDoc.exists) {
return null;
}
List salt = (userDoc.data() as Map<String, dynamic>)[_kSaltKey];
return Salt.fromRawValue(value: Uint8List.fromList(salt.cast<int>()));
}

@override
Future<void> saveSecretsSalt(Salt salt) async {
DocumentReference<Map<String, dynamic>> userDoc = _userDocument;
await userDoc.set(
{
_kSaltKey: salt.value,
_kUpdatedKey: FieldValue.serverTimestamp(),
},
SetOptions(merge: true),
);
}

@override
Future<void> deleteSecretsSalt() async {
DocumentSnapshot<Map<String, dynamic>> userDoc = await _userDocument.get();
if (!userDoc.exists) {
return;
}
Map<String, dynamic> data = userDoc.data() as Map<String, dynamic>;
data.remove(_kSaltKey);
if (data.isEmpty || (data.keys.length == 1 && data.keys.first == _kUpdatedKey)) {
await _userDocument.delete();
} else {
await _userDocument.set(data);
}
}

@override
Future<void> close() async {
_cancelSubscription();
Expand Down
81 changes: 21 additions & 60 deletions lib/model/storage/storage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ class StorageNotifier extends AutoDisposeAsyncNotifier<Storage> {
return const ResultSuccess();
}

Salt? oldSalt = await currentStorage.readSecretsSalt();
if (oldSalt == null) {
throw SaltError();
}

if (backupPassword != null) {
Result<Backup> backupResult = await ref.read(backupStoreProvider.notifier).doBackup(backupPassword);
if (backupResult is! ResultSuccess) {
Expand All @@ -74,17 +69,22 @@ class StorageNotifier extends AutoDisposeAsyncNotifier<Storage> {
}
}

CryptoStore currentCryptoStore = await CryptoStore.fromPassword(masterPassword, oldSalt);
Salt? salt = await newStorage.readSecretsSalt();
List<Totp> totps = await currentStorage.listTotps();
List<Totp> currentTotps = await currentStorage.listTotps();
Totp? firstTotp = (await newStorage.listTotps(limit: 1)).firstOrNull;
List<Totp> toAdd = [];
if (salt == null) {
await newStorage.saveSecretsSalt(oldSalt);
toAdd.addAll(totps);
if (firstTotp == null) {
toAdd.addAll(currentTotps);
} else {
CryptoStore newCryptoStore = await CryptoStore.fromPassword(masterPassword, salt);
for (Totp totp in totps) {
DecryptedTotp? decryptedTotp = await totp.changeEncryptionKey(currentCryptoStore, newCryptoStore);
CryptoStore? currentCryptoStore = ref.read(cryptoStoreProvider).value;
CryptoStore newCryptoStore = await CryptoStore.fromPassword(masterPassword, firstTotp.encryptedData.encryptionSalt);
for (Totp totp in currentTotps) {
CryptoStore oldCryptoStore = currentCryptoStore?.salt == totp.encryptedData.encryptionSalt
? currentCryptoStore!
: await CryptoStore.fromPassword(
masterPassword,
totp.encryptedData.encryptionSalt,
);
DecryptedTotp? decryptedTotp = await totp.changeEncryptionKey(oldCryptoStore, newCryptoStore);
toAdd.add(decryptedTotp ?? totp);
}
await ref.read(cryptoStoreProvider.notifier).saveAndUse(newCryptoStore);
Expand Down Expand Up @@ -135,7 +135,7 @@ class GenericMigrationError extends StorageMigrationException {
/// Whether we should ask for a different [StorageMigrationDeletedTotpPolicy].
class ShouldAskForDifferentDeletedTotpPolicyException extends StorageMigrationException {
/// The error code.
static const String _code = 'genericError';
static const String _code = 'shouldAskForDifferentDeletedTotpPolicy';

/// Creates a new storage migration policy exception instance.
ShouldAskForDifferentDeletedTotpPolicyException()
Expand All @@ -144,28 +144,13 @@ class ShouldAskForDifferentDeletedTotpPolicyException extends StorageMigrationEx
);

@override
String toString() => 'StorageMigrationDeletedTotpPolicy error';
}

/// When there is a salt error.
class SaltError extends StorageMigrationException {
/// The error code.
static const String _code = 'genericError';

/// Creates a new salt error instance.
SaltError()
: super(
code: _code,
);

@override
String toString() => 'Salt error';
String toString() => 'Another deleted TOTP policy should be used';
}

/// When we haven't succeeded to do the asked backup.
class BackupException extends StorageMigrationException {
/// The error code.
static const String _code = 'genericError';
static const String _code = 'backupError';

/// Creates a new backup exception instance.
BackupException()
Expand All @@ -180,7 +165,7 @@ class BackupException extends StorageMigrationException {
/// When the provided password don't match the one that has been using on the old storage.
class CurrentStoragePasswordMismatchException extends StorageMigrationException {
/// The error code.
static const String _code = 'genericError';
static const String _code = 'currentStoragePasswordMismatch';

/// Creates a new current storage password mismatch exception instance.
CurrentStoragePasswordMismatchException()
Expand All @@ -192,25 +177,10 @@ class CurrentStoragePasswordMismatchException extends StorageMigrationException
String toString() => 'Current storage password is incorrect';
}

/// When the provided password don't match the one that has been using on the new storage.
class NewStoragePasswordMismatchException extends StorageMigrationException {
/// The error code.
static const String _code = 'genericError';

/// Creates a new new storage password mismatch exception instance.
NewStoragePasswordMismatchException()
: super(
code: _code,
);

@override
String toString() => 'New storage password is incorrect';
}

/// When there is an error while trying to change the encryption key of the old storage.
class EncryptionKeyChangeFailedError extends StorageMigrationException {
/// The error code.
static const String _code = 'genericError';
static const String _code = 'encryptionKeyChangeFailed';

/// Creates a new encryption key change error instance.
EncryptionKeyChangeFailedError()
Expand Down Expand Up @@ -264,26 +234,17 @@ mixin Storage {
Future<Totp?> getTotp(String uuid);

/// Lists all TOTPs.
Future<List<Totp>> listTotps();
Future<List<Totp>> listTotps({int? limit});

/// Lists all TOTPs UUID.
Future<List<String>> listUuids();
Future<List<String>> listUuids({int? limit});

/// Replace all current TOTPs by [newTotps].
Future<void> replaceTotps(List<Totp> newTotps) async {
await clearTotps();
await addTotps(newTotps);
}

/// Loads the salt that allows to encrypt secrets.
Future<Salt?> readSecretsSalt();

/// Saves the salt that allows to encrypt secrets.
Future<void> saveSecretsSalt(Salt salt);

/// Deletes the salt that allows to encrypt secrets.
Future<void> deleteSecretsSalt();

/// Closes this storage instance.
Future<void> close();

Expand Down
Loading

0 comments on commit f23577a

Please sign in to comment.