fix(auth): a code is counted before it is checked
Codes sent at once were all checked before the tenth refused began a wait (the re-review's first finding: 30 codes from three networks, all checked), so whoever knew the password could open sign-ins ahead and guess far more than ten before codes waited. Every attempt at a code for an account is now counted before its code is checked, one at a time per account (a transaction holding an advisory lock on the account's key), and the tenth in a row begins the wait as it is counted: of codes sent at once at most ten are checked, the rest refused (codesWait). A code accepted clears it all, that wait too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tizian 「ティツィアーン」 <raisondetredev@pm.me>
This commit is contained in:
parent
1908e4f362
commit
ae41937e0b
4 changed files with 158 additions and 74 deletions
|
|
@ -168,13 +168,13 @@ Future<List<String>> confirmAuthenticator(
|
|||
final confirmedBefore = await confirmedAuthenticatorsOf(session, person);
|
||||
if (confirmedBefore.isNotEmpty) {
|
||||
// Tried here once already (_countTry above): not counted again.
|
||||
await refuseWhileCodesWait(session, person);
|
||||
final typed = existingCode?.trim() ?? '';
|
||||
if (typed.isEmpty ||
|
||||
!await acceptTotpCode(session, keys, confirmedBefore, typed)) {
|
||||
if (typed.isNotEmpty) {
|
||||
await codeRefused(session, person);
|
||||
}
|
||||
if (typed.isEmpty) {
|
||||
throw refusedFor(PasswordRefusal.wrongExistingCode);
|
||||
}
|
||||
final waitBegan = await takeCodeAttempt(session, person);
|
||||
if (!await acceptTotpCode(session, keys, confirmedBefore, typed)) {
|
||||
await noticeCodesWait(session, waitBegan);
|
||||
throw refusedFor(PasswordRefusal.wrongExistingCode);
|
||||
}
|
||||
await clearRefusedCodes(session, person);
|
||||
|
|
@ -379,16 +379,16 @@ Future<void> _requireAuthenticatorCode(
|
|||
PasswordRefusal refusal = PasswordRefusal.wrongCode,
|
||||
}) async {
|
||||
await _countTry(session, person);
|
||||
await refuseWhileCodesWait(session, person);
|
||||
final typed = code?.trim() ?? '';
|
||||
if (typed.isNotEmpty &&
|
||||
await acceptTotpCode(session, keys, confirmed, typed)) {
|
||||
if (typed.isEmpty) {
|
||||
throw refusedFor(refusal);
|
||||
}
|
||||
final waitBegan = await takeCodeAttempt(session, person);
|
||||
if (await acceptTotpCode(session, keys, confirmed, typed)) {
|
||||
await clearRefusedCodes(session, person);
|
||||
return;
|
||||
}
|
||||
if (typed.isNotEmpty) {
|
||||
await codeRefused(session, person);
|
||||
}
|
||||
await noticeCodesWait(session, waitBegan);
|
||||
throw refusedFor(refusal);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -884,7 +884,7 @@ Future<bool> secondFactor(
|
|||
throw refusedFor(PasswordRefusal.setupIncomplete);
|
||||
}
|
||||
if (totp.isNotEmpty || recovery.isNotEmpty) {
|
||||
await refuseWhileCodesWait(session, account);
|
||||
final waitBegan = await takeCodeAttempt(session, account);
|
||||
if (totp.isNotEmpty
|
||||
? await acceptTotpCode(session, keys, confirmed, totp)
|
||||
: await _useRecoveryCode(session, keys, account, recovery)) {
|
||||
|
|
@ -892,7 +892,7 @@ Future<bool> secondFactor(
|
|||
await _callOffResets(session, account);
|
||||
return false;
|
||||
}
|
||||
await codeRefused(session, account, notice: notice);
|
||||
await noticeCodesWait(session, waitBegan, notice: notice);
|
||||
} else if (resetTookEffect) {
|
||||
await _carryOutAuthenticatorReset(session, account);
|
||||
await _callOffAccountReset(session, account);
|
||||
|
|
@ -911,25 +911,27 @@ Future<bool> secondFactor(
|
|||
/// instance keeps none) and the language to word it in.
|
||||
typedef CodesWaitNotice = ({String to, String language});
|
||||
|
||||
/// Counts a code refused for [account] ([countRefusedCode]), and mails
|
||||
/// [notice]'s address when the first wait since the last accepted code
|
||||
/// begins: whoever guesses knows the password. A mail that can't be sent
|
||||
/// is logged by its type and refuses nothing.
|
||||
Future<void> codeRefused(
|
||||
/// Mails [notice]'s address that codes wait, where [waitBegan] (from
|
||||
/// [takeCodeAttempt], for a code then refused) began the first wait since
|
||||
/// the last accepted code: whoever guesses knows the password. A mail that
|
||||
/// can't be sent is logged by its type and refuses nothing.
|
||||
Future<void> noticeCodesWait(
|
||||
Session session,
|
||||
UuidValue account, {
|
||||
({DateTime until, bool first})? waitBegan, {
|
||||
CodesWaitNotice? notice,
|
||||
}) async {
|
||||
final began = await countRefusedCode(session, account);
|
||||
final mailer = EmailCodes.of(session.serverpod).mailer;
|
||||
if (began == null || !began.first || notice == null || mailer == null) {
|
||||
if (waitBegan == null ||
|
||||
!waitBegan.first ||
|
||||
notice == null ||
|
||||
mailer == null) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
await mailer.sendCodesWaitNotice(
|
||||
session,
|
||||
to: notice.to,
|
||||
until: began.until,
|
||||
until: waitBegan.until,
|
||||
language: notice.language == 'de' ? 'de' : 'en',
|
||||
);
|
||||
} on Exception catch (error) {
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import 'dart:convert';
|
||||
import 'dart:math';
|
||||
|
||||
import 'package:clock/clock.dart';
|
||||
|
|
@ -7,8 +8,9 @@ import 'package:serverpod/serverpod.dart';
|
|||
// Codes guessed with the password known (ADR 0017; the second reading's M4):
|
||||
// every code checked against an account's authenticators, from a sign-in,
|
||||
// a reset with the recovery phrase or a signed-in change, and every
|
||||
// recovery code, counts while refused, per account, from every network.
|
||||
// After every [codeFailuresBeforeWait] refused in a row no code is taken
|
||||
// recovery code, counts as it is tried (before it is checked, so codes sent
|
||||
// at once can't slip past), per account, from every network.
|
||||
// After every [codeFailuresBeforeWait] tried in a row no code is taken
|
||||
// for a while, an hour the first time and twice as long each time after,
|
||||
// [longestCodeWait] at the most, and the owner is mailed when the first wait
|
||||
// begins where their address is at hand: whoever guesses knows the
|
||||
|
|
@ -28,59 +30,89 @@ const firstCodeWait = Duration(hours: 1);
|
|||
/// the first wait, to change the password.
|
||||
const longestCodeWait = Duration(days: 1);
|
||||
|
||||
/// Refused ([PasswordRefusal.codesWait], with when codes are taken again)
|
||||
/// while [account]'s codes wait.
|
||||
Future<void> refuseWhileCodesWait(Session session, UuidValue account) async {
|
||||
// SELECT * FROM "second_factor_failures" WHERE "authUserId" = $1
|
||||
final row = await SecondFactorFailures.db.findFirstRow(
|
||||
session,
|
||||
where: (t) => t.authUserId.equals(account),
|
||||
);
|
||||
final until = row?.waitUntil;
|
||||
if (until != null && until.isAfter(clock.now())) {
|
||||
throw PasswordRefusedException(
|
||||
reason: PasswordRefusal.codesWait,
|
||||
takesEffectAt: until,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Counts a code refused for [account], and returns when codes are taken
|
||||
/// again where this one began a wait (null otherwise), and whether it is
|
||||
/// the first wait since the last accepted code (the owner is mailed then).
|
||||
/// The count is one statement, so two refused at once count two.
|
||||
Future<({DateTime until, bool first})?> countRefusedCode(
|
||||
/// Takes an attempt at a code for [account], counted before the code is
|
||||
/// checked (the re-review's first finding): codes sent at once can't all be
|
||||
/// checked before a wait begins. Refused ([PasswordRefusal.codesWait], with
|
||||
/// when codes are taken again) while [account]'s codes wait. Every
|
||||
/// [codeFailuresBeforeWait]th attempt in a row begins the next wait as it
|
||||
/// is counted, and is checked still; it returns when that wait ends and
|
||||
/// whether it is the first since the last accepted code, null for any
|
||||
/// other attempt. A code accepted clears it all ([clearRefusedCodes]),
|
||||
/// that wait too.
|
||||
///
|
||||
/// One attempt at a time per account (a transaction holding an advisory
|
||||
/// lock on the account's key), so of codes sent at once at most the ten
|
||||
/// before a wait are checked.
|
||||
Future<({DateTime until, bool first})?> takeCodeAttempt(
|
||||
Session session,
|
||||
UuidValue account,
|
||||
) async {
|
||||
) {
|
||||
final now = clock.now().toUtc();
|
||||
final counted = await session.db.unsafeQuery(
|
||||
'INSERT INTO "second_factor_failures" '
|
||||
'("authUserId", "failures", "waits", "updatedAt") '
|
||||
'VALUES (@account, 1, 0, @now) '
|
||||
'ON CONFLICT ("authUserId") DO UPDATE SET '
|
||||
'"failures" = "second_factor_failures"."failures" + 1, '
|
||||
'"updatedAt" = @now '
|
||||
'RETURNING "failures", "waits"',
|
||||
parameters: QueryParameters.named({'account': account.uuid, 'now': now}),
|
||||
);
|
||||
final failures = counted.first[0]! as int;
|
||||
final waits = counted.first[1]! as int;
|
||||
if (failures % codeFailuresBeforeWait != 0) {
|
||||
return null;
|
||||
return session.db.transaction((transaction) async {
|
||||
// SELECT pg_advisory_xact_lock($1)
|
||||
await session.db.unsafeQuery(
|
||||
'SELECT pg_advisory_xact_lock(@key)',
|
||||
parameters: QueryParameters.named({
|
||||
'key': _lockKeyOf('second_factor:${account.uuid}'),
|
||||
}),
|
||||
transaction: transaction,
|
||||
);
|
||||
// SELECT * FROM "second_factor_failures" WHERE "authUserId" = $1
|
||||
final row = await SecondFactorFailures.db.findFirstRow(
|
||||
session,
|
||||
where: (t) => t.authUserId.equals(account),
|
||||
transaction: transaction,
|
||||
);
|
||||
final waiting = row?.waitUntil;
|
||||
if (waiting != null && waiting.isAfter(now)) {
|
||||
throw PasswordRefusedException(
|
||||
reason: PasswordRefusal.codesWait,
|
||||
takesEffectAt: waiting,
|
||||
);
|
||||
}
|
||||
final failures = (row?.failures ?? 0) + 1;
|
||||
final waits = row?.waits ?? 0;
|
||||
final begins = failures % codeFailuresBeforeWait == 0;
|
||||
final until = begins ? now.add(codeWait(waits)) : null;
|
||||
final counted = SecondFactorFailures(
|
||||
id: row?.id,
|
||||
authUserId: account,
|
||||
failures: failures,
|
||||
waits: begins ? waits + 1 : waits,
|
||||
waitUntil: until ?? waiting,
|
||||
updatedAt: now,
|
||||
);
|
||||
if (row == null) {
|
||||
// INSERT INTO "second_factor_failures" (…) VALUES (…)
|
||||
await SecondFactorFailures.db.insertRow(
|
||||
session,
|
||||
counted,
|
||||
transaction: transaction,
|
||||
);
|
||||
} else {
|
||||
// UPDATE "second_factor_failures" SET … WHERE "id" = $1
|
||||
await SecondFactorFailures.db.updateRow(
|
||||
session,
|
||||
counted,
|
||||
transaction: transaction,
|
||||
);
|
||||
}
|
||||
return until == null ? null : (until: until, first: waits == 0);
|
||||
});
|
||||
}
|
||||
|
||||
/// A key for Postgres' advisory locks from [text] (FNV-1a, 64 bits), as
|
||||
/// Serverpod's rate limiter makes its own.
|
||||
int _lockKeyOf(String text) {
|
||||
// A server's code, never compiled to JavaScript: 64-bit integers hold.
|
||||
// ignore: avoid_js_rounded_ints
|
||||
const offsetBasis = 0xcbf29ce484222325;
|
||||
const prime = 0x100000001b3;
|
||||
var hash = offsetBasis;
|
||||
for (final unit in utf8.encode(text)) {
|
||||
hash = (hash ^ unit) * prime;
|
||||
}
|
||||
final until = now.add(codeWait(waits));
|
||||
// UPDATE "second_factor_failures" SET "waits" = "waits" + 1,
|
||||
// "waitUntil" = $1 WHERE "authUserId" = $2
|
||||
await session.db.unsafeExecute(
|
||||
'UPDATE "second_factor_failures" SET "waits" = "waits" + 1, '
|
||||
'"waitUntil" = @until WHERE "authUserId" = @account',
|
||||
parameters: QueryParameters.named({
|
||||
'account': account.uuid,
|
||||
'until': until,
|
||||
}),
|
||||
);
|
||||
return (until: until, first: waits == 0);
|
||||
return hash;
|
||||
}
|
||||
|
||||
/// How long codes wait after [waits] waits before: an hour times two to
|
||||
|
|
|
|||
|
|
@ -844,6 +844,56 @@ void main() {
|
|||
expect(mailbox.codesWait, hasLength(2));
|
||||
});
|
||||
|
||||
test('sent at once are counted before they are checked: ten '
|
||||
'checked, the rest wait', () async {
|
||||
await annaComplete();
|
||||
final challenges = [
|
||||
for (var network = 0; network < 3; network++)
|
||||
for (var i = 0; i < 10; i++)
|
||||
await atNow(
|
||||
() => beginPasswordSignIn(
|
||||
session,
|
||||
address: 'anna@example.org',
|
||||
network: '203.0.113.$network',
|
||||
),
|
||||
),
|
||||
];
|
||||
|
||||
final reasons = await Future.wait([
|
||||
for (final (i, challenge) in challenges.indexed)
|
||||
atNow(() async {
|
||||
final answer = device.answer(challenge);
|
||||
try {
|
||||
await endpoints.passwordSignIn.complete(
|
||||
notSignedIn(),
|
||||
challenge: challenge.challenge,
|
||||
clientValue: answer.clientValue,
|
||||
clientProof: answer.clientProof,
|
||||
totpCode: '${100000 + i}',
|
||||
host: testHost,
|
||||
address: 'anna@example.org',
|
||||
language: 'en',
|
||||
);
|
||||
return null;
|
||||
} on PasswordRefusedException catch (refusal) {
|
||||
return refusal.reason;
|
||||
}
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(
|
||||
reasons.where((r) => r == PasswordRefusal.wrongCredentials),
|
||||
hasLength(10),
|
||||
);
|
||||
expect(
|
||||
reasons.where((r) => r == PasswordRefusal.codesWait),
|
||||
hasLength(20),
|
||||
);
|
||||
final row = await SecondFactorFailures.db.findFirstRow(session);
|
||||
expect(row?.failures, 10);
|
||||
expect(row?.waits, 1);
|
||||
});
|
||||
|
||||
test('wait a day at the most', () {
|
||||
expect(codeWait(0), const Duration(hours: 1));
|
||||
expect(codeWait(1), const Duration(hours: 2));
|
||||
|
|
|
|||
Loading…
Reference in a new issue