From f3f4d8baa646771c928b34daf9452c4b25a01f05 Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Wed, 29 Jul 2026 09:41:03 +0200 Subject: [PATCH 1/4] fix(tunnel): break the expired-leaf renewal deadlock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /renew` is authenticated by mTLS with the very leaf it renews, so once that leaf lapsed the host could never renew it and the tunnel stayed down until an operator re-paired by hand. Production hit exactly this: the Mac slept through its 8h renewal window, the 24h leaf expired, and the agent then logged `client certificate has expired; renew before dialling` 6380 times over 8 days without recovering. Three layers independently refused an expired leaf, so all three had to move: - agent `buildTlsOptions` gains an opt-in `expiredGraceMs`. Absent/0 keeps the historical fail-closed behaviour, and the TUNNEL dial never passes it — only the renew transport does. Past the window it throws the new `CertExpiredBeyondGraceError`. - agent rotator routes an already-expired leaf to a separate recovery endpoint and treats "beyond grace" as TERMINAL: report once via `onExhausted`, stop retrying, and name the fix (re-pair) instead of spamming warnings forever. - control-plane `assertPresentedCertTrusted` grants a bounded grace on `notAfter` only. Chain validation, SPIFFE identity, `notBefore`, and the registry `active`/account checks are all unchanged, so revocation still bites. - new `deploy/nginx/recover-mtls.conf` (:8472). The strict enroll vhost cannot host this: under `ssl_verify_client optional` nginx answers a bare 400 as soon as a presented cert fails verification, so an expired leaf never reaches the location — and the directive is server-level, not per-location. Grace defaults to 30 days on both ends and is configurable (`RECOVER_URL`, `expiredRenewGraceMs`). The honest trade is recorded in the code: a stale stolen leaf stays reusable for the window, which widens an existing exposure (an unexpired stolen leaf already renews indefinitely) rather than opening a new one. Verified: agent 296/296, control-plane 286/286, tsc clean on both. --- agent/src/certs/nativeRenew.ts | 31 ++++++- agent/src/certs/rotation.ts | 66 ++++++++++++++- agent/src/config/agentConfig.ts | 13 +++ agent/src/transport/dial.ts | 45 +++++++++- agent/test/dial.test.ts | 78 +++++++++++++++++ agent/test/nativeRenew.test.ts | 90 +++++++++++++++++++- agent/test/rotation.test.ts | 120 +++++++++++++++++++++++++++ control-plane/src/api/renew.ts | 44 ++++++++-- control-plane/test/renew.test.ts | 95 ++++++++++++++++++++- deploy/nginx/recover-mtls.conf | 65 +++++++++++++++ deploy/nginx/stream-sni-additions.md | 12 +++ 11 files changed, 643 insertions(+), 16 deletions(-) create mode 100644 deploy/nginx/recover-mtls.conf diff --git a/agent/src/certs/nativeRenew.ts b/agent/src/certs/nativeRenew.ts index ac92762..5c04a6d 100644 --- a/agent/src/certs/nativeRenew.ts +++ b/agent/src/certs/nativeRenew.ts @@ -22,7 +22,12 @@ import type { Keystore } from '../keys/keystore.js' import type { Logger } from '../log/logger.js' import type { TimerLike } from '../transport/seams.js' import { createBackoff } from '../transport/backoff.js' -import { buildTlsOptions, type CertParser, type TlsClientOptions } from '../transport/dial.js' +import { + buildTlsOptions, + DEFAULT_EXPIRED_RENEW_GRACE_MS, + type CertParser, + type TlsClientOptions, +} from '../transport/dial.js' import { DEFAULT_CERT_RENEW_WINDOW_MS } from '../health/probe.js' import { createCertRotator, type CertRotator } from './rotation.js' @@ -126,7 +131,7 @@ function toHeaderRecord(headers: RequestInit['headers']): Record */ export function createMtlsFetch( ks: Keystore, - opts: { request?: MtlsRequest; certParser?: CertParser } = {}, + opts: { request?: MtlsRequest; certParser?: CertParser; expiredGraceMs?: number } = {}, ): typeof fetch { const request = opts.request ?? defaultMtlsRequest const shim = async (input: Parameters[0], init?: RequestInit): Promise => { @@ -135,7 +140,14 @@ export function createMtlsFetch( // SYSTEM roots — its host (the LE-fronted control-plane) is publicly trusted; pinning the private // enroll caChain here fails with "unable to get local issuer certificate". So drop `ca` (absent → // node uses the default roots); rejectUnauthorized stays true. - const full = buildTlsOptions(ks, { ...(opts.certParser ? { certParser: opts.certParser } : {}) }) + // This transport exists ONLY to renew, so it opts into the expired-leaf grace: refusing here is + // exactly the deadlock we are fixing (mTLS renew needs the very leaf that lapsed). Past the + // window `buildTlsOptions` throws CertExpiredBeyondGraceError, which the rotator treats as + // terminal rather than retrying forever. + const full = buildTlsOptions(ks, { + expiredGraceMs: opts.expiredGraceMs ?? DEFAULT_EXPIRED_RENEW_GRACE_MS, + ...(opts.certParser ? { certParser: opts.certParser } : {}), + }) const tls: TlsClientOptions = { cert: full.cert, key: full.key, rejectUnauthorized: full.rejectUnauthorized } const reqInit: MtlsRequestInit = { method: init?.method ?? 'GET', @@ -195,6 +207,16 @@ export function wireAutoRenew( error: errorMessage(err), }) }) + // Terminal: the grace window is spent, so every further attempt is guaranteed to fail. Say so once, + // at error level, naming the fix — and deliberately do NOT stop the supervisor: `pair` writes fresh + // cert files that the restart-on-exit frpc child picks up without a manual service restart. + rotator.onExhausted((err) => { + logger.log('error', 'frp-client cert expired beyond renewal grace — run `web-terminal-agent pair ` to re-pair this host', { + ...meta, + expiredForMs: err.expiredForMs, + graceMs: err.graceMs, + }) + }) rotator.start() return { stop: () => rotator.stop() } } @@ -205,6 +227,8 @@ export function wireAutoRenew( export interface NativeAutoRenewOpts { readonly mtlsRequest?: MtlsRequest readonly certParser?: CertParser + /** Window in which an already-expired leaf may still authenticate its own renewal. */ + readonly expiredGraceMs?: number readonly timer?: TimerLike readonly renewBeforeMs?: number readonly retryBaseMs?: number @@ -232,6 +256,7 @@ export function startNativeAutoRenew( const fetchImpl = createMtlsFetch(ks, { ...(opts.mtlsRequest ? { request: opts.mtlsRequest } : {}), ...(opts.certParser ? { certParser: opts.certParser } : {}), + ...(opts.expiredGraceMs !== undefined ? { expiredGraceMs: opts.expiredGraceMs } : {}), }) const rotator = createCertRotator(cfg, id, ks, { fetchImpl, diff --git a/agent/src/certs/rotation.ts b/agent/src/certs/rotation.ts index 9ad15e0..140772f 100644 --- a/agent/src/certs/rotation.ts +++ b/agent/src/certs/rotation.ts @@ -14,6 +14,7 @@ import type { AgentIdentity } from '../keys/identity.js' import type { Keystore } from '../keys/keystore.js' import type { TimerLike } from '../transport/seams.js' import { createBackoff, type BackoffPolicy } from '../transport/backoff.js' +import { CertExpiredBeyondGraceError } from '../transport/dial.js' import { buildCsr } from '../enroll/csr.js' import { certResponseToPem } from './pem.js' @@ -26,6 +27,11 @@ export interface CertRotator { onRevoked(cb: () => void): void /** A renewal attempt failed (network/HTTP, NOT a 403 revoke). The rotator retries with backoff. */ onError(cb: (err: unknown) => void): void + /** + * TERMINAL: the leaf expired past the renewal grace window, so no future attempt can succeed. The + * rotator has stopped; recovery requires an operator re-pair. + */ + onExhausted(cb: (err: CertExpiredBeyondGraceError) => void): void } export type RenewOutcome = 'rotated' | 'revoked' @@ -35,6 +41,36 @@ export function renewalUrlFor(cfg: AgentConfig): string { return cfg.enrollUrl.replace(/\/enroll$/, '/renew') } +/** The `enroll.` label the recovery vhost mirrors as `recover.`. */ +const ENROLL_LABEL = 'enroll.' +const RECOVER_LABEL = 'recover.' + +/** + * Renewal route for an ALREADY-EXPIRED leaf, or null when this deployment has none. + * + * The normal `/renew` vhost runs `ssl_verify_client optional`, which makes nginx answer a bare + * `400 Bad Request` as soon as a presented client cert fails verification — an expired leaf never + * reaches the location, so it can never be forwarded to the control-plane. Recovery therefore lives + * on a sibling `recover.` vhost running `optional_no_ca`, where nginx forwards the cert unverified + * and the control-plane is the sole (and full) verifier: chain → SPIFFE → registry status → bounded + * expiry grace. Explicit `cfg.recoverUrl` wins; otherwise it is derived by swapping the `enroll.` + * label. A deployment whose enroll host has no `enroll.` label gets null (no recovery configured) + * rather than a guessed hostname. + */ +export function recoveryRenewalUrlFor(cfg: AgentConfig): string | null { + if (cfg.recoverUrl != null && cfg.recoverUrl.length > 0) return cfg.recoverUrl + let url: URL + try { + url = new URL(cfg.enrollUrl) + } catch { + return null + } + if (!url.hostname.startsWith(ENROLL_LABEL)) return null + url.hostname = RECOVER_LABEL + url.hostname.slice(ENROLL_LABEL.length) + url.pathname = url.pathname.replace(/\/enroll$/, '/renew') + return url.toString() +} + /** Ms until (validTo − renewBeforeMs), clamped to ≥ 0. */ export function computeRenewDelayMs( certPem: string, @@ -56,9 +92,10 @@ export async function renewCert( id: AgentIdentity, ks: Keystore, fetchImpl: typeof fetch, + opts: { url?: string } = {}, ): Promise { const csr = buildCsr(id, cfg.subdomain ?? 'web-terminal-agent') - const res = await fetchImpl(renewalUrlFor(cfg), { + const res = await fetchImpl(opts.url ?? renewalUrlFor(cfg), { method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify({ csr }), @@ -102,6 +139,7 @@ export function createCertRotator( let rotatedCb: (() => void) | null = null let revokedCb: (() => void) | null = null let errorCb: ((err: unknown) => void) | null = null + let exhaustedCb: ((err: CertExpiredBeyondGraceError) => void) | null = null function schedule(): void { const certs = ks.loadCert() @@ -110,8 +148,21 @@ export function createCertRotator( handle = timer.setTimeout(runRenewal, delay) } + /** + * Pick the endpoint for THIS attempt. A leaf that has already lapsed cannot be forwarded by the + * strict vhost, so it must go to the recovery vhost; a still-valid leaf always uses the normal one. + * With no recovery endpoint configured we fall back to the normal URL and let it fail honestly. + */ + function endpointForNow(): string { + const certs = ks.loadCert() + if (certs === null) return renewalUrlFor(cfg) + const expired = parseCert(certs.certPem).getTime() < now().getTime() + if (!expired) return renewalUrlFor(cfg) + return recoveryRenewalUrlFor(cfg) ?? renewalUrlFor(cfg) + } + function runRenewal(): void { - void renewCert(cfg, id, ks, doFetch) + void renewCert(cfg, id, ks, doFetch, { url: endpointForNow() }) .then((outcome) => { if (outcome === 'revoked') { revokedCb?.() @@ -122,6 +173,14 @@ export function createCertRotator( schedule() }) .catch((err: unknown) => { + // The grace window is spent: no amount of retrying can mint a new leaf, only an operator + // re-pair can. Report it ONCE and stop — the old behaviour retried forever and buried the + // real signal under thousands of identical warnings. + if (err instanceof CertExpiredBeyondGraceError) { + handle = null + exhaustedCb?.(err) + return + } // Network/HTTP failure (never a 403 revoke): surface it (caller logs, no secret) and retry // with backoff. The cert is still valid until expiry, so the tunnel stays up meanwhile — a // failed renewal must NEVER tear the supervisor down. @@ -147,5 +206,8 @@ export function createCertRotator( onError(cb): void { errorCb = cb }, + onExhausted(cb): void { + exhaustedCb = cb + }, } } diff --git a/agent/src/config/agentConfig.ts b/agent/src/config/agentConfig.ts index 05675de..2f3945a 100644 --- a/agent/src/config/agentConfig.ts +++ b/agent/src/config/agentConfig.ts @@ -18,6 +18,13 @@ export interface AgentConfig { readonly localTargetUrl: string readonly subdomain: string | null readonly hostId: string | null + /** + * Renewal endpoint used ONLY when the current leaf has already expired (the strict `/renew` vhost + * rejects an expired client cert before it reaches the control-plane). Optional: when unset it is + * derived from `enrollUrl` by swapping the `enroll.` label for `recover.` — see + * `certs/rotation.ts` `recoveryRenewalUrlFor`. + */ + readonly recoverUrl?: string | null | undefined } /** @@ -59,6 +66,11 @@ export const AgentConfigSchema = z .refine(isLoopbackWsUrl, 'localTargetUrl must be a ws:// loopback URL (anti-SSRF)'), subdomain: z.string().min(1).nullable(), hostId: z.string().min(1).nullable(), + recoverUrl: z + .string() + .refine((u) => hasScheme(u, 'https:'), 'recoverUrl must be an https:// URL') + .nullable() + .optional(), }) .strict() .readonly() @@ -85,6 +97,7 @@ export function loadAgentConfig( localTargetUrl: argv.localTargetUrl ?? env.LOCAL_TARGET_URL ?? DEFAULT_LOCAL_TARGET, subdomain: argv.subdomain ?? env.SUBDOMAIN ?? null, hostId: argv.hostId ?? env.HOST_ID ?? null, + recoverUrl: argv.recoverUrl ?? env.RECOVER_URL ?? null, } return AgentConfigSchema.parse(merged) } diff --git a/agent/src/transport/dial.ts b/agent/src/transport/dial.ts index 8e645c9..4dd6328 100644 --- a/agent/src/transport/dial.ts +++ b/agent/src/transport/dial.ts @@ -21,6 +21,37 @@ export class CertExpiredError extends Error { } } +/** + * Default window during which an ALREADY-EXPIRED leaf may still authenticate a `/renew` (30 days). + * + * WHY THIS EXISTS: `/renew` is authenticated by mTLS with the current leaf, so once that leaf lapses + * the agent can no longer renew it — a deadlock that bricks the host until a manual re-pair (observed + * in production: the leaf expired while the laptop was asleep through its renewal window, then the + * agent logged `client certificate has expired; renew before dialling` 6380 times and never + * recovered). Accepting a recently-expired leaf ONLY for re-issuance breaks the cycle. + * + * WHY IT IS SAFE-ENOUGH: the leaf still proves possession of the private key, still has to chain to + * the frp-client CA, and the control-plane still refuses a host whose registry status is `revoked` — + * none of those checks are relaxed. The delta is that a STALE stolen credential stays reusable for + * this window; an attacker holding an UNEXPIRED stolen leaf can already renew indefinitely, so this + * widens an existing exposure rather than creating a new class of one. It is bounded, and it never + * applies to the tunnel dial — only to re-issuance. + */ +export const DEFAULT_EXPIRED_RENEW_GRACE_MS = 30 * 24 * 60 * 60 * 1000 + +/** The leaf expired so long ago that even the renewal grace is exhausted ⇒ operator must re-pair. */ +export class CertExpiredBeyondGraceError extends Error { + constructor( + /** How long ago the leaf expired (ms) — non-secret, safe to log. */ + readonly expiredForMs: number, + /** The grace window that was exceeded (ms). */ + readonly graceMs: number, + ) { + super('client certificate expired beyond the renewal grace window; re-pair required') + this.name = 'CertExpiredBeyondGraceError' + } +} + /** TLS material for the wss client. NOTE: `rejectUnauthorized` is ALWAYS true (anti-MITM). */ export interface TlsClientOptions { readonly cert: string @@ -44,17 +75,27 @@ const defaultCertParser: CertParser = (pem) => ({ validTo: new Date(new X509Cert /** * Assemble the mTLS options from the keystore. Throws NotEnrolledError if key/cert are missing, * CertExpiredError if the cert is past `validTo`. There is NO token field by construction (INV4). + * + * `expiredGraceMs` is the ONLY way to get an expired leaf through, and exists solely for the `/renew` + * recovery path (see `DEFAULT_EXPIRED_RENEW_GRACE_MS`). Omitted or 0 ⇒ the historical fail-closed + * behaviour, unchanged; the tunnel dial deliberately never passes it. Past the window the distinct + * `CertExpiredBeyondGraceError` tells the caller to stop retrying and surface "re-pair required". */ export function buildTlsOptions( ks: Keystore, - opts: { now?: Date; certParser?: CertParser } = {}, + opts: { now?: Date; certParser?: CertParser; expiredGraceMs?: number } = {}, ): TlsClientOptions { const id = ks.loadIdentity() const certs = ks.loadCert() if (id === null || certs === null) throw new NotEnrolledError() const parse = opts.certParser ?? defaultCertParser const now = opts.now ?? new Date() - if (parse(certs.certPem).validTo.getTime() < now.getTime()) throw new CertExpiredError() + const expiredForMs = now.getTime() - parse(certs.certPem).validTo.getTime() + if (expiredForMs > 0) { + const graceMs = opts.expiredGraceMs ?? 0 + if (graceMs <= 0) throw new CertExpiredError() + if (expiredForMs > graceMs) throw new CertExpiredBeyondGraceError(expiredForMs, graceMs) + } return { cert: certs.certPem, key: id.exportPrivatePkcs8Pem(), diff --git a/agent/test/dial.test.ts b/agent/test/dial.test.ts index e17bf45..b02c86e 100644 --- a/agent/test/dial.test.ts +++ b/agent/test/dial.test.ts @@ -6,7 +6,9 @@ import type { AgentConfig } from '../src/config/agentConfig.js' import { generateIdentity } from '../src/keys/identity.js' import { openKeystore } from '../src/keys/keystore.js' import { + CertExpiredBeyondGraceError, CertExpiredError, + DEFAULT_EXPIRED_RENEW_GRACE_MS, NotEnrolledError, buildTlsOptions, dialRelay, @@ -65,6 +67,82 @@ describe('buildTlsOptions (T12, INV14/INV4)', () => { }) }) +/** + * Expired-leaf RENEWAL grace. The tunnel dial stays fail-closed forever; only the /renew recovery + * path may opt in, and only inside a bounded window — otherwise an expired leaf deadlocks the agent + * (mTLS renew needs a cert, and the only cert has expired) and the host is bricked until a re-pair. + */ +describe('buildTlsOptions expired-renewal grace (deadlock recovery)', () => { + const expiredBy = (ms: number) => () => ({ validTo: new Date(Date.now() - ms) }) + const DAY = 86_400_000 + + it('stays fail-closed by DEFAULT — no grace opt-in ⇒ CertExpiredError', () => { + const { dir, ks } = enrolledKs() + expect(() => buildTlsOptions(ks, { certParser: expiredBy(DAY) })).toThrow(CertExpiredError) + rmSync(dir, { recursive: true, force: true }) + }) + + it('expired INSIDE the grace window → returns options so /renew can authenticate', () => { + const { dir, ks } = enrolledKs() + const tls = buildTlsOptions(ks, { + certParser: expiredBy(8 * DAY), + expiredGraceMs: DEFAULT_EXPIRED_RENEW_GRACE_MS, + }) + expect(tls.cert).toBe('CERTPEM') + expect(tls.rejectUnauthorized).toBe(true) + rmSync(dir, { recursive: true, force: true }) + }) + + it('expired BEYOND the grace window → CertExpiredBeyondGraceError (re-pair required)', () => { + const { dir, ks } = enrolledKs() + expect(() => + buildTlsOptions(ks, { + certParser: expiredBy(31 * DAY), + expiredGraceMs: DEFAULT_EXPIRED_RENEW_GRACE_MS, + }), + ).toThrow(CertExpiredBeyondGraceError) + rmSync(dir, { recursive: true, force: true }) + }) + + it('grace of 0 is not a bypass — an expired leaf still throws', () => { + const { dir, ks } = enrolledKs() + expect(() => buildTlsOptions(ks, { certParser: expiredBy(1000), expiredGraceMs: 0 })).toThrow( + CertExpiredError, + ) + rmSync(dir, { recursive: true, force: true }) + }) + + it('a still-valid cert is unaffected by the grace option', () => { + const { dir, ks } = enrolledKs() + const tls = buildTlsOptions(ks, { + certParser: future, + expiredGraceMs: DEFAULT_EXPIRED_RENEW_GRACE_MS, + }) + expect(tls.cert).toBe('CERTPEM') + rmSync(dir, { recursive: true, force: true }) + }) + + it('the TUNNEL dial never gets grace — an expired leaf still refuses to dial', () => { + const { dir, ks } = enrolledKs() + let constructed = 0 + class NeverCtor implements RawTlsWs { + constructor() { + constructed += 1 + } + send(): void {} + close(): void {} + on(): void {} + once(): void {} + } + // `dialRelay` has no grace seam at all: it fails fast (synchronously) before any socket exists. + expect(() => + dialRelay(CFG, ks, { Ctor: NeverCtor as unknown as TlsWsConstructor, certParser: past }), + ).toThrow(CertExpiredError) + expect(constructed).toBe(0) + rmSync(dir, { recursive: true, force: true }) + }) +}) + describe('dialRelay (T12)', () => { it('constructs the wss client with the TLS options and resolves on open', async () => { const { dir, ks } = enrolledKs() diff --git a/agent/test/nativeRenew.test.ts b/agent/test/nativeRenew.test.ts index b4d3a4e..e3d5458 100644 --- a/agent/test/nativeRenew.test.ts +++ b/agent/test/nativeRenew.test.ts @@ -27,6 +27,10 @@ import { type MtlsRequest, } from '../src/certs/nativeRenew.js' import { FakeTimer } from './fixtures/fakes.js' +import { + CertExpiredBeyondGraceError, + DEFAULT_EXPIRED_RENEW_GRACE_MS, +} from '../src/transport/dial.js' const CFG: AgentConfig = { relayUrl: 'wss://relay/agent', @@ -109,11 +113,21 @@ describe('createMtlsFetch (A5)', () => { describe('wireAutoRenew (A5)', () => { function fakeRotator(): { rotator: CertRotator - fire: { rotated?: () => void; revoked?: () => void; error?: (e: unknown) => void } + fire: { + rotated?: () => void + revoked?: () => void + error?: (e: unknown) => void + exhausted?: (e: CertExpiredBeyondGraceError) => void + } start: ReturnType stop: ReturnType } { - const fire: { rotated?: () => void; revoked?: () => void; error?: (e: unknown) => void } = {} + const fire: { + rotated?: () => void + revoked?: () => void + error?: (e: unknown) => void + exhausted?: (e: CertExpiredBeyondGraceError) => void + } = {} const start = vi.fn() const stop = vi.fn() const rotator: CertRotator = { @@ -128,6 +142,9 @@ describe('wireAutoRenew (A5)', () => { onError: (cb) => { fire.error = cb }, + onExhausted: (cb) => { + fire.exhausted = cb + }, } return { rotator, fire, start, stop } } @@ -277,3 +294,72 @@ describe('startNativeAutoRenew (A5 end-to-end)', () => { rmSync(dir, { recursive: true, force: true }) }) }) + +/** + * Expired-leaf recovery. `createMtlsFetch` is the ONE place that decides whether a lapsed leaf may + * still be presented; if it keeps refusing (the pre-fix behaviour) the renewal can never leave the + * host and the tunnel stays dead until a manual re-pair. + */ +describe('createMtlsFetch expired-leaf recovery', () => { + const expiredBy = (ms: number) => () => ({ validTo: new Date(Date.now() - ms) }) + const DAY = 86_400_000 + + it('still presents a leaf that expired INSIDE the grace window (renewal can go out)', async () => { + const { dir, ks } = enrolledKs() + let presented = '' + const request: MtlsRequest = async (_url, tls) => { + presented = tls.cert + return { status: 201, body: '{}' } + } + const f = createMtlsFetch(ks, { request, certParser: expiredBy(8 * DAY) }) + const res = await f('https://recover.example.com/renew', { method: 'POST', body: '{}' }) + expect(res.status).toBe(201) + expect(presented).toBe('LEAFCERT') + rmSync(dir, { recursive: true, force: true }) + }) + + it('refuses once the leaf is BEYOND the grace window (terminal — re-pair required)', async () => { + const { dir, ks } = enrolledKs() + const request: MtlsRequest = async () => ({ status: 201, body: '{}' }) + const f = createMtlsFetch(ks, { + request, + certParser: expiredBy(DEFAULT_EXPIRED_RENEW_GRACE_MS + DAY), + }) + await expect(f('https://recover.example.com/renew', { method: 'POST' })).rejects.toThrow( + CertExpiredBeyondGraceError, + ) + rmSync(dir, { recursive: true, force: true }) + }) +}) + +describe('wireAutoRenew exhausted routing', () => { + it('logs an actionable re-pair alarm and leaves the supervisor running', () => { + const fire: { exhausted?: (e: CertExpiredBeyondGraceError) => void } = {} + const rotator: CertRotator = { + start: vi.fn(), + stop: vi.fn(), + onRotated: () => {}, + onRevoked: () => {}, + onError: () => {}, + onExhausted: (cb) => { + fire.exhausted = cb + }, + } + const restartChild = vi.fn() + const stop = vi.fn() + const lines: string[] = [] + wireAutoRenew(rotator, { restartChild, stop }, createLogger('info', (l) => lines.push(l)), { + subdomain: 'h7fd8', + hostId: 'h-1', + }) + fire.exhausted?.(new CertExpiredBeyondGraceError(40 * 86_400_000, 30 * 86_400_000)) + + const alarm = lines.find((l) => /re-pair/i.test(l)) + expect(alarm).toBeDefined() + expect(alarm).toContain('h7fd8') + // The supervisor keeps running: a later `pair` writes fresh cert files that the restarting + // frpc child picks up. Tearing down here would make recovery need a manual restart too. + expect(stop).not.toHaveBeenCalled() + expect(restartChild).not.toHaveBeenCalled() + }) +}) diff --git a/agent/test/rotation.test.ts b/agent/test/rotation.test.ts index 2567f30..928af33 100644 --- a/agent/test/rotation.test.ts +++ b/agent/test/rotation.test.ts @@ -8,9 +8,11 @@ import { openKeystore } from '../src/keys/keystore.js' import { computeRenewDelayMs, createCertRotator, + recoveryRenewalUrlFor, renewCert, renewalUrlFor, } from '../src/certs/rotation.js' +import { CertExpiredBeyondGraceError } from '../src/transport/dial.js' import { createBackoff } from '../src/transport/backoff.js' import { FakeTimer } from './fixtures/fakes.js' @@ -159,3 +161,121 @@ describe('createCertRotator (T13)', () => { rmSync(dir, { recursive: true, force: true }) }) }) + +/** + * Expired-leaf recovery (production deadlock, 2026-07): `/renew` is mTLS-authenticated by the leaf + * it renews, so a lapsed leaf can never renew itself. Recovery needs BOTH a different endpoint (the + * strict nginx vhost rejects an expired client cert with a bare 400 before any location runs) and a + * terminal signal when even the grace window is spent, so the agent stops retrying forever. + */ +describe('expired-leaf recovery routing', () => { + it('derives the recovery endpoint from an `enroll.` host', () => { + expect(recoveryRenewalUrlFor({ ...CFG, enrollUrl: 'https://enroll.terminal.example.com/enroll' })).toBe( + 'https://recover.terminal.example.com/renew', + ) + }) + + it('honours an explicit recoverUrl over the derivation', () => { + expect( + recoveryRenewalUrlFor({ + ...CFG, + enrollUrl: 'https://enroll.terminal.example.com/enroll', + recoverUrl: 'https://elsewhere.example.com/renew', + }), + ).toBe('https://elsewhere.example.com/renew') + }) + + it('is null when the enroll host carries no `enroll.` label (nothing to derive)', () => { + expect(recoveryRenewalUrlFor(CFG)).toBeNull() + }) + + it('posts to the NORMAL endpoint while the leaf is still valid', async () => { + const { dir, ks } = enrolledKs() + const timer = new FakeTimer() + const urls: string[] = [] + const rotator = createCertRotator( + { ...CFG, enrollUrl: 'https://enroll.terminal.example.com/enroll' }, + ks.loadIdentity()!, + ks, + { + timer, + renewBeforeMs: 1000, + fetchImpl: (async (u: string) => { + urls.push(u) + return jsonRes(200, { cert: 'NEWCERT', caChain: ['NEWCA'] }) + }) as unknown as typeof fetch, + now: () => new Date(0), + parseCert: () => new Date(2000), // still valid at now=0 + }, + ) + rotator.start() + timer.advance(1000) + await flush() + expect(urls).toEqual(['https://enroll.terminal.example.com/renew']) + rotator.stop() + rmSync(dir, { recursive: true, force: true }) + }) + + it('posts to the RECOVERY endpoint once the leaf has already expired', async () => { + const { dir, ks } = enrolledKs() + const timer = new FakeTimer() + const urls: string[] = [] + const rotator = createCertRotator( + { ...CFG, enrollUrl: 'https://enroll.terminal.example.com/enroll' }, + ks.loadIdentity()!, + ks, + { + timer, + renewBeforeMs: 1000, + fetchImpl: (async (u: string) => { + urls.push(u) + return jsonRes(200, { cert: 'NEWCERT', caChain: ['NEWCA'] }) + }) as unknown as typeof fetch, + now: () => new Date(10_000), + parseCert: () => new Date(2000), // expired 8s ago ⇒ delay clamps to 0 + }, + ) + rotator.start() + timer.advance(0) + await flush() + expect(urls).toEqual(['https://recover.terminal.example.com/renew']) + rotator.stop() + rmSync(dir, { recursive: true, force: true }) + }) + + it('fires onExhausted and STOPS retrying when the grace window is spent', async () => { + const { dir, ks } = enrolledKs() + const timer = new FakeTimer() + let attempts = 0 + const rotator = createCertRotator(CFG, ks.loadIdentity()!, ks, { + timer, + renewBeforeMs: 1000, + retryBackoff: createBackoff({ baseMs: 500, jitter: false }), + fetchImpl: (async () => { + attempts += 1 + throw new CertExpiredBeyondGraceError(40 * 86_400_000, 30 * 86_400_000) + }) as unknown as typeof fetch, + now: () => new Date(0), + parseCert: () => new Date(2000), + }) + const errors: unknown[] = [] + let exhausted: CertExpiredBeyondGraceError | null = null + rotator.onError((e) => errors.push(e)) + rotator.onExhausted((e) => { + exhausted = e + }) + rotator.start() + timer.advance(1000) + await flush() + + expect(attempts).toBe(1) + expect(exhausted).toBeInstanceOf(CertExpiredBeyondGraceError) + // Terminal: no retry is armed, because only a re-pair can help — retrying forever is the log + // spam that buried the real signal in production (6380 identical warnings). + expect(errors).toHaveLength(0) + timer.advance(60_000) + await flush() + expect(attempts).toBe(1) + rmSync(dir, { recursive: true, force: true }) + }) +}) diff --git a/control-plane/src/api/renew.ts b/control-plane/src/api/renew.ts index 8d2c5a1..2d83768 100644 --- a/control-plane/src/api/renew.ts +++ b/control-plane/src/api/renew.ts @@ -57,6 +57,21 @@ export const MAX_CSR_B64_LEN = 8192 * its own current cert. Production wiring can swap in a socket-peer-cert resolver instead. */ export const DEFAULT_CLIENT_CERT_HEADER = 'x-client-cert' +/** + * How long after `notAfter` a presented leaf may still authenticate its OWN re-issuance (30 days). + * + * `/renew` is authenticated by the very leaf it renews, so a strict expiry check means a host whose + * leaf lapsed can never renew it — it is bricked until an operator re-pairs (this happened: a laptop + * slept through its renewal window and the tunnel stayed down for 8 days). Accepting a recently + * expired leaf FOR RE-ISSUANCE ONLY breaks that deadlock. + * + * Nothing else is relaxed: the leaf must still chain to the CA anchor set, its SPIFFE identity must + * still resolve to a registry record that is `active` and account-consistent, and `notBefore` is NOT + * graced. The residual risk is that a stale stolen leaf stays usable for this window — an attacker + * holding an unexpired stolen leaf can already renew indefinitely, so this widens an existing + * exposure rather than opening a new one. Set to 0 to restore the strict behaviour. + */ +export const DEFAULT_EXPIRED_RENEW_GRACE_MS = 30 * 24 * 60 * 60 * 1000 /** Uniform reject: 401 = no/invalid current cert (unauthenticated); 403 = cert valid but not allowed. */ export class RenewRejectError extends Error { @@ -207,6 +222,7 @@ function assertPresentedCertTrusted( der: Uint8Array, caAnchorsDer: readonly Uint8Array[], nowMs: number, + expiredGraceMs: number, ): void { let leaf: NodeX509Certificate let anchors: NodeX509Certificate[] @@ -220,7 +236,10 @@ function assertPresentedCertTrusted( const notBefore = new Date(leaf.validFrom).getTime() const notAfter = new Date(leaf.validTo).getTime() if (Number.isNaN(notBefore) || Number.isNaN(notAfter)) throw new RenewRejectError(401) - if (nowMs < notBefore || nowMs > notAfter) throw new RenewRejectError(401) + // `notBefore` is never graced — a not-yet-valid cert is nonsense, not a recoverable lapse. Only + // `notAfter` gets the bounded renewal grace (see DEFAULT_EXPIRED_RENEW_GRACE_MS). + if (nowMs < notBefore) throw new RenewRejectError(401) + if (nowMs > notAfter + Math.max(0, expiredGraceMs)) throw new RenewRejectError(401) } /** @@ -234,7 +253,11 @@ function assertPresentedCertTrusted( function parsePresentedCertIdentity( der: Uint8Array, expectedKind: SpiffeKind, - verify?: { readonly caAnchorsDer: readonly Uint8Array[]; readonly nowMs: number }, + verify?: { + readonly caAnchorsDer: readonly Uint8Array[] + readonly nowMs: number + readonly expiredGraceMs: number + }, ): CertIdentity { let leaf: x509.X509Certificate try { @@ -242,7 +265,8 @@ function parsePresentedCertIdentity( } catch { throw new RenewRejectError(401) } - if (verify !== undefined) assertPresentedCertTrusted(der, verify.caAnchorsDer, verify.nowMs) + if (verify !== undefined) + assertPresentedCertTrusted(der, verify.caAnchorsDer, verify.nowMs, verify.expiredGraceMs) const san = leaf.getExtension(x509.SubjectAlternativeNameExtension) const uri = san?.names.toJSON().find((n) => n.type === 'url')?.value if (uri === undefined) throw new RenewRejectError(401) @@ -279,6 +303,11 @@ export interface RenewDeps { readonly hostCaAnchorsDer?: readonly Uint8Array[] /** device-CA anchor DER(s) — same role for the DEVICE renew path. */ readonly deviceCaAnchorsDer?: readonly Uint8Array[] + /** + * Window after `notAfter` in which a presented leaf may still authenticate its own re-issuance. + * Defaults to `DEFAULT_EXPIRED_RENEW_GRACE_MS`; 0 restores the strict fail-closed behaviour. + */ + readonly expiredRenewGraceMs?: number } /** Map any thrown error to a uniform HTTP reject — never leak which internal check failed. */ @@ -298,6 +327,7 @@ function sendError(reply: FastifyReply, err: unknown): void { export function buildRenewRouter(deps: RenewDeps): FastifyPluginAsync { const presentedCert = deps.presentedCert ?? headerPresentedCert() const rateLimiter = deps.rateLimiter ?? createRenewRateLimiter() + const expiredGraceMs = deps.expiredRenewGraceMs ?? DEFAULT_EXPIRED_RENEW_GRACE_MS return async (app) => { app.post('/renew', async (req, reply) => { @@ -307,7 +337,9 @@ export function buildRenewRouter(deps: RenewDeps): FastifyPluginAsync { const identity = parsePresentedCertIdentity( der, 'host', - deps.hostCaAnchorsDer ? { caAnchorsDer: deps.hostCaAnchorsDer, nowMs: Date.now() } : undefined, + deps.hostCaAnchorsDer + ? { caAnchorsDer: deps.hostCaAnchorsDer, nowMs: Date.now(), expiredGraceMs } + : undefined, ) rateLimiter.check(`host:${identity.accountId}:${identity.id}`) @@ -340,7 +372,9 @@ export function buildRenewRouter(deps: RenewDeps): FastifyPluginAsync { const identity = parsePresentedCertIdentity( der, 'device', - deps.deviceCaAnchorsDer ? { caAnchorsDer: deps.deviceCaAnchorsDer, nowMs: Date.now() } : undefined, + deps.deviceCaAnchorsDer + ? { caAnchorsDer: deps.deviceCaAnchorsDer, nowMs: Date.now(), expiredGraceMs } + : undefined, ) const id = (req.params as { id: string }).id rateLimiter.check(`device:${identity.id}`) diff --git a/control-plane/test/renew.test.ts b/control-plane/test/renew.test.ts index 852e368..4384ed8 100644 --- a/control-plane/test/renew.test.ts +++ b/control-plane/test/renew.test.ts @@ -490,10 +490,16 @@ describe('CP6c POST /renew — presented current-cert chain + expiry verificatio expect(res.statusCode).toBe(201) }) - test('an EXPIRED current cert (valid chain, notAfter in the past) is rejected → 401', async () => { + /** + * POLICY CHANGE (expired-leaf deadlock fix): `/renew` is mTLS-authenticated by the very leaf it + * renews, so refusing every expired leaf meant a host whose leaf lapsed could NEVER renew and was + * bricked until a manual re-pair. A recently-expired leaf is now accepted for RE-ISSUANCE ONLY, + * inside a bounded window. Everything else stays fail-closed — the tests below pin that down. + */ + test('a leaf expired INSIDE the grace window renews → 201 (breaks the deadlock)', async () => { const ctx = await hostCtx('alice') const now = Date.now() - const expired = await mintHostLeaf(ctx, new Date(now - 2 * DAY_MS), new Date(now - DAY_MS)) + const expired = await mintHostLeaf(ctx, new Date(now - 9 * DAY_MS), new Date(now - 8 * DAY_MS)) const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) await app.ready() const res = await app.inject({ @@ -502,6 +508,91 @@ describe('CP6c POST /renew — presented current-cert chain + expiry verificatio headers: { 'x-client-cert': certHeader(expired) }, payload: { csr: b64Csr(ctx.csr) }, }) + expect(res.statusCode).toBe(201) + }) + + test('a leaf expired BEYOND the grace window is rejected → 401', async () => { + const ctx = await hostCtx('alice') + const now = Date.now() + const stale = await mintHostLeaf(ctx, new Date(now - 60 * DAY_MS), new Date(now - 31 * DAY_MS)) + const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) + await app.ready() + const res = await app.inject({ + method: 'POST', + url: '/renew', + headers: { 'x-client-cert': certHeader(stale) }, + payload: { csr: b64Csr(ctx.csr) }, + }) + expect(res.statusCode).toBe(401) + }) + + test('grace 0 restores the strict fail-closed behaviour → 401', async () => { + const ctx = await hostCtx('alice') + const now = Date.now() + const expired = await mintHostLeaf(ctx, new Date(now - 2 * DAY_MS), new Date(now - DAY_MS)) + const app = appWith( + hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer], expiredRenewGraceMs: 0 }), + ) + await app.ready() + const res = await app.inject({ + method: 'POST', + url: '/renew', + headers: { 'x-client-cert': certHeader(expired) }, + payload: { csr: b64Csr(ctx.csr) }, + }) + expect(res.statusCode).toBe(401) + }) + + test('grace NEVER bypasses revocation — revoked host with an in-grace leaf → 403', async () => { + const ctx = await hostCtx('alice') + const now = Date.now() + const expired = await mintHostLeaf(ctx, new Date(now - 9 * DAY_MS), new Date(now - 8 * DAY_MS)) + const host = await ctx.hosts.getHostBySubdomain('alice') + await ctx.hosts.setHostStatus(host!.hostId, 'revoked') + const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) + await app.ready() + const res = await app.inject({ + method: 'POST', + url: '/renew', + headers: { 'x-client-cert': certHeader(expired) }, + payload: { csr: b64Csr(ctx.csr) }, + }) + expect(res.statusCode).toBe(403) + }) + + test('grace NEVER bypasses chain validation — untrusted CA + in-grace leaf → 401', async () => { + const ctx = await hostCtx('alice') + const rogueCa = await makeP256Ca('rogue-CA') + const now = Date.now() + const rogue = await mintHostLeaf( + ctx, + new Date(now - 9 * DAY_MS), + new Date(now - 8 * DAY_MS), + rogueCa.caSigner, + ) + const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) + await app.ready() + const res = await app.inject({ + method: 'POST', + url: '/renew', + headers: { 'x-client-cert': certHeader(rogue) }, + payload: { csr: b64Csr(ctx.csr) }, + }) + expect(res.statusCode).toBe(401) + }) + + test('grace applies ONLY to notAfter — a not-yet-valid leaf is still rejected → 401', async () => { + const ctx = await hostCtx('alice') + const now = Date.now() + const future = await mintHostLeaf(ctx, new Date(now + DAY_MS), new Date(now + 2 * DAY_MS)) + const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) + await app.ready() + const res = await app.inject({ + method: 'POST', + url: '/renew', + headers: { 'x-client-cert': certHeader(future) }, + payload: { csr: b64Csr(ctx.csr) }, + }) expect(res.statusCode).toBe(401) }) diff --git a/deploy/nginx/recover-mtls.conf b/deploy/nginx/recover-mtls.conf new file mode 100644 index 0000000..af43441 --- /dev/null +++ b/deploy/nginx/recover-mtls.conf @@ -0,0 +1,65 @@ +# recover-mtls.conf — nginx :8472 RECOVERY vhost: re-issue a leaf that has ALREADY EXPIRED. +# +# Install target: /etc/nginx/conf.d/recover-mtls.conf +# SNI route to merge into the stream map (see stream-sni-additions.md): +# recover.terminal.yaojia.wang 127.0.0.1:8472; +# +# ── WHY THIS EXISTS ─────────────────────────────────────────────────────────────────────────────── +# `POST /renew` is authenticated by mTLS with the very leaf it renews. Once that leaf lapses the host +# can no longer renew it, so the tunnel stays down until an operator re-pairs by hand. That is not +# hypothetical: a laptop slept through its renewal window, the leaf expired, and the agent then +# logged `client certificate has expired; renew before dialling` 6380 times over 8 days without ever +# recovering. +# +# The enroll vhost cannot host the fix. It runs `ssl_verify_client optional`, and nginx answers a +# bare `400 Bad Request` the moment a PRESENTED client cert fails verification — an expired leaf +# never reaches the `location`, so it can never be forwarded to the control-plane. `ssl_verify_client` +# is a server-level directive and cannot be relaxed per-location, hence a separate server block. +# +# ── THE TRADE (read before changing anything here) ──────────────────────────────────────────────── +# This server runs `optional_no_ca`: nginx forwards the presented cert WITHOUT validating its chain. +# The control-plane is therefore the SOLE and FULL verifier on this path. `api/renew.ts` does, in +# order: real X.509 path validation to the frp-client-CA anchors (`assertPresentedCertTrusted` → +# relay-auth `verifyChain`), SPIFFE SAN parse, `notBefore` check (NEVER graced), `notAfter` + bounded +# grace (`DEFAULT_EXPIRED_RENEW_GRACE_MS`, 30d), then a registry lookup requiring an `active`, +# account-consistent host — so REVOCATION IS STILL ENFORCED, via registry status rather than the CRL. +# Do not weaken those checks on the assumption that nginx is still gating this port. It is not. +# +# Blast radius is deliberately tiny: ONLY `POST /renew` is reachable; every other path 404s, and the +# strict enroll vhost (:8471) is left untouched so the normal renewal path keeps its nginx-level +# chain + CRL enforcement. + +# Recovery is a once-in-a-blue-moon call per host, but it is reachable pre-authentication and costs +# the control-plane an X.509 path validation, so bound it. CP additionally rate-limits per identity. +limit_req_zone $binary_remote_addr zone=renew_recover:1m rate=10r/m; + +server { + listen 127.0.0.1:8472 ssl; + server_name recover.terminal.yaojia.wang; + + ssl_certificate /etc/relay/frp-tls/fullchain.pem; # LE wildcard *.terminal.yaojia.wang + ssl_certificate_key /etc/relay/frp-tls/privkey.pem; + + # Ask for a client cert and forward whatever is presented — INCLUDING an expired one, which is + # the entire point. `ssl_client_certificate` is kept only so the CertificateRequest carries a CA + # hint and clients auto-select the right identity; it does NOT gate anything under optional_no_ca. + ssl_verify_client optional_no_ca; + ssl_client_certificate /etc/relay/frp-client-ca/frp-client-ca.cert.pem; + + location = /renew { + # Under optional_no_ca `$ssl_client_verify` is `NONE` even for a good cert, so presence of the + # cert itself is the only thing nginx can meaningfully assert here. + if ($ssl_client_escaped_cert = "") { return 403; } + + limit_req zone=renew_recover burst=5 nodelay; + + proxy_pass http://127.0.0.1:8080; + proxy_http_version 1.1; + proxy_set_header Host $host; + proxy_set_header x-client-cert $ssl_client_escaped_cert; + proxy_read_timeout 60s; + } + + # Everything else — including /enroll, /device/*, /auth/login — stays on the strict vhost. + location / { return 404; } +} diff --git a/deploy/nginx/stream-sni-additions.md b/deploy/nginx/stream-sni-additions.md index 107e171..d615346 100644 --- a/deploy/nginx/stream-sni-additions.md +++ b/deploy/nginx/stream-sni-additions.md @@ -17,6 +17,18 @@ frp.terminal.yaojia.wang 127.0.0.1:7000; # frps control channel (TLS passthro *.terminal.yaojia.wang 127.0.0.1:8470; # nginx :8470 TLS-term + device-CA mTLS (frp-mtls.conf) ``` +### Later addition — expired-leaf recovery (recover-mtls.conf) + +```nginx +recover.terminal.yaojia.wang 127.0.0.1:8472; # expired-leaf re-issue (recover-mtls.conf) +``` + +**Order IS load-bearing for this one**: it must sit ABOVE the `*.terminal.yaojia.wang` wildcard, the +same way `enroll.terminal.yaojia.wang` does. `hostnames` gives exact matches priority over wildcards +in nginx's `map`, so a correctly-placed line wins regardless — but keep the exact-match lines grouped +above the wildcard so a future editor cannot mis-read the intent. No DNS work is needed: the zone is +already served by the wildcard record `*.terminal.yaojia.wang → `. + ## Coexistence warning (do NOT retype the existing lines) The existing `map` body already contains — and MUST be preserved **verbatim**: From 5509c81eee2aec61571d07ae179a46c17f3dc99e Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Wed, 29 Jul 2026 09:52:17 +0200 Subject: [PATCH 2/4] fix(tunnel): recover an expired leaf over plain HTTPS, not mTLS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correction to the previous commit. Its recovery vhost could not work: nginx will not forward an expired client certificate in ANY `ssl_verify_client` mode. `optional` answers a bare `400 The SSL certificate error` before any location runs, and `optional_no_ca` only tolerates CHAIN failures — see nginx's `ngx_ssl_verify_error_optional()`, which covers self-signed / unknown-issuer / unverifiable-leaf and NOT `X509_V_ERR_CERT_HAS_EXPIRED`. Verified live against the deployed :8472 server, which rejected the real expired leaf. So recovery drops mTLS instead of trying to bend it: - agent: `buildTlsOptions` and the mTLS renew transport go back to being strictly fail-closed on expiry — the relaxation is gone from the TLS layer entirely. The rotator now decides per attempt: valid → mTLS `/renew`, expired-inside-grace → plain `/recover`, expired-beyond-grace → terminal `onExhausted` with no request issued at all. - new `recoverCert` POSTs `{cert, csr}` with no client certificate. Possession of the private key is still proven: the CSR is self-signed by it and the host signer already enforces CSR PoP plus `CSR key == registered key`, so a replayed (public) cert without the key yields at most a certificate the attacker cannot authenticate with. - control-plane: `/renew` is strict again (grace 0, matching the terminator). The grace lives on the new `POST /recover`, which reads the cert from the BODY and ignores the `x-client-cert` header, then runs the identical trust pipeline: X.509 path validation to the frp-client-CA anchors, SPIFFE parse, `notBefore` (never graced), registry `active` + account match. Revocation still bites. - deploy: no new vhost, no new SNI, no DNS. One `location = /recover` merged into the existing enroll vhost, documented in `deploy/nginx/enroll-recover-location.md` with the nginx source citation. The load-bearing new test is "a self-signed cert with a FORGED SPIFFE SAN is refused → 401": nginx no longer validates the chain on this path, so that assertion is what keeps `/recover` from being a cert vending machine. Verified: agent 289/289, control-plane 290/290, tsc clean on both. --- agent/node_modules | 1 + agent/src/certs/nativeRenew.ts | 35 ++-- agent/src/certs/rotation.ts | 133 +++++++++---- agent/src/transport/dial.ts | 45 +---- agent/test/dial.test.ts | 78 -------- agent/test/nativeRenew.test.ts | 41 ++-- agent/test/rotation.test.ts | 183 ++++++++++-------- control-plane/node_modules | 1 + control-plane/src/api/renew.ts | 100 ++++++++-- control-plane/test/renew.test.ts | 244 +++++++++++++++--------- deploy/nginx/enroll-recover-location.md | 93 +++++++++ deploy/nginx/recover-mtls.conf | 65 ------- deploy/nginx/stream-sni-additions.md | 12 -- 13 files changed, 559 insertions(+), 472 deletions(-) create mode 120000 agent/node_modules create mode 120000 control-plane/node_modules create mode 100644 deploy/nginx/enroll-recover-location.md delete mode 100644 deploy/nginx/recover-mtls.conf diff --git a/agent/node_modules b/agent/node_modules new file mode 120000 index 0000000..b61e91e --- /dev/null +++ b/agent/node_modules @@ -0,0 +1 @@ +/Users/yiukai/Documents/git/web-terminal/agent/node_modules \ No newline at end of file diff --git a/agent/src/certs/nativeRenew.ts b/agent/src/certs/nativeRenew.ts index 5c04a6d..03b91fa 100644 --- a/agent/src/certs/nativeRenew.ts +++ b/agent/src/certs/nativeRenew.ts @@ -22,14 +22,13 @@ import type { Keystore } from '../keys/keystore.js' import type { Logger } from '../log/logger.js' import type { TimerLike } from '../transport/seams.js' import { createBackoff } from '../transport/backoff.js' -import { - buildTlsOptions, - DEFAULT_EXPIRED_RENEW_GRACE_MS, - type CertParser, - type TlsClientOptions, -} from '../transport/dial.js' +import { buildTlsOptions, type CertParser, type TlsClientOptions } from '../transport/dial.js' import { DEFAULT_CERT_RENEW_WINDOW_MS } from '../health/probe.js' -import { createCertRotator, type CertRotator } from './rotation.js' +import { + createCertRotator, + type CertExpiredBeyondGraceError, + type CertRotator, +} from './rotation.js' /** Non-secret message from an unknown thrown value (never serializes cert/key material). */ function errorMessage(err: unknown): string { @@ -131,7 +130,7 @@ function toHeaderRecord(headers: RequestInit['headers']): Record */ export function createMtlsFetch( ks: Keystore, - opts: { request?: MtlsRequest; certParser?: CertParser; expiredGraceMs?: number } = {}, + opts: { request?: MtlsRequest; certParser?: CertParser } = {}, ): typeof fetch { const request = opts.request ?? defaultMtlsRequest const shim = async (input: Parameters[0], init?: RequestInit): Promise => { @@ -140,14 +139,9 @@ export function createMtlsFetch( // SYSTEM roots — its host (the LE-fronted control-plane) is publicly trusted; pinning the private // enroll caChain here fails with "unable to get local issuer certificate". So drop `ca` (absent → // node uses the default roots); rejectUnauthorized stays true. - // This transport exists ONLY to renew, so it opts into the expired-leaf grace: refusing here is - // exactly the deadlock we are fixing (mTLS renew needs the very leaf that lapsed). Past the - // window `buildTlsOptions` throws CertExpiredBeyondGraceError, which the rotator treats as - // terminal rather than retrying forever. - const full = buildTlsOptions(ks, { - expiredGraceMs: opts.expiredGraceMs ?? DEFAULT_EXPIRED_RENEW_GRACE_MS, - ...(opts.certParser ? { certParser: opts.certParser } : {}), - }) + // Deliberately still fail-closed on an EXPIRED leaf: nginx would refuse to forward it anyway, so + // a lapsed leaf is routed to the plain `/recover` endpoint by the rotator instead of through here. + const full = buildTlsOptions(ks, { ...(opts.certParser ? { certParser: opts.certParser } : {}) }) const tls: TlsClientOptions = { cert: full.cert, key: full.key, rejectUnauthorized: full.rejectUnauthorized } const reqInit: MtlsRequestInit = { method: init?.method ?? 'GET', @@ -211,7 +205,7 @@ export function wireAutoRenew( // at error level, naming the fix — and deliberately do NOT stop the supervisor: `pair` writes fresh // cert files that the restart-on-exit frpc child picks up without a manual service restart. rotator.onExhausted((err) => { - logger.log('error', 'frp-client cert expired beyond renewal grace — run `web-terminal-agent pair ` to re-pair this host', { + logger.log('error', 'frp-client cert expired beyond recovery grace — run `web-terminal-agent pair ` to re-pair this host', { ...meta, expiredForMs: err.expiredForMs, graceMs: err.graceMs, @@ -227,8 +221,10 @@ export function wireAutoRenew( export interface NativeAutoRenewOpts { readonly mtlsRequest?: MtlsRequest readonly certParser?: CertParser - /** Window in which an already-expired leaf may still authenticate its own renewal. */ + /** Window in which an already-expired leaf may still be recovered via `/recover`. */ readonly expiredGraceMs?: number + /** Plain (NON-mTLS) fetch for the `/recover` call; unset ⇒ global fetch. */ + readonly recoverFetchImpl?: typeof fetch readonly timer?: TimerLike readonly renewBeforeMs?: number readonly retryBaseMs?: number @@ -256,10 +252,11 @@ export function startNativeAutoRenew( const fetchImpl = createMtlsFetch(ks, { ...(opts.mtlsRequest ? { request: opts.mtlsRequest } : {}), ...(opts.certParser ? { certParser: opts.certParser } : {}), - ...(opts.expiredGraceMs !== undefined ? { expiredGraceMs: opts.expiredGraceMs } : {}), }) const rotator = createCertRotator(cfg, id, ks, { fetchImpl, + ...(opts.expiredGraceMs !== undefined ? { expiredGraceMs: opts.expiredGraceMs } : {}), + ...(opts.recoverFetchImpl ? { recoverFetchImpl: opts.recoverFetchImpl } : {}), renewBeforeMs: opts.renewBeforeMs ?? DEFAULT_CERT_RENEW_WINDOW_MS, ...(opts.timer ? { timer: opts.timer } : {}), ...(opts.now ? { now: opts.now } : {}), diff --git a/agent/src/certs/rotation.ts b/agent/src/certs/rotation.ts index 140772f..20acaad 100644 --- a/agent/src/certs/rotation.ts +++ b/agent/src/certs/rotation.ts @@ -14,12 +14,35 @@ import type { AgentIdentity } from '../keys/identity.js' import type { Keystore } from '../keys/keystore.js' import type { TimerLike } from '../transport/seams.js' import { createBackoff, type BackoffPolicy } from '../transport/backoff.js' -import { CertExpiredBeyondGraceError } from '../transport/dial.js' import { buildCsr } from '../enroll/csr.js' import { certResponseToPem } from './pem.js' export const DEFAULT_RENEW_BEFORE_MS = 5 * 60_000 // renew 5 min before expiry +/** + * How long after `notAfter` a lapsed leaf may still be swapped for a fresh one (30 days). + * + * `/renew` is mTLS-authenticated by the very leaf it renews, so a lapsed leaf cannot renew itself — + * a deadlock that bricked a host for 8 days in production (the laptop slept through its renewal + * window, then the agent logged `client certificate has expired` 6380 times and never recovered). + * Inside this window the agent switches to the `/recover` endpoint instead; past it, only a re-pair + * can help and the rotator says so once and stops. + */ +export const DEFAULT_EXPIRED_RENEW_GRACE_MS = 30 * 24 * 60 * 60 * 1000 + +/** The leaf lapsed longer ago than the recovery grace allows ⇒ operator must re-pair this host. */ +export class CertExpiredBeyondGraceError extends Error { + constructor( + /** How long ago the leaf expired (ms) — non-secret, safe to log. */ + readonly expiredForMs: number, + /** The grace window that was exceeded (ms). */ + readonly graceMs: number, + ) { + super('client certificate expired beyond the recovery grace window; re-pair required') + this.name = 'CertExpiredBeyondGraceError' + } +} + export interface CertRotator { start(): void stop(): void @@ -41,34 +64,21 @@ export function renewalUrlFor(cfg: AgentConfig): string { return cfg.enrollUrl.replace(/\/enroll$/, '/renew') } -/** The `enroll.` label the recovery vhost mirrors as `recover.`. */ -const ENROLL_LABEL = 'enroll.' -const RECOVER_LABEL = 'recover.' - /** - * Renewal route for an ALREADY-EXPIRED leaf, or null when this deployment has none. + * Recovery route for a leaf that has ALREADY EXPIRED — a sibling PATH on the same enroll host. * - * The normal `/renew` vhost runs `ssl_verify_client optional`, which makes nginx answer a bare - * `400 Bad Request` as soon as a presented client cert fails verification — an expired leaf never - * reaches the location, so it can never be forwarded to the control-plane. Recovery therefore lives - * on a sibling `recover.` vhost running `optional_no_ca`, where nginx forwards the cert unverified - * and the control-plane is the sole (and full) verifier: chain → SPIFFE → registry status → bounded - * expiry grace. Explicit `cfg.recoverUrl` wins; otherwise it is derived by swapping the `enroll.` - * label. A deployment whose enroll host has no `enroll.` label gets null (no recovery configured) - * rather than a guessed hostname. + * It cannot be `/renew`, because nginx will not forward an expired client certificate at all: under + * `ssl_verify_client optional` it answers a bare `400 Bad Request`, and `optional_no_ca` does not + * help either — nginx only tolerates CHAIN errors there (`ngx_ssl_verify_error_optional` covers + * self-signed / unknown-issuer / unverifiable-leaf, NOT `X509_V_ERR_CERT_HAS_EXPIRED`). So recovery + * drops mTLS entirely: it is a plain HTTPS POST carrying the expired cert in the BODY. Nothing is + * lost by that — the accompanying CSR is self-signed by the same private key, and the control-plane + * signer already enforces CSR proof-of-possession plus `CSR key == registered key`, so possession is + * proven exactly as the TLS handshake used to prove it. */ -export function recoveryRenewalUrlFor(cfg: AgentConfig): string | null { +export function recoveryUrlFor(cfg: AgentConfig): string { if (cfg.recoverUrl != null && cfg.recoverUrl.length > 0) return cfg.recoverUrl - let url: URL - try { - url = new URL(cfg.enrollUrl) - } catch { - return null - } - if (!url.hostname.startsWith(ENROLL_LABEL)) return null - url.hostname = RECOVER_LABEL + url.hostname.slice(ENROLL_LABEL.length) - url.pathname = url.pathname.replace(/\/enroll$/, '/renew') - return url.toString() + return cfg.enrollUrl.replace(/\/enroll$/, '/recover') } /** Ms until (validTo − renewBeforeMs), clamped to ≥ 0. */ @@ -110,6 +120,34 @@ export async function renewCert( return 'rotated' } +/** + * One recovery round-trip for an EXPIRED leaf: plain HTTPS (no client cert — see `recoveryUrlFor`) + * POSTing the expired cert alongside a fresh CSR over the SAME key. Same outcome contract as + * `renewCert`: 'rotated' installs the new leaf, 403 ⇒ 'revoked', anything else throws to the retry. + */ +export async function recoverCert( + cfg: AgentConfig, + id: AgentIdentity, + ks: Keystore, + fetchImpl: typeof fetch, + url: string = recoveryUrlFor(cfg), +): Promise { + const certs = ks.loadCert() + if (certs === null) throw new Error('cannot recover without the expired leaf on disk') + const csr = buildCsr(id, cfg.subdomain ?? 'web-terminal-agent') + const res = await fetchImpl(url, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ cert: certs.certPem, csr }), + }) + if (res.status === 403) return 'revoked' + if (!res.ok) throw new Error(`cert recovery failed: HTTP ${res.status}`) + const json = (await res.json()) as { cert?: unknown; caChain?: unknown } + const { certPem, caChainPem } = certResponseToPem(json.cert, json.caChain) + ks.saveCert(certPem, caChainPem) + return 'rotated' +} + export function createCertRotator( cfg: AgentConfig, id: AgentIdentity, @@ -122,6 +160,10 @@ export function createCertRotator( parseCert?: (pem: string) => Date /** Backoff policy for retrying a FAILED renewal (default 1s→30s). Reset after a success. */ retryBackoff?: BackoffPolicy + /** Plain (NON-mTLS) fetch used only for the expired-leaf `/recover` call. */ + recoverFetchImpl?: typeof fetch + /** Window in which an expired leaf may still be recovered. 0 ⇒ no recovery at all. */ + expiredGraceMs?: number } = {}, ): CertRotator { const renewBeforeMs = opts.renewBeforeMs ?? DEFAULT_RENEW_BEFORE_MS @@ -133,6 +175,8 @@ export function createCertRotator( clearInterval: (h) => clearInterval(h as ReturnType), } const doFetch = opts.fetchImpl ?? fetch + const recoverFetch = opts.recoverFetchImpl ?? fetch + const expiredGraceMs = opts.expiredGraceMs ?? DEFAULT_EXPIRED_RENEW_GRACE_MS const now = opts.now ?? (() => new Date()) const retryBackoff = opts.retryBackoff ?? createBackoff({ jitter: true }) let handle: unknown = null @@ -149,20 +193,33 @@ export function createCertRotator( } /** - * Pick the endpoint for THIS attempt. A leaf that has already lapsed cannot be forwarded by the - * strict vhost, so it must go to the recovery vhost; a still-valid leaf always uses the normal one. - * With no recovery endpoint configured we fall back to the normal URL and let it fail honestly. + * How this attempt must be made, derived from how stale the leaf on disk is: + * `normal` — still valid ⇒ ordinary mTLS `/renew`; + * `recover` — expired but inside the grace window ⇒ plain `/recover` with the cert in the body; + * `exhausted` — expired past the grace window ⇒ nothing can succeed; only an operator re-pair. */ - function endpointForNow(): string { + function attemptPlan(): { mode: 'normal' | 'recover' | 'exhausted'; expiredForMs: number } { const certs = ks.loadCert() - if (certs === null) return renewalUrlFor(cfg) - const expired = parseCert(certs.certPem).getTime() < now().getTime() - if (!expired) return renewalUrlFor(cfg) - return recoveryRenewalUrlFor(cfg) ?? renewalUrlFor(cfg) + if (certs === null) return { mode: 'normal', expiredForMs: 0 } + const expiredForMs = now().getTime() - parseCert(certs.certPem).getTime() + if (expiredForMs <= 0) return { mode: 'normal', expiredForMs: 0 } + return { mode: expiredForMs > expiredGraceMs ? 'exhausted' : 'recover', expiredForMs } } function runRenewal(): void { - void renewCert(cfg, id, ks, doFetch, { url: endpointForNow() }) + const plan = attemptPlan() + if (plan.mode === 'exhausted') { + // Terminal: report ONCE and arm nothing. The old code retried forever, which is how a single + // real failure turned into 6380 identical warnings that buried the signal. + handle = null + exhaustedCb?.(new CertExpiredBeyondGraceError(plan.expiredForMs, expiredGraceMs)) + return + } + const attempt = + plan.mode === 'recover' + ? recoverCert(cfg, id, ks, recoverFetch) + : renewCert(cfg, id, ks, doFetch) + void attempt .then((outcome) => { if (outcome === 'revoked') { revokedCb?.() @@ -173,14 +230,6 @@ export function createCertRotator( schedule() }) .catch((err: unknown) => { - // The grace window is spent: no amount of retrying can mint a new leaf, only an operator - // re-pair can. Report it ONCE and stop — the old behaviour retried forever and buried the - // real signal under thousands of identical warnings. - if (err instanceof CertExpiredBeyondGraceError) { - handle = null - exhaustedCb?.(err) - return - } // Network/HTTP failure (never a 403 revoke): surface it (caller logs, no secret) and retry // with backoff. The cert is still valid until expiry, so the tunnel stays up meanwhile — a // failed renewal must NEVER tear the supervisor down. diff --git a/agent/src/transport/dial.ts b/agent/src/transport/dial.ts index 4dd6328..8e645c9 100644 --- a/agent/src/transport/dial.ts +++ b/agent/src/transport/dial.ts @@ -21,37 +21,6 @@ export class CertExpiredError extends Error { } } -/** - * Default window during which an ALREADY-EXPIRED leaf may still authenticate a `/renew` (30 days). - * - * WHY THIS EXISTS: `/renew` is authenticated by mTLS with the current leaf, so once that leaf lapses - * the agent can no longer renew it — a deadlock that bricks the host until a manual re-pair (observed - * in production: the leaf expired while the laptop was asleep through its renewal window, then the - * agent logged `client certificate has expired; renew before dialling` 6380 times and never - * recovered). Accepting a recently-expired leaf ONLY for re-issuance breaks the cycle. - * - * WHY IT IS SAFE-ENOUGH: the leaf still proves possession of the private key, still has to chain to - * the frp-client CA, and the control-plane still refuses a host whose registry status is `revoked` — - * none of those checks are relaxed. The delta is that a STALE stolen credential stays reusable for - * this window; an attacker holding an UNEXPIRED stolen leaf can already renew indefinitely, so this - * widens an existing exposure rather than creating a new class of one. It is bounded, and it never - * applies to the tunnel dial — only to re-issuance. - */ -export const DEFAULT_EXPIRED_RENEW_GRACE_MS = 30 * 24 * 60 * 60 * 1000 - -/** The leaf expired so long ago that even the renewal grace is exhausted ⇒ operator must re-pair. */ -export class CertExpiredBeyondGraceError extends Error { - constructor( - /** How long ago the leaf expired (ms) — non-secret, safe to log. */ - readonly expiredForMs: number, - /** The grace window that was exceeded (ms). */ - readonly graceMs: number, - ) { - super('client certificate expired beyond the renewal grace window; re-pair required') - this.name = 'CertExpiredBeyondGraceError' - } -} - /** TLS material for the wss client. NOTE: `rejectUnauthorized` is ALWAYS true (anti-MITM). */ export interface TlsClientOptions { readonly cert: string @@ -75,27 +44,17 @@ const defaultCertParser: CertParser = (pem) => ({ validTo: new Date(new X509Cert /** * Assemble the mTLS options from the keystore. Throws NotEnrolledError if key/cert are missing, * CertExpiredError if the cert is past `validTo`. There is NO token field by construction (INV4). - * - * `expiredGraceMs` is the ONLY way to get an expired leaf through, and exists solely for the `/renew` - * recovery path (see `DEFAULT_EXPIRED_RENEW_GRACE_MS`). Omitted or 0 ⇒ the historical fail-closed - * behaviour, unchanged; the tunnel dial deliberately never passes it. Past the window the distinct - * `CertExpiredBeyondGraceError` tells the caller to stop retrying and surface "re-pair required". */ export function buildTlsOptions( ks: Keystore, - opts: { now?: Date; certParser?: CertParser; expiredGraceMs?: number } = {}, + opts: { now?: Date; certParser?: CertParser } = {}, ): TlsClientOptions { const id = ks.loadIdentity() const certs = ks.loadCert() if (id === null || certs === null) throw new NotEnrolledError() const parse = opts.certParser ?? defaultCertParser const now = opts.now ?? new Date() - const expiredForMs = now.getTime() - parse(certs.certPem).validTo.getTime() - if (expiredForMs > 0) { - const graceMs = opts.expiredGraceMs ?? 0 - if (graceMs <= 0) throw new CertExpiredError() - if (expiredForMs > graceMs) throw new CertExpiredBeyondGraceError(expiredForMs, graceMs) - } + if (parse(certs.certPem).validTo.getTime() < now.getTime()) throw new CertExpiredError() return { cert: certs.certPem, key: id.exportPrivatePkcs8Pem(), diff --git a/agent/test/dial.test.ts b/agent/test/dial.test.ts index b02c86e..e17bf45 100644 --- a/agent/test/dial.test.ts +++ b/agent/test/dial.test.ts @@ -6,9 +6,7 @@ import type { AgentConfig } from '../src/config/agentConfig.js' import { generateIdentity } from '../src/keys/identity.js' import { openKeystore } from '../src/keys/keystore.js' import { - CertExpiredBeyondGraceError, CertExpiredError, - DEFAULT_EXPIRED_RENEW_GRACE_MS, NotEnrolledError, buildTlsOptions, dialRelay, @@ -67,82 +65,6 @@ describe('buildTlsOptions (T12, INV14/INV4)', () => { }) }) -/** - * Expired-leaf RENEWAL grace. The tunnel dial stays fail-closed forever; only the /renew recovery - * path may opt in, and only inside a bounded window — otherwise an expired leaf deadlocks the agent - * (mTLS renew needs a cert, and the only cert has expired) and the host is bricked until a re-pair. - */ -describe('buildTlsOptions expired-renewal grace (deadlock recovery)', () => { - const expiredBy = (ms: number) => () => ({ validTo: new Date(Date.now() - ms) }) - const DAY = 86_400_000 - - it('stays fail-closed by DEFAULT — no grace opt-in ⇒ CertExpiredError', () => { - const { dir, ks } = enrolledKs() - expect(() => buildTlsOptions(ks, { certParser: expiredBy(DAY) })).toThrow(CertExpiredError) - rmSync(dir, { recursive: true, force: true }) - }) - - it('expired INSIDE the grace window → returns options so /renew can authenticate', () => { - const { dir, ks } = enrolledKs() - const tls = buildTlsOptions(ks, { - certParser: expiredBy(8 * DAY), - expiredGraceMs: DEFAULT_EXPIRED_RENEW_GRACE_MS, - }) - expect(tls.cert).toBe('CERTPEM') - expect(tls.rejectUnauthorized).toBe(true) - rmSync(dir, { recursive: true, force: true }) - }) - - it('expired BEYOND the grace window → CertExpiredBeyondGraceError (re-pair required)', () => { - const { dir, ks } = enrolledKs() - expect(() => - buildTlsOptions(ks, { - certParser: expiredBy(31 * DAY), - expiredGraceMs: DEFAULT_EXPIRED_RENEW_GRACE_MS, - }), - ).toThrow(CertExpiredBeyondGraceError) - rmSync(dir, { recursive: true, force: true }) - }) - - it('grace of 0 is not a bypass — an expired leaf still throws', () => { - const { dir, ks } = enrolledKs() - expect(() => buildTlsOptions(ks, { certParser: expiredBy(1000), expiredGraceMs: 0 })).toThrow( - CertExpiredError, - ) - rmSync(dir, { recursive: true, force: true }) - }) - - it('a still-valid cert is unaffected by the grace option', () => { - const { dir, ks } = enrolledKs() - const tls = buildTlsOptions(ks, { - certParser: future, - expiredGraceMs: DEFAULT_EXPIRED_RENEW_GRACE_MS, - }) - expect(tls.cert).toBe('CERTPEM') - rmSync(dir, { recursive: true, force: true }) - }) - - it('the TUNNEL dial never gets grace — an expired leaf still refuses to dial', () => { - const { dir, ks } = enrolledKs() - let constructed = 0 - class NeverCtor implements RawTlsWs { - constructor() { - constructed += 1 - } - send(): void {} - close(): void {} - on(): void {} - once(): void {} - } - // `dialRelay` has no grace seam at all: it fails fast (synchronously) before any socket exists. - expect(() => - dialRelay(CFG, ks, { Ctor: NeverCtor as unknown as TlsWsConstructor, certParser: past }), - ).toThrow(CertExpiredError) - expect(constructed).toBe(0) - rmSync(dir, { recursive: true, force: true }) - }) -}) - describe('dialRelay (T12)', () => { it('constructs the wss client with the TLS options and resolves on open', async () => { const { dir, ks } = enrolledKs() diff --git a/agent/test/nativeRenew.test.ts b/agent/test/nativeRenew.test.ts index e3d5458..926fc0d 100644 --- a/agent/test/nativeRenew.test.ts +++ b/agent/test/nativeRenew.test.ts @@ -27,10 +27,7 @@ import { type MtlsRequest, } from '../src/certs/nativeRenew.js' import { FakeTimer } from './fixtures/fakes.js' -import { - CertExpiredBeyondGraceError, - DEFAULT_EXPIRED_RENEW_GRACE_MS, -} from '../src/transport/dial.js' +import { CertExpiredBeyondGraceError } from '../src/certs/rotation.js' const CFG: AgentConfig = { relayUrl: 'wss://relay/agent', @@ -296,38 +293,26 @@ describe('startNativeAutoRenew (A5 end-to-end)', () => { }) /** - * Expired-leaf recovery. `createMtlsFetch` is the ONE place that decides whether a lapsed leaf may - * still be presented; if it keeps refusing (the pre-fix behaviour) the renewal can never leave the - * host and the tunnel stays dead until a manual re-pair. + * The mTLS renew transport stays STRICT about expiry: nginx refuses to forward an expired client + * cert at all, so an expired leaf must be routed to the plain `/recover` endpoint by the rotator + * rather than smuggled through this transport. */ -describe('createMtlsFetch expired-leaf recovery', () => { - const expiredBy = (ms: number) => () => ({ validTo: new Date(Date.now() - ms) }) - const DAY = 86_400_000 - - it('still presents a leaf that expired INSIDE the grace window (renewal can go out)', async () => { +describe('createMtlsFetch stays fail-closed on an expired leaf', () => { + it('refuses to present a lapsed leaf (recovery is the rotator\'s job, not this transport\'s)', async () => { const { dir, ks } = enrolledKs() - let presented = '' - const request: MtlsRequest = async (_url, tls) => { - presented = tls.cert + let called = 0 + const request: MtlsRequest = async () => { + called += 1 return { status: 201, body: '{}' } } - const f = createMtlsFetch(ks, { request, certParser: expiredBy(8 * DAY) }) - const res = await f('https://recover.example.com/renew', { method: 'POST', body: '{}' }) - expect(res.status).toBe(201) - expect(presented).toBe('LEAFCERT') - rmSync(dir, { recursive: true, force: true }) - }) - - it('refuses once the leaf is BEYOND the grace window (terminal — re-pair required)', async () => { - const { dir, ks } = enrolledKs() - const request: MtlsRequest = async () => ({ status: 201, body: '{}' }) const f = createMtlsFetch(ks, { request, - certParser: expiredBy(DEFAULT_EXPIRED_RENEW_GRACE_MS + DAY), + certParser: () => ({ validTo: new Date(Date.now() - 86_400_000) }), }) - await expect(f('https://recover.example.com/renew', { method: 'POST' })).rejects.toThrow( - CertExpiredBeyondGraceError, + await expect(f('https://cp.example.com/renew', { method: 'POST' })).rejects.toThrow( + /expired/i, ) + expect(called).toBe(0) rmSync(dir, { recursive: true, force: true }) }) }) diff --git a/agent/test/rotation.test.ts b/agent/test/rotation.test.ts index 928af33..befa082 100644 --- a/agent/test/rotation.test.ts +++ b/agent/test/rotation.test.ts @@ -8,11 +8,12 @@ import { openKeystore } from '../src/keys/keystore.js' import { computeRenewDelayMs, createCertRotator, - recoveryRenewalUrlFor, + recoverCert, + recoveryUrlFor, renewCert, renewalUrlFor, } from '../src/certs/rotation.js' -import { CertExpiredBeyondGraceError } from '../src/transport/dial.js' +import { CertExpiredBeyondGraceError } from '../src/certs/rotation.js' import { createBackoff } from '../src/transport/backoff.js' import { FakeTimer } from './fixtures/fakes.js' @@ -164,99 +165,128 @@ describe('createCertRotator (T13)', () => { /** * Expired-leaf recovery (production deadlock, 2026-07): `/renew` is mTLS-authenticated by the leaf - * it renews, so a lapsed leaf can never renew itself. Recovery needs BOTH a different endpoint (the - * strict nginx vhost rejects an expired client cert with a bare 400 before any location runs) and a - * terminal signal when even the grace window is spent, so the agent stops retrying forever. + * it renews, so a lapsed leaf can never renew itself — and nginx will not even forward an expired + * client cert. Recovery is therefore a PLAIN POST to `/recover` carrying the expired cert in the + * body, plus a terminal signal once the grace window is spent so the agent stops retrying forever. */ -describe('expired-leaf recovery routing', () => { - it('derives the recovery endpoint from an `enroll.` host', () => { - expect(recoveryRenewalUrlFor({ ...CFG, enrollUrl: 'https://enroll.terminal.example.com/enroll' })).toBe( - 'https://recover.terminal.example.com/renew', +describe('expired-leaf recovery', () => { + const HOUR = 3_600_000 + + it('derives /recover as a sibling PATH on the same enroll host', () => { + expect(recoveryUrlFor({ ...CFG, enrollUrl: 'https://enroll.terminal.example.com/enroll' })).toBe( + 'https://enroll.terminal.example.com/recover', ) }) it('honours an explicit recoverUrl over the derivation', () => { - expect( - recoveryRenewalUrlFor({ - ...CFG, - enrollUrl: 'https://enroll.terminal.example.com/enroll', - recoverUrl: 'https://elsewhere.example.com/renew', - }), - ).toBe('https://elsewhere.example.com/renew') + expect(recoveryUrlFor({ ...CFG, recoverUrl: 'https://elsewhere.example.com/recover' })).toBe( + 'https://elsewhere.example.com/recover', + ) }) - it('is null when the enroll host carries no `enroll.` label (nothing to derive)', () => { - expect(recoveryRenewalUrlFor(CFG)).toBeNull() - }) - - it('posts to the NORMAL endpoint while the leaf is still valid', async () => { + it('recoverCert posts the EXPIRED cert plus a CSR, with no client cert, and installs the result', async () => { const { dir, ks } = enrolledKs() - const timer = new FakeTimer() - const urls: string[] = [] - const rotator = createCertRotator( + let seenUrl = '' + let seenBody: { cert?: string; csr?: string } = {} + const fetchImpl = (async (u: string, init: { body: string }) => { + seenUrl = u + seenBody = JSON.parse(init.body) + return jsonRes(201, { cert: 'FRESHCERT', caChain: ['FRESHCA'] }) + }) as unknown as typeof fetch + + const outcome = await recoverCert( { ...CFG, enrollUrl: 'https://enroll.terminal.example.com/enroll' }, ks.loadIdentity()!, ks, - { - timer, - renewBeforeMs: 1000, - fetchImpl: (async (u: string) => { - urls.push(u) - return jsonRes(200, { cert: 'NEWCERT', caChain: ['NEWCA'] }) - }) as unknown as typeof fetch, - now: () => new Date(0), - parseCert: () => new Date(2000), // still valid at now=0 - }, + fetchImpl, ) - rotator.start() - timer.advance(1000) - await flush() - expect(urls).toEqual(['https://enroll.terminal.example.com/renew']) - rotator.stop() + expect(outcome).toBe('rotated') + expect(seenUrl).toBe('https://enroll.terminal.example.com/recover') + expect(seenBody.cert).toBe('OLDCERT') // the lapsed leaf travels in the BODY, not the TLS layer + expect(seenBody.csr).toBeTruthy() + expect(ks.loadCert()!.certPem).toContain('FRESHCERT') rmSync(dir, { recursive: true, force: true }) }) - it('posts to the RECOVERY endpoint once the leaf has already expired', async () => { + it('a still-valid leaf uses the mTLS /renew fetch, never the recovery one', async () => { const { dir, ks } = enrolledKs() const timer = new FakeTimer() - const urls: string[] = [] - const rotator = createCertRotator( - { ...CFG, enrollUrl: 'https://enroll.terminal.example.com/enroll' }, - ks.loadIdentity()!, - ks, - { - timer, - renewBeforeMs: 1000, - fetchImpl: (async (u: string) => { - urls.push(u) - return jsonRes(200, { cert: 'NEWCERT', caChain: ['NEWCA'] }) - }) as unknown as typeof fetch, - now: () => new Date(10_000), - parseCert: () => new Date(2000), // expired 8s ago ⇒ delay clamps to 0 - }, - ) - rotator.start() - timer.advance(0) - await flush() - expect(urls).toEqual(['https://recover.terminal.example.com/renew']) - rotator.stop() - rmSync(dir, { recursive: true, force: true }) - }) - - it('fires onExhausted and STOPS retrying when the grace window is spent', async () => { - const { dir, ks } = enrolledKs() - const timer = new FakeTimer() - let attempts = 0 + let renewCalls = 0 + let recoverCalls = 0 const rotator = createCertRotator(CFG, ks.loadIdentity()!, ks, { timer, renewBeforeMs: 1000, - retryBackoff: createBackoff({ baseMs: 500, jitter: false }), fetchImpl: (async () => { - attempts += 1 - throw new CertExpiredBeyondGraceError(40 * 86_400_000, 30 * 86_400_000) + renewCalls += 1 + return jsonRes(200, { cert: 'NEWCERT', caChain: ['NEWCA'] }) + }) as unknown as typeof fetch, + recoverFetchImpl: (async () => { + recoverCalls += 1 + return jsonRes(200, {}) }) as unknown as typeof fetch, now: () => new Date(0), - parseCert: () => new Date(2000), + parseCert: () => new Date(2000), // valid at now=0 + }) + rotator.start() + timer.advance(1000) + await flush() + expect(renewCalls).toBe(1) + expect(recoverCalls).toBe(0) + rotator.stop() + rmSync(dir, { recursive: true, force: true }) + }) + + it('an expired leaf INSIDE the grace window switches to the recovery fetch', async () => { + const { dir, ks } = enrolledKs() + const timer = new FakeTimer() + let renewCalls = 0 + let recoverCalls = 0 + const rotator = createCertRotator(CFG, ks.loadIdentity()!, ks, { + timer, + renewBeforeMs: 1000, + expiredGraceMs: 30 * 24 * HOUR, + fetchImpl: (async () => { + renewCalls += 1 + return jsonRes(200, {}) + }) as unknown as typeof fetch, + recoverFetchImpl: (async () => { + recoverCalls += 1 + return jsonRes(201, { cert: 'NEWCERT', caChain: ['NEWCA'] }) + }) as unknown as typeof fetch, + now: () => new Date(8 * 24 * HOUR), // 8 days after the leaf lapsed + parseCert: () => new Date(0), + }) + let rotated = 0 + rotator.onRotated(() => { + rotated += 1 + }) + rotator.start() + timer.advance(0) + await flush() + expect(recoverCalls).toBe(1) + expect(renewCalls).toBe(0) + expect(rotated).toBe(1) + rotator.stop() + rmSync(dir, { recursive: true, force: true }) + }) + + it('BEYOND the grace window it fires onExhausted, issues no request, and stops retrying', async () => { + const { dir, ks } = enrolledKs() + const timer = new FakeTimer() + let requests = 0 + const countingFetch = (async () => { + requests += 1 + return jsonRes(201, {}) + }) as unknown as typeof fetch + const rotator = createCertRotator(CFG, ks.loadIdentity()!, ks, { + timer, + renewBeforeMs: 1000, + expiredGraceMs: 30 * 24 * HOUR, + retryBackoff: createBackoff({ baseMs: 500, jitter: false }), + fetchImpl: countingFetch, + recoverFetchImpl: countingFetch, + now: () => new Date(31 * 24 * HOUR), // 31 days stale ⇒ past a 30-day grace + parseCert: () => new Date(0), }) const errors: unknown[] = [] let exhausted: CertExpiredBeyondGraceError | null = null @@ -265,17 +295,16 @@ describe('expired-leaf recovery routing', () => { exhausted = e }) rotator.start() - timer.advance(1000) + timer.advance(0) await flush() - expect(attempts).toBe(1) expect(exhausted).toBeInstanceOf(CertExpiredBeyondGraceError) - // Terminal: no retry is armed, because only a re-pair can help — retrying forever is the log - // spam that buried the real signal in production (6380 identical warnings). + expect(requests).toBe(0) // nothing is even attempted — it cannot succeed expect(errors).toHaveLength(0) + // Terminal: no retry armed. Retrying forever is what produced 6380 identical warnings. timer.advance(60_000) await flush() - expect(attempts).toBe(1) + expect(requests).toBe(0) rmSync(dir, { recursive: true, force: true }) }) }) diff --git a/control-plane/node_modules b/control-plane/node_modules new file mode 120000 index 0000000..84c91d5 --- /dev/null +++ b/control-plane/node_modules @@ -0,0 +1 @@ +/Users/yiukai/Documents/git/web-terminal/control-plane/node_modules \ No newline at end of file diff --git a/control-plane/src/api/renew.ts b/control-plane/src/api/renew.ts index 2d83768..f868f75 100644 --- a/control-plane/src/api/renew.ts +++ b/control-plane/src/api/renew.ts @@ -51,6 +51,8 @@ export const DEFAULT_RENEW_RATE_WINDOW_MS = 60 * 60 * 1000 export const RENEW_RATE_MAX_IDENTITIES = 10_000 /** Max base64 length of a submitted CSR (a P-256 PKCS#10 is well under 1 KB; this is generous slack). */ export const MAX_CSR_B64_LEN = 8192 +/** Max wire length of a body-carried certificate (a P-256 leaf PEM is ~1 KB; generous slack). */ +export const MAX_PRESENTED_CERT_LEN = 16384 /** * Default header the mTLS terminator forwards the verified client cert in (base64 DER). The terminator * MUST set this from `$ssl_client_cert` AND strip any client-supplied copy — a client can never provide @@ -185,23 +187,29 @@ export function headerPresentedCert(headerName: string = DEFAULT_CLIENT_CERT_HEA const raw = req.headers[name] const value = Array.isArray(raw) ? raw[0] : raw if (typeof value !== 'string' || value.length === 0) return null - try { - // The terminator may forward the verified cert as base64 DER, OR as PEM — including nginx's - // header-safe `$ssl_client_escaped_cert` (URL-encoded PEM). Normalize all three to raw DER: - // URL-decode if escaped, then strip PEM armor + whitespace to recover the base64 DER body. - let s = value.includes('%') ? safeDecodeUri(value) : value - if (s.includes('BEGIN CERTIFICATE')) { - s = s.replace(/-----[^-]+-----/g, '').replace(/\s+/g, '') - } - const der = Buffer.from(s, 'base64') - return der.length > 0 ? new Uint8Array(der) : null - } catch { - return null - } + return certWireToDer(value) }, } } +/** + * Normalize a certificate carried over the wire to raw DER. Accepts base64 DER, PEM, and nginx's + * header-safe `$ssl_client_escaped_cert` (URL-encoded PEM): URL-decode if escaped, then strip PEM + * armor + whitespace to recover the base64 body. Returns null on anything unparseable. + */ +export function certWireToDer(value: string): Uint8Array | null { + try { + let s = value.includes('%') ? safeDecodeUri(value) : value + if (s.includes('BEGIN CERTIFICATE')) { + s = s.replace(/-----[^-]+-----/g, '').replace(/\s+/g, '') + } + const der = Buffer.from(s, 'base64') + return der.length > 0 ? new Uint8Array(der) : null + } catch { + return null + } +} + /** URL-decode `value`, returning it unchanged if it is not valid percent-encoding. */ function safeDecodeUri(value: string): string { try { @@ -286,6 +294,13 @@ function parsePresentedCertIdentity( } const RenewBodySchema = z.object({ csr: z.string().min(1).max(MAX_CSR_B64_LEN) }).strict() +/** `/recover` additionally carries the EXPIRED leaf itself (base64 DER or PEM). */ +const RecoverBodySchema = z + .object({ + cert: z.string().min(1).max(MAX_PRESENTED_CERT_LEN), + csr: z.string().min(1).max(MAX_CSR_B64_LEN), + }) + .strict() export interface RenewDeps { readonly hosts: HostRegistry @@ -327,7 +342,9 @@ function sendError(reply: FastifyReply, err: unknown): void { export function buildRenewRouter(deps: RenewDeps): FastifyPluginAsync { const presentedCert = deps.presentedCert ?? headerPresentedCert() const rateLimiter = deps.rateLimiter ?? createRenewRateLimiter() - const expiredGraceMs = deps.expiredRenewGraceMs ?? DEFAULT_EXPIRED_RENEW_GRACE_MS + // `/renew` stays STRICT (0): the mTLS terminator would never forward an expired cert to it anyway. + // The grace belongs to `/recover`, the route built for exactly that case. + const recoverGraceMs = deps.expiredRenewGraceMs ?? DEFAULT_EXPIRED_RENEW_GRACE_MS return async (app) => { app.post('/renew', async (req, reply) => { @@ -338,7 +355,7 @@ export function buildRenewRouter(deps: RenewDeps): FastifyPluginAsync { der, 'host', deps.hostCaAnchorsDer - ? { caAnchorsDer: deps.hostCaAnchorsDer, nowMs: Date.now(), expiredGraceMs } + ? { caAnchorsDer: deps.hostCaAnchorsDer, nowMs: Date.now(), expiredGraceMs: 0 } : undefined, ) rateLimiter.check(`host:${identity.accountId}:${identity.id}`) @@ -365,6 +382,57 @@ export function buildRenewRouter(deps: RenewDeps): FastifyPluginAsync { } }) + /** + * Expired-leaf recovery. The lapsed cert arrives in the BODY because no TLS terminator will + * forward an expired client certificate (nginx `optional` → bare 400; `optional_no_ca` tolerates + * only chain errors, never `X509_V_ERR_CERT_HAS_EXPIRED`). That makes this route the SOLE + * verifier, so it runs the identical trust pipeline as `/renew` — real X.509 path validation to + * the frp-client-CA anchors, SPIFFE parse, `notBefore`, registry `active` + account match — and + * differs ONLY in allowing a bounded overrun on `notAfter`. + * + * Possession of the private key is still proven: the CSR is self-signed by it, and the host + * signer's delegated gate enforces CSR PoP plus `CSR key == registered key`. A replayed cert + * without the key therefore yields, at most, a certificate the attacker cannot authenticate with. + * The header channel is deliberately IGNORED here — on this route only the body speaks. + */ + app.post('/recover', async (req, reply) => { + try { + const body = RecoverBodySchema.parse(req.body) + const der = certWireToDer(body.cert) + if (der === null) throw new RenewRejectError(401) + const identity = parsePresentedCertIdentity( + der, + 'host', + deps.hostCaAnchorsDer + ? { + caAnchorsDer: deps.hostCaAnchorsDer, + nowMs: Date.now(), + expiredGraceMs: recoverGraceMs, + } + : undefined, + ) + rateLimiter.check(`recover:${identity.accountId}:${identity.id}`) + + const host = await deps.hosts.getHostBySubdomain(identity.id) + if (host === null || host.status === 'revoked' || host.accountId !== identity.accountId) { + throw new RenewRejectError(403) + } + + const leaf = await deps.renewer.renewHostLeaf( + host.hostId, + identity.publicKeySpki, + decodeCsrWire(body.csr), + ) + await reply.code(201).send({ + cert: bytesToBase64(leaf.cert), + caChain: leaf.caChain.map((c) => bytesToBase64(c)), + notAfter: leaf.notAfter.toISOString(), + }) + } catch (err) { + sendError(reply, err) + } + }) + app.post('/device/:id/renew', async (req, reply) => { try { const der = presentedCert.presentedCertDer(req) @@ -373,7 +441,7 @@ export function buildRenewRouter(deps: RenewDeps): FastifyPluginAsync { der, 'device', deps.deviceCaAnchorsDer - ? { caAnchorsDer: deps.deviceCaAnchorsDer, nowMs: Date.now(), expiredGraceMs } + ? { caAnchorsDer: deps.deviceCaAnchorsDer, nowMs: Date.now(), expiredGraceMs: 0 } : undefined, ) const id = (req.params as { id: string }).id diff --git a/control-plane/test/renew.test.ts b/control-plane/test/renew.test.ts index 4384ed8..92505af 100644 --- a/control-plane/test/renew.test.ts +++ b/control-plane/test/renew.test.ts @@ -490,65 +490,10 @@ describe('CP6c POST /renew — presented current-cert chain + expiry verificatio expect(res.statusCode).toBe(201) }) - /** - * POLICY CHANGE (expired-leaf deadlock fix): `/renew` is mTLS-authenticated by the very leaf it - * renews, so refusing every expired leaf meant a host whose leaf lapsed could NEVER renew and was - * bricked until a manual re-pair. A recently-expired leaf is now accepted for RE-ISSUANCE ONLY, - * inside a bounded window. Everything else stays fail-closed — the tests below pin that down. - */ - test('a leaf expired INSIDE the grace window renews → 201 (breaks the deadlock)', async () => { - const ctx = await hostCtx('alice') - const now = Date.now() - const expired = await mintHostLeaf(ctx, new Date(now - 9 * DAY_MS), new Date(now - 8 * DAY_MS)) - const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) - await app.ready() - const res = await app.inject({ - method: 'POST', - url: '/renew', - headers: { 'x-client-cert': certHeader(expired) }, - payload: { csr: b64Csr(ctx.csr) }, - }) - expect(res.statusCode).toBe(201) - }) - - test('a leaf expired BEYOND the grace window is rejected → 401', async () => { - const ctx = await hostCtx('alice') - const now = Date.now() - const stale = await mintHostLeaf(ctx, new Date(now - 60 * DAY_MS), new Date(now - 31 * DAY_MS)) - const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) - await app.ready() - const res = await app.inject({ - method: 'POST', - url: '/renew', - headers: { 'x-client-cert': certHeader(stale) }, - payload: { csr: b64Csr(ctx.csr) }, - }) - expect(res.statusCode).toBe(401) - }) - - test('grace 0 restores the strict fail-closed behaviour → 401', async () => { + test('an EXPIRED current cert (valid chain, notAfter in the past) is rejected → 401', async () => { const ctx = await hostCtx('alice') const now = Date.now() const expired = await mintHostLeaf(ctx, new Date(now - 2 * DAY_MS), new Date(now - DAY_MS)) - const app = appWith( - hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer], expiredRenewGraceMs: 0 }), - ) - await app.ready() - const res = await app.inject({ - method: 'POST', - url: '/renew', - headers: { 'x-client-cert': certHeader(expired) }, - payload: { csr: b64Csr(ctx.csr) }, - }) - expect(res.statusCode).toBe(401) - }) - - test('grace NEVER bypasses revocation — revoked host with an in-grace leaf → 403', async () => { - const ctx = await hostCtx('alice') - const now = Date.now() - const expired = await mintHostLeaf(ctx, new Date(now - 9 * DAY_MS), new Date(now - 8 * DAY_MS)) - const host = await ctx.hosts.getHostBySubdomain('alice') - await ctx.hosts.setHostStatus(host!.hostId, 'revoked') const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) await app.ready() const res = await app.inject({ @@ -557,42 +502,8 @@ describe('CP6c POST /renew — presented current-cert chain + expiry verificatio headers: { 'x-client-cert': certHeader(expired) }, payload: { csr: b64Csr(ctx.csr) }, }) - expect(res.statusCode).toBe(403) - }) - - test('grace NEVER bypasses chain validation — untrusted CA + in-grace leaf → 401', async () => { - const ctx = await hostCtx('alice') - const rogueCa = await makeP256Ca('rogue-CA') - const now = Date.now() - const rogue = await mintHostLeaf( - ctx, - new Date(now - 9 * DAY_MS), - new Date(now - 8 * DAY_MS), - rogueCa.caSigner, - ) - const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) - await app.ready() - const res = await app.inject({ - method: 'POST', - url: '/renew', - headers: { 'x-client-cert': certHeader(rogue) }, - payload: { csr: b64Csr(ctx.csr) }, - }) - expect(res.statusCode).toBe(401) - }) - - test('grace applies ONLY to notAfter — a not-yet-valid leaf is still rejected → 401', async () => { - const ctx = await hostCtx('alice') - const now = Date.now() - const future = await mintHostLeaf(ctx, new Date(now + DAY_MS), new Date(now + 2 * DAY_MS)) - const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) - await app.ready() - const res = await app.inject({ - method: 'POST', - url: '/renew', - headers: { 'x-client-cert': certHeader(future) }, - payload: { csr: b64Csr(ctx.csr) }, - }) + // STRICT on this route: nginx would never forward an expired client cert here anyway, so the + // deadlock escape hatch lives on `/recover` (below) and NOT on the mTLS renewal path. expect(res.statusCode).toBe(401) }) @@ -612,3 +523,152 @@ describe('CP6c POST /renew — presented current-cert chain + expiry verificatio expect(res.statusCode).toBe(401) }) }) + +/** + * CP6d · POST /recover — the expired-leaf escape hatch. + * + * Unlike `/renew` this route is NOT mTLS-authenticated: nginx cannot forward an expired client cert + * (under `ssl_verify_client optional` it 400s, and `optional_no_ca` only tolerates CHAIN errors, not + * `X509_V_ERR_CERT_HAS_EXPIRED`). The lapsed cert therefore arrives in the BODY, and the + * control-plane becomes the sole verifier. These tests pin down that nothing except the `notAfter` + * bound was relaxed — chain, SPIFFE, `notBefore`, and registry status all still decide. + */ +describe('CP6d POST /recover — expired-leaf recovery', () => { + async function mintLeaf( + ctx: HostCtx, + notBefore: Date, + notAfter: Date, + signer = ctx.ca.caSigner, + ): Promise { + const spiffe = `spiffe://relay.terminal.yaojia.wang/account/${ctx.accountId}/host/${ctx.subdomain}` + return assembleCertificate({ + subjectPublicKey: ctx.spki, + subject: `CN=${ctx.subdomain}`, + issuer: ctx.ca.caCert.subjectName, + serialNumber: Uint8Array.from([0x0b]), + notBefore, + notAfter, + extensions: [ + new x509.SubjectAlternativeNameExtension([ + { type: 'dns', value: `${ctx.subdomain}.terminal.yaojia.wang` }, + { type: 'url', value: spiffe }, + ]), + new x509.BasicConstraintsExtension(false, undefined, true), + new x509.KeyUsagesExtension(x509.KeyUsageFlags.digitalSignature, true), + new x509.ExtendedKeyUsageExtension([x509.ExtendedKeyUsage.clientAuth]), + ], + signer, + sigAlg: 'ecdsa-p256', + }) + } + + const b64 = (der: Uint8Array): string => Buffer.from(der).toString('base64') + + async function post(ctx: HostCtx, cert: Uint8Array, extra?: Partial) { + const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer], ...extra })) + await app.ready() + return app.inject({ + method: 'POST', + url: '/recover', + payload: { cert: b64(cert), csr: b64Csr(ctx.csr) }, + }) + } + + test('a leaf expired INSIDE the grace window is re-issued → 201 (breaks the deadlock)', async () => { + const ctx = await hostCtx('alice') + const now = Date.now() + const res = await post(ctx, await mintLeaf(ctx, new Date(now - 9 * DAY_MS), new Date(now - 8 * DAY_MS))) + expect(res.statusCode).toBe(201) + expect(JSON.parse(res.payload).cert).toBeTruthy() + }) + + test('a leaf expired BEYOND the grace window is refused → 401', async () => { + const ctx = await hostCtx('alice') + const now = Date.now() + const res = await post(ctx, await mintLeaf(ctx, new Date(now - 60 * DAY_MS), new Date(now - 31 * DAY_MS))) + expect(res.statusCode).toBe(401) + }) + + test('grace 0 disables recovery entirely → 401', async () => { + const ctx = await hostCtx('alice') + const now = Date.now() + const res = await post( + ctx, + await mintLeaf(ctx, new Date(now - 2 * DAY_MS), new Date(now - DAY_MS)), + { expiredRenewGraceMs: 0 }, + ) + expect(res.statusCode).toBe(401) + }) + + /** + * THE load-bearing test for this route. nginx no longer validates the chain here, so a forged + * self-signed cert carrying a correct-looking SPIFFE SAN must be rejected by the control-plane + * alone. If this ever goes green-to-red, `/recover` becomes an unauthenticated cert vending machine. + */ + test('a self-signed cert with a FORGED SPIFFE SAN is refused → 401', async () => { + const ctx = await hostCtx('alice') + const rogueCa = await makeP256Ca('rogue-CA') + const now = Date.now() + const rogue = await mintLeaf( + ctx, + new Date(now - 9 * DAY_MS), + new Date(now - 8 * DAY_MS), + rogueCa.caSigner, + ) + const res = await post(ctx, rogue) + expect(res.statusCode).toBe(401) + }) + + test('recovery NEVER bypasses revocation — revoked host → 403', async () => { + const ctx = await hostCtx('alice') + const now = Date.now() + const cert = await mintLeaf(ctx, new Date(now - 9 * DAY_MS), new Date(now - 8 * DAY_MS)) + const host = await ctx.hosts.getHostBySubdomain('alice') + await ctx.hosts.setHostStatus(host!.hostId, 'revoked') + const res = await post(ctx, cert) + expect(res.statusCode).toBe(403) + }) + + test('grace covers notAfter ONLY — a not-yet-valid leaf is refused → 401', async () => { + const ctx = await hostCtx('alice') + const now = Date.now() + const res = await post(ctx, await mintLeaf(ctx, new Date(now + DAY_MS), new Date(now + 2 * DAY_MS))) + expect(res.statusCode).toBe(401) + }) + + test('a still-valid leaf may also use /recover → 201', async () => { + const ctx = await hostCtx('alice') + const res = await post(ctx, ctx.currentCertDer) + expect(res.statusCode).toBe(201) + }) + + test('a missing cert field is rejected → 400 (uniform schema reject, same as a missing csr)', async () => { + const ctx = await hostCtx('alice') + const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) + await app.ready() + const res = await app.inject({ method: 'POST', url: '/recover', payload: { csr: b64Csr(ctx.csr) } }) + // The module answers with uniform rejects that never say which check failed: 400 for a malformed + // body, 401 for a cert that parses but is not trusted, 403 for a trusted cert that is not allowed. + expect(res.statusCode).toBe(400) + }) + + /** + * The header is the mTLS terminator's channel. On `/recover` the cert comes from the body, so a + * client-supplied header must not be able to substitute an identity. + */ + test('an x-client-cert HEADER cannot override the body identity → 401', async () => { + const ctx = await hostCtx('alice') + const rogueCa = await makeP256Ca('rogue-CA') + const now = Date.now() + const rogue = await mintLeaf(ctx, new Date(now - 9 * DAY_MS), new Date(now - 8 * DAY_MS), rogueCa.caSigner) + const app = appWith(hostDeps(ctx, { hostCaAnchorsDer: [ctx.ca.caDer] })) + await app.ready() + const res = await app.inject({ + method: 'POST', + url: '/recover', + headers: { 'x-client-cert': certHeader(ctx.currentCertDer) }, + payload: { cert: b64(rogue), csr: b64Csr(ctx.csr) }, + }) + expect(res.statusCode).toBe(401) + }) +}) diff --git a/deploy/nginx/enroll-recover-location.md b/deploy/nginx/enroll-recover-location.md new file mode 100644 index 0000000..6a611fe --- /dev/null +++ b/deploy/nginx/enroll-recover-location.md @@ -0,0 +1,93 @@ +# Enroll vhost addition — `/recover` (expired-leaf recovery) + +One `location` block to **merge into the existing** enroll vhost on the VPS +(`/etc/nginx/conf.d/enroll.conf`, the `127.0.0.1:8471` server). No new server, no new SNI route, no +DNS work. + +> **This is a MERGE, not a file to ship.** The enroll vhost also carries `/enroll`, `/device/enroll`, +> `/auth/login`, `/crl/`, `/renew` and `/device/:id/renew` — do not replace it. + +## Why the route exists + +`POST /renew` is mTLS-authenticated by the very leaf it renews, so once that leaf lapses the host can +never renew it and the tunnel stays down until an operator re-pairs. That is not hypothetical: a +laptop slept through its 8h renewal window, its 24h leaf expired, and the agent then logged +`client certificate has expired; renew before dialling` 6380 times over 8 days without recovering. + +## Why it cannot be fixed on `/renew` itself + +**nginx will not forward an expired client certificate, under any `ssl_verify_client` mode.** + +- `optional` → nginx answers a bare `400 The SSL certificate error` as soon as verification fails. + The request never reaches the `location`, so no `if ($ssl_client_verify …)` can rescue it. +- `optional_no_ca` → does **not** help either. It only tolerates *chain* failures; see nginx's + `ngx_ssl_verify_error_optional()`, which covers `DEPTH_ZERO_SELF_SIGNED_CERT`, + `SELF_SIGNED_CERT_IN_CHAIN`, `UNABLE_TO_GET_ISSUER_CERT_LOCALLY` and + `UNABLE_TO_VERIFY_LEAF_SIGNATURE` — and **not** `X509_V_ERR_CERT_HAS_EXPIRED`. +- `ssl_verify_client` is a `server`-level directive, so it cannot be relaxed per-location anyway. + +So recovery drops mTLS: `/recover` takes **no client certificate**, and the lapsed cert travels in +the request body instead. + +## Why that is still authenticated + +A certificate is public, so the body alone proves nothing — possession of the **private key** does, +and it is still proven end to end: + +- the accompanying CSR is **self-signed by that key**, and the host signer's delegated gate enforces + CSR proof-of-possession plus `CSR key == registered key` (`control-plane/src/ca/csr.ts` + `verifyCsrPoP`); +- `control-plane/src/api/renew.ts` runs the *same* trust pipeline as `/renew` — real X.509 path + validation to the frp-client-CA anchors, SPIFFE SAN parse, `notBefore`, and a registry lookup + requiring an `active`, account-consistent host — differing **only** in a bounded overrun allowance + on `notAfter` (`DEFAULT_EXPIRED_RENEW_GRACE_MS`, 30 days). + +Worst case for a replayed cert without the key: the attacker receives a certificate they cannot +authenticate with. Revocation still bites, via registry status. + +## The block to add + +```nginx + # Expired-leaf recovery: NO client cert (see enroll-recover-location.md). The control-plane is + # the sole verifier; strip any client-supplied cert header so only the body can speak. + location = /recover { + limit_req zone=renew_recover burst=5 nodelay; + proxy_pass http://127.0.0.1:8080; + proxy_http_version 1.1; + proxy_set_header Host $host; + proxy_set_header x-client-cert ""; + proxy_read_timeout 60s; + } +``` + +And once, in the `http` context (top of `enroll.conf` is fine) — the route is reachable +pre-authentication and costs the control-plane an X.509 path validation, so bound it. The +control-plane additionally rate-limits per identity (`createRenewRateLimiter`, 30/hour): + +```nginx +limit_req_zone $binary_remote_addr zone=renew_recover:1m rate=10r/m; +``` + +## Deploy gate + +```bash +cp /etc/nginx/conf.d/enroll.conf{,.bak.$(date +%s)} # snapshot first +# ...merge the block above... +nginx -t && systemctl restart nginx # NEVER reload on a failed -t +``` + +`restart`, not `reload`: a graceful reload has been observed keeping old workers alive for seconds, +which makes post-deploy verification race the change. + +## Verify + +```bash +# no cert needed — an in-grace expired leaf is re-issued +curl -sS --resolve enroll.terminal.yaojia.wang:443: \ + -X POST -H 'content-type: application/json' \ + -d "{\"cert\":\"$(base64 -w0 expired.cert.pem)\",\"csr\":\"$(base64 -w0 new.csr.der)\"}" \ + https://enroll.terminal.yaojia.wang/recover # → 201 {cert,caChain,notAfter} + +# a forged self-signed cert with a correct-looking SPIFFE SAN must still be refused +# → 401 (this is the load-bearing check; nginx is no longer validating the chain here) +``` diff --git a/deploy/nginx/recover-mtls.conf b/deploy/nginx/recover-mtls.conf deleted file mode 100644 index af43441..0000000 --- a/deploy/nginx/recover-mtls.conf +++ /dev/null @@ -1,65 +0,0 @@ -# recover-mtls.conf — nginx :8472 RECOVERY vhost: re-issue a leaf that has ALREADY EXPIRED. -# -# Install target: /etc/nginx/conf.d/recover-mtls.conf -# SNI route to merge into the stream map (see stream-sni-additions.md): -# recover.terminal.yaojia.wang 127.0.0.1:8472; -# -# ── WHY THIS EXISTS ─────────────────────────────────────────────────────────────────────────────── -# `POST /renew` is authenticated by mTLS with the very leaf it renews. Once that leaf lapses the host -# can no longer renew it, so the tunnel stays down until an operator re-pairs by hand. That is not -# hypothetical: a laptop slept through its renewal window, the leaf expired, and the agent then -# logged `client certificate has expired; renew before dialling` 6380 times over 8 days without ever -# recovering. -# -# The enroll vhost cannot host the fix. It runs `ssl_verify_client optional`, and nginx answers a -# bare `400 Bad Request` the moment a PRESENTED client cert fails verification — an expired leaf -# never reaches the `location`, so it can never be forwarded to the control-plane. `ssl_verify_client` -# is a server-level directive and cannot be relaxed per-location, hence a separate server block. -# -# ── THE TRADE (read before changing anything here) ──────────────────────────────────────────────── -# This server runs `optional_no_ca`: nginx forwards the presented cert WITHOUT validating its chain. -# The control-plane is therefore the SOLE and FULL verifier on this path. `api/renew.ts` does, in -# order: real X.509 path validation to the frp-client-CA anchors (`assertPresentedCertTrusted` → -# relay-auth `verifyChain`), SPIFFE SAN parse, `notBefore` check (NEVER graced), `notAfter` + bounded -# grace (`DEFAULT_EXPIRED_RENEW_GRACE_MS`, 30d), then a registry lookup requiring an `active`, -# account-consistent host — so REVOCATION IS STILL ENFORCED, via registry status rather than the CRL. -# Do not weaken those checks on the assumption that nginx is still gating this port. It is not. -# -# Blast radius is deliberately tiny: ONLY `POST /renew` is reachable; every other path 404s, and the -# strict enroll vhost (:8471) is left untouched so the normal renewal path keeps its nginx-level -# chain + CRL enforcement. - -# Recovery is a once-in-a-blue-moon call per host, but it is reachable pre-authentication and costs -# the control-plane an X.509 path validation, so bound it. CP additionally rate-limits per identity. -limit_req_zone $binary_remote_addr zone=renew_recover:1m rate=10r/m; - -server { - listen 127.0.0.1:8472 ssl; - server_name recover.terminal.yaojia.wang; - - ssl_certificate /etc/relay/frp-tls/fullchain.pem; # LE wildcard *.terminal.yaojia.wang - ssl_certificate_key /etc/relay/frp-tls/privkey.pem; - - # Ask for a client cert and forward whatever is presented — INCLUDING an expired one, which is - # the entire point. `ssl_client_certificate` is kept only so the CertificateRequest carries a CA - # hint and clients auto-select the right identity; it does NOT gate anything under optional_no_ca. - ssl_verify_client optional_no_ca; - ssl_client_certificate /etc/relay/frp-client-ca/frp-client-ca.cert.pem; - - location = /renew { - # Under optional_no_ca `$ssl_client_verify` is `NONE` even for a good cert, so presence of the - # cert itself is the only thing nginx can meaningfully assert here. - if ($ssl_client_escaped_cert = "") { return 403; } - - limit_req zone=renew_recover burst=5 nodelay; - - proxy_pass http://127.0.0.1:8080; - proxy_http_version 1.1; - proxy_set_header Host $host; - proxy_set_header x-client-cert $ssl_client_escaped_cert; - proxy_read_timeout 60s; - } - - # Everything else — including /enroll, /device/*, /auth/login — stays on the strict vhost. - location / { return 404; } -} diff --git a/deploy/nginx/stream-sni-additions.md b/deploy/nginx/stream-sni-additions.md index d615346..107e171 100644 --- a/deploy/nginx/stream-sni-additions.md +++ b/deploy/nginx/stream-sni-additions.md @@ -17,18 +17,6 @@ frp.terminal.yaojia.wang 127.0.0.1:7000; # frps control channel (TLS passthro *.terminal.yaojia.wang 127.0.0.1:8470; # nginx :8470 TLS-term + device-CA mTLS (frp-mtls.conf) ``` -### Later addition — expired-leaf recovery (recover-mtls.conf) - -```nginx -recover.terminal.yaojia.wang 127.0.0.1:8472; # expired-leaf re-issue (recover-mtls.conf) -``` - -**Order IS load-bearing for this one**: it must sit ABOVE the `*.terminal.yaojia.wang` wildcard, the -same way `enroll.terminal.yaojia.wang` does. `hostnames` gives exact matches priority over wildcards -in nginx's `map`, so a correctly-placed line wins regardless — but keep the exact-match lines grouped -above the wildcard so a future editor cannot mis-read the intent. No DNS work is needed: the zone is -already served by the wildcard record `*.terminal.yaojia.wang → `. - ## Coexistence warning (do NOT retype the existing lines) The existing `map` body already contains — and MUST be preserved **verbatim**: From 0970c623eb2707f3bcf61876520c238e417bdcda Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Wed, 29 Jul 2026 10:08:58 +0200 Subject: [PATCH 3/4] docs(progress): log the expired-leaf renewal deadlock fix + live self-heal proof --- docs/PROGRESS_LOG.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/docs/PROGRESS_LOG.md b/docs/PROGRESS_LOG.md index 9069d46..f4f1675 100644 --- a/docs/PROGRESS_LOG.md +++ b/docs/PROGRESS_LOG.md @@ -24,6 +24,32 @@ > 新会话读到的第一块。保持准确,只描述"此刻"。 +### 🔥 [x] 隧道断了 8 天 —— 证书过期后**自动续期死锁**,已修 + 已上线(2026-07-29) + +- **现象**: 用户问"远端 relay 是否在运行"。服务端**全绿**(nginx/frps/cp-zte/panel-zte/xray/pg/redis 全 active,LE 通配证书有效至 2026-10-05,`*.terminal.yaojia.wang` 通配 DNS 正常),但 **frps `:7000` 已建立连接数 = 0**,`h7fd8.terminal.yaojia.wang` 打不通。Mac 上 frpc 在跑,每 20s 重试一次,全部 `connect to server error: EOF`(frps 在 mTLS 阶段拒)。 +- **根因(死锁)**: Mac 的 frp-client 叶证书 **2026-07-21 13:55 UTC 过期**(TTL 仅 ~24h)。`POST /renew` 是**用它自己要续的那张证书做 mTLS 鉴权**的 —— 证书一过期就再也换不了证书。触发链:续期窗口(T-8h)那天 Mac 在睡眠,醒来时 DNS 未就绪 → 24 次 `getaddrinfo ENOTFOUND enroll.terminal.yaojia.wang` 错过窗口 → 过期后改报 `client certificate has expired; renew before dialling`,**重复 6380 次、8 天、永不自愈**(agent.log 涨到 3MB)。 +- **三层都 fail-closed**,只改一层没用:① agent `transport/dial.ts` `buildTlsOptions` 抛 `CertExpiredError`,连 socket 都不开;② nginx `enroll.conf` 的 `location = /renew` 要求 `$ssl_client_verify = SUCCESS`;③ CP `api/renew.ts` `assertPresentedCertTrusted` 再查一次有效期 → 401。 +- **两个假设被实测推翻(重要,别再踩)**: + 1. `ssl_verify_client optional` 在客户端证书**验证失败时直接返回裸 400**(`400 The SSL certificate error`),请求**根本到不了 location** —— 所以任何 `if ($ssl_client_verify …)` 都救不了。 + 2. `optional_no_ca` **也不行**:它只容忍**链**错误。见 nginx 的 `ngx_ssl_verify_error_optional()` —— 只含 `DEPTH_ZERO_SELF_SIGNED_CERT` / `SELF_SIGNED_CERT_IN_CHAIN` / `UNABLE_TO_GET_ISSUER_CERT_LOCALLY` / `UNABLE_TO_VERIFY_LEAF_SIGNATURE`,**不含 `X509_V_ERR_CERT_HAS_EXPIRED`**。已在 VPS 上用真过期证书打 :8472 实测确认。结论:**nginx 在任何模式下都不会转发过期客户端证书**。 +- **最终方案(用户选定:30 天宽限窗口 + 超窗回落配对码)**: 恢复路径**放弃 mTLS**,改为**不带客户端证书的普通 HTTPS POST,过期证书放进 body**。不丢鉴权 —— **CSR 是用同一把私钥自签的**,而签名网关本来就强制 CSR PoP(`ca/csr.ts` `verifyCsrPoP`)+ `CSR key == 注册 key`,持有私钥的证明与原来 TLS 握手给的完全等价。重放一张(公开的)证书而没有私钥,最多拿到一张**自己用不了**的证书。 + - **agent**: `dial.ts` 与 mTLS renew 通道**保持严格 fail-closed**(放宽已从 TLS 层完全移除)。改由 rotator 逐次决策:有效 → mTLS `/renew`;过期但在宽限内 → 普通 `/recover`;超出宽限 → 终态 `onExhausted`,**一次告警后停止重试**(不再发任何请求),日志直接给出 `web-terminal-agent pair `。新增 `recoverCert()`、`recoveryUrlFor()`、`DEFAULT_EXPIRED_RENEW_GRACE_MS`(30d)、`CertExpiredBeyondGraceError`;`AgentConfig` 加可选 `recoverUrl`(env `RECOVER_URL`)。 + - **CP**: `/renew` **恢复严格**(grace 0,与终端器行为一致)。宽限只给新增的 `POST /recover` —— 证书从 **body** 读、**忽略 `x-client-cert` 头**,然后跑**与 `/renew` 完全相同**的信任链:frp-client-CA 锚点做真 X.509 路径验证 → SPIFFE 解析 → `notBefore`(**永不宽限**)→ registry `active` + 账户一致。**吊销照样生效**。 + - **deploy**: 不需要新 vhost / 新 SNI / 新 DNS —— 只往现有 enroll vhost 加一个 `location = /recover`(+ `limit_req` 10r/m)。文档 `deploy/nginx/enroll-recover-location.md`(含 nginx 源码引证)。 +- **验证**: + - 单测:agent **289/289**、control-plane **290/290**,两侧 `tsc --noEmit` 干净。TDD 全程先 RED 后 GREEN。 + - 承重测试:**「山寨 CA 签的、SPIFFE SAN 与真证书逐字节相同的伪造证书 → 401」**。这条最关键 —— 这条路径上 nginx 已不再验链,它一旦变红,`/recover` 就成了无鉴权发证机。已在**线上实测 401**。 + - **线上端到端自愈实测**:把那张真·过期 8 天的证书塞回 keystore → 重启 agent → 日志 `frp-client cert rotated; restarting frpc onto the fresh leaf` → 证书自动换成新的 24h 叶(Jul30 08:06)→ frpc `login to server success` / `start proxy success` → 设备证书走完整链路 `:443 → SNI → :8470 mTLS → frps → frpc → Mac base app` 拿到 **HTTP 200**(返回本机真实 session 列表)→ 健康探针 `healthy: true`。**全程零人工介入** —— 正是生产卡死 8 天的那个场景。 + - 回归:`/renew` 对过期证书仍 400(nginx 拦住,未放宽);4 区 SNI 探针(frp/通配/enroll/xray-Reality)全部未受影响。 +- **应急恢复(修复前先做的)**: 用 frp-client CA 给 h7fd8 补签了一张 7 天过桥证书(同一把私钥、同子域名、同 SPIFFE SAN,经 `openssl ca` 登记进 index.txt 因此可吊销),先把隧道救活。自愈生效后它已被换成 CP 正常签发的 24h 叶。 +- **遗留 / 待办**: + - **`agent/src/dist/buildBinary.ts` 是源码却被 `.gitignore` 的 `dist/` 规则吞掉,从未提交**(既有问题,与本次无关):任何全新 clone/worktree 都缺它 → `agent/src/index.ts` 类型检查失败、已提交的 `test/buildBinary.test.ts` 直接导入失败。建议加 `!agent/src/dist/` 例外。 + - agent 日志里 `subdomain: (none)` / `host_id: (none)`:enroll 后未把 subdomain/hostId 落进配置,导致告警缺主机标识(既有问题)。 + - 手机轨(device)目前**没有** `/recover` 对应路径,设备证书过期仍需重新 enroll;CP 侧宽限代码是通用的,补一条路由即可。 + - 24h TTL + 单次定时续期 + 笔记本睡眠 的组合仍偏脆,宽限窗口是兜底而非根治。 +- **文件**: `agent/src/certs/rotation.ts` · `agent/src/certs/nativeRenew.ts` · `agent/src/config/agentConfig.ts` · `control-plane/src/api/renew.ts` · `deploy/nginx/enroll-recover-location.md`(新) · 各自测试。 +- **commit**: `f3f4d8b`(第一版,方向对但 nginx 那条路走不通)+ `5509c81`(实测后改为 body 传证书,最终版)。 + ### 🐛 [x] Mac 版中文全变 `_` + 框线渲染错乱 —— PTY 缺 UTF-8 locale(2026-07-28) - **现象**(用户截图,Mac 版):Claude Code TUI 里所有中文变成 `_`;banner/输入框只剩零散横线,圆角边框消失。 From 7064a39bf139ac09232704ce7d03fe52cdbd32af Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Wed, 29 Jul 2026 10:09:28 +0200 Subject: [PATCH 4/4] chore: drop accidentally committed node_modules symlinks They were local worktree conveniences (symlinks into the main checkout so vitest could resolve the file: workspace deps), swept in by a broad `git add agent`. --- agent/node_modules | 1 - control-plane/node_modules | 1 - 2 files changed, 2 deletions(-) delete mode 120000 agent/node_modules delete mode 120000 control-plane/node_modules diff --git a/agent/node_modules b/agent/node_modules deleted file mode 120000 index b61e91e..0000000 --- a/agent/node_modules +++ /dev/null @@ -1 +0,0 @@ -/Users/yiukai/Documents/git/web-terminal/agent/node_modules \ No newline at end of file diff --git a/control-plane/node_modules b/control-plane/node_modules deleted file mode 120000 index 84c91d5..0000000 --- a/control-plane/node_modules +++ /dev/null @@ -1 +0,0 @@ -/Users/yiukai/Documents/git/web-terminal/control-plane/node_modules \ No newline at end of file