From 5509c81eee2aec61571d07ae179a46c17f3dc99e Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Wed, 29 Jul 2026 09:52:17 +0200 Subject: [PATCH] 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**: