fix(app): no sheet is left busy by an error nothing expected

An instance's salt of another length made the device's cryptography throw
an ArgumentError, a wrapping's nonce a RangeError, and libsodium or the
keyring may throw their own; none was a refusal, so the step's error
escaped and the sheet stayed busy, which can't be closed. halkyon_keys
now checks the lengths of what comes from elsewhere before libsodium sees
it (KeyRefusal.malformed: a salt, a wrapping, a sealed key's box, a sealed
payload), the device's cryptography turns anything else into the general
failure, and so does every password cubit, the error logged by its type
alone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Tizian 「ティツィアーン」 <raisondetredev@pm.me>
This commit is contained in:
Tizian 「ティツィアーン」 2026-10-21 14:41:28 +02:00
commit cf69e62ad4
Signed by: raisondetredev
GPG key ID: 07F6FCF405488B64
16 changed files with 318 additions and 32 deletions

View file

@ -1,5 +1,6 @@
import 'package:equatable/equatable.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:halkyon_app/src/password/bloc/unexpected_failure.dart';
import 'package:halkyon_app/src/password/data/password_repository.dart';
import 'package:halkyon_app/src/password/model/mail_code_purpose.dart';
import 'package:halkyon_app/src/password/model/password_failed.dart';
@ -162,7 +163,8 @@ class AccountRecoveryCubit(
if (!isClosed) {
emit(state.copyWith(busy: false));
}
} on PasswordFailed catch (failed) {
} on Object catch (error) {
final failed = asPasswordFailed(error);
if (!isClosed) {
emit(state.copyWith(busy: false, problem: failed));
}

View file

@ -2,6 +2,7 @@ import 'dart:async';
import 'package:equatable/equatable.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:halkyon_app/src/password/bloc/unexpected_failure.dart';
import 'package:halkyon_app/src/password/data/password_repository.dart';
import 'package:halkyon_app/src/password/model/account_authenticator.dart';
import 'package:halkyon_app/src/password/model/account_security.dart';
@ -43,7 +44,10 @@ class AccountSecurityCubit(final PasswordRepository _passwords)
if (!isClosed) {
emit(AccountSecurityState(security: security));
}
} on PasswordFailed {
} on Object catch (error) {
if (error is! PasswordFailed) {
logUnexpected(error);
}
if (!isClosed) {
emit(AccountSecurityState(security: state.security, failed: true));
}
@ -79,7 +83,8 @@ class SecurityStepCubit<T>() extends Cubit<SecurityStepState<T>> {
emit(SecurityStepState<T>(result: result));
}
return true;
} on PasswordFailed catch (failed) {
} on Object catch (error) {
final failed = asPasswordFailed(error);
if (!isClosed) {
emit(SecurityStepState<T>(problem: failed, result: state.result));
}
@ -140,7 +145,8 @@ class AddAuthenticatorCubit(
AddAuthenticatorState(step: AddAuthenticatorStep.code, setup: setup),
);
}
} on PasswordFailed catch (failed) {
} on Object catch (error) {
final failed = asPasswordFailed(error);
if (!isClosed) {
emit(AddAuthenticatorState(problem: failed));
}
@ -180,7 +186,8 @@ class AddAuthenticatorCubit(
),
);
}
} on PasswordFailed catch (failed) {
} on Object catch (error) {
final failed = asPasswordFailed(error);
if (!isClosed) {
emit(
AddAuthenticatorState(
@ -250,7 +257,8 @@ class NewPhraseCubit(final PasswordRepository _passwords)
if (!isClosed) {
emit(NewPhraseState(step: NewPhraseStep.shown, phrase: made.phrase));
}
} on PasswordFailed catch (failed) {
} on Object catch (error) {
final failed = asPasswordFailed(error);
if (!isClosed) {
emit(NewPhraseState(problem: failed));
}
@ -287,7 +295,8 @@ class NewPhraseCubit(final PasswordRepository _passwords)
if (!isClosed) {
emit(NewPhraseState(step: NewPhraseStep.done, phrase: state.phrase));
}
} on PasswordFailed catch (failed) {
} on Object catch (error) {
final failed = asPasswordFailed(error);
if (!isClosed) {
emit(_again(problem: failed));
}

View file

@ -1,5 +1,6 @@
import 'package:equatable/equatable.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:halkyon_app/src/password/bloc/unexpected_failure.dart';
import 'package:halkyon_app/src/password/data/account_crypto.dart';
import 'package:halkyon_app/src/password/data/password_repository.dart';
import 'package:halkyon_app/src/password/model/account_authenticator.dart';
@ -338,7 +339,8 @@ class AccountSetupCubit(
if (!isClosed) {
emit(state.copyWith(busy: false));
}
} on PasswordFailed catch (failed) {
} on Object catch (error) {
final failed = asPasswordFailed(error);
if (isClosed) {
return;
}

View file

@ -1,5 +1,6 @@
import 'package:equatable/equatable.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:halkyon_app/src/password/bloc/unexpected_failure.dart';
import 'package:halkyon_app/src/password/data/password_repository.dart';
import 'package:halkyon_app/src/password/model/account_authenticator.dart';
import 'package:halkyon_app/src/password/model/mail_code_purpose.dart';
@ -351,7 +352,8 @@ class PasswordSignInCubit(
if (!isClosed) {
emit(state.copyWith(busy: false));
}
} on PasswordFailed catch (failed) {
} on Object catch (error) {
final failed = asPasswordFailed(error);
if (!isClosed) {
emit(state.copyWith(busy: false, problem: failed));
}

View file

@ -0,0 +1,20 @@
import 'dart:developer';
import 'package:halkyon_app/src/password/model/password_failed.dart';
/// [error] as the refusal a sheet shows: a [PasswordFailed] as it is,
/// anything else (an instance's malformed answer, libsodium's or the
/// keyring's own errors) as [PasswordProblem.failed], logged by its type
/// alone, so a sheet is never left busy and never closes on an error it
/// didn't expect (ADR 0017's full reading).
PasswordFailed asPasswordFailed(Object error) {
if (error is PasswordFailed) {
return error;
}
logUnexpected(error);
return const PasswordFailed(PasswordProblem.failed);
}
/// Logs [error] by its type alone: what it says may hold a secret.
void logUnexpected(Object error) =>
log('Failed unexpectedly: ${error.runtimeType}', name: 'Passwords');

View file

@ -1,3 +1,4 @@
import 'dart:developer';
import 'dart:isolate';
import 'dart:typed_data';
@ -301,8 +302,20 @@ Future<T> _guarded<T>(Future<T> Function() step) async {
PasswordRefusal.empty ||
PasswordRefusal.controlCharacter => PasswordProblem.passwordUnusable,
});
} on KeyRefused {
throw const PasswordFailed(PasswordProblem.keysMissing);
} on KeyRefused catch (refused) {
throw PasswordFailed(switch (refused.reason) {
// What the instance sent isn't of the lengths it must have.
KeyRefusal.malformed => PasswordProblem.failed,
_ => PasswordProblem.keysMissing,
});
} on PasswordFailed {
rethrow;
} on Object catch (error) {
// Anything else (libsodium's own errors, Argon2id without the memory
// it asks for) fails the step rather than escaping it, which left the
// sheet busy for good; logged by its type alone.
log('The keys failed: ${error.runtimeType}', name: 'Keys');
throw const PasswordFailed(PasswordProblem.failed);
}
}

View file

@ -0,0 +1,144 @@
import 'dart:typed_data';
import 'package:flutter_test/flutter_test.dart';
import 'package:halkyon_app/halkyon_app.dart';
import 'package:halkyon_app/testing.dart';
import 'package:halkyon_base_client/halkyon_base_client.dart' as protocol;
import 'states.dart';
/// Cryptography failing as nothing above expects: an instance's salt of
/// another length once made libsodium's wrapper throw an `ArgumentError`,
/// a keyring a `PlatformException`.
class _BrokenCrypto() extends FakeAccountCrypto {
@override
Future<PasswordUnlock> unlock(
String password, {
required Uint8List salt,
required protocol.PasswordCosts costs,
}) async => throw ArgumentError.value(15, 'salt', 'must be 16 bytes');
@override
Future<NewAccountMaterial> newAccount(String password) async =>
throw StateError('no memory for Argon2id');
@override
Future<UnlockedAccountKeys> openWithPhrase(
String phrase,
protocol.WrappedKeys wrapped, {
required Uint8List encryptionPublicKey,
required Uint8List signingPublicKey,
}) async => throw RangeError('a nonce of 23 bytes');
}
/// A gateway failing as nothing above expects.
class _BrokenGateway(super.auth) extends FakePasswordGateway {
@override
Future<({bool offered, DateTime? gitHubEnds})> offers() async =>
throw const FormatException('an answer no client reads');
}
/// A refusal of [PasswordProblem.failed].
Matcher get failedGenerally => isA<PasswordFailed>().having(
(failed) => failed.problem,
'problem',
PasswordProblem.failed,
);
void main() {
late FakeAuthGateway auth;
late PasswordRepository passwords;
PasswordRepository repository({
FakePasswordGateway? gateway,
FakeAccountCrypto? crypto,
}) => PasswordRepository(
gateway ?? FakePasswordGateway(auth),
crypto ?? _BrokenCrypto(),
MemoryAccountKeyStore(),
auth,
MemorySignInAddresses(),
);
setUp(() {
auth = FakeAuthGateway();
passwords = repository();
});
tearDown(() => passwords.dispose());
test('signing in: the sheet is never left busy, whatever fails', () async {
final cubit = PasswordSignInCubit(passwords, language: 'en');
addTearDown(cubit.close);
await cubit.credentialsEntered('anna@example.org', 'correct horse');
expect(cubit.state.busy, isFalse, reason: 'the sheet closes again');
expect(cubit.state.problem, failedGenerally);
expect(cubit.state.step, PasswordSignInStep.credentials);
});
test('setting up: the sheet is never left busy, whatever fails', () async {
final cubit = AccountSetupCubit(
passwords,
mode: AccountSetupMode.signUp,
language: 'en',
);
addTearDown(cubit.close);
cubit.addressEntered('anna@example.org');
await cubit.passwordEntered(
'correct horse battery',
'correct horse battery',
);
expect(cubit.state.busy, isFalse);
expect(cubit.state.problem, failedGenerally);
expect(cubit.state.step, AccountSetupStep.password);
});
test('getting back in: the sheet is never left busy, whatever '
'fails', () async {
final gateway = _BrokenGateway(auth)
..addAccount(
'anna@example.org',
password: 'forgotten',
crypto: FakeAccountCrypto(),
);
final broken = repository(gateway: gateway);
addTearDown(broken.dispose);
final cubit = AccountRecoveryCubit(broken, language: 'en');
addTearDown(cubit.close);
await cubit.phraseEntered(
address: 'anna@example.org',
phrase: 'the phrase',
password: 'a new password',
again: 'a new password',
code: fakeTotpCode,
);
expect(cubit.state.busy, isFalse);
expect(cubit.state.problem, failedGenerally);
});
test('a step of the account section is never left busy, and what it '
'reads shows as failed', () async {
final step = SecurityStepCubit<void>();
addTearDown(step.close);
expect(
await step.run(() async => throw StateError('keyring locked')),
isFalse,
);
expect(step.state.busy, isFalse);
expect(step.state.problem, failedGenerally);
final gateway = _BrokenGateway(auth);
final broken = repository(gateway: gateway, crypto: FakeAccountCrypto());
addTearDown(broken.dispose);
await auth.signInAnonymously();
final section = AccountSecurityCubit(broken);
addTearDown(section.close);
final states = await statesOf(section, section.load);
expect(states.last.failed, isTrue);
});
}

View file

@ -308,4 +308,28 @@ void main() {
expect(answer.clientProof.lengthInBytes, 32);
expect(answer.verifiesServer(ByteData(32)), isFalse);
});
test('what an instance sends of the wrong length fails the step, never '
'escaping it', () async {
expect(
crypto.unlock(
'correct horse battery staple',
salt: Uint8List(15),
costs: account.setup.costs,
),
failsWith(PasswordProblem.failed),
);
// After the password proved right: the instance isn't trusted.
final unlocked = await unlock('correct horse battery staple');
expect(
open(
unlocked,
protocol.WrappedKeys(
nonce: ByteData(23),
cipherText: account.setup.passwordKeys.cipherText,
),
),
failsWith(PasswordProblem.instanceUnproven),
);
});
}

View file

@ -157,6 +157,16 @@ AccountKeys unwrapAccountKeys(
signingPublicKey.length != sodium.crypto.sign.publicKeyBytes) {
throw const KeyRefused(KeyRefusal.notTheAccountsKeys);
}
// Checked before libsodium sees them: of another length it throws a
// RangeError, which nothing above expects.
final aead = sodium.crypto.aeadXChaCha20Poly1305IETF;
if (wrapped.nonce.length != aead.nonceBytes ||
wrapped.cipherText.length !=
sodium.crypto.box.secretKeyBytes +
sodium.crypto.sign.secretKeyBytes +
aead.aBytes) {
throw const KeyRefused(KeyRefusal.malformed);
}
final Uint8List plainText;
try {
plainText = sodium.crypto.aeadXChaCha20Poly1305IETF.decrypt(

View file

@ -88,6 +88,11 @@ DataKey openDataKey(
throw const KeyRefused(KeyRefusal.forgedSignature);
}
final box = Uint8List.sublistView(sealed, signatureBytes);
// Of a box's least length or longer, as libsodium throws a RangeError for
// shorter ones, which nothing above expects.
if (box.length < sodium.crypto.box.sealBytes) {
throw const KeyRefused(KeyRefusal.malformed);
}
if (!sodium.crypto.sign.verifyDetached(
message: _signed(keys.encryptionPublicKey, box),
signature: Uint8List.sublistView(sealed, 0, signatureBytes),

View file

@ -27,6 +27,11 @@ enum KeyRefusal() {
/// isn't the author's.
forgedSignature,
/// Bytes from elsewhere (an instance's salt, a wrapping, a sealed key)
/// aren't of the length they must have: refused before anything reads
/// them, as a device must never trust what an instance sends.
malformed,
/// Argon2id's costs named for an account are below what any account may
/// have, or of another algorithm.
weakPasswordKdf,

View file

@ -129,11 +129,7 @@ PasswordKeys derivePasswordKeys(
PasswordKdf kdf = PasswordKdf.current,
}) {
if (salt.length != sodium.crypto.pwhash.saltBytes) {
throw ArgumentError.value(
salt.length,
'salt',
'must be ${sodium.crypto.pwhash.saltBytes} bytes',
);
throw const KeyRefused(KeyRefusal.malformed);
}
final masterKey = sodium.crypto.pwhash.callStr(
outLen: _keyBytes,

View file

@ -86,7 +86,8 @@ Uint8List openContent(
throw const KeyRefused(KeyRefusal.forgedSignature);
}
final aead = sodium.crypto.aeadXChaCha20Poly1305IETF;
if (sealed.nonce.length != aead.nonceBytes) {
if (sealed.nonce.length != aead.nonceBytes ||
sealed.payload.length < aead.aBytes) {
throw const KeyRefused(KeyRefusal.wrongKeyOrTampered);
}
try {

View file

@ -137,10 +137,6 @@ Future<void> main() async {
for (final tampered in [
WrappedAccountKeys(nonce: wrapped.nonce, cipherText: cipherText),
WrappedAccountKeys(nonce: nonce, cipherText: wrapped.cipherText),
WrappedAccountKeys(
nonce: wrapped.nonce,
cipherText: wrapped.cipherText.sublist(1),
),
]) {
expect(
() => unwrap(tampered, password.keyEncryptionKey),
@ -177,6 +173,33 @@ Future<void> main() async {
refusedFor(KeyRefusal.notTheAccountsKeys),
);
});
test('wrappings of other lengths, as an instance might send them, are '
'refused rather than thrown on', () {
for (final (nonce, cipherText) in [
(Uint8List(23), wrapped.cipherText),
(Uint8List(25), wrapped.cipherText),
(Uint8List(0), Uint8List(0)),
(wrapped.nonce, Uint8List(15)),
(wrapped.nonce, wrapped.cipherText.sublist(1)),
(wrapped.nonce, Uint8List(wrapped.cipherText.length + 1)),
]) {
expect(
() => unwrap(
WrappedAccountKeys(nonce: nonce, cipherText: cipherText),
password.keyEncryptionKey,
),
throwsA(
isA<KeyRefused>().having(
(refused) => refused.reason,
'reason',
KeyRefusal.malformed,
),
),
reason: '${nonce.length} / ${cipherText.length}',
);
}
});
});
test('wrapped again under the recovery key, the phrase opens them', () {

View file

@ -109,6 +109,24 @@ Future<void> main() async {
throwsArgumentError,
);
});
test(
'a sealed key too short to hold a box is refused, never thrown on',
() {
// Signed by the sealer, so only the box's own length can fail it.
final signature = sodium.crypto.sign.detached(
message: Uint8List(0),
secretKey: account.signing.secretKey,
);
for (final box in [Uint8List(1), Uint8List(47)]) {
expect(
() => open(Uint8List.fromList([...signature, ...box])),
throwsA(isA<KeyRefused>()),
reason: '${box.length}',
);
}
},
);
});
group('content, sealed and signed', () {

View file

@ -54,17 +54,29 @@ Future<void> main() async {
expect(keys.srpPrivateValue, greaterThan(BigInt.zero));
});
test('a salt of another length is refused', () {
expect(
() => derivePasswordKeys(
sodium,
password: 'a password',
salt: Uint8List(15),
kdf: cheap,
),
throwsArgumentError,
);
});
test(
'a salt of another length, as an instance might send one, is refused',
() {
for (final length in [0, 15, 17]) {
expect(
() => derivePasswordKeys(
sodium,
password: 'a password',
salt: Uint8List(length),
kdf: cheap,
),
throwsA(
isA<KeyRefused>().having(
(refused) => refused.reason,
'reason',
KeyRefusal.malformed,
),
),
reason: '$length',
);
}
},
);
test('new salts are 16 random bytes', () {
final first = newPasswordSalt(sodium);