Renew registrar leaves and honour the reload contract (#768)

[sehkone]

Closes #768

Part of #770

Summary

Both leaves the registrar surface runs on expire, and nothing renewed them: ensure_registrar_surface_certificates mints them once at start and deliberately leaves a usable pair alone, so a lapse took every enrollment in the deployment down and the registrar could not re-mint itself. This adds one daemon-owned renewal adapter that keeps both valid, and delivers both halves of the reload contract. Nothing goes on the wire: no response field, serialization fixture, wire-visible renewal state or lapse classification is added.

The adapter

Created only where [registrar_endpoint] enabled = true, from the activated endpoint the daemon already holds, and armed before anything at all is spawned — beside the accept task's other start-up dependencies — so the per-leaf state accessor is initialized from the certificates already on disk before the endpoint serves anything. Its handle joins with the daemon's other tasks and it takes the same shutdown watch. With the endpoint disabled it is never built: no adapter, no state entry, no pass, no OpenBao request and no CA request.

Arming it is part of starting, not something logged over a running endpoint. An enabled endpoint owes both leaves an adapter and both accessor entries, and a failed attempt has no lifetime to make a missing entry with, so a preparation that cannot be completed — a rendered internal agent.toml that is missing or no longer loads, a deployment state file that cannot be resolved, a leaf certificate on disk that no longer parses — fails the invocation with a named error, exactly as an absent internal credential or an unloadable provisioning config already does. Serving two certificates nothing will renew is the outage this whole change exists to prevent.

Cadence, lead time and the issuance-retry budget all come from the rendered internal agent configuration's sole profile. No second scheduler, no lead-time constant of its own, no second retry policy: eligibility is the daemon's existing predicate, split out as daemon::should_renew_certificate so the two registrar leaves — which have no [[profiles]] entry — are judged by the same rule rather than by a copy of it, no-bundle opt-out and all. The retry budget is selected the same way, through daemon::select_retry_backoff: that profile may carry a [profiles.retry] of its own, and where it does it wins over the config's top-level [retry], exactly as it does for a service profile.

Publication as a transaction

Nothing touches a live path until everything that can fail has already succeeded:

1. Issue off-live. acme::flow is split at exactly the point publication used to begin, so issue_certificate_material returns the chain, the candidate certificate and a fresh key instead of writing them. issue_certificate_with_bootstrap is now that call plus publish_issued_material, so the two paths cannot drift.
2. Validate the candidate. Key-to-leaf; the reserved SAN, with a client candidate recognising to the same instance, host and domain, and a server candidate carrying the exact endpoint SAN and chaining to an anchor the endpoint pin file already names. The pin file is read and never written.
3. Stage the merged bundle, computed but not written.
4. Build the whole next ServerConfig — from the candidate server pair when that is the leaf being renewed and the live one when it is not, and from the staged bundle in either case. The incoming WebPkiClientVerifier is rebuilt from the post-merge bundle's pinned subset and from nothing else.
5. Snapshot, then publish: the bundle, then the certificate and key through the existing two-rename contract.
6. Exchange the active configuration, infallibly, because the replacement was built in step 4.

A failed write restores every path the publication reached — bytes, mode and ownership — leaves the old configuration installed and records a failed attempt. A rollback that itself fails carries both errors and explicitly does not describe the files as restored. Candidate files and snapshots live in a 0700 working directory removed on every exit.

One writer at a time on the shared CA bundle

[trust] ca_bundle_path is not the registrar's file. The per-profile publication reads it, merges its issued chain into it and writes it back — up to three profiles concurrently by default — and the fast-poll trust apply replaces it wholesale when a trust update lands. Nothing serialized any of them, and ProfileLocks cannot: its keys are profile labels, and this file has no profile. So a merge computed from bytes another writer had already replaced overwrote that writer's anchor, and a merge landing after the registrar transaction's snapshot was discarded by that transaction's rollback — either way the host serves and validates against a bundle missing a CA it was told to trust, with nothing recording that it went missing.

All three writers now take one lock per bundle path (ca_bundle_lock) across their whole span: the registrar's from the staged read through the last restore its rollback can perform, the profile writer's around its read-merge-write, the fast-poll apply's around its replacement. The lock is deliberately not inside fs_util::write_ca_bundle, which the holding transactions call. The lost-update window predates this branch for the two pre-existing writers, so CHANGELOG.md records it under ### Fixed.

The server-side swap

ActivatedEndpoint now holds its acceptor and resolver together behind an RwLock<Arc<ActiveTls>>, and the accept loop loads it immediately before each handshake, holding no lock across an await. Renewal exchanges the whole configuration: the old acceptor retains the old client verifier, so replacing EndpointCertResolver alone would leave a trust-anchor rotation undecided until a restart. No restart, no signal, no socket rebind, no dropped in-flight connection.

The caller-side reload

load_matching_client_material rereads the pair on every dial and checks the key is the leaf's. A reader that lands between the writer's two renames re-reads instead of presenting a mismatch: five reads total, with 1/2/4/8 ms waits after the first four, then a typed ClientMaterialError::KeyMismatch. That is a fixed local policy of the reader's and deliberately not the daemon's issuance-retry backoff. It is documented as the caller contract in docs/en/operations.md and docs/ko/operations.md, naming the in-repository client as the reference behaviour.

Test plan

• Enabled/disabled adapter creation, accessor initialization before serving, shutdown join, and the absence of any disabled-path OpenBao or CA activity
• An enabled endpoint whose renewal cannot be armed fails run_daemon with a named error before anything is spawned, so the accept task is never reached
• Immediate first pass and scheduled later passes under tokio::time::pause, with a driven pass reading no role_id/secret_id and using no AppRole
• Initialization records observed notAfter, never_attempted and no timestamp without issuing anything, and a leaf it cannot observe refuses the adapter instead of leaving the endpoint one entry short
• The cadence's retry budget: the internal profile's own [profiles.retry] wins, and the rendered [retry] table is the fallback
• Lead-time renewal: candidate key/SAN/pin validation, staged bundle, next TLS configuration and snapshots all precede the first live write; fresh key, preserved client instance/host/domain, exact server SAN, unchanged pin file, accessor success
• Chain drift: client replacement, server publication behind a pinned anchor, unpinned-server refusal leaving live bundle, pair, active TLS and pin file byte-for-byte unchanged with notAfter retained, and the no-bundle opt-out plus the activation refusal it cannot stand behind
• The incoming verifier's exact source set, rejecting pin-file-only, unpinned-bundle, returned-chain-only and no-bundle substitutes
• The shared bundle lock: one path is one lock and two paths are two; the registrar transaction, the per-profile merge and the fast-poll trust apply each write nothing while another in-process writer holds the bundle and proceed once it is released; and a profile merge interleaved into a registrar publication survives that publication's rollback
• Whole-configuration exchange against the production accept loop: the endpoint is served by serve::run, a publication runs through RegistrarCertRenewal, and the next real mTLS handshake is presented the renewed server leaf and admits a caller under an anchor only the rebuilt verifier holds — with an unchanged socket inode and an in-flight connection that finishes under the configuration it handshook with. Both are read off the wire rather than off the disk: remove swap_active_tls and each fails, one on the presented leaf and one on an UnknownCA alert
• Injected bundle, certificate and key publication failures — rollback, old acceptor retained, failed state, artifacts cleaned — and an injected rollback failure recording both errors without claiming restoration
• An ordinary internal-credential authentication failure recorded as a failed attempt, and an unchanged no-op pass
• Next-dial client renewal without recreating the caller, and the deterministic torn-pair success and five-read exhaustion paths with their 1/2/4/8 ms waits
• cargo test --lib (1311 passed) and cargo test --bin bootroot (1295 passed), plus fmt, clippy -D warnings and rustdoc, all run on Linux in a rust:1-bookworm container as a non-root user, because src/registrar/endpoint/* is Linux-gated and never compiles on macOS
• scripts/preflight/ci/check.sh: fmt, clippy, rustdoc, ruff, biome, markdownlint, mkdocs build --strict, cargo audit (only the two pre-existing allowed advisories)
• scripts/preflight/ci/e2e-matrix.sh — did not run on this machine. The matrix needs passwordless sudo for its bootroot init step, which this host does not supply, and port 8200 is held by an unrelated local process. tests/bootroot_rotate.rs likewise fails inside the Linux container for want of a docker binary; those tests never reach any code this branch changes. CI's Docker E2E and test-core jobs gate the arm that could not run here.

[sehkone]

[Reviewer Round 1]

Found two issues:

• [P1] Do not serve an enabled endpoint after renewal setup fails. src/daemon.rs logs a RegistrarCertRenewal::prepare error and continues to spawn the accept loop. For example, a missing or malformed rendered internal agent.toml leaves an enabled endpoint running permanently without the required adapter, state entries, or renewal work—contrary to the issue’s enabled-endpoint contract. Similarly, assemble tolerates certificate-observation failures, while record_failure will not create the missing entry. Fail preparation before serving, or otherwise guarantee both state entries and the adapter.

• [P2] Honor the internal profile’s retry override. RenewalCadence::from_internal_config always copies internal.retry.backoff_secs. The daemon’s established policy uses a profile-level override when present (select_retry_backoff). The internal-config loader permits such an override, so registrar issuance can use a different retry budget from its sole profile. Select profile.retry first and add a coverage case for it.

[sehkone]

[Review Verdict Round 1: NOT_APPROVED]

[sehkone]

[Author Round 1]

CI is running on the pushed commit. Here is the disposition of each review item.

[P1] Do not serve an enabled endpoint after renewal setup fails — Fixed

Accepted in full; the issue's contract is explicit that an enabled endpoint has an adapter and both accessor entries, and the old code could satisfy neither while still serving.

• run_daemon now arms the adapter with prepare_registrar_cert_renewal immediately after resolve_registrar_service, before anything is spawned, and propagates the error with ?. This is the rule the accept task's other dependencies already follow (resolve_registrar_service's own doc: a dependency it cannot resolve fails the invocation rather than leaving the adopted socket with no accept task). spawn_registrar_cert_renewal is now infallible and just spawns the already-armed loop; the error!-and-continue path is gone.
• RegistrarCertRenewal::assemble returns Result and fails when a leaf's certificate cannot be read or parsed, so record_failure's inability to create a missing entry is no longer reachable — the two entries and the adapter are guaranteed together or the daemon does not start. Docs on record_success/record_failure updated accordingly.
• Practical risk is small: build_registrar_handler → InternalCredential::load already calls load_internal_config, so a missing or malformed rendered agent.toml was already a hard startup failure. The genuinely new refusal is an unparsable leaf certificate — a file start-time issuance has just made usable.
• Tests: an_unarmable_registrar_renewal_stops_the_daemon (clobbered client leaf → run_daemon returns an error naming both the arming step and the path), a_leaf_that_cannot_be_observed_refuses_the_adapter, and the_adapter_is_armed_before_anything_is_spawned (source-order assertion mirroring the existing the_registrar_service_is_resolved_before_anything_is_spawned).
• Two existing daemon tests called run_daemon with enabled = true and no registrar-endpoint certificate paths at all — a state configuration validation refuses in production, and one the test seam allowed only because the old code shrugged. They now get a fixture, configure_registrar_surface_material, that provisions the four paths the way start-time issuance would.

[P2] Honour the internal profile's retry override — Fixed

Valid: load_internal_config permits a [profiles.retry] on the sole profile, and the daemon's established policy prefers it. RenewalCadence::from_internal_config now calls daemon::select_retry_backoff(&internal, profile) (widened to pub(crate), with a doc comment recording why it is shared) instead of copying internal.retry.backoff_secs. Covered by the_cadence_takes_the_profiles_own_retry_budget and the_cadence_falls_back_to_the_top_level_retry_budget, which render a real internal agent config and read the cadence back off it.

Docs

docs/en/operations.md and docs/ko/operations.md gained a paragraph stating that arming the loop is part of starting, and the CHANGELOG.md entry now says a host that enables the endpoint but cannot arm renewal fails to start instead of serving certificates nothing would renew. PR body updated to match (both Closes #768 and the inherited Part of #770 prose retained).

Verification

Linux container (rust:1-bookworm, non-root, since src/registrar/endpoint/* never compiles on macOS): cargo fmt --check (with group_imports=StdExternalCrate), cargo clippy --all-targets -D warnings, RUSTDOCFLAGS=-D warnings cargo doc, cargo test --lib (1306 passed, 0 failed), cargo test --bin bootroot (1295 passed, 0 failed). On macOS: markdownlint-cli2 and ./scripts/check-docs.sh (mkdocs --strict) both clean. e2e-matrix.sh still cannot run on this host — unchanged from the PR body's existing note.

[sehkone]

[Reviewer Round 2]

[Reviewer Round 2: NOT_APPROVED]

• [P1] Serialize the shared CA-bundle transaction with other in-process writers. registrar_renewal.rs stages and snapshots trust.ca_bundle_path, then publishes or rolls it back at 1166–1181. Meanwhile the normal internal-profile renewal independently merges and writes that same bundle in acme/flow.rs. Both loops are spawned independently in daemon.rs and 268, and the existing locks are per profile, not per bundle.

  Consequently, an internal-profile write can land after the registrar snapshot; if the registrar pair write then fails, its rollback restores the stale snapshot and discards the other renewal’s bundle update. A successful registrar write can similarly overwrite an intervening merge. Put all in-process bundle read/merge, snapshot, publication, and rollback paths behind one shared bundle lock (including the normal profile writer), and add a deterministic interleaving test.

Round 1’s two findings are resolved: preparation now fails before spawning the endpoint, and cadence correctly uses the internal profile’s retry override. PR linkage and test-plan hygiene look correct.

[sehkone]

[Review Verdict Round 2: NOT_APPROVED]

[sehkone]

[Author Round 2]

CI is running on baf29b2d. Here is the disposition of Round 2's single item.

[P1] Serialize the shared CA-bundle transaction with other in-process writers — Fixed

The finding is correct, and it is worse than the two writers it names — there are three, and one of the races predates this branch.

What was wrong. [trust] ca_bundle_path has no owner. acme::flow::write_merged_ca_bundle reads it, merges the issued chain in and writes it back — and up to three profiles issue concurrently by default (max_concurrent_issuances = 3). fast_poll::apply_trust_to_disk replaces the same file wholesale on a trust update. registrar_renewal::publish_candidate stages a merge, snapshots it, publishes it and restores it when the pair write that follows fails. ProfileLocks serializes none of them: its keys are profile labels, and this file has no profile.

The fix. A new src/ca_bundle_lock.rs — a path-keyed registry (LazyLock<BundleLocks>, the shape registrar::verbs' ID_LOCKS already uses) handing out one tokio::Mutex per bundle path. All three writers hold it across their whole read-merge-write span: the registrar's from before stage_bundle's read through the last restore roll_back can perform, the profile writer's inside write_merged_ca_bundle, the fast-poll apply's scoped to its replacement (not its agent.toml rewrite).

A static rather than a plumbed-through registry, deliberately: the per-profile publication is reached from the CLI as well as the daemon loop, several frames below anything that knows a daemon exists, and an argument every future call site must remember to pass is a lock whose correctness compiles away when forgotten. It is also deliberately not inside fs_util::write_ca_bundle, which the holding transactions call — that would deadlock them against themselves; both that function's docs and the module's say so.

Deterministic interleaving test, plus three more:

• an_interleaved_profile_merge_survives_a_registrar_rollback — a LivePaths double gates the pair write open, a real write_merged_ca_bundle is spawned against the same bundle, and the assertion is the invariant the lock creates: whichever way the two are scheduled, the second anchor is in the bundle afterwards. The wait is condition-based (merge.is_finished(), capped), not a clock. I measured it: with the hold removed from publish_candidate it fails 10 runs out of 10; with the lock it passes 10 out of 10. (The fixed-yield version I wrote first caught it only 1 in 5 — that is why the wait is a condition.)
• the_registrar_publication_waits_for_the_shared_bundle_lock and the_profile_bundle_writer_waits_for_the_shared_bundle_lock — with the guard held externally, neither writes a byte to any live path, and both proceed on release.
• apply_trust_to_disk_waits_for_the_shared_bundle_lock in fast_poll, and one_path_is_one_lock_and_two_paths_are_two in the new module.

Scope note: including fast_poll goes one writer beyond what the review named. It replaces the same file from an independently spawned loop and would land inside either transaction's span in exactly the same way, so leaving it out would have left the reported bug half-fixed.

Docs. CHANGELOG.md gains a ### Fixed entry — the lost update between two concurrent profile merges is observable to a user of the last release, unlike the registrar half of it — and docs/en/concepts.md / docs/ko/concepts.md note beside the existing merge explanation that the bundle is exclusively held per write and that readers are never blocked. PR body updated with an "One writer at a time on the shared CA bundle" section and a test-plan bullet; Closes #768 and the inherited Part of #770 prose both retained.

Verification (Linux container, rust:1-bookworm, non-root, since src/registrar/endpoint/* never compiles on macOS): cargo fmt --check with group_imports=StdExternalCrate, cargo clippy --all-targets -D warnings, RUSTDOCFLAGS=-D warnings cargo doc — all clean; cargo test --lib 1311 passed, cargo test --bin bootroot 1295 passed. One testbin run failed on preflight_compose_published_ports_checks_openbao_localhost_during_install — the known port-8200 flake on this host, not reproducible on rerun. On macOS: markdownlint-cli2 and ./scripts/check-docs.sh (mkdocs --strict) clean. e2e-matrix.sh still cannot run here, unchanged from the PR body's note.

[sehkone]

[Reviewer Round 3]

[Reviewer Round 3: NOT_APPROVED]

• [P2] Exercise the reload contract through the renewal path. the_next_handshake_uses_the_renewed_material_without_a_restart never runs the accept loop or completes a TLS handshake. It only opens an unserved raw Unix socket and checks that the certificate file changed. Consequently, it would still pass if publish_candidate stopped calling swap_active_tls, leaving live files renewed but the active acceptor on the old server certificate/client verifier. Add an integration test that serves the harness endpoint, publishes through RegistrarCertRenewal, and verifies a subsequent real mTLS handshake presents the renewed server leaf and accepts a client under the rebuilt trust anchor.

Round 2’s P1 is resolved: the path-keyed bundle lock now covers registrar staging/snapshot/rollback, profile merge publication, and fast-poll trust replacement, with a deterministic rollback interleaving test.

[sehkone]

[Review Verdict Round 3: NOT_APPROVED]

[sehkone]

[Author Round 3]

CI is running on b748619e. Disposition of Round 3's single item:

[P2] Exercise the reload contract through the renewal path — Fixed

The finding is correct. the_next_handshake_uses_the_renewed_material_without_a_restart opened a raw, unserved UnixStream and then compared PEM text on disk. Nothing in it touched the acceptor, so deleting swap_active_tls from publish_candidate would have left it green while the endpoint served renewed files behind the old server certificate and the old client verifier.

It is replaced by two tests that run the production accept loop (serve::run) over the harness endpoint and read every conclusion off the wire:

• the_next_handshake_presents_the_renewed_server_leaf — dials and completes a real mTLS handshake before the swap, holds that connection open with its request unsent, publishes a server candidate through RegistrarCertRenewal::publish_candidate, then asserts (a) the socket inode is unchanged, (b) the in-flight connection still completes its round trip under the configuration it handshook with, and (c) a fresh handshake is presented the renewed leaf — the fingerprint comes from peer_certificates(), not from the file.
• the_next_handshake_accepts_a_caller_under_the_rebuilt_anchor — pins a rotated CA in trusted_ca_sha256 but leaves it out of the live bundle, dials with a caller under it and asserts the live verifier refuses it with UnknownCA, publishes exactly that material as the client-leaf renewal, then dials again with the now-live pair and is admitted. The only thing that changed is the verifier the publication rebuilt from the staged bundle.

Acceptance is asserted through a round trip rather than through connect, because in TLS 1.3 the client finishes its handshake before the server has validated the client certificate. The handler behind the endpoint answers with the transport-authenticated caller identity, so a returned answer proves both the WebPkiClientVerifier accepted the chain and the accept loop recognized the SAN. The caller's TLS configuration is composed by endpoint::client::build_client_config — the one place in the tree that builds it — over material read from the live paths each dial, so a dial after a renewal carries no caller state across.

Negative control, measured: with self.endpoint.swap_active_tls(...) replaced by a no-op, both tests fail — the first on the presented leaf, the second on AlertReceived(UnknownCA). The production line was restored before the run below.

Docs

None. This round is test-only; no behaviour, interface, or user-visible change, so CHANGELOG.md and the manuals are untouched. The PR body's test-plan bullet was rewritten to say what is now actually exercised (production accept loop, wire-read assertions, the negative control), with Closes #768 and the inherited Part of #770 prose both retained.

Verification

Linux container (rust:1-bookworm, non-root, since src/registrar/endpoint/* never compiles on macOS): cargo fmt --check with group_imports=StdExternalCrate, cargo clippy --all-targets -D warnings, RUSTDOCFLAGS=-D warnings cargo doc --no-deps — all clean; cargo test --lib 1312 passed / 0 failed, cargo test --bin bootroot 1295 passed / 0 failed. e2e-matrix.sh still cannot run on this host, unchanged from the PR body's note — and this round changes no code any E2E path reaches.

[sehkone]

[Reviewer Round 4]

[Reviewer Round 4: APPROVED]

Round 3’s P2 is resolved. The new tests use the production accept loop and real mTLS handshakes to prove both server-leaf replacement and client-verifier replacement after publication (tests, tests). The accept loop loads the active acceptor immediately before the handshake, and publication swaps it only after all live writes succeed (serve.rs, registrar_renewal.rs).

All prior findings are resolved. I found no new issues. PR linkage and test-plan hygiene are correct.

[sehkone]

[Review Verdict Round 4: APPROVED]

[sehkone]

Suggested squash commit

Title

Renew the registrar surface's two leaves

Body

Both leaves the registrar surface runs on expire, and nothing renewed
them: the surface mints them once at start and deliberately leaves a
usable pair alone, so a lapse took every enrollment in the deployment
down and left the registrar unable to re-mint itself.

One daemon-owned adapter now keeps both valid. It is built only where
the endpoint is enabled, armed before anything is spawned so the
per-leaf state is initialized from the certificates already on disk
before the endpoint serves, and joined with the daemon's other tasks
under the same shutdown watch. Cadence, lead time and the issuance
retry budget all come from the rendered internal agent configuration's
sole profile, and eligibility is the daemon's own predicate rather
than a copy of it.

A pass touches no live path until everything that can fail has already
succeeded: issue off-live, validate the candidate's key, its SAN and a
server candidate's anchor against the pin file, stage the merged
bundle, build the whole next server configuration from it, snapshot,
publish, and only then exchange the active configuration. A failed
write restores every path the publication reached and leaves the old
configuration installed; a rollback that itself fails carries both
errors and does not claim the files were restored. The pin file is
read and never written.

The endpoint holds its acceptor and resolver together and the accept
loop loads them immediately before each handshake, so a renewal needs
no restart, no signal and no socket rebind, and drops no connection in
flight. Replacing the certificate resolver alone would not do: the old
acceptor retains the old client verifier, leaving a trust-anchor
rotation undecided until a restart.

The shared CA bundle gained a third writer here, and nothing had ever
serialized the two it already had — the per-profile publication's
read-merge-write and the fast-poll trust apply's wholesale replacement
— so a merge computed from bytes another writer had already replaced
could drop an anchor the host was told to trust, with nothing
recording that it went missing. All three now take one lock per bundle
path across their whole span.

Callers reload through the per-dial loader, which rereads the pair on
every dial and re-reads rather than present a mismatch when it lands
between the writer's two renames. That contract is documented for
callers in English and Korean, naming the in-repository client as the
reference behaviour.

Nothing here goes on the wire: no response field, no serialization
fixture, no wire-visible renewal state and no lapse classification.

Closes #768

Part of #770