fix(app): the short reading's nits on the device
- Deleting an identified account whose security can't be read (the instance out of reach) says so and deletes nothing; it went on to a deletion by the session alone, refused for an account with a password, and the person saw a spinner, then nothing. A refusal still leaves the session's own deletion. - InstanceChoice's sharesStorage is required, so no app leaves it out and removes a folder another instance uses. - A PasswordUnlock forgotten is spent: its secrets read again fail (StateError) rather than make a proof or a wrapping of zeros. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tizian 「ティツィアーン」 <raisondetredev@pm.me>
This commit is contained in:
parent
7ac6eb75b4
commit
012f2375a5
10 changed files with 106 additions and 21 deletions
|
|
@ -41,6 +41,7 @@ void main() {
|
||||||
store,
|
store,
|
||||||
builtIn: builtIn,
|
builtIn: builtIn,
|
||||||
forget: (instance) async => forgotten.add(instance),
|
forget: (instance) async => forgotten.add(instance),
|
||||||
|
sharesStorage: (a, b) => false,
|
||||||
);
|
);
|
||||||
await choice.load(usedBefore: usedBefore);
|
await choice.load(usedBefore: usedBefore);
|
||||||
await tester.pumpWidget(
|
await tester.pumpWidget(
|
||||||
|
|
|
||||||
|
|
@ -36,6 +36,7 @@ void main() {
|
||||||
store,
|
store,
|
||||||
builtIn: builtIn,
|
builtIn: builtIn,
|
||||||
forget: (instance) async {},
|
forget: (instance) async {},
|
||||||
|
sharesStorage: (a, b) => false,
|
||||||
);
|
);
|
||||||
await choice.load(usedBefore: usedBefore);
|
await choice.load(usedBefore: usedBefore);
|
||||||
await signedOut.write(signedOut: wasSignedOut);
|
await signedOut.write(signedOut: wasSignedOut);
|
||||||
|
|
|
||||||
|
|
@ -132,13 +132,23 @@ class const AccountTiles({
|
||||||
try {
|
try {
|
||||||
security = identified ? await passwords.security() : null;
|
security = identified ? await passwords.security() : null;
|
||||||
} on Object catch (error) {
|
} on Object catch (error) {
|
||||||
// Unreachable, or anything else (the reading of the fixes: a failure
|
if (error is PasswordFailed && error.problem != PasswordProblem.failed) {
|
||||||
// that isn't a refusal escaped, and the tap did nothing): deleting
|
// The instance refused to say (no passwords offered there): the
|
||||||
// goes on, and fails as the session bloc says if it must.
|
// session's own deletion, which the instance checks.
|
||||||
|
session.add(const SessionEventDeleteRequested());
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Out of reach, or anything else: said so, nothing deleted (the short
|
||||||
|
// reading: a deletion by the session alone, refused for an account
|
||||||
|
// with a password, showed a spinner, then nothing).
|
||||||
if (error is! PasswordFailed) {
|
if (error is! PasswordFailed) {
|
||||||
log('Reading the account failed: ${error.runtimeType}', name: 'Auth');
|
log('Reading the account failed: ${error.runtimeType}', name: 'Auth');
|
||||||
}
|
}
|
||||||
session.add(const SessionEventDeleteRequested());
|
if (context.mounted) {
|
||||||
|
ScaffoldMessenger.maybeOf(
|
||||||
|
context,
|
||||||
|
)?.showSnackBar(SnackBar(content: Text(l10n.passwordFailed)));
|
||||||
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (security == null || !security.hasPassword) {
|
if (security == null || !security.hasPassword) {
|
||||||
|
|
|
||||||
|
|
@ -25,7 +25,9 @@ class InstanceChoice(
|
||||||
|
|
||||||
/// Whether the device keeps two instances in one place
|
/// Whether the device keeps two instances in one place
|
||||||
/// (`InstanceStorage.sharesWith`): such addresses are one account here.
|
/// (`InstanceStorage.sharesWith`): such addresses are one account here.
|
||||||
final bool Function(Instance a, Instance b) sharesStorage = _keptApart,
|
/// Required, so no app forgets it and removes a folder another instance
|
||||||
|
/// uses (the short reading).
|
||||||
|
required final bool Function(Instance a, Instance b) sharesStorage,
|
||||||
}) extends ChangeNotifier {
|
}) extends ChangeNotifier {
|
||||||
Instance? _current;
|
Instance? _current;
|
||||||
var _known = const <Instance>[];
|
var _known = const <Instance>[];
|
||||||
|
|
@ -112,5 +114,3 @@ class InstanceChoice(
|
||||||
notifyListeners();
|
notifyListeners();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
bool _keptApart(Instance a, Instance b) => false;
|
|
||||||
|
|
|
||||||
|
|
@ -7,24 +7,43 @@ import 'package:halkyon_base_client/srp.dart' as srp;
|
||||||
/// What a password unlocks on the device (ADR 0017), for one account's salt
|
/// What a password unlocks on the device (ADR 0017), for one account's salt
|
||||||
/// and costs: SRP's private value and the key that wraps the account's
|
/// and costs: SRP's private value and the key that wraps the account's
|
||||||
/// private keys, kept only while a sign-in, reset or change needs them.
|
/// private keys, kept only while a sign-in, reset or change needs them.
|
||||||
final class const PasswordUnlock({
|
final class PasswordUnlock({
|
||||||
/// The salt and costs they were derived with.
|
/// The salt and costs they were derived with.
|
||||||
required final Uint8List salt,
|
required final Uint8List salt,
|
||||||
required final protocol.PasswordCosts costs,
|
required final protocol.PasswordCosts costs,
|
||||||
|
|
||||||
/// SRP's x, 32 bytes, big-endian.
|
/// SRP's x, 32 bytes, big-endian ([srpSecret]).
|
||||||
required final Uint8List srpSecret,
|
required final Uint8List _srpSecret,
|
||||||
|
|
||||||
/// The key-encryption key, 32 bytes.
|
/// The key-encryption key, 32 bytes ([keyEncryptionKey]).
|
||||||
required final Uint8List keyEncryptionKey,
|
required final Uint8List _keyEncryptionKey,
|
||||||
}) {
|
}) {
|
||||||
|
var _forgotten = false;
|
||||||
|
|
||||||
|
/// SRP's x, 32 bytes, big-endian; a [StateError] once [forget] wiped it.
|
||||||
|
Uint8List get srpSecret => _unlessForgotten(_srpSecret);
|
||||||
|
|
||||||
|
/// The key-encryption key, 32 bytes; a [StateError] once [forget] wiped
|
||||||
|
/// it.
|
||||||
|
Uint8List get keyEncryptionKey => _unlessForgotten(_keyEncryptionKey);
|
||||||
|
|
||||||
|
/// Whether [forget] wiped them.
|
||||||
|
bool get isForgotten => _forgotten;
|
||||||
|
|
||||||
/// Zeroes [srpSecret] and [keyEncryptionKey] once nothing needs them (the
|
/// Zeroes [srpSecret] and [keyEncryptionKey] once nothing needs them (the
|
||||||
/// reading of the fixes): Dart's own bytes, which no collector wipes.
|
/// reading of the fixes): Dart's own bytes, which no collector wipes. The
|
||||||
|
/// unlock is spent then (the short reading): read again, they fail
|
||||||
|
/// rather than make a proof or a wrapping of zeros.
|
||||||
void forget() {
|
void forget() {
|
||||||
srpSecret.fillRange(0, srpSecret.length, 0);
|
_srpSecret.fillRange(0, _srpSecret.length, 0);
|
||||||
keyEncryptionKey.fillRange(0, keyEncryptionKey.length, 0);
|
_keyEncryptionKey.fillRange(0, _keyEncryptionKey.length, 0);
|
||||||
|
_forgotten = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Uint8List _unlessForgotten(Uint8List secret) => _forgotten
|
||||||
|
? throw StateError('What the password unlocked was forgotten')
|
||||||
|
: secret;
|
||||||
|
|
||||||
/// Whether these were derived for [salt] and [costs].
|
/// Whether these were derived for [salt] and [costs].
|
||||||
bool fits(Uint8List salt, protocol.PasswordCosts costs) =>
|
bool fits(Uint8List salt, protocol.PasswordCosts costs) =>
|
||||||
_same(this.salt, salt) &&
|
_same(this.salt, salt) &&
|
||||||
|
|
|
||||||
|
|
@ -60,6 +60,7 @@ void main() {
|
||||||
store,
|
store,
|
||||||
builtIn: builtIn,
|
builtIn: builtIn,
|
||||||
forget: (instance) async => forgotten.add(instance),
|
forget: (instance) async => forgotten.add(instance),
|
||||||
|
sharesStorage: (a, b) => false,
|
||||||
);
|
);
|
||||||
await choice.load(usedBefore: usedBefore);
|
await choice.load(usedBefore: usedBefore);
|
||||||
await tester.pumpWidget(
|
await tester.pumpWidget(
|
||||||
|
|
@ -322,6 +323,7 @@ void main() {
|
||||||
store,
|
store,
|
||||||
builtIn: builtIn,
|
builtIn: builtIn,
|
||||||
forget: (instance) async {},
|
forget: (instance) async {},
|
||||||
|
sharesStorage: (a, b) => false,
|
||||||
);
|
);
|
||||||
await choice.load(usedBefore: false);
|
await choice.load(usedBefore: false);
|
||||||
var attempts = 0;
|
var attempts = 0;
|
||||||
|
|
|
||||||
|
|
@ -95,6 +95,7 @@ void main() {
|
||||||
MemoryInstanceStore((current: instance, known: [instance])),
|
MemoryInstanceStore((current: instance, known: [instance])),
|
||||||
builtIn: instance,
|
builtIn: instance,
|
||||||
forget: (instance) async {},
|
forget: (instance) async {},
|
||||||
|
sharesStorage: (a, b) => false,
|
||||||
);
|
);
|
||||||
await choice.load(usedBefore: false);
|
await choice.load(usedBefore: false);
|
||||||
await start(tester, choice: choice);
|
await start(tester, choice: choice);
|
||||||
|
|
@ -174,6 +175,31 @@ void main() {
|
||||||
expect(find.text('Signed out'), findsOneWidget);
|
expect(find.text('Signed out'), findsOneWidget);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
testWidgets('deleting an identified account while the instance is out of '
|
||||||
|
'reach says so, and deletes nothing (the short reading)', (tester) async {
|
||||||
|
gateway = FakeAuthGateway(
|
||||||
|
status: const SignedIn(authUserId: 'github-1', isIdentified: true),
|
||||||
|
);
|
||||||
|
await start(tester);
|
||||||
|
dependencies.passwordGateway.failing = const PasswordFailed(
|
||||||
|
PasswordProblem.failed,
|
||||||
|
);
|
||||||
|
|
||||||
|
await tester.tap(find.text('Delete account'));
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
await tester.tap(find.text('Delete account').last);
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
|
||||||
|
expect(
|
||||||
|
find.text(
|
||||||
|
"Couldn't reach the instance. Check the connection and try again.",
|
||||||
|
),
|
||||||
|
findsOneWidget,
|
||||||
|
);
|
||||||
|
expect(gateway.accountDeletions, 0);
|
||||||
|
expect(find.text('App of github-1'), findsOneWidget);
|
||||||
|
});
|
||||||
|
|
||||||
testWidgets('deleting an account whose setup never finished asks for the '
|
testWidgets('deleting an account whose setup never finished asks for the '
|
||||||
'password and a code by mail', (tester) async {
|
'password and a code by mail', (tester) async {
|
||||||
// The first person the fake password gateway makes.
|
// The first person the fake password gateway makes.
|
||||||
|
|
|
||||||
|
|
@ -93,6 +93,7 @@ void main() {
|
||||||
store,
|
store,
|
||||||
builtIn: insecure,
|
builtIn: insecure,
|
||||||
forget: (instance) async {},
|
forget: (instance) async {},
|
||||||
|
sharesStorage: (a, b) => false,
|
||||||
);
|
);
|
||||||
expect(development.allows(builtIn), isTrue);
|
expect(development.allows(builtIn), isTrue);
|
||||||
expect(
|
expect(
|
||||||
|
|
@ -110,6 +111,7 @@ void main() {
|
||||||
builtIn: insecure,
|
builtIn: insecure,
|
||||||
forget: (instance) async {},
|
forget: (instance) async {},
|
||||||
allowsInsecureBuiltIn: false,
|
allowsInsecureBuiltIn: false,
|
||||||
|
sharesStorage: (a, b) => false,
|
||||||
);
|
);
|
||||||
expect(release.allows(insecure), isFalse);
|
expect(release.allows(insecure), isFalse);
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -53,9 +53,8 @@ void main() {
|
||||||
finished: finished,
|
finished: finished,
|
||||||
);
|
);
|
||||||
|
|
||||||
bool wiped(PasswordUnlock unlock) =>
|
/// Whether [unlock] was wiped, and so spent (the short reading).
|
||||||
unlock.srpSecret.every((byte) => byte == 0) &&
|
bool wiped(PasswordUnlock unlock) => unlock.isForgotten;
|
||||||
unlock.keyEncryptionKey.every((byte) => byte == 0);
|
|
||||||
|
|
||||||
test(
|
test(
|
||||||
'wipes what the password unlocked once signed in, once another '
|
'wipes what the password unlocked once signed in, once another '
|
||||||
|
|
|
||||||
|
|
@ -16,9 +16,34 @@ void main() {
|
||||||
),
|
),
|
||||||
srpSecret: Uint8List.fromList(List.filled(32, 7)),
|
srpSecret: Uint8List.fromList(List.filled(32, 7)),
|
||||||
keyEncryptionKey: Uint8List.fromList(List.filled(32, 9)),
|
keyEncryptionKey: Uint8List.fromList(List.filled(32, 9)),
|
||||||
)..forget();
|
);
|
||||||
|
final x = unlock.srpSecret;
|
||||||
|
final key = unlock.keyEncryptionKey;
|
||||||
|
|
||||||
expect(unlock.srpSecret, everyElement(0));
|
unlock.forget();
|
||||||
expect(unlock.keyEncryptionKey, everyElement(0));
|
|
||||||
|
expect(x, everyElement(0));
|
||||||
|
expect(key, everyElement(0));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a forgotten unlock is spent: its secrets are read no more, so no '
|
||||||
|
'proof is made of zeros (the short reading)', () {
|
||||||
|
final unlock = PasswordUnlock(
|
||||||
|
salt: Uint8List(16),
|
||||||
|
costs: protocol.PasswordCosts(
|
||||||
|
algorithm: 'argon2id13',
|
||||||
|
opsLimit: 3,
|
||||||
|
memLimit: 64 * 1024 * 1024,
|
||||||
|
),
|
||||||
|
srpSecret: Uint8List.fromList(List.filled(32, 7)),
|
||||||
|
keyEncryptionKey: Uint8List.fromList(List.filled(32, 9)),
|
||||||
|
);
|
||||||
|
expect(unlock.isForgotten, isFalse);
|
||||||
|
|
||||||
|
unlock.forget();
|
||||||
|
|
||||||
|
expect(unlock.isForgotten, isTrue);
|
||||||
|
expect(() => unlock.srpSecret, throwsStateError);
|
||||||
|
expect(() => unlock.keyEncryptionKey, throwsStateError);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue