fix(auth): a capped wait, a long instance secret, writes of their own columns

Three of the second reading's lows:
- A member waits at most 5 seconds (maxClockSkew) for the register's
  moment before making a session after a reset; a clock further behind
  is refused at once (unavailable, logged), rather than holding the
  request for as long as the clocks differ.
- instanceSecret, which lets a service set passwords and delete accounts
  for anyone, stops the server when set shorter than 32 characters, as
  the password keys do.
- A change of password, a reset keeping the keys and a new phrase each
  write their own columns alone (writeNewPassword, columns: in
  setRecoveryKeys), so one made from the row as it was before another
  never undoes it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Tizian 「ティツィアーン」 <raisondetredev@pm.me>

packages/halkyon_base_server/lib/src/password/account_recovery.dart

@@ -146,24 +146,7 @@ Future<AccountSessionsEnded> resetPasswordWithPhrase(
     recoveryCode: recoveryCode,
   );
   final endedBefore = await session.db.transaction((transaction) async {
-    // UPDATE "account_password" SET "salt" = $1, "kdfAlgorithm" = $2,
-    //   "kdfOpsLimit" = $3, "kdfMemLimit" = $4, "verifier" = $5,
-    //   "passwordKeysNonce" = $6, "passwordKeysCipherText" = $7,
-    //   "updatedAt" = $8 WHERE "id" = $9
-    await AccountPassword.db.updateRow(
-      session,
-      row.copyWith(
-        salt: password.salt,
-        kdfAlgorithm: password.costs.algorithm,
-        kdfOpsLimit: password.costs.opsLimit,
-        kdfMemLimit: password.costs.memLimit,
-        verifier: password.verifier,
-        passwordKeysNonce: password.passwordKeys.nonce,
-        passwordKeysCipherText: password.passwordKeys.cipherText,
-        updatedAt: clock.now().toUtc(),
-      ),
-      transaction: transaction,
-    );
+    await writeNewPassword(session, row, password, transaction: transaction);
     return await endSessionsEverywhere(
       session,
       account,

packages/halkyon_base_server/lib/src/password/password_change.dart

@@ -56,24 +56,7 @@ Future<void> changePassword(
     host: host,
   );
   await session.db.transaction((transaction) async {
-    // UPDATE "account_password" SET "salt" = $1, "kdfAlgorithm" = $2,
-    //   "kdfOpsLimit" = $3, "kdfMemLimit" = $4, "verifier" = $5,
-    //   "passwordKeysNonce" = $6, "passwordKeysCipherText" = $7,
-    //   "updatedAt" = $8 WHERE "id" = $9
-    await AccountPassword.db.updateRow(
-      session,
-      row.copyWith(
-        salt: password.salt,
-        kdfAlgorithm: password.costs.algorithm,
-        kdfOpsLimit: password.costs.opsLimit,
-        kdfMemLimit: password.costs.memLimit,
-        verifier: password.verifier,
-        passwordKeysNonce: password.passwordKeys.nonce,
-        passwordKeysCipherText: password.passwordKeys.cipherText,
-        updatedAt: clock.now().toUtc(),
-      ),
-      transaction: transaction,
-    );
+    await writeNewPassword(session, row, password, transaction: transaction);
     await endSessionsEverywhere(
       session,
       person,
@@ -125,6 +108,7 @@ Future<void> setRecoveryKeys(
   // UPDATE "account_password" SET "recoveryKeysNonce" = $1,
   //   "recoveryKeysCipherText" = $2, "recoveryProofKey" = $3,
   //   "updatedAt" = $4 WHERE "id" = $5
+  // (Those columns alone: a change of password meanwhile stays.)
   await AccountPassword.db.updateRow(
     session,
     row.copyWith(
@@ -133,6 +117,12 @@ Future<void> setRecoveryKeys(
       recoveryProofKey: recoveryProofKey,
       updatedAt: clock.now().toUtc(),
     ),
+    columns: (t) => [
+      t.recoveryKeysNonce,
+      t.recoveryKeysCipherText,
+      t.recoveryProofKey,
+      t.updatedAt,
+    ],
   );
 }
 

packages/halkyon_base_server/lib/src/password/passwords.dart

@@ -474,6 +474,45 @@ Future<UuidValue> signUpWithCode(
   }
 }
 
+/// Writes [password] (a change, or a reset keeping the keys) over [row]'s
+/// password alone: the salt, the costs, the verifier and the keys wrapped
+/// under it. Never the row's other columns, which [row] may hold as they
+/// were before a change made meanwhile (a new phrase) that must stay.
+Future<void> writeNewPassword(
+  Session session,
+  AccountPassword row,
+  NewPassword password, {
+  Transaction? transaction,
+}) =>
+    // UPDATE "account_password" SET "salt" = $1, "kdfAlgorithm" = $2,
+    //   "kdfOpsLimit" = $3, "kdfMemLimit" = $4, "verifier" = $5,
+    //   "passwordKeysNonce" = $6, "passwordKeysCipherText" = $7,
+    //   "updatedAt" = $8 WHERE "id" = $9
+    AccountPassword.db.updateRow(
+      session,
+      row.copyWith(
+        salt: password.salt,
+        kdfAlgorithm: password.costs.algorithm,
+        kdfOpsLimit: password.costs.opsLimit,
+        kdfMemLimit: password.costs.memLimit,
+        verifier: password.verifier,
+        passwordKeysNonce: password.passwordKeys.nonce,
+        passwordKeysCipherText: password.passwordKeys.cipherText,
+        updatedAt: clock.now().toUtc(),
+      ),
+      columns: (t) => [
+        t.salt,
+        t.kdfAlgorithm,
+        t.kdfOpsLimit,
+        t.kdfMemLimit,
+        t.verifier,
+        t.passwordKeysNonce,
+        t.passwordKeysCipherText,
+        t.updatedAt,
+      ],
+      transaction: transaction,
+    );
+
 /// The first step of signing in with a password: SRP's B for the account
 /// with [address], with its salt and costs, all alike for an address nobody
 /// has (see above). At most 10 sign-ins begin for an address and 30 from a

packages/halkyon_base_server/lib/src/register/instance_routes.dart

@@ -66,12 +66,25 @@ void addInstanceRoutes(Serverpod pod) {
     );
 }
 
+/// The fewest characters the instance's shared secret has: it sets
+/// passwords and deletes accounts for anyone, as the password keys
+/// (`passwordKeyMinLength`) protect what they protect.
+const instanceSecretMinLength = 32;
+
 /// Throws a [StateError] where [pod] would call the instance's other
 /// services without the shared secret (`instanceSecret`): a member asks its
 /// register everything, and a register with members tells them when an
-/// account goes.
+/// account goes; and for a secret shorter than [instanceSecretMinLength].
 void checkInstanceSecret(Serverpod pod) {
-  if (instanceSecretOf(pod) != null) {
+  final secret = instanceSecretOf(pod);
+  if (secret != null) {
+    if (secret.length < instanceSecretMinLength) {
+      throw StateError(
+        'INSTANCE_SECRET (instanceSecret in the passwords) is shorter than '
+        '$instanceSecretMinLength characters: it lets a service act for any '
+        'account of the instance. Generate one: openssl rand -hex 32',
+      );
+    }
     return;
   }
   switch (HalkyonBase.maybeOf(pod)?.accounts) {

packages/halkyon_base_server/lib/src/register/people.dart

@@ -1,6 +1,7 @@
 import 'package:clock/clock.dart';
 import 'package:halkyon_base_server/src/auth/identified.dart';
 import 'package:halkyon_base_server/src/auth/scopes.dart';
+import 'package:halkyon_base_server/src/generated/protocol.dart';
 import 'package:halkyon_base_server/src/register/sessions_ended.dart';
 import 'package:serverpod/serverpod.dart';
 import 'package:serverpod_auth_idp_server/core.dart';
@@ -81,12 +82,28 @@ Future<AuthSuccess> issueSessionAfter(
 }) async {
   await endSessionsHere(session, account, endedBefore: endedBefore);
   final wait = endedBefore.difference(clock.now());
+  if (wait > maxClockSkew) {
+    // Its type of trouble only, never whose: the clocks need setting right.
+    session.log(
+      "This server's clock is ${wait.inSeconds} s behind the register's: "
+      'set both by NTP.',
+      level: LogLevel.warning,
+    );
+    throw PasswordRefusedException(reason: PasswordRefusal.unavailable);
+  }
   if (!wait.isNegative) {
     await Future<void>.delayed(wait + const Duration(milliseconds: 1));
   }
   return await issueSession(session, account, method: method);
 }
 
+/// How far behind the register's clock a member's may run for
+/// [issueSessionAfter] to wait for it: further, the request would hang as
+/// long, so it is refused. Clocks set by NTP differ by milliseconds; a
+/// member's clock running ahead of the register's by up to this much
+/// leaves sessions made in that gap, which the ADR accepts.
+const maxClockSkew = Duration(seconds: 5);
+
 /// What [person] becomes here once a way in is theirs, as linking makes them
 /// on the register in the same transaction: `identified` (on their sessions
 /// too), and no longer anonymous. A member does it after the register linked

packages/halkyon_base_server/test/integration/password/password_change_test.dart

@@ -3,6 +3,8 @@ import 'dart:typed_data';
 import 'package:clock/clock.dart';
 import 'package:cryptography/cryptography.dart';
 import 'package:halkyon_base_server/base.dart';
+import 'package:halkyon_base_server/src/password/passwords.dart'
+    show writeNewPassword;
 import 'package:serverpod/serverpod.dart';
 import 'package:serverpod_auth_idp_server/core.dart';
 import 'package:test/test.dart';
@@ -412,6 +414,39 @@ void main() {
           expect(await proves(freshProof), isTrue);
         });
 
+        test('and a change of password made from the row as it was before '
+            'each write their own columns alone: neither undoes the '
+            'other', () async {
+          final before = (await AccountPassword.db.findFirstRow(
+            session,
+            where: (t) => t.authUserId.equals(anna),
+          ))!;
+          await newPhrase();
+          final newPassword = device.newPassword();
+
+          await writeNewPassword(session, before, newPassword);
+
+          final after = (await AccountPassword.db.findFirstRow(
+            session,
+            where: (t) => t.authUserId.equals(anna),
+          ))!;
+          expect(
+            WrappedKeys(
+              nonce: after.recoveryKeysNonce,
+              cipherText: after.recoveryKeysCipherText,
+            ).toJson(),
+            fresh.toJson(),
+          );
+          expect(
+            Uint8List.sublistView(after.recoveryProofKey!),
+            Uint8List.sublistView(freshProofKey),
+          );
+          expect(
+            Uint8List.sublistView(after.verifier),
+            Uint8List.sublistView(newPassword.verifier),
+          );
+        });
+
         test("is refused alike without the password's proof, for a wrong "
             "one, another person's, and a missing or wrong code, keeping "
             'the wrapping', () async {

packages/halkyon_base_server/test/integration/register/member_test.dart

@@ -4,6 +4,7 @@ import 'dart:typed_data';
 import 'package:halkyon_base_server/base.dart';
 import 'package:halkyon_base_server/src/register/deletion_notices.dart';
 import 'package:halkyon_base_server/src/register/instance_api.dart';
+import 'package:halkyon_base_server/src/register/people.dart' show maxClockSkew;
 import 'package:halkyon_base_server/src/register/register_protocol.dart';
 import 'package:halkyon_base_server/src/register/remote_register.dart';
 import 'package:serverpod/serverpod.dart';
@@ -1068,6 +1069,61 @@ void main() {
             isFalse,
           );
         });
+
+        test("waits a few seconds at most for the register's moment: a "
+            'clock further behind is refused at once, and signs nobody in, '
+            'rather than holding the request', () async {
+          register
+            ..proof = PasswordProof(
+              account: ben,
+              serverProof: ByteData(32),
+              encryptionPublicKey: ByteData(32),
+              signingPublicKey: ByteData(32),
+              passwordKeys: WrappedKeys(
+                nonce: ByteData(24),
+                cipherText: ByteData(112),
+              ),
+              needsAuthenticator: false,
+            )
+            ..endedBefore = DateTime.now().toUtc().add(
+              const Duration(minutes: 1),
+            );
+          final watch = Stopwatch()..start();
+
+          await expectLater(
+            endpoints.accountRecovery.resetPassword(
+              notSignedIn(),
+              challenge: const Uuid().v7obj(),
+              signature: ByteData(64),
+              password: NewPassword(
+                salt: ByteData(16),
+                costs: newPasswordCosts,
+                verifier: ByteData(384),
+                passwordKeys: WrappedKeys(
+                  nonce: ByteData(24),
+                  cipherText: ByteData(112),
+                ),
+              ),
+              totpCode: '123456',
+              host: testHost,
+            ),
+            throwsA(
+              isA<PasswordRefusedException>().having(
+                (error) => error.reason,
+                'reason',
+                PasswordRefusal.unavailable,
+              ),
+            ),
+          );
+          expect(watch.elapsed, lessThan(maxClockSkew));
+          expect(
+            await ServerSideSession.db.count(
+              session,
+              where: (t) => t.authUserId.equals(ben),
+            ),
+            0,
+          );
+        });
       });
 
       group('pruning', () {
@@ -1135,6 +1191,21 @@ void main() {
         expect(() => checkInstanceSecret(pod), returnsNormally);
       });
 
+      test('does not start with a secret shorter than 32 characters, alone '
+          'or not: it gates every account of the instance', () {
+        for (final accounts in [
+          const HoldsRegister(),
+          HoldsRegister(members: [_learn]),
+          MemberOfRegister(_learn),
+        ]) {
+          useBase(sessionBuilder, accounts: accounts);
+          useInstanceSecret(pod, 'x' * 31);
+          expect(() => checkInstanceSecret(pod), throwsStateError);
+          useInstanceSecret(pod, 'x' * 32);
+          expect(() => checkInstanceSecret(pod), returnsNormally);
+        }
+      });
+
       test('asks the register at its address with the secret, and asks '
           'nothing without one', () {
         AccountRegister.use(pod, null);