fix(agent): make native cert auto-renew actually work end-to-end

Two real-deploy renew bugs (found running the live /renew):
- the /renew mTLS request pinned the enroll caChain as the server CA → TLS
  'unable to get local issuer certificate' (the LE-fronted CP is publicly
  trusted). Verify the server against SYSTEM roots (drop ca), keep the client
  cert + rejectUnauthorized:true. (TlsClientOptions.ca now optional.)
- renewCert parsed {cert, caChain:string}, but the CP returns cert=base64(DER)
  + caChain=base64(DER)[]; normalize to PEM (shared certs/pem.ts, reused by
  native enroll). Verified live: cert rotated 13:41→next-day, frpc restarted,
  tunnel stayed up. 281 tests pass.
This commit is contained in:
Yaojia Wang
2026-07-19 07:56:01 +02:00
parent 55d177e9ee
commit 1e398c7561
8 changed files with 71 additions and 30 deletions

View File

@@ -72,7 +72,9 @@ const defaultMtlsRequest: MtlsRequest = (url, tls, init) =>
headers: init.headers,
cert: tls.cert,
key: tls.key,
ca: tls.ca,
// ca omitted ⇒ verify the server against the system roots (LE-fronted CP). Present only when a
// private CA is pinned (not for /renew).
...(tls.ca !== undefined ? { ca: tls.ca } : {}),
rejectUnauthorized: tls.rejectUnauthorized, // always true (anti-MITM, INV14)
},
(res) => {
@@ -129,7 +131,12 @@ export function createMtlsFetch(
const request = opts.request ?? defaultMtlsRequest
const shim = async (input: Parameters<typeof fetch>[0], init?: RequestInit): Promise<Response> => {
const url = typeof input === 'string' ? input : input.toString()
const tls = buildTlsOptions(ks, { ...(opts.certParser ? { certParser: opts.certParser } : {}) })
// Present the current frp-client leaf (client auth), but verify the /renew SERVER cert against the
// 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 } : {}) })
const tls: TlsClientOptions = { cert: full.cert, key: full.key, rejectUnauthorized: full.rejectUnauthorized }
const reqInit: MtlsRequestInit = {
method: init?.method ?? 'GET',
headers: toHeaderRecord(init?.headers),

33
agent/src/certs/pem.ts Normal file
View File

@@ -0,0 +1,33 @@
/**
* PEM helpers shared by the native enroll (enroll/pair.ts) and renew (certs/rotation.ts) paths.
*
* The control-plane returns the frp-client leaf + CA chain as base64-encoded DER (cert as a string,
* caChain as a string[]); the keystore + frpc need PEM files. `derBase64ToPem` wraps a base64 DER body
* back into CERTIFICATE armor at 64 columns.
*/
/** base64(DER) → PEM (CERTIFICATE armor, 64-col wrapped). */
export function derBase64ToPem(derBase64: string, label = 'CERTIFICATE'): string {
const body = derBase64.replace(/\s+/g, '')
const lines = body.match(/.{1,64}/g) ?? []
return `-----BEGIN ${label}-----\n${lines.join('\n')}\n-----END ${label}-----\n`
}
/**
* Normalize a control-plane cert response ({cert: base64 DER, caChain: base64 DER[]}) to PEM strings
* for the keystore. Throws if the shape is wrong. Shared by enroll + renew so both stay in lockstep.
*/
export function certResponseToPem(cert: unknown, caChain: unknown): { certPem: string; caChainPem: string } {
if (
typeof cert !== 'string' ||
!Array.isArray(caChain) ||
caChain.length === 0 ||
!caChain.every((c) => typeof c === 'string')
) {
throw new Error('cert response missing cert/caChain')
}
return {
certPem: derBase64ToPem(cert),
caChainPem: (caChain as string[]).map((c) => derBase64ToPem(c)).join(''),
}
}

View File

@@ -15,6 +15,7 @@ import type { Keystore } from '../keys/keystore.js'
import type { TimerLike } from '../transport/seams.js'
import { createBackoff, type BackoffPolicy } from '../transport/backoff.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
@@ -64,11 +65,11 @@ export async function renewCert(
})
if (res.status === 403) return 'revoked'
if (!res.ok) throw new Error(`cert renewal failed: HTTP ${res.status}`)
const json = (await res.json()) as { cert?: string; caChain?: string }
if (typeof json.cert !== 'string' || typeof json.caChain !== 'string') {
throw new Error('cert renewal response missing cert/caChain')
}
ks.saveCert(json.cert, json.caChain) // atomic whole-file install
// The control-plane returns cert=base64(DER) + caChain=base64(DER)[]; normalize to PEM for the
// keystore + frpc (same shape as native enroll).
const json = (await res.json()) as { cert?: unknown; caChain?: unknown }
const { certPem, caChainPem } = certResponseToPem(json.cert, json.caChain)
ks.saveCert(certPem, caChainPem) // atomic whole-file install
return 'rotated'
}

View File

@@ -15,6 +15,7 @@ import type { EnrollResult } from 'relay-contracts'
import type { AgentIdentity } from '../keys/identity.js'
import type { Keystore } from '../keys/keystore.js'
import { buildCsr } from './csr.js'
import { derBase64ToPem } from '../certs/pem.js'
/** v0.8 shared-token gate vs v0.9+ per-host Ed25519. Default is `'ed25519'` from v0.9. */
export type EnrollMode = 'token' | 'ed25519'
@@ -69,16 +70,6 @@ interface EnrollResponseJson {
hostContentSecret: string // base64url over the wire
}
/**
* base64(DER) → PEM. The native /enroll returns the leaf + CA certs as base64-encoded DER; the
* keystore + frpc need PEM files, so wrap the base64 body at 64 columns in CERTIFICATE armor.
*/
function derBase64ToPem(derBase64: string, label = 'CERTIFICATE'): string {
const body = derBase64.replace(/\s+/g, '')
const lines = body.match(/.{1,64}/g) ?? []
return `-----BEGIN ${label}-----\n${lines.join('\n')}\n-----END ${label}-----\n`
}
function parseEnrollResult(json: unknown, allowMissingContentSecret = false): EnrollResult {
const j = json as Partial<EnrollResponseJson>
if (typeof j.hostContentSecret !== 'string') {

View File

@@ -25,7 +25,13 @@ export class CertExpiredError extends Error {
export interface TlsClientOptions {
readonly cert: string
readonly key: string
readonly ca: string
/**
* CA(s) to verify the SERVER cert against. Set to the private tunnel CA for the relay dial; left
* undefined for a request whose server is publicly trusted (the LE-fronted control-plane /renew) so
* Node verifies against the system roots — pinning the private CA there fails with "unable to get
* local issuer certificate".
*/
readonly ca?: string
readonly rejectUnauthorized: true
}

View File

@@ -69,7 +69,9 @@ describe('createMtlsFetch (A5)', () => {
expect(await res.json()).toEqual({ cert: 'NEW', caChain: 'NEWCA' })
expect(seen[0]!.url).toBe('https://cp.example.com/renew')
expect(seen[0]!.tls.cert).toBe('LEAFCERT')
expect(seen[0]!.tls.ca).toBe('CACHAIN')
// /renew verifies the LE-fronted control-plane against the SYSTEM roots, so no private CA is pinned
// (pinning the enroll caChain here fails with "unable to get local issuer certificate").
expect(seen[0]!.tls.ca).toBeUndefined()
expect(seen[0]!.tls.rejectUnauthorized).toBe(true)
expect(String(seen[0]!.tls.key)).toContain('PRIVATE KEY') // in-process PKCS#8 key, mTLS only
expect(seen[0]!.init.method).toBe('POST')
@@ -166,7 +168,7 @@ describe('startNativeAutoRenew (A5 end-to-end)', () => {
const timer = new FakeTimer()
const request: MtlsRequest = async () => ({
status: 200,
body: JSON.stringify({ cert: 'FRESHLEAF', caChain: 'CACHAIN' }),
body: JSON.stringify({ cert: 'FRESHLEAF', caChain: ['CACHAIN'] }),
})
const restartChild = vi.fn()
const stop = vi.fn()
@@ -183,7 +185,7 @@ describe('startNativeAutoRenew (A5 end-to-end)', () => {
timer.advance(1000) // renewal fires at ~2/3 TTL
await flush()
expect(ks.loadCert()!.certPem).toBe('FRESHLEAF') // atomic persist
expect(ks.loadCert()!.certPem).toContain('FRESHLEAF') // atomic persist
expect(restartChild).toHaveBeenCalledTimes(1) // frpc restarted onto the fresh leaf
expect(stop).not.toHaveBeenCalled()
controller.stop()
@@ -222,7 +224,7 @@ describe('startNativeAutoRenew (A5 end-to-end)', () => {
const request: MtlsRequest = async () => {
calls += 1
if (calls === 1) throw new Error('ECONNREFUSED')
return { status: 200, body: JSON.stringify({ cert: 'FRESHLEAF', caChain: 'CACHAIN' }) }
return { status: 200, body: JSON.stringify({ cert: 'FRESHLEAF', caChain: ['CACHAIN'] }) }
}
const restartChild = vi.fn()
const stop = vi.fn()
@@ -252,7 +254,7 @@ describe('startNativeAutoRenew (A5 end-to-end)', () => {
timer.advance(500) // backoff retry fires and succeeds
await flush()
expect(ks.loadCert()!.certPem).toBe('FRESHLEAF')
expect(ks.loadCert()!.certPem).toContain('FRESHLEAF')
expect(restartChild).toHaveBeenCalledTimes(1)
expect(lines.join('\n')).not.toContain('LEAFCERT')
controller.stop()

View File

@@ -115,7 +115,8 @@ describe('defaultMtlsRequest transport (A5 — real node:https)', () => {
expect(seenOpts.rejectUnauthorized).toBe(true) // anti-MITM (INV14)
expect(seenOpts.method).toBe('POST')
expect(seenOpts.cert).toBe('LEAFCERT')
expect(seenOpts.ca).toBe('CACHAIN')
// ca omitted ⇒ verify the LE-fronted control-plane against the system roots (not the private CA).
expect(seenOpts.ca).toBeUndefined()
expect(String(seenOpts.key)).toContain('PRIVATE KEY') // in-process PKCS#8 key
// HIGH fix: a socket timeout is armed with the sane default so a stall can never hang forever.
expect(seenReq!.setTimeoutCalls).toHaveLength(1)

View File

@@ -53,10 +53,10 @@ describe('renewCert (T13)', () => {
it('installs a fresh cert atomically on success (same key)', async () => {
const { dir, ks } = enrolledKs()
const before = ks.loadIdentity()!.publicKey
const fetchImpl = vi.fn(async () => jsonRes(200, { cert: 'NEWCERT', caChain: 'NEWCA' }))
const fetchImpl = vi.fn(async () => jsonRes(200, { cert: 'NEWCERT', caChain: ['NEWCA'] }))
const out = await renewCert(CFG, ks.loadIdentity()!, ks, fetchImpl as unknown as typeof fetch)
expect(out).toBe('rotated')
expect(ks.loadCert()).toEqual({ certPem: 'NEWCERT', caChainPem: 'NEWCA' })
expect(ks.loadCert()!.certPem).toContain('NEWCERT'); expect(ks.loadCert()!.caChainPem).toContain('NEWCA')
// pubkey unchanged — only the cert rotated
expect(Buffer.from(ks.loadIdentity()!.publicKey).equals(Buffer.from(before))).toBe(true)
rmSync(dir, { recursive: true, force: true })
@@ -102,7 +102,7 @@ describe('createCertRotator (T13)', () => {
const rotator = createCertRotator(CFG, ks.loadIdentity()!, ks, {
timer,
renewBeforeMs: 1000,
fetchImpl: (async () => jsonRes(200, { cert: 'NEWCERT', caChain: 'NEWCA' })) as unknown as typeof fetch,
fetchImpl: (async () => jsonRes(200, { cert: 'NEWCERT', caChain: ['NEWCA'] })) as unknown as typeof fetch,
now: () => new Date(0),
parseCert: () => new Date(2000),
})
@@ -114,7 +114,7 @@ describe('createCertRotator (T13)', () => {
timer.advance(1000)
await flush()
expect(rotated).toBe(1)
expect(ks.loadCert()!.certPem).toBe('NEWCERT')
expect(ks.loadCert()!.certPem).toContain('NEWCERT')
rotator.stop()
rmSync(dir, { recursive: true, force: true })
})
@@ -126,7 +126,7 @@ describe('createCertRotator (T13)', () => {
const fetchImpl = (async () => {
calls += 1
if (calls === 1) throw new Error('network down')
return jsonRes(200, { cert: 'NEWCERT', caChain: 'NEWCA' })
return jsonRes(200, { cert: 'NEWCERT', caChain: ['NEWCA'] })
}) as unknown as typeof fetch
const errors: unknown[] = []
let rotated = 0
@@ -154,7 +154,7 @@ describe('createCertRotator (T13)', () => {
timer.advance(500)
await flush()
expect(rotated).toBe(1)
expect(ks.loadCert()!.certPem).toBe('NEWCERT')
expect(ks.loadCert()!.certPem).toContain('NEWCERT')
rotator.stop()
rmSync(dir, { recursive: true, force: true })
})