Skip to content

Commit 95de96a

Browse files
dahliaclaude
andcommitted
Use constant-time comparison for static credentials
Replace plain === string comparison for static password and username/password auth with timingSafeEqual from node:crypto to prevent timing side-channel attacks on credential checks. Also add tests for the full session lifecycle: logging in with a valid password, using the cookie to access protected pages, and verifying that forged cookies are rejected. #561 #564 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 026bee0 commit 95de96a

2 files changed

Lines changed: 86 additions & 3 deletions

File tree

packages/debugger/src/mod.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -887,6 +887,66 @@ test("auth password: logout cookie omits Secure on HTTP", async () => {
887887
);
888888
});
889889

890+
// ---------- Auth: session lifecycle tests ----------
891+
892+
test("auth password: valid session cookie grants access", async () => {
893+
const { federation } = createMockFederation();
894+
const exporter = createMockExporter();
895+
const auth: FederationDebuggerAuth = {
896+
type: "password",
897+
password: "secret123",
898+
};
899+
const dbg = createFederationDebugger(federation, { exporter, auth });
900+
// Step 1: login to get a session cookie
901+
const loginBody = new URLSearchParams({ password: "secret123" });
902+
const loginResponse = await dbg.fetch(
903+
new Request("https://example.com/__debug__/login", {
904+
method: "POST",
905+
headers: { "Content-Type": "application/x-www-form-urlencoded" },
906+
body: loginBody.toString(),
907+
}),
908+
{ contextData: undefined },
909+
);
910+
strictEqual(loginResponse.status, 303);
911+
const setCookie = loginResponse.headers.get("set-cookie")!;
912+
// Extract the cookie value
913+
const cookieValue = setCookie.split(";")[0];
914+
915+
// Step 2: use the cookie to access a protected page
916+
const dashboardResponse = await dbg.fetch(
917+
new Request("https://example.com/__debug__/", {
918+
headers: { Cookie: cookieValue },
919+
}),
920+
{ contextData: undefined },
921+
);
922+
strictEqual(dashboardResponse.status, 200);
923+
const html = await dashboardResponse.text();
924+
ok(html.includes("Fedify Debug Dashboard"));
925+
});
926+
927+
test("auth password: forged session cookie is rejected", async () => {
928+
const { federation } = createMockFederation();
929+
const exporter = createMockExporter();
930+
const auth: FederationDebuggerAuth = {
931+
type: "password",
932+
password: "secret123",
933+
};
934+
const dbg = createFederationDebugger(federation, { exporter, auth });
935+
// Use a fake/forged cookie value
936+
const response = await dbg.fetch(
937+
new Request("https://example.com/__debug__/", {
938+
headers: {
939+
Cookie: "__fedify_debug_session=deadbeefdeadbeefdeadbeefdeadbeef",
940+
},
941+
}),
942+
{ contextData: undefined },
943+
);
944+
// Should show login form (401), not grant access
945+
strictEqual(response.status, 401);
946+
const html = await response.text();
947+
ok(html.includes("Login Required"));
948+
});
949+
890950
// ---------- Sink property tests ----------
891951

892952
test("createFederationDebugger exposes a sink property", () => {

packages/debugger/src/mod.tsx

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
import { Hono } from "hono";
2727
import { getCookie } from "hono/cookie";
2828
import { AsyncLocalStorage } from "node:async_hooks";
29+
import { timingSafeEqual } from "node:crypto";
2930
import { LoginPage } from "./views/login.tsx";
3031
import { TracesListPage } from "./views/traces-list.tsx";
3132
import { TraceDetailPage } from "./views/trace-detail.tsx";
@@ -465,6 +466,23 @@ async function verifySession(
465466
}
466467
}
467468

469+
/**
470+
* Constant-time string comparison to prevent timing attacks on credential
471+
* checks. Uses {@link timingSafeEqual} from `node:crypto` under the hood.
472+
*/
473+
function constantTimeEqual(a: string, b: string): boolean {
474+
const encoder = new TextEncoder();
475+
const bufA = encoder.encode(a);
476+
const bufB = encoder.encode(b);
477+
if (bufA.byteLength !== bufB.byteLength) {
478+
// Still compare to burn the same amount of time regardless, but
479+
// the result is always false when lengths differ.
480+
timingSafeEqual(bufA, new Uint8Array(bufA.byteLength));
481+
return false;
482+
}
483+
return timingSafeEqual(bufA, bufB);
484+
}
485+
468486
async function checkAuth(
469487
auth: FederationDebuggerAuth,
470488
formData: { username?: string; password: string },
@@ -473,7 +491,7 @@ async function checkAuth(
473491
if ("authenticate" in auth) {
474492
return await auth.authenticate(formData.password);
475493
}
476-
return formData.password === auth.password;
494+
return constantTimeEqual(formData.password, auth.password);
477495
}
478496
if (auth.type === "usernamePassword") {
479497
if ("authenticate" in auth) {
@@ -482,8 +500,13 @@ async function checkAuth(
482500
formData.password,
483501
);
484502
}
485-
return formData.username === auth.username &&
486-
formData.password === auth.password;
503+
// Check both fields in constant time (don't short-circuit)
504+
const usernameMatch = constantTimeEqual(
505+
formData.username ?? "",
506+
auth.username,
507+
);
508+
const passwordMatch = constantTimeEqual(formData.password, auth.password);
509+
return usernameMatch && passwordMatch;
487510
}
488511
return false;
489512
}

0 commit comments

Comments
 (0)