fix(tunnel): recover an expired leaf over plain HTTPS, not mTLS
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.
This commit is contained in:
@@ -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<string, string>
|
||||
*/
|
||||
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<typeof fetch>[0], init?: RequestInit): Promise<Response> => {
|
||||
@@ -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 <CODE>` to re-pair this host', {
|
||||
logger.log('error', 'frp-client cert expired beyond recovery grace — run `web-terminal-agent pair <CODE>` 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 } : {}),
|
||||
|
||||
Reference in New Issue
Block a user