Matrix: auth/cross-signing flow hardening

extensions/matrix/src/matrix/client.test.ts

@@ -1,6 +1,11 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
 import type { CoreConfig } from "../types.js";
-import { resolveMatrixAuth, resolveMatrixConfig, resolveMatrixConfigForAccount } from "./client.js";
+import {
+  resolveMatrixAuth,
+  resolveMatrixAuthContext,
+  resolveMatrixConfig,
+  resolveMatrixConfigForAccount,
+} from "./client.js";
 import * as credentialsModule from "./credentials.js";
 import * as sdkModule from "./sdk.js";
 
@@ -89,6 +94,40 @@ describe("resolveMatrixConfig", () => {
     expect(resolved.accessToken).toBe("ops-token");
     expect(resolved.deviceName).toBe("Ops Device");
   });
+
+  it("prefers channels.matrix.accounts.default over global env for the default account", () => {
+    const cfg = {
+      channels: {
+        matrix: {
+          accounts: {
+            default: {
+              homeserver: "https://matrix.gumadeiras.com",
+              userId: "@pinguini:matrix.gumadeiras.com",
+              password: "cfg-pass", // pragma: allowlist secret
+              deviceName: "OpenClaw Gateway Pinguini",
+              encryption: true,
+            },
+          },
+        },
+      },
+    } as CoreConfig;
+    const env = {
+      MATRIX_HOMESERVER: "https://env.example.org",
+      MATRIX_USER_ID: "@env:example.org",
+      MATRIX_PASSWORD: "env-pass",
+      MATRIX_DEVICE_NAME: "EnvDevice",
+    } as NodeJS.ProcessEnv;
+
+    const resolved = resolveMatrixAuthContext({ cfg, env });
+    expect(resolved.accountId).toBe("default");
+    expect(resolved.resolved).toMatchObject({
+      homeserver: "https://matrix.gumadeiras.com",
+      userId: "@pinguini:matrix.gumadeiras.com",
+      password: "cfg-pass",
+      deviceName: "OpenClaw Gateway Pinguini",
+      encryption: true,
+    });
+  });
 });
 
 describe("resolveMatrixAuth", () => {

extensions/matrix/src/matrix/client/config.ts

@@ -328,16 +328,9 @@ export function resolveMatrixAuthContext(params?: {
   const cfg = params?.cfg ?? (getMatrixRuntime().config.loadConfig() as CoreConfig);
   const env = params?.env ?? process.env;
   const explicitAccountId = normalizeOptionalAccountId(params?.accountId);
-  const defaultResolved = resolveMatrixConfig(cfg, env);
   const effectiveAccountId =
-    explicitAccountId ??
-    (defaultResolved.homeserver
-      ? DEFAULT_ACCOUNT_ID
-      : (resolveImplicitMatrixAccountId(cfg, env) ?? DEFAULT_ACCOUNT_ID));
-  const resolved =
-    effectiveAccountId === DEFAULT_ACCOUNT_ID && defaultResolved.homeserver
-      ? defaultResolved
-      : resolveMatrixConfigForAccount(cfg, effectiveAccountId, env);
+    explicitAccountId ?? resolveImplicitMatrixAccountId(cfg, env) ?? DEFAULT_ACCOUNT_ID;
+  const resolved = resolveMatrixConfigForAccount(cfg, effectiveAccountId, env);
 
   return {
     cfg,

extensions/matrix/src/matrix/sdk.test.ts

@@ -1043,6 +1043,7 @@ describe("MatrixClient crypto bootstrapping", () => {
     matrixJsClient.getDeviceId = vi.fn(() => "DEVICE123");
     const bootstrapSecretStorage = vi.fn(async () => {});
     const bootstrapCrossSigning = vi.fn(async () => {});
+    const checkKeyBackupAndEnable = vi.fn(async () => {});
     const getSecretStorageStatus = vi.fn(async () => ({
       ready: true,
       defaultKeyId: "SSSSKEY",
@@ -1061,6 +1062,7 @@ describe("MatrixClient crypto bootstrapping", () => {
       requestOwnUserVerification: vi.fn(async () => null),
       getSecretStorageStatus,
       getDeviceVerificationStatus,
+      checkKeyBackupAndEnable,
     }));
 
     const recoveryDir = fs.mkdtempSync(path.join(os.tmpdir(), "matrix-sdk-verify-key-"));
@@ -1077,6 +1079,7 @@ describe("MatrixClient crypto bootstrapping", () => {
     expect(matrixJsClient.startClient).toHaveBeenCalledTimes(1);
     expect(bootstrapSecretStorage).toHaveBeenCalled();
     expect(bootstrapCrossSigning).toHaveBeenCalled();
+    expect(checkKeyBackupAndEnable).toHaveBeenCalledTimes(1);
   });
 
   it("fails recovery-key verification when the device is only locally trusted", async () => {
@@ -1237,11 +1240,13 @@ describe("MatrixClient crypto bootstrapping", () => {
       .mockResolvedValueOnce("9")
       .mockResolvedValue("9");
     const loadSessionBackupPrivateKeyFromSecretStorage = vi.fn(async () => {});
+    const checkKeyBackupAndEnable = vi.fn(async () => {});
     const restoreKeyBackup = vi.fn(async () => ({ imported: 4, total: 10 }));
     matrixJsClient.getCrypto = vi.fn(() => ({
       on: vi.fn(),
       getActiveSessionBackupVersion,
       loadSessionBackupPrivateKeyFromSecretStorage,
+      checkKeyBackupAndEnable,
       restoreKeyBackup,
       getSessionBackupPrivateKey: vi.fn(async () => new Uint8Array([1])),
       getKeyBackupInfo: vi.fn(async () => ({
@@ -1267,6 +1272,46 @@ describe("MatrixClient crypto bootstrapping", () => {
     expect(result.loadedFromSecretStorage).toBe(true);
     expect(matrixJsClient.startClient).toHaveBeenCalledTimes(1);
     expect(loadSessionBackupPrivateKeyFromSecretStorage).toHaveBeenCalledTimes(1);
+    expect(checkKeyBackupAndEnable).toHaveBeenCalledTimes(1);
+    expect(restoreKeyBackup).toHaveBeenCalledTimes(1);
+  });
+
+  it("activates backup after loading the key from secret storage before restore", async () => {
+    const getActiveSessionBackupVersion = vi
+      .fn()
+      .mockResolvedValueOnce(null)
+      .mockResolvedValueOnce("5256")
+      .mockResolvedValue("5256");
+    const loadSessionBackupPrivateKeyFromSecretStorage = vi.fn(async () => {});
+    const checkKeyBackupAndEnable = vi.fn(async () => {});
+    const restoreKeyBackup = vi.fn(async () => ({ imported: 0, total: 0 }));
+    matrixJsClient.getCrypto = vi.fn(() => ({
+      on: vi.fn(),
+      getActiveSessionBackupVersion,
+      getSessionBackupPrivateKey: vi.fn(async () => new Uint8Array([1])),
+      loadSessionBackupPrivateKeyFromSecretStorage,
+      checkKeyBackupAndEnable,
+      restoreKeyBackup,
+      getKeyBackupInfo: vi.fn(async () => ({
+        algorithm: "m.megolm_backup.v1.curve25519-aes-sha2",
+        auth_data: {},
+        version: "5256",
+      })),
+      isKeyBackupTrusted: vi.fn(async () => ({
+        trusted: true,
+        matchesDecryptionKey: true,
+      })),
+    }));
+
+    const client = new MatrixClient("https://matrix.example.org", "token", undefined, undefined, {
+      encryption: true,
+    });
+
+    const result = await client.restoreRoomKeyBackup();
+    expect(result.success).toBe(true);
+    expect(result.backupVersion).toBe("5256");
+    expect(loadSessionBackupPrivateKeyFromSecretStorage).toHaveBeenCalledTimes(1);
+    expect(checkKeyBackupAndEnable).toHaveBeenCalledTimes(1);
     expect(restoreKeyBackup).toHaveBeenCalledTimes(1);
   });
 

extensions/matrix/src/matrix/sdk.ts

@@ -798,6 +798,7 @@ export class MatrixClient {
     await this.cryptoBootstrapper.bootstrap(crypto, {
       allowAutomaticCrossSigningReset: false,
     });
+    await this.enableTrustedRoomKeyBackupIfPossible(crypto);
     const status = await this.getOwnDeviceVerificationStatus();
     if (!status.verified) {
       return {
@@ -871,6 +872,7 @@ export class MatrixClient {
         }
         await crypto.loadSessionBackupPrivateKeyFromSecretStorage(); // pragma: allowlist secret
         loadedFromSecretStorage = true;
+        await this.enableTrustedRoomKeyBackupIfPossible(crypto);
         activeVersion = await this.resolveActiveRoomKeyBackupVersion(crypto);
       }
       if (!activeVersion) {
@@ -1136,6 +1138,15 @@ export class MatrixClient {
     }
   }
 
+  private async enableTrustedRoomKeyBackupIfPossible(
+    crypto: MatrixCryptoBootstrapApi,
+  ): Promise<void> {
+    if (typeof crypto.checkKeyBackupAndEnable !== "function") {
+      return;
+    }
+    await crypto.checkKeyBackupAndEnable();
+  }
+
   private async ensureRoomKeyBackupEnabled(crypto: MatrixCryptoBootstrapApi): Promise<void> {
     const existingVersion = await this.resolveRoomKeyBackupVersion();
     if (existingVersion) {

extensions/matrix/src/matrix/sdk/crypto-bootstrap.test.ts

@@ -208,6 +208,43 @@ describe("MatrixCryptoBootstrapper", () => {
     );
   });
 
+  it("recreates secret storage and retries cross-signing when explicit bootstrap hits bad MAC", async () => {
+    const deps = createBootstrapperDeps();
+    const bootstrapCrossSigning = vi
+      .fn<() => Promise<void>>()
+      .mockRejectedValueOnce(new Error("Error decrypting secret m.cross_signing.master: bad MAC"))
+      .mockResolvedValueOnce(undefined);
+    const crypto = createCryptoApi({
+      bootstrapCrossSigning,
+      isCrossSigningReady: vi.fn(async () => true),
+      userHasCrossSigningKeys: vi.fn(async () => true),
+      getDeviceVerificationStatus: vi.fn(async () => ({
+        isVerified: () => true,
+        localVerified: true,
+        crossSigningVerified: true,
+        signedByOwner: true,
+      })),
+    });
+    const bootstrapper = new MatrixCryptoBootstrapper(
+      deps as unknown as MatrixCryptoBootstrapperDeps<MatrixRawEvent>,
+    );
+
+    await bootstrapper.bootstrap(crypto, {
+      strict: true,
+      allowSecretStorageRecreateWithoutRecoveryKey: true,
+      allowAutomaticCrossSigningReset: false,
+    });
+
+    expect(deps.recoveryKeyStore.bootstrapSecretStorageWithRecoveryKey).toHaveBeenCalledWith(
+      crypto,
+      {
+        allowSecretStorageRecreateWithoutRecoveryKey: true,
+        forceNewSecretStorage: true,
+      },
+    );
+    expect(bootstrapCrossSigning).toHaveBeenCalledTimes(2);
+  });
+
   it("fails in strict mode when cross-signing keys are still unpublished", async () => {
     const deps = createBootstrapperDeps();
     const crypto = createCryptoApi({

extensions/matrix/src/matrix/sdk/crypto-bootstrap.ts

@@ -3,6 +3,7 @@ import { VerificationPhase } from "matrix-js-sdk/lib/crypto-api/verification.js"
 import type { MatrixDecryptBridge } from "./decrypt-bridge.js";
 import { LogService } from "./logger.js";
 import type { MatrixRecoveryKeyStore } from "./recovery-key-store.js";
+import { isRepairableSecretStorageAccessError } from "./recovery-key-store.js";
 import type {
   MatrixAuthDict,
   MatrixCryptoBootstrapApi,
@@ -176,12 +177,11 @@ export class MatrixCryptoBootstrapper<TRawEvent extends MatrixRawEvent> {
     } catch (err) {
       const shouldRepairSecretStorage =
         options.allowSecretStorageRecreateWithoutRecoveryKey &&
-        err instanceof Error &&
-        err.message.includes("getSecretStorageKey callback returned falsey");
+        isRepairableSecretStorageAccessError(err);
       if (shouldRepairSecretStorage) {
         LogService.warn(
           "MatrixClientLite",
-          "Cross-signing bootstrap could not access secret storage; recreating secret storage during explicit bootstrap and retrying.",
+          "Cross-signing bootstrap could not unlock secret storage; recreating secret storage during explicit bootstrap and retrying.",
         );
         await this.deps.recoveryKeyStore.bootstrapSecretStorageWithRecoveryKey(crypto, {
           allowSecretStorageRecreateWithoutRecoveryKey: true,

extensions/matrix/src/matrix/sdk/recovery-key-store.test.ts

@@ -227,6 +227,54 @@ describe("MatrixRecoveryKeyStore", () => {
     });
   });
 
+  it("recreates secret storage during explicit bootstrap when decrypting a stored secret fails with bad MAC", async () => {
+    const recoveryKeyPath = createTempRecoveryKeyPath();
+    const store = new MatrixRecoveryKeyStore(recoveryKeyPath);
+    const generated = {
+      keyId: "REPAIRED",
+      keyInfo: { name: "repaired" },
+      privateKey: new Uint8Array([7, 7, 8, 9]),
+      encodedPrivateKey: "encoded-repaired-key", // pragma: allowlist secret
+    };
+    const createRecoveryKeyFromPassphrase = vi.fn(async () => generated);
+    const bootstrapSecretStorage = vi.fn(
+      async (opts?: {
+        setupNewSecretStorage?: boolean;
+        createSecretStorageKey?: () => Promise<unknown>;
+      }) => {
+        if (opts?.setupNewSecretStorage) {
+          await opts.createSecretStorageKey?.();
+          return;
+        }
+        throw new Error("Error decrypting secret m.cross_signing.master: bad MAC");
+      },
+    );
+    const crypto = {
+      on: vi.fn(),
+      bootstrapCrossSigning: vi.fn(async () => {}),
+      bootstrapSecretStorage,
+      createRecoveryKeyFromPassphrase,
+      getSecretStorageStatus: vi.fn(async () => ({
+        ready: true,
+        defaultKeyId: "LEGACY",
+        secretStorageKeyValidityMap: { LEGACY: true },
+      })),
+      requestOwnUserVerification: vi.fn(async () => null),
+    } as unknown as MatrixCryptoBootstrapApi;
+
+    await store.bootstrapSecretStorageWithRecoveryKey(crypto, {
+      allowSecretStorageRecreateWithoutRecoveryKey: true,
+    });
+
+    expect(createRecoveryKeyFromPassphrase).toHaveBeenCalledTimes(1);
+    expect(bootstrapSecretStorage).toHaveBeenCalledTimes(2);
+    expect(bootstrapSecretStorage).toHaveBeenLastCalledWith(
+      expect.objectContaining({
+        setupNewSecretStorage: true,
+      }),
+    );
+  });
+
   it("stores an encoded recovery key and decodes its private key material", () => {
     const recoveryKeyPath = createTempRecoveryKeyPath();
     const store = new MatrixRecoveryKeyStore(recoveryKeyPath);

extensions/matrix/src/matrix/sdk/recovery-key-store.ts

@@ -10,6 +10,23 @@ import type {
   MatrixStoredRecoveryKey,
 } from "./types.js";
 
+export function isRepairableSecretStorageAccessError(err: unknown): boolean {
+  const message = (err instanceof Error ? err.message : String(err)).toLowerCase();
+  if (!message) {
+    return false;
+  }
+  if (message.includes("getsecretstoragekey callback returned falsey")) {
+    return true;
+  }
+  // The homeserver still has secret storage, but the local recovery key cannot
+  // authenticate/decrypt a required secret. During explicit bootstrap we can
+  // recreate secret storage and continue with a new local baseline.
+  if (message.includes("decrypting secret") && message.includes("bad mac")) {
+    return true;
+  }
+  return false;
+}
+
 export class MatrixRecoveryKeyStore {
   private readonly secretStorageKeyCache = new Map<
     string,
@@ -215,17 +232,16 @@ export class MatrixRecoveryKeyStore {
     } catch (err) {
       const shouldRecreateWithoutRecoveryKey =
         options.allowSecretStorageRecreateWithoutRecoveryKey === true &&
-        !recoveryKey &&
         hasDefaultSecretStorageKey &&
-        err instanceof Error &&
-        err.message.includes("getSecretStorageKey callback returned falsey");
+        isRepairableSecretStorageAccessError(err);
       if (!shouldRecreateWithoutRecoveryKey) {
         throw err;
       }
 
+      recoveryKey = null;
       LogService.warn(
         "MatrixClientLite",
-        "Secret storage exists on the server but no local recovery key is available; recreating secret storage and generating a new recovery key during explicit bootstrap.",
+        "Secret storage exists on the server but local recovery material cannot unlock it; recreating secret storage during explicit bootstrap.",
       );
       await crypto.bootstrapSecretStorage({
         setupNewSecretStorage: true,