mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 11:10:45 +00:00
fix: harden Matrix recovery trust flow
This commit is contained in:
@@ -370,7 +370,6 @@ describe("matrix verification actions", () => {
|
||||
expect(crypto.confirmVerificationSas).toHaveBeenCalledWith("verification-1");
|
||||
expect(bootstrapOwnDeviceVerification).toHaveBeenCalledWith({
|
||||
allowAutomaticCrossSigningReset: false,
|
||||
verifyOwnIdentity: true,
|
||||
});
|
||||
expect(getOwnDeviceVerificationStatus).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -210,7 +210,6 @@ async function completeMatrixSelfVerification(params: {
|
||||
}): Promise<MatrixSelfVerificationResult> {
|
||||
const bootstrap = await params.client.bootstrapOwnDeviceVerification({
|
||||
allowAutomaticCrossSigningReset: false,
|
||||
verifyOwnIdentity: true,
|
||||
});
|
||||
if (!bootstrap.verification.verified) {
|
||||
throw new Error(
|
||||
|
||||
@@ -1779,6 +1779,78 @@ describe("MatrixClient crypto bootstrapping", () => {
|
||||
expect(persisted.encodedPrivateKey).toBe(previousEncoded);
|
||||
});
|
||||
|
||||
it("does not persist a staged recovery key that secret storage did not validate", async () => {
|
||||
const previousEncoded = encodeRecoveryKey(
|
||||
new Uint8Array(Array.from({ length: 32 }, (_, i) => i + 5)),
|
||||
);
|
||||
const attemptedEncoded = encodeRecoveryKey(
|
||||
new Uint8Array(Array.from({ length: 32 }, (_, i) => i + 55)),
|
||||
);
|
||||
|
||||
matrixJsClient.getUserId = vi.fn(() => "@bot:example.org");
|
||||
matrixJsClient.getDeviceId = vi.fn(() => "DEVICE123");
|
||||
matrixJsClient.getCrypto = vi.fn(() => ({
|
||||
on: vi.fn(),
|
||||
bootstrapCrossSigning: vi.fn(async () => {}),
|
||||
bootstrapSecretStorage: vi.fn(consumeMatrixSecretStorageKey),
|
||||
requestOwnUserVerification: vi.fn(async () => null),
|
||||
getSecretStorageStatus: vi.fn(async () => ({
|
||||
ready: true,
|
||||
defaultKeyId: "SSSSKEY",
|
||||
secretStorageKeyValidityMap: { SSSSKEY: false },
|
||||
})),
|
||||
getDeviceVerificationStatus: vi.fn(async () => ({
|
||||
isVerified: () => true,
|
||||
localVerified: true,
|
||||
crossSigningVerified: false,
|
||||
signedByOwner: false,
|
||||
})),
|
||||
checkKeyBackupAndEnable: vi.fn(async () => {}),
|
||||
getActiveSessionBackupVersion: vi.fn(async () => "11"),
|
||||
getSessionBackupPrivateKey: vi.fn(async () => new Uint8Array([1])),
|
||||
getKeyBackupInfo: vi.fn(async () => ({
|
||||
algorithm: "m.megolm_backup.v1.curve25519-aes-sha2",
|
||||
auth_data: {},
|
||||
version: "11",
|
||||
})),
|
||||
isKeyBackupTrusted: vi.fn(async () => ({
|
||||
trusted: true,
|
||||
matchesDecryptionKey: true,
|
||||
})),
|
||||
}));
|
||||
|
||||
const recoveryDir = fs.mkdtempSync(path.join(os.tmpdir(), "matrix-sdk-verify-invalid-"));
|
||||
const recoveryKeyPath = path.join(recoveryDir, "recovery-key.json");
|
||||
fs.writeFileSync(
|
||||
recoveryKeyPath,
|
||||
JSON.stringify({
|
||||
version: 1,
|
||||
createdAt: new Date().toISOString(),
|
||||
keyId: "SSSSKEY",
|
||||
encodedPrivateKey: previousEncoded,
|
||||
privateKeyBase64: Buffer.from(
|
||||
new Uint8Array(Array.from({ length: 32 }, (_, i) => i + 5)),
|
||||
).toString("base64"),
|
||||
}),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
const client = new MatrixClient("https://matrix.example.org", "token", {
|
||||
encryption: true,
|
||||
recoveryKeyPath,
|
||||
});
|
||||
|
||||
const result = await client.verifyWithRecoveryKey(attemptedEncoded as string);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.recoveryKeyAccepted).toBe(false);
|
||||
expect(result.backupUsable).toBe(true);
|
||||
const persisted = JSON.parse(fs.readFileSync(recoveryKeyPath, "utf8")) as {
|
||||
encodedPrivateKey?: string;
|
||||
};
|
||||
expect(persisted.encodedPrivateKey).toBe(previousEncoded);
|
||||
});
|
||||
|
||||
it("fails recovery-key verification when backup remains untrusted after device verification", async () => {
|
||||
const encoded = encodeRecoveryKey(new Uint8Array(Array.from({ length: 32 }, (_, i) => i + 1)));
|
||||
|
||||
|
||||
@@ -165,13 +165,11 @@ const MATRIX_AUTOMATIC_REPAIR_BOOTSTRAP_OPTIONS = {
|
||||
function createMatrixExplicitBootstrapOptions(params?: {
|
||||
allowAutomaticCrossSigningReset?: boolean;
|
||||
forceResetCrossSigning?: boolean;
|
||||
verifyOwnIdentity?: boolean;
|
||||
}): MatrixCryptoBootstrapOptions {
|
||||
return {
|
||||
forceResetCrossSigning: params?.forceResetCrossSigning === true,
|
||||
allowAutomaticCrossSigningReset: params?.allowAutomaticCrossSigningReset !== false,
|
||||
allowSecretStorageRecreateWithoutRecoveryKey: true,
|
||||
verifyOwnIdentity: params?.verifyOwnIdentity === true,
|
||||
strict: true,
|
||||
};
|
||||
}
|
||||
@@ -1168,10 +1166,12 @@ export class MatrixClient {
|
||||
return await fail("Matrix recovery key is required");
|
||||
}
|
||||
|
||||
let stagedKeyId: string | null = null;
|
||||
try {
|
||||
stagedKeyId = (await this.resolveDefaultSecretStorageKeyId(crypto)) ?? null;
|
||||
this.recoveryKeyStore.stageEncodedRecoveryKey({
|
||||
encodedPrivateKey: trimmedRecoveryKey,
|
||||
keyId: await this.resolveDefaultSecretStorageKeyId(crypto),
|
||||
keyId: stagedKeyId,
|
||||
});
|
||||
} catch (err) {
|
||||
return await fail(formatMatrixErrorMessage(err));
|
||||
@@ -1195,11 +1195,20 @@ export class MatrixClient {
|
||||
requireServerBackup: true,
|
||||
}) === null;
|
||||
const stagedRecoveryKeyUsed = this.recoveryKeyStore.hasStagedRecoveryKeyBeenUsed();
|
||||
const recoveryKeyAccepted = stagedRecoveryKeyUsed && (status.verified || backupUsable);
|
||||
const secretStorageStatus =
|
||||
typeof crypto.getSecretStorageStatus === "function"
|
||||
? await crypto.getSecretStorageStatus().catch(() => null)
|
||||
: null;
|
||||
const stagedRecoveryKeyValidated = Boolean(
|
||||
stagedRecoveryKeyUsed &&
|
||||
stagedKeyId &&
|
||||
secretStorageStatus?.secretStorageKeyValidityMap?.[stagedKeyId] === true,
|
||||
);
|
||||
const recoveryKeyAccepted = stagedRecoveryKeyValidated && (status.verified || backupUsable);
|
||||
if (!status.verified) {
|
||||
if (backupUsable && stagedRecoveryKeyUsed) {
|
||||
if (backupUsable && stagedRecoveryKeyValidated) {
|
||||
this.recoveryKeyStore.commitStagedRecoveryKey({
|
||||
keyId: await this.resolveDefaultSecretStorageKeyId(crypto),
|
||||
keyId: stagedKeyId,
|
||||
});
|
||||
} else {
|
||||
this.recoveryKeyStore.discardStagedRecoveryKey();
|
||||
@@ -1228,7 +1237,7 @@ export class MatrixClient {
|
||||
...status,
|
||||
};
|
||||
}
|
||||
if (!stagedRecoveryKeyUsed) {
|
||||
if (!stagedRecoveryKeyValidated) {
|
||||
this.recoveryKeyStore.discardStagedRecoveryKey();
|
||||
return {
|
||||
success: false,
|
||||
@@ -1242,7 +1251,7 @@ export class MatrixClient {
|
||||
}
|
||||
|
||||
this.recoveryKeyStore.commitStagedRecoveryKey({
|
||||
keyId: await this.resolveDefaultSecretStorageKeyId(crypto),
|
||||
keyId: stagedKeyId,
|
||||
});
|
||||
const committedStatus = await this.getOwnDeviceVerificationStatus();
|
||||
return {
|
||||
@@ -1477,7 +1486,6 @@ export class MatrixClient {
|
||||
allowAutomaticCrossSigningReset?: boolean;
|
||||
recoveryKey?: string;
|
||||
forceResetCrossSigning?: boolean;
|
||||
verifyOwnIdentity?: boolean;
|
||||
}): Promise<MatrixVerificationBootstrapResult> {
|
||||
const pendingVerifications = async (): Promise<number> =>
|
||||
this.crypto ? (await this.crypto.listVerifications()).length : 0;
|
||||
|
||||
@@ -253,7 +253,7 @@ describe("MatrixCryptoBootstrapper", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("can mark the own Matrix identity verified before cross-signing the current device", async () => {
|
||||
it("does not mark the own Matrix identity verified before cross-signing the current device", async () => {
|
||||
const verifyOwnIdentity = vi.fn(async () => undefined);
|
||||
const freeOwnIdentity = vi.fn();
|
||||
const setDeviceVerified = vi.fn(async () => {});
|
||||
@@ -287,11 +287,10 @@ describe("MatrixCryptoBootstrapper", () => {
|
||||
|
||||
await bootstrapper.bootstrap(crypto, {
|
||||
allowAutomaticCrossSigningReset: false,
|
||||
verifyOwnIdentity: true,
|
||||
});
|
||||
|
||||
expect(verifyOwnIdentity).toHaveBeenCalledTimes(1);
|
||||
expect(freeOwnIdentity).toHaveBeenCalledTimes(1);
|
||||
expect(verifyOwnIdentity).not.toHaveBeenCalled();
|
||||
expect(freeOwnIdentity).not.toHaveBeenCalled();
|
||||
expect(setDeviceVerified).toHaveBeenCalledWith("@bot:example.org", "DEVICE123", true);
|
||||
expect(crossSignDevice).toHaveBeenCalledWith("DEVICE123");
|
||||
});
|
||||
|
||||
@@ -28,7 +28,6 @@ export type MatrixCryptoBootstrapOptions = {
|
||||
forceResetCrossSigning?: boolean;
|
||||
allowAutomaticCrossSigningReset?: boolean;
|
||||
allowSecretStorageRecreateWithoutRecoveryKey?: boolean;
|
||||
verifyOwnIdentity?: boolean;
|
||||
strict?: boolean;
|
||||
};
|
||||
|
||||
@@ -86,7 +85,6 @@ export class MatrixCryptoBootstrapper<TRawEvent extends MatrixRawEvent> {
|
||||
}
|
||||
const ownDeviceVerified = await this.ensureOwnDeviceTrust(crypto, {
|
||||
strict,
|
||||
verifyOwnIdentity: options.verifyOwnIdentity === true,
|
||||
});
|
||||
return {
|
||||
crossSigningReady: crossSigning.ready,
|
||||
@@ -351,29 +349,10 @@ export class MatrixCryptoBootstrapper<TRawEvent extends MatrixRawEvent> {
|
||||
LogService.info("MatrixClientLite", "Verification request handler registered");
|
||||
}
|
||||
|
||||
private async verifyOwnIdentityTrust(crypto: MatrixCryptoBootstrapApi): Promise<void> {
|
||||
if (typeof crypto.getOwnIdentity !== "function") {
|
||||
return;
|
||||
}
|
||||
const identity = await crypto.getOwnIdentity();
|
||||
if (!identity) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
if (identity.isVerified?.() === true) {
|
||||
return;
|
||||
}
|
||||
await identity.verify?.();
|
||||
} finally {
|
||||
identity.free?.();
|
||||
}
|
||||
}
|
||||
|
||||
private async ensureOwnDeviceTrust(
|
||||
crypto: MatrixCryptoBootstrapApi,
|
||||
options: {
|
||||
strict: boolean;
|
||||
verifyOwnIdentity: boolean;
|
||||
},
|
||||
): Promise<boolean | null> {
|
||||
const deviceId = this.deps.getDeviceId()?.trim();
|
||||
@@ -392,10 +371,6 @@ export class MatrixCryptoBootstrapper<TRawEvent extends MatrixRawEvent> {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (options.verifyOwnIdentity) {
|
||||
await this.verifyOwnIdentityTrust(crypto);
|
||||
}
|
||||
|
||||
if (typeof crypto.setDeviceVerified === "function") {
|
||||
await crypto.setDeviceVerified(userId, deviceId, true);
|
||||
}
|
||||
|
||||
@@ -1,5 +1,12 @@
|
||||
import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { formatMatrixQaCliCommand, redactMatrixQaCliOutput } from "./scenario-runtime-cli.js";
|
||||
import {
|
||||
formatMatrixQaCliCommand,
|
||||
redactMatrixQaCliOutput,
|
||||
resolveMatrixQaOpenClawCliEntryPath,
|
||||
} from "./scenario-runtime-cli.js";
|
||||
|
||||
describe("Matrix QA CLI runtime", () => {
|
||||
it("redacts secret CLI arguments in diagnostic command text", () => {
|
||||
@@ -23,4 +30,15 @@ describe("Matrix QA CLI runtime", () => {
|
||||
redactMatrixQaCliOutput("GET /_matrix/client/v3/sync?access_token=abcdef1234567890ghij"),
|
||||
).toBe("GET /_matrix/client/v3/sync?access_token=abcdef…ghij");
|
||||
});
|
||||
|
||||
it("prefers the ESM OpenClaw CLI entrypoint when present", async () => {
|
||||
const root = await mkdtemp(path.join(tmpdir(), "matrix-qa-cli-entry-"));
|
||||
try {
|
||||
await mkdir(path.join(root, "dist"));
|
||||
await writeFile(path.join(root, "dist", "index.mjs"), "");
|
||||
expect(resolveMatrixQaOpenClawCliEntryPath(root)).toBe(path.join(root, "dist", "index.mjs"));
|
||||
} finally {
|
||||
await rm(root, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { spawn as startOpenClawCliProcess } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { setTimeout as sleep } from "node:timers/promises";
|
||||
import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime";
|
||||
@@ -48,6 +49,14 @@ export function formatMatrixQaCliCommand(args: string[]) {
|
||||
return `openclaw ${redactMatrixQaCliArgs(args).join(" ")}`;
|
||||
}
|
||||
|
||||
export function resolveMatrixQaOpenClawCliEntryPath(cwd: string): string {
|
||||
const mjsEntryPath = path.join(cwd, "dist", "index.mjs");
|
||||
if (existsSync(mjsEntryPath)) {
|
||||
return mjsEntryPath;
|
||||
}
|
||||
return path.join(cwd, "dist", "index.js");
|
||||
}
|
||||
|
||||
function buildMatrixQaCliResult(params: {
|
||||
args: string[];
|
||||
exitCode: number;
|
||||
@@ -78,7 +87,7 @@ export function startMatrixQaOpenClawCli(params: {
|
||||
timeoutMs: number;
|
||||
}): MatrixQaCliSession {
|
||||
const cwd = params.cwd ?? process.cwd();
|
||||
const distEntryPath = path.join(cwd, "dist", "index.js");
|
||||
const distEntryPath = resolveMatrixQaOpenClawCliEntryPath(cwd);
|
||||
const stdout: Buffer[] = [];
|
||||
const stderr: Buffer[] = [];
|
||||
let closed = false;
|
||||
|
||||
@@ -14,11 +14,13 @@ const { createMatrixQaE2eeScenarioClient, runMatrixQaE2eeBootstrap, startMatrixQ
|
||||
const {
|
||||
formatMatrixQaCliCommand,
|
||||
redactMatrixQaCliOutput,
|
||||
resolveMatrixQaOpenClawCliEntryPath,
|
||||
runMatrixQaOpenClawCli,
|
||||
startMatrixQaOpenClawCli,
|
||||
} = vi.hoisted(() => ({
|
||||
formatMatrixQaCliCommand: (args: string[]) => `openclaw ${args.join(" ")}`,
|
||||
redactMatrixQaCliOutput: (text: string) => text,
|
||||
resolveMatrixQaOpenClawCliEntryPath: (cwd: string) => `${cwd}/dist/index.js`,
|
||||
runMatrixQaOpenClawCli: vi.fn(),
|
||||
startMatrixQaOpenClawCli: vi.fn(),
|
||||
}));
|
||||
@@ -36,6 +38,7 @@ vi.mock("../../substrate/fault-proxy.js", () => ({
|
||||
vi.mock("./scenario-runtime-cli.js", () => ({
|
||||
formatMatrixQaCliCommand,
|
||||
redactMatrixQaCliOutput,
|
||||
resolveMatrixQaOpenClawCliEntryPath,
|
||||
runMatrixQaOpenClawCli,
|
||||
startMatrixQaOpenClawCli,
|
||||
}));
|
||||
|
||||
Reference in New Issue
Block a user