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**: