mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(encryption): sample several stored values before declaring a key mismatch
The boot check decided on the first value it found under a key id, so one damaged or foreign-sealed row could take a correctly keyed server down with 503 on every request. The probe now samples up to five values, one per column first. One value that opens clears the key; a mismatch needs at least two values and none opening. A single value that does not open is inconclusive: the server keeps serving, writes no canary, logs a warning and probes again at the next start. Integration setup truncates encryption_key_canaries between tests.
This commit is contained in:
1 parent
ced17569bf
commit
b04b1f2995
5 files changed
+215
-37
No files matched your search
@@ -33,10 +33,19 @@ export async function runBootKeyCheck(client: CanaryClient): Promise<boolean> {
|
||||
console.warn(
|
||||
`[boot] Encryption key check skipped: ${outcome.message.slice(0, 300)}`,
|
||||
);
|
||||
} else if (outcome.written.length > 0) {
|
||||
console.info(
|
||||
`[boot] Encryption key check: recorded key id(s) ${outcome.written.join(", ")}`,
|
||||
);
|
||||
} else {
|
||||
if (outcome.inconclusive.length > 0) {
|
||||
console.warn(
|
||||
`[boot] Encryption key check inconclusive for key id(s) ${outcome.inconclusive.join(", ")}: ` +
|
||||
"the only stored value found under that id did not open. Serving; " +
|
||||
"no canary written; the check runs again at the next start.",
|
||||
);
|
||||
}
|
||||
if (outcome.written.length > 0) {
|
||||
console.info(
|
||||
`[boot] Encryption key check: recorded key id(s) ${outcome.written.join(", ")}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
@@ -29,7 +29,8 @@ function useKey(hex: string) {
|
||||
/** An in-memory stand-in for the two raw statements and the probe delegates. */
|
||||
function fakeClient(opts: {
|
||||
canaries?: Record<string, string>;
|
||||
probe?: Record<string, Record<string, string>>;
|
||||
/** delegate → field → stored values (in row order). */
|
||||
probe?: Record<string, Record<string, string[]>>;
|
||||
}) {
|
||||
const canaries = new Map(Object.entries(opts.canaries ?? {}));
|
||||
const inserts: string[] = [];
|
||||
@@ -49,9 +50,16 @@ function fakeClient(opts: {
|
||||
} as unknown as CanaryClient & Record<string, unknown>;
|
||||
for (const [delegate, fields] of Object.entries(opts.probe ?? {})) {
|
||||
client[delegate] = {
|
||||
async findFirst(args: { select: Record<string, boolean> }) {
|
||||
async findMany(args: {
|
||||
select: Record<string, boolean>;
|
||||
take: number;
|
||||
skip?: number;
|
||||
}) {
|
||||
const field = Object.keys(args.select)[0];
|
||||
return field in fields ? { [field]: fields[field] } : null;
|
||||
const skip = args.skip ?? 0;
|
||||
return (fields[field] ?? [])
|
||||
.slice(skip, skip + args.take)
|
||||
.map((value) => ({ [field]: value }));
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -67,7 +75,12 @@ describe("encryption key canary", () => {
|
||||
it("writes a canary for a configured key with no row and no data", async () => {
|
||||
const { client, canaries, inserts } = fakeClient({});
|
||||
const outcome = await checkEncryptionKeyCanaries(client);
|
||||
expect(outcome).toEqual({ state: "ok", written: ["v1"], verified: [] });
|
||||
expect(outcome).toEqual({
|
||||
state: "ok",
|
||||
written: ["v1"],
|
||||
verified: [],
|
||||
inconclusive: [],
|
||||
});
|
||||
expect(inserts).toEqual(["v1"]);
|
||||
expect(canaries.get("v1")?.startsWith("v1.")).toBe(true);
|
||||
});
|
||||
@@ -77,7 +90,12 @@ describe("encryption key canary", () => {
|
||||
canaries: { v1: encryptUnderKeyId(canaryPlaintext("v1"), "v1") },
|
||||
});
|
||||
const outcome = await checkEncryptionKeyCanaries(client);
|
||||
expect(outcome).toEqual({ state: "ok", written: [], verified: ["v1"] });
|
||||
expect(outcome).toEqual({
|
||||
state: "ok",
|
||||
written: [],
|
||||
verified: ["v1"],
|
||||
inconclusive: [],
|
||||
});
|
||||
expect(inserts).toEqual([]);
|
||||
});
|
||||
|
||||
@@ -100,10 +118,15 @@ describe("encryption key canary", () => {
|
||||
});
|
||||
|
||||
it("refuses to seal a wrong key when existing data predates the canary", async () => {
|
||||
const existing = encrypt("a stored token");
|
||||
const existing = [encrypt("a stored token"), encrypt("another token")];
|
||||
useKey(KEY_B);
|
||||
const { client, inserts } = fakeClient({
|
||||
probe: { user: { codexAccessTokenEncrypted: existing } },
|
||||
probe: {
|
||||
user: {
|
||||
codexAccessTokenEncrypted: [existing[0]],
|
||||
codexRefreshTokenEncrypted: [existing[1]],
|
||||
},
|
||||
},
|
||||
});
|
||||
const outcome = await checkEncryptionKeyCanaries(client);
|
||||
expect(outcome).toEqual({ state: "mismatch", keyIds: ["v1"] });
|
||||
@@ -112,13 +135,59 @@ describe("encryption key canary", () => {
|
||||
|
||||
it("writes the canary when existing data opens under the key", async () => {
|
||||
const { client, inserts } = fakeClient({
|
||||
probe: { user: { codexAccessTokenEncrypted: encrypt("a stored token") } },
|
||||
probe: {
|
||||
user: { codexAccessTokenEncrypted: [encrypt("a stored token")] },
|
||||
},
|
||||
});
|
||||
const outcome = await checkEncryptionKeyCanaries(client);
|
||||
expect(outcome.state).toBe("ok");
|
||||
expect(inserts).toEqual(["v1"]);
|
||||
});
|
||||
|
||||
it("one damaged row among good ones does not take the server down", async () => {
|
||||
// A row that claims key id v1 but does not open (damaged, or sealed by
|
||||
// something else), found FIRST, then values that open.
|
||||
const damaged = "v1." + Buffer.alloc(40, 7).toString("base64");
|
||||
const { client, inserts } = fakeClient({
|
||||
probe: {
|
||||
user: {
|
||||
codexAccessTokenEncrypted: [damaged],
|
||||
codexRefreshTokenEncrypted: [encrypt("a stored token")],
|
||||
},
|
||||
},
|
||||
});
|
||||
const outcome = await checkEncryptionKeyCanaries(client);
|
||||
expect(outcome.state).toBe("ok");
|
||||
expect(inserts).toEqual(["v1"]);
|
||||
});
|
||||
|
||||
it("several values in one column count, so a wrong key with one busy column still refuses", async () => {
|
||||
const values = [encrypt("one"), encrypt("two"), encrypt("three")];
|
||||
useKey(KEY_B);
|
||||
const { client, inserts } = fakeClient({
|
||||
probe: { user: { codexAccessTokenEncrypted: values } },
|
||||
});
|
||||
const outcome = await checkEncryptionKeyCanaries(client);
|
||||
expect(outcome).toEqual({ state: "mismatch", keyIds: ["v1"] });
|
||||
expect(inserts).toEqual([]);
|
||||
});
|
||||
|
||||
it("a single value that does not open is inconclusive: no mismatch, no canary", async () => {
|
||||
const existing = encrypt("the only stored token");
|
||||
useKey(KEY_B);
|
||||
const { client, inserts } = fakeClient({
|
||||
probe: { user: { codexAccessTokenEncrypted: [existing] } },
|
||||
});
|
||||
const outcome = await checkEncryptionKeyCanaries(client);
|
||||
expect(outcome).toEqual({
|
||||
state: "ok",
|
||||
written: [],
|
||||
verified: [],
|
||||
inconclusive: ["v1"],
|
||||
});
|
||||
expect(inserts).toEqual([]);
|
||||
});
|
||||
|
||||
it("reports an error, not a mismatch, when the table cannot be read", async () => {
|
||||
const client = {
|
||||
$queryRaw: async () => {
|
||||
|
||||
+85
-25
@@ -12,16 +12,33 @@
|
||||
*
|
||||
* - row present: it must decrypt to the expected value, else the key is not
|
||||
* the one the row was written with;
|
||||
* - row absent: before writing one, look for ANY existing ciphertext under
|
||||
* that key id in the registered string columns and try it. Without this
|
||||
* - row absent: before writing one, sample existing ciphertext under that
|
||||
* key id from the registered string columns and try it. Without this
|
||||
* probe, the first boot after an upgrade (or a re-keyed boot over a
|
||||
* database that predates the canary) would seal the wrong key as the
|
||||
* right one. A value that opens, or no value at all, lets the canary be
|
||||
* written (`ON CONFLICT DO NOTHING`, so two processes racing is fine).
|
||||
* right one.
|
||||
*
|
||||
* The probe is bounded by a time budget; past it, the canary is written
|
||||
* without a verdict from existing data, which is exactly the behaviour before
|
||||
* this check existed.
|
||||
* The probe samples up to `PROBE_SAMPLE_LIMIT` values, one per column first
|
||||
* (different columns are different writers, so one bad writer cannot speak
|
||||
* for the whole database), then further values from columns that had any.
|
||||
* Its verdict, and what each one does:
|
||||
*
|
||||
* - `opens`: at least one value opened. A wrong key opens nothing, so the
|
||||
* key is right and any value that failed is a damaged or foreign row, not
|
||||
* a key problem. The canary is written.
|
||||
* - `fails`: two or more values were tried and none opened. That is what a
|
||||
* wrong key looks like; the process refuses (mismatch).
|
||||
* - `inconclusive`: exactly one value was found and it did not open. One
|
||||
* row cannot tell a wrong key from one damaged row, and the two possible
|
||||
* mistakes are not equal: declaring a mismatch takes a correctly keyed
|
||||
* server down with 503 on every request, while serving keeps today's
|
||||
* behaviour. So the process serves, but writes NO canary either (that
|
||||
* would seal a key that may be wrong), logs a warning, and probes again
|
||||
* at the next boot, by which time more data may decide it.
|
||||
* - `none`: nothing found inside the time budget. The canary is written,
|
||||
* which is exactly the behaviour before this check existed.
|
||||
*
|
||||
* Writes are `ON CONFLICT DO NOTHING`, so two processes racing is fine.
|
||||
*
|
||||
* Never logs or returns key material. The canary is written by raw SQL and is
|
||||
* deliberately not in the rotation registry: it belongs to its key id.
|
||||
@@ -40,6 +57,9 @@ export function canaryPlaintext(keyId: string): string {
|
||||
return `${CANARY_PREFIX}${keyId}`;
|
||||
}
|
||||
|
||||
/** How many existing values the probe tries at most per key id. */
|
||||
export const PROBE_SAMPLE_LIMIT = 5;
|
||||
|
||||
/** How long the existing-data probe may take before the canary is written anyway. */
|
||||
export const PROBE_BUDGET_MS = 10_000;
|
||||
|
||||
@@ -56,12 +76,16 @@ export interface CanaryClient {
|
||||
}
|
||||
|
||||
type ProbeDelegate = {
|
||||
findFirst: (args: {
|
||||
findMany: (args: {
|
||||
where: Record<string, unknown>;
|
||||
select: Record<string, boolean>;
|
||||
}) => Promise<Record<string, unknown> | null>;
|
||||
take: number;
|
||||
skip?: number;
|
||||
}) => Promise<Array<Record<string, unknown>>>;
|
||||
};
|
||||
|
||||
export type ProbeVerdict = "opens" | "fails" | "inconclusive" | "none";
|
||||
|
||||
export type KeyCheckOutcome =
|
||||
| {
|
||||
state: "ok";
|
||||
@@ -69,6 +93,11 @@ export type KeyCheckOutcome =
|
||||
written: string[];
|
||||
/** Key ids whose canary existed and opened. */
|
||||
verified: string[];
|
||||
/**
|
||||
* Key ids with no canary whose only existing value did not open: not a
|
||||
* mismatch, no canary written, probed again at the next boot.
|
||||
*/
|
||||
inconclusive: string[];
|
||||
}
|
||||
| { state: "mismatch"; keyIds: string[] }
|
||||
| { state: "error"; message: string };
|
||||
@@ -84,36 +113,65 @@ function opensAs(ciphertext: string, keyId: string, expected?: string) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Look for one existing value sealed under `keyId` and try it. Returns
|
||||
* `"opens"`, `"fails"`, or `"none"` (no value found inside the budget).
|
||||
* Sample existing values sealed under `keyId` and try them; see the module
|
||||
* comment for what each verdict means and why.
|
||||
*/
|
||||
export async function probeExistingData(
|
||||
client: unknown,
|
||||
keyId: string,
|
||||
deadline: number,
|
||||
): Promise<"opens" | "fails" | "none"> {
|
||||
): Promise<ProbeVerdict> {
|
||||
const delegates = client as Record<string, ProbeDelegate | undefined>;
|
||||
for (const column of ENCRYPTED_COLUMNS) {
|
||||
if (Date.now() > deadline) return "none";
|
||||
if (column.kind !== "string" || column.codec || column.codecField) {
|
||||
continue;
|
||||
}
|
||||
const columns = ENCRYPTED_COLUMNS.filter(
|
||||
(c) => c.kind === "string" && !c.codec && !c.codecField,
|
||||
);
|
||||
let tried = 0;
|
||||
const columnsWithData: typeof columns = [];
|
||||
|
||||
const sample = async (
|
||||
column: (typeof columns)[number],
|
||||
skip: number,
|
||||
take: number,
|
||||
): Promise<string[]> => {
|
||||
const delegate =
|
||||
delegates[column.model.charAt(0).toLowerCase() + column.model.slice(1)];
|
||||
if (!delegate || typeof delegate.findFirst !== "function") continue;
|
||||
let row: Record<string, unknown> | null;
|
||||
if (!delegate || typeof delegate.findMany !== "function") return [];
|
||||
try {
|
||||
row = await delegate.findFirst({
|
||||
const rows = await delegate.findMany({
|
||||
where: { [column.field]: { startsWith: `${keyId}.` } },
|
||||
select: { [column.field]: true },
|
||||
take,
|
||||
...(skip > 0 ? { skip } : {}),
|
||||
});
|
||||
return rows
|
||||
.map((row) => row[column.field])
|
||||
.filter((v): v is string => typeof v === "string");
|
||||
} catch {
|
||||
continue;
|
||||
return [];
|
||||
}
|
||||
const value = row?.[column.field];
|
||||
if (typeof value !== "string") continue;
|
||||
return opensAs(value, keyId) ? "opens" : "fails";
|
||||
};
|
||||
|
||||
// Pass 1: one value per column, so different writers are heard first.
|
||||
for (const column of columns) {
|
||||
if (tried >= PROBE_SAMPLE_LIMIT || Date.now() > deadline) break;
|
||||
const values = await sample(column, 0, 1);
|
||||
if (values.length === 0) continue;
|
||||
columnsWithData.push(column);
|
||||
tried += 1;
|
||||
if (opensAs(values[0], keyId)) return "opens";
|
||||
}
|
||||
// Pass 2: more values from the columns that had any, up to the limit.
|
||||
for (const column of columnsWithData) {
|
||||
if (tried >= PROBE_SAMPLE_LIMIT || Date.now() > deadline) break;
|
||||
const values = await sample(column, 1, PROBE_SAMPLE_LIMIT - tried);
|
||||
for (const value of values) {
|
||||
tried += 1;
|
||||
if (opensAs(value, keyId)) return "opens";
|
||||
}
|
||||
}
|
||||
|
||||
if (tried >= 2) return "fails";
|
||||
if (tried === 1) return "inconclusive";
|
||||
return "none";
|
||||
}
|
||||
|
||||
@@ -159,9 +217,11 @@ export async function checkEncryptionKeyCanaries(
|
||||
|
||||
const deadline = Date.now() + (options.probeBudgetMs ?? PROBE_BUDGET_MS);
|
||||
const toWrite: string[] = [];
|
||||
const inconclusive: string[] = [];
|
||||
for (const keyId of missing) {
|
||||
const probe = await probeExistingData(client, keyId, deadline);
|
||||
if (probe === "fails") mismatched.push(keyId);
|
||||
else if (probe === "inconclusive") inconclusive.push(keyId);
|
||||
else toWrite.push(keyId);
|
||||
}
|
||||
|
||||
@@ -180,7 +240,7 @@ export async function checkEncryptionKeyCanaries(
|
||||
return { state: "error", message: (err as Error).message };
|
||||
}
|
||||
|
||||
return { state: "ok", written: toWrite, verified };
|
||||
return { state: "ok", written: toWrite, verified, inconclusive };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -170,6 +170,7 @@ describe("boot encryption key check (real Postgres)", () => {
|
||||
username: "existing",
|
||||
email: "existing@example.test",
|
||||
codexAccessTokenEncrypted: encrypt("a stored token"),
|
||||
codexRefreshTokenEncrypted: encrypt("another stored token"),
|
||||
},
|
||||
});
|
||||
|
||||
@@ -186,6 +187,42 @@ describe("boot encryption key check (real Postgres)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("boot key check probe over existing data (real Postgres)", () => {
|
||||
it("one damaged row among good ones still lets the right key through", async () => {
|
||||
const prisma = getPrismaClient();
|
||||
await prisma.user.create({
|
||||
data: {
|
||||
username: "damaged",
|
||||
email: "damaged@example.test",
|
||||
codexAccessTokenEncrypted:
|
||||
"v1." + Buffer.alloc(40, 7).toString("base64"),
|
||||
codexRefreshTokenEncrypted: encrypt("a good token"),
|
||||
},
|
||||
});
|
||||
expect(await runBootKeyCheck(prisma)).toBe(false);
|
||||
expect(isKeyMismatch()).toBe(false);
|
||||
expect(await prisma.encryptionKeyCanary.count()).toBe(1);
|
||||
});
|
||||
|
||||
it("a single value that does not open neither refuses nor writes a canary", async () => {
|
||||
const prisma = getPrismaClient();
|
||||
await prisma.user.create({
|
||||
data: {
|
||||
username: "single",
|
||||
email: "single@example.test",
|
||||
codexAccessTokenEncrypted: encrypt("the only token"),
|
||||
},
|
||||
});
|
||||
useKey(KEY_B);
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
expect(await runBootKeyCheck(prisma)).toBe(false);
|
||||
expect(warn.mock.calls[0]?.[0]).toContain("inconclusive");
|
||||
warn.mockRestore();
|
||||
expect(isKeyMismatch()).toBe(false);
|
||||
expect(await prisma.encryptionKeyCanary.count()).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("encryption key backup step (real Postgres)", () => {
|
||||
it("is due, refuses a stale confirmation, records the right one, and is due again after a re-key", async () => {
|
||||
const prisma = getPrismaClient();
|
||||
|
||||
@@ -51,6 +51,9 @@ export async function truncateAllTables(client: PrismaClient): Promise<void> {
|
||||
"coach_usage",
|
||||
"data_backups",
|
||||
"devices",
|
||||
// Instance-wide, keyed by encryption key id: a canary left by one test
|
||||
// would make the next test's boot check verify instead of write.
|
||||
"encryption_key_canaries",
|
||||
"host_metrics",
|
||||
"idempotency_keys",
|
||||
"integration_statuses",
|
||||
|
||||
Reference in new issue
Block a user