diff --git a/extensions/nextcloud-talk/src/core.test.ts b/extensions/nextcloud-talk/src/core.test.ts index 10601bcb80a..8234314b9ab 100644 --- a/extensions/nextcloud-talk/src/core.test.ts +++ b/extensions/nextcloud-talk/src/core.test.ts @@ -131,6 +131,99 @@ describe("nextcloud talk core", () => { ).toBeNull(); }); + it("still runs timingSafeEqual when the supplied signature length mismatches", async () => { + const timingSafeEqualMock = vi.fn(); + + vi.resetModules(); + vi.doMock("node:crypto", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + timingSafeEqual: vi.fn((left: NodeJS.ArrayBufferView, right: NodeJS.ArrayBufferView) => { + timingSafeEqualMock(left, right); + return actual.timingSafeEqual(left, right); + }), + }; + }); + + const { generateNextcloudTalkSignature, verifyNextcloudTalkSignature } = + await import("./signature.js"); + const body = JSON.stringify({ hello: "world" }); + const generated = generateNextcloudTalkSignature({ + body, + secret: "secret-123", + }); + const shortSignature = generated.signature.slice(0, 12); + + expect( + verifyNextcloudTalkSignature({ + signature: shortSignature, + random: generated.random, + body, + secret: "secret-123", + }), + ).toBe(false); + + expect(timingSafeEqualMock).toHaveBeenCalledOnce(); + const [leftBuffer, rightBuffer] = timingSafeEqualMock.mock.calls[0] ?? []; + expect(Buffer.isBuffer(leftBuffer)).toBe(true); + expect(Buffer.isBuffer(rightBuffer)).toBe(true); + if (!Buffer.isBuffer(leftBuffer) || !Buffer.isBuffer(rightBuffer)) { + throw new TypeError("Expected timingSafeEqual to receive Buffer arguments"); + } + expect(leftBuffer).toHaveLength(rightBuffer.length); + + vi.doUnmock("node:crypto"); + vi.resetModules(); + }); + + it("rejects tampered bodies, wrong secrets, and tampered signatures", () => { + const body = JSON.stringify({ hello: "world" }); + const generated = generateNextcloudTalkSignature({ + body, + secret: "secret-123", + }); + + expect( + verifyNextcloudTalkSignature({ + signature: generated.signature, + random: generated.random, + body: JSON.stringify({ hello: "tampered" }), + secret: "secret-123", + }), + ).toBe(false); + expect( + verifyNextcloudTalkSignature({ + signature: generated.signature, + random: generated.random, + body, + secret: "wrong-secret", + }), + ).toBe(false); + expect( + verifyNextcloudTalkSignature({ + signature: "a".repeat(generated.signature.length), + random: generated.random, + body, + secret: "secret-123", + }), + ).toBe(false); + }); + + it("takes the first value from array-backed headers", () => { + expect( + extractNextcloudTalkHeaders({ + "x-nextcloud-talk-signature": ["sig1", "sig2"], + "x-nextcloud-talk-random": ["rand1", "rand2"], + "x-nextcloud-talk-backend": ["backend1", "backend2"], + }), + ).toEqual({ + signature: "sig1", + random: "rand1", + backend: "backend1", + }); + }); + it("persists replay decisions across guard instances and scopes account namespaces", async () => { const stateDir = await makeTempDir(); diff --git a/extensions/nextcloud-talk/src/signature.ts b/extensions/nextcloud-talk/src/signature.ts index 4f84417a82c..3df6e1309c5 100644 --- a/extensions/nextcloud-talk/src/signature.ts +++ b/extensions/nextcloud-talk/src/signature.ts @@ -1,4 +1,4 @@ -import { createHmac, randomBytes } from "node:crypto"; +import { createHmac, randomBytes, timingSafeEqual } from "node:crypto"; import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/text-runtime"; import type { NextcloudTalkWebhookHeaders } from "./types.js"; @@ -25,14 +25,23 @@ export function verifyNextcloudTalkSignature(params: { .update(random + body) .digest("hex"); - if (signature.length !== expected.length) { - return false; - } - let result = 0; - for (let i = 0; i < signature.length; i++) { - result |= signature.charCodeAt(i) ^ expected.charCodeAt(i); - } - return result === 0; + const expectedBuf = Buffer.from(expected, "utf8"); + const signatureBuf = Buffer.from(signature, "utf8"); + + // Pad to equal length before constant-time comparison to prevent + // leaking length information via early-return timing. + // Note: digest("hex") always produces lowercase ASCII (64 bytes for SHA-256), + // so expectedBuf is always 64 bytes — no variable-length concern on the expected side. + const maxLen = Math.max(expectedBuf.length, signatureBuf.length); + const paddedExpected = Buffer.alloc(maxLen); + const paddedSignature = Buffer.alloc(maxLen); + expectedBuf.copy(paddedExpected); + signatureBuf.copy(paddedSignature); + + // Use crypto.timingSafeEqual instead of manual XOR loop to avoid + // potential JIT-optimisation timing leaks in the JavaScript engine. + const timingResult = timingSafeEqual(paddedExpected, paddedSignature); + return expectedBuf.length === signatureBuf.length && timingResult; } /**