From 4487940831772c8e60e221b4871a739205ccd131 Mon Sep 17 00:00:00 2001 From: Aco Piper Date: Wed, 2 Sep 2026 16:13:18 +0900 Subject: [PATCH 1/4] Remove the insecure TLS override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `danger_accept_invalid_certs(true)`, reached through `bootroot-agent --insecure`, disabled certificate verification outright. The repository's certificate-verification policy does not admit that: verification is never disabled or weakened to make a handshake work, and there is no temporary exception — only a permanent one introduced temporarily. So the flag goes rather than being narrowed. No runtime mode now accepts a certificate the configured trust cannot anchor, and `insecure_mode` stops being threaded through the ACME client, the issuance flow, the registrar surface and the daemon. The oneshot path carried an `IssuanceRuntime` for that one field and never read the rest of it, so it and the `config_path` it was built from go with it; the daemon path, which does reload config per retry, keeps both. The compose smoke path was the only caller that ran without trust material at all. `agent.toml.compose` now carries a `[trust]` section to fill in from the deployment's own `secrets/certs/`, and the agent scenarios build the bundle and both pins from there for every runtime config they write. They also name the loopback rather than address it: step-ca's certificate carries `localhost` as a DNS SAN and no IP SAN, so dialling `127.0.0.1` would fail hostname verification against the very certificate the run was told to trust. Closes #983 --- CHANGELOG.md | 17 ++ agent.toml.compose | 19 +++ docs/en/cli.md | 5 +- docs/en/configuration.md | 21 ++- docs/en/installation.md | 38 +++-- docs/en/operations.md | 5 +- docs/en/troubleshooting.md | 5 +- docs/ko/cli.md | 6 +- docs/ko/configuration.md | 21 ++- docs/ko/installation.md | 38 +++-- docs/ko/operations.md | 4 +- docs/ko/troubleshooting.md | 5 +- scripts/preflight/extra/agent-scenarios.sh | 89 +++++++++- src/acme/client.rs | 39 +---- src/acme/flow.rs | 42 +---- src/agent_args.rs | 27 ++- src/bin/bootroot-agent.rs | 12 +- src/commands/init/steps/registrar_internal.rs | 2 +- src/config.rs | 1 - src/daemon.rs | 28 +--- src/daemon/registrar_handler_tests.rs | 5 - src/lib.rs | 5 +- src/registrar_certs.rs | 29 +--- src/registrar_certs/tests.rs | 155 ++++++++---------- src/registrar_renewal.rs | 19 +-- src/tls.rs | 23 +-- tests/bootroot_agent_hardening.rs | 41 +++-- 27 files changed, 361 insertions(+), 340 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6f939c1..235e258c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -274,6 +274,23 @@ service host. For a component installed once per deployment, reusing its old `service_name` as the key reproduces every previous path and name byte for byte. +### Removed + +- `bootroot-agent --insecure` is gone. The flag disabled TLS certificate + verification for the ACME server, so a run that carried it accepted any + certificate at all, and the daemon offered no narrower way to get past a + handshake failure. Certificate verification now has no off switch: the + agent verifies against `[trust]` when it is configured and against the + system CA store otherwise, and a run that passes `--insecure` is refused + at argument parsing rather than starting with verification off. A + deployment that relied on the flag needs its trust material in place + before the first run — `bootroot service add` and `bootroot-remote + bootstrap` already write `trust.ca_bundle_path` and + `trust.trusted_ca_sha256` for the managed onboarding flow. The compose + smoke path is the one place that ran without them, and + `agent.toml.compose` now carries a `[trust]` section to fill in from + `secrets/certs/`. + ### Security - Bumped `h2` from 0.4.15 to 0.4.16 to address RUSTSEC-2026-0258 diff --git a/agent.toml.compose b/agent.toml.compose index da7a0159..963e46b9 100644 --- a/agent.toml.compose +++ b/agent.toml.compose @@ -26,6 +26,25 @@ http_responder_hmac = "CHANGE-ME" http_responder_timeout_secs = 5 http_responder_token_ttl_secs = 300 +# The compose stack's step-ca is self-signed, and nothing in bootroot-agent +# skips verifying it, so this run needs the deployment's own trust material. +# Build the bundle and read the two fingerprints out of secrets/certs/: +# +# mkdir -p certs +# cat secrets/certs/root_ca.crt secrets/certs/intermediate_ca.crt \ +# > certs/compose-ca-bundle.pem +# for cert in secrets/certs/root_ca.crt secrets/certs/intermediate_ca.crt; do +# openssl x509 -in "$cert" -noout -fingerprint -sha256 \ +# | cut -d= -f2 | tr -d ':' | tr 'A-Z' 'a-z' +# done +# +# trusted_ca_sha256 must hold those two values before deployment; the +# placeholders below match nothing and fail the handshake closed. +# scripts/preflight/extra/agent-scenarios.sh stamps both in for its own runs. +[trust] +ca_bundle_path = "certs/compose-ca-bundle.pem" +trusted_ca_sha256 = ["CHANGE-ME-ROOT", "CHANGE-ME-INTERMEDIATE"] + [retry] backoff_secs = [5, 10, 30] diff --git a/docs/en/cli.md b/docs/en/cli.md index 0a2992d3..c53665c7 100644 --- a/docs/en/cli.md +++ b/docs/en/cli.md @@ -1101,8 +1101,9 @@ automatically as part of onboarding. - `remote-bootstrap`: `bootroot service add` prepares the service trust payload in OpenBao, and `bootroot-remote bootstrap` applies it on the remote host. -- `--insecure` is a per-run break-glass override. For full rules and - operating flow, see [Configuration > Trust](configuration.md#trust). +- There is no override that skips verifying the ACME server. Trust must be in + place before the first run. For full rules and operating flow, see + [Configuration > Trust](configuration.md#trust). #### 4-3) Preview mode (`--print-only`/`--dry-run`) diff --git a/docs/en/configuration.md b/docs/en/configuration.md index 195829bc..15fd68f5 100644 --- a/docs/en/configuration.md +++ b/docs/en/configuration.md @@ -230,7 +230,7 @@ This section covers both mTLS trust and **ACME server TLS verification**. - when both trust keys are configured, bootroot-agent verifies the ACME server with that bundle and fingerprint set - when trust is not configured, bootroot-agent falls back to the system CA - store unless `--insecure` is used for a one-off override + store; there is no flag that relaxes verification below that `trusted_ca_sha256` must match real CA certificate fingerprints (not arbitrary values). @@ -247,9 +247,11 @@ arbitrary values). #### 4) Runtime flag behavior -- `--insecure`: disable ACME server TLS verification for that run only -- no override flag: verify normally using the configured trust material or - the system CA store +- there is no runtime flag that disables or relaxes ACME server TLS + verification. bootroot-agent always verifies, using the configured trust + material when `[trust]` is set and the system CA store otherwise +- a handshake that fails is fixed by correcting the trust anchors, the SANs + or the clock, never by accepting the certificate anyway #### 5) Recommended operating flow @@ -264,9 +266,10 @@ enabled" `ca-bundle.pem` locally. - `remote-bootstrap`: run `bootroot-remote bootstrap` once to apply the same trust payload on the remote host. -4. Start `bootroot-agent` without `--insecure`; in the managed onboarding - flow trust should already be in place for normal verification. -5. Reserve `--insecure` for temporary diagnosis or other break-glass cases. +4. Start `bootroot-agent`; in the managed onboarding flow trust should + already be in place for normal verification. A run that starts before + trust is applied fails the ACME handshake instead of proceeding + unverified. In the default Bootroot deployment, step-ca may present its CA certificate directly on the HTTPS endpoint. When `trusted_ca_sha256` is configured, @@ -276,7 +279,8 @@ bundle or a directly presented certificate whose fingerprint is pinned in #### 6) Failure/caution notes -- `--insecure` bypasses verification only for that run +- a TLS failure against the ACME server means the trust material is wrong or + missing; apply it and retry rather than looking for a bypass - in single-step-ca setups, reusing one `ca_bundle_path` for both mTLS and ACME verification is acceptable @@ -1067,7 +1071,6 @@ Options: - `--eab-hmac `: EAB HMAC key - `--eab-file `: EAB JSON file path - `--oneshot`: issue once and exit (disable daemon loop, default `false`) -- `--insecure`: disable ACME server TLS verification (default `false`) All other settings (profiles, retry, scheduler, hooks, CA bundle paths, etc.) must be defined in `agent.toml`. diff --git a/docs/en/installation.md b/docs/en/installation.md index 855bfe4c..bf1b7116 100644 --- a/docs/en/installation.md +++ b/docs/en/installation.md @@ -591,14 +591,15 @@ updates and `rotate eab-clear` are silent no-ops for that agent. See [Operations > systemd operations procedure](operations.md#systemd-operations-procedure-recommended-for-bootroot-agent) for a hardened unit example. -TLS verification override: +TLS verification: For detailed behavior and the recommended operating flow, see [Configuration > Trust](configuration.md#trust). -- `--insecure` disables verification for that run (**insecure**, overrides - normal behavior). In the normal managed onboarding flow, trust is prepared - before the first `bootroot-agent` run so verification can already be on. +- bootroot-agent always verifies the ACME server's certificate, and there is + no flag that turns that off. In the normal managed onboarding flow, trust is + prepared before the first `bootroot-agent` run, so the first run verifies + against material that is already in place. #### CA bundle consumer permissions @@ -613,18 +614,33 @@ there is nothing to `docker compose up`. To exercise a **one-shot** issuance against the compose stack, build the binary and point it at the ports the stack publishes to the host: +The compose stack's CA is self-signed and nothing skips verifying it, so +prepare the trust material first from the deployment's own certificates: + +```bash +mkdir -p certs +cat secrets/certs/root_ca.crt secrets/certs/intermediate_ca.crt \ + > certs/compose-ca-bundle.pem +for cert in secrets/certs/root_ca.crt secrets/certs/intermediate_ca.crt; do + openssl x509 -in "$cert" -noout -fingerprint -sha256 \ + | cut -d= -f2 | tr -d ':' | tr 'A-Z' 'a-z' +done +``` + +Put those two fingerprints in `agent.toml.compose` under +`trust.trusted_ca_sha256`, then run: + ```bash cargo build --bin bootroot-agent -./target/debug/bootroot-agent --oneshot --insecure --config agent.toml.compose +./target/debug/bootroot-agent --oneshot --config agent.toml.compose ``` `agent.toml.compose` is the config for exactly this run model — a native -binary talking to the compose stack over `localhost`. `--insecure` is needed -because the compose stack's CA is self-signed and this config carries no -trust bundle; the managed onboarding flow prepares trust first and does not -need it. This is a demo/smoke path, not an onboarding path: production -services run the bootroot-agent host daemon described above with the config -`bootroot service add` writes. +binary talking to the compose stack over `localhost`, verifying step-ca +against the bundle and pins prepared above. This is a demo/smoke path, not an +onboarding path: production services run the bootroot-agent host daemon +described above with the config `bootroot service add` writes, and +`bootroot service add` prepares the same two trust keys for them. `scripts/preflight/extra/agent-scenarios.sh` drives the same binary the same way across its scenarios. diff --git a/docs/en/operations.md b/docs/en/operations.md index a05f0f17..a8e8cd76 100644 --- a/docs/en/operations.md +++ b/docs/en/operations.md @@ -3258,8 +3258,9 @@ This section covers how to operate two trust settings together: - bootroot-agent normally verifies the ACME server (step-ca) TLS certificate. If trust settings are configured, it uses the managed CA bundle and pins; otherwise it uses the system CA store. -- CLI override: `bootroot-agent --insecure` disables verification only for - that run. +- There is no CLI override that disables that verification. A run whose trust + material is missing or wrong fails the handshake rather than continuing + unverified. - In the managed onboarding flow, trust is prepared before the first `bootroot-agent` run: - `local-file`: `bootroot service add` writes trust settings and diff --git a/docs/en/troubleshooting.md b/docs/en/troubleshooting.md index b7427d33..d9b5b8ae 100644 --- a/docs/en/troubleshooting.md +++ b/docs/en/troubleshooting.md @@ -249,7 +249,10 @@ older builds, add `http_responder_hmac` to the `[acme]` section of - Ensure `server` URL is `https://` (`http://` is rejected) - Validate system trust or `trust.ca_bundle_path` -- Use `bootroot-agent --insecure` only for temporary diagnosis +- Verification cannot be turned off: fix the trust anchors, the certificate's + SANs, or the clock. Confirm the anchor the agent is configured with matches + the one that signed the ACME server's certificate: + `openssl s_client -connect : -showcerts 신뢰](configuration.md#신뢰) 섹션을 - 참고하세요. +- ACME 서버 검증을 건너뛰는 오버라이드는 없습니다. 첫 실행 전에 trust가 + 준비돼 있어야 합니다. 자세한 규칙/운영 흐름은 + [설정 > 신뢰](configuration.md#신뢰) 섹션을 참고하세요. #### 4-3) preview 모드(`--print-only`/`--dry-run`) diff --git a/docs/ko/configuration.md b/docs/ko/configuration.md index 0095295f..f2946191 100644 --- a/docs/ko/configuration.md +++ b/docs/ko/configuration.md @@ -221,8 +221,8 @@ mTLS 신뢰와 **ACME 서버 TLS 검증**을 함께 다루는 섹션입니다. - `trusted_ca_sha256`: 신뢰할 CA 인증서 지문 목록(SHA-256 hex) - trust 두 값이 모두 있으면 bootroot-agent가 해당 번들과 지문으로 ACME 서버를 검증합니다 -- trust가 비어 있으면 `--insecure`를 쓰지 않는 한 시스템 CA 저장소로 - 일반 검증을 수행합니다 +- trust가 비어 있으면 시스템 CA 저장소로 일반 검증을 수행합니다. 검증 + 수준을 그 아래로 낮추는 플래그는 없습니다 `trusted_ca_sha256`는 임의 값이 아니라 실제 CA 인증서 지문이어야 합니다. @@ -238,8 +238,11 @@ mTLS 신뢰와 **ACME 서버 TLS 검증**을 함께 다루는 섹션입니다. #### 4) 실행 플래그 동작 -- `--insecure`: 해당 실행에서만 ACME 서버 TLS 검증 비활성화 -- 오버라이드가 없으면: 구성된 trust 또는 시스템 CA 저장소로 일반 검증 +- ACME 서버 TLS 검증을 비활성화하거나 완화하는 실행 플래그는 없습니다. + bootroot-agent는 `[trust]`가 설정돼 있으면 그 trust 자료로, 아니면 + 시스템 CA 저장소로 항상 검증합니다 +- 핸드셰이크가 실패하면 trust 앵커, 인증서 SAN, 시계를 바로잡아 + 해결합니다. 인증서를 그대로 수용하는 우회 경로는 없습니다 #### 5) 권장 운영 절차 @@ -253,9 +256,9 @@ mTLS 신뢰와 **ACME 서버 TLS 검증**을 함께 다루는 섹션입니다. `ca-bundle.pem`을 로컬에 기록합니다. - `remote-bootstrap`: `bootroot-remote bootstrap`을 1회 실행해 같은 trust payload를 원격 호스트에 반영합니다. -4. `--insecure` 없이 `bootroot-agent`를 시작합니다. managed onboarding - 흐름에서는 이미 정상 검증에 필요한 trust가 준비돼 있어야 합니다. -5. `--insecure`는 임시 진단이나 break-glass 상황에서만 사용합니다. +4. `bootroot-agent`를 시작합니다. managed onboarding 흐름에서는 이미 정상 + 검증에 필요한 trust가 준비돼 있어야 합니다. trust 반영 전에 시작한 + 실행은 검증 없이 진행되지 않고 ACME 핸드셰이크에서 실패합니다. 기본 Bootroot 배포에서는 step-ca가 HTTPS 엔드포인트에서 CA 인증서를 직접 제시할 수 있습니다. `trusted_ca_sha256`가 설정되면 bootroot-agent는 @@ -264,7 +267,8 @@ mTLS 신뢰와 **ACME 서버 TLS 검증**을 함께 다루는 섹션입니다. #### 6) 실패/주의 사항 -- `--insecure` 실행은 해당 실행에서만 검증을 우회 +- ACME 서버 TLS 실패는 trust 자료가 없거나 잘못됐다는 뜻입니다. 우회 + 수단을 찾는 대신 trust를 반영하고 다시 실행합니다 - 단일 step-ca 환경에서는 mTLS 번들과 ACME 검증에 같은 `ca_bundle_path` 재사용 가능 #### 7) 점검 체크리스트 @@ -1110,7 +1114,6 @@ backoff_secs = [5, 10, 30] - `--eab-hmac `: EAB HMAC Key - `--eab-file `: EAB JSON 파일 경로 - `--oneshot`: 1회 발급 후 종료(데몬 루프 비활성화, 기본값 `false`) -- `--insecure`: ACME 서버 TLS 검증 비활성화(기본값 `false`) 그 외 설정(프로필, 재시도, 스케줄러, 훅, CA 번들 경로 등)은 `agent.toml`에 정의해야 합니다. diff --git a/docs/ko/installation.md b/docs/ko/installation.md index 1049bbdb..8bf6aedb 100644 --- a/docs/ko/installation.md +++ b/docs/ko/installation.md @@ -597,15 +597,14 @@ EAB 회전이 적용되려면 `--eab-file`이 필수입니다 — 없으면 EAB 방법은 **설정** 섹션과 [운영 > systemd 운영 절차](operations.md)의 하드닝된 유닛 예시를 참고하세요. -TLS 검증 오버라이드: +TLS 검증: 자세한 동작 원리와 권장 운용 순서는 [설정 > 신뢰](configuration.md)를 참고하세요. -- `--insecure`: 해당 실행에서만 ACME 서버 TLS 검증 비활성화 - (비보안 오버라이드). 일반적인 managed onboarding 흐름에서는 첫 - `bootroot-agent` 실행 전에 trust가 준비되므로 처음부터 검증을 켤 수 - 있습니다. +- bootroot-agent는 ACME 서버 인증서를 항상 검증하며, 이를 끄는 플래그는 + 없습니다. 일반적인 managed onboarding 흐름에서는 첫 `bootroot-agent` + 실행 전에 trust가 준비되므로, 첫 실행부터 이미 반영된 자료로 검증합니다. #### CA 번들 소비 서비스 권한 @@ -620,18 +619,33 @@ bootroot-agent에는 컨테이너 이미지가 없습니다. 항상 호스트 상대로 **1회 발급**(`--oneshot`)을 확인하려면 바이너리를 빌드한 뒤 스택이 호스트에 게시한 포트로 연결합니다: +compose 스택의 CA는 자체 서명이고 이를 건너뛰는 수단은 없으므로, 배포가 +가진 인증서로 trust 자료를 먼저 준비합니다: + +```bash +mkdir -p certs +cat secrets/certs/root_ca.crt secrets/certs/intermediate_ca.crt \ + > certs/compose-ca-bundle.pem +for cert in secrets/certs/root_ca.crt secrets/certs/intermediate_ca.crt; do + openssl x509 -in "$cert" -noout -fingerprint -sha256 \ + | cut -d= -f2 | tr -d ':' | tr 'A-Z' 'a-z' +done +``` + +출력된 두 지문을 `agent.toml.compose`의 `trust.trusted_ca_sha256`에 넣은 뒤 +실행합니다: + ```bash cargo build --bin bootroot-agent -./target/debug/bootroot-agent --oneshot --insecure --config agent.toml.compose +./target/debug/bootroot-agent --oneshot --config agent.toml.compose ``` `agent.toml.compose`는 바로 이 실행 모델(네이티브 바이너리가 `localhost`로 -compose 스택에 접속)을 위한 설정입니다. compose 스택의 CA는 자체 서명이고 -이 설정에는 trust 번들이 없으므로 `--insecure`가 필요합니다. managed -onboarding 흐름은 trust를 먼저 준비하므로 이 옵션이 필요하지 않습니다. -데모/스모크 경로일 뿐 온보딩 경로가 **아니며**, 운영 서비스는 위에서 설명한 -대로 `bootroot service add`가 작성한 설정으로 bootroot-agent 호스트 데몬을 -실행합니다. +compose 스택에 접속)을 위한 설정이며, 위에서 준비한 번들과 핀으로 step-ca를 +검증합니다. 데모/스모크 경로일 뿐 온보딩 경로가 **아니며**, 운영 서비스는 +위에서 설명한 대로 `bootroot service add`가 작성한 설정으로 bootroot-agent +호스트 데몬을 실행합니다. `bootroot service add`도 같은 trust 두 값을 +준비해 줍니다. `scripts/preflight/extra/agent-scenarios.sh`도 동일한 바이너리를 같은 방식으로 실행합니다. diff --git a/docs/ko/operations.md b/docs/ko/operations.md index 741cf403..2b7173d9 100644 --- a/docs/ko/operations.md +++ b/docs/ko/operations.md @@ -3087,8 +3087,8 @@ bootroot rotate \ - bootroot-agent는 기본적으로 ACME 서버(step-ca)의 TLS 인증서를 검증합니다. trust 설정이 있으면 관리되는 번들과 지문을 사용하고, 없으면 시스템 CA 저장소를 사용합니다. -- CLI 오버라이드: `bootroot-agent --insecure` - (해당 실행에서만 검증 비활성화). +- 이 검증을 비활성화하는 CLI 오버라이드는 없습니다. trust 자료가 없거나 + 잘못된 실행은 검증 없이 진행되지 않고 핸드셰이크에서 실패합니다. - managed onboarding 흐름에서는 첫 `bootroot-agent` 실행 전에 trust를 준비합니다. - `local-file`: `bootroot service add`가 trust 설정과 diff --git a/docs/ko/troubleshooting.md b/docs/ko/troubleshooting.md index 4a2bee74..d54d4bf8 100644 --- a/docs/ko/troubleshooting.md +++ b/docs/ko/troubleshooting.md @@ -247,7 +247,10 @@ bootroot service add \ - `server` URL이 `https://`인지 확인 (`http://` 거부) - 시스템 trust 또는 `trust.ca_bundle_path`가 올바른지 확인 -- 임시 진단 용도로만 `bootroot-agent --insecure` 사용 (운영 비권장) +- 검증은 끌 수 없습니다. trust 앵커, 인증서 SAN, 시계를 바로잡습니다. + 에이전트에 설정한 앵커가 ACME 서버 인증서를 서명한 앵커와 같은지 + 확인하세요: + `openssl s_client -connect : -showcerts ` fails hostname verification against the very + # certificate it was configured to trust. The compose file binds these + # ports to 127.0.0.1, so that is the mapping Compose reports. + addr="${addr/#0.0.0.0:/localhost:}" + addr="${addr/#127.0.0.1:/localhost:}" + addr="${addr/#\[::\]:/localhost:}" + addr="${addr/#\[::1\]:/localhost:}" + printf '%s\n' "$addr" } # Renders a runtime agent config for the native binary. @@ -161,6 +173,68 @@ materialize_agent_config() { -e "s|\"/app/certs/|\"$ROOT_DIR/certs/|g" \ -e "s|^http_responder_hmac = \".*\"$|http_responder_hmac = \"$responder_hmac\"|" \ "$src" > "$dest" + + stamp_trust_block "$dest" +} + +# Points a runtime config at the stack's own CA material. +# +# There is no flag that skips verifying step-ca, so every agent run below +# needs real trust. The compose stack's CA is self-signed, and the only +# copy of it that exists is the one `bootroot init` rendered under +# secrets/certs/, so build the bundle and the pins from there. +# +# Both certificates are pinned, not just the root: step-ca serves a leaf +# chaining to the intermediate in the ordinary case, and a deployment that +# presents the intermediate directly is still verified rather than +# accepted blindly. +# +# The block is appended, so a `[trust]` the source config already carries +# would be a duplicate table the parser rejects. Drop it first and let +# this be the single writer. +stamp_trust_block() { + local dest="$1" + + ensure_compose_ca_bundle + + local stripped="$dest.notrust" + awk ' + /^\[trust\]$/ { skip = 1; next } + /^\[/ { skip = 0 } + skip != 1 { print } + ' "$dest" > "$stripped" + mv "$stripped" "$dest" + + { + printf '\n[trust]\n' + printf 'ca_bundle_path = "%s"\n' "$COMPOSE_CA_BUNDLE" + printf 'trusted_ca_sha256 = ["%s", "%s"]\n' \ + "$(cert_sha256 "$ROOT_DIR/secrets/certs/root_ca.crt")" \ + "$(cert_sha256 "$ROOT_DIR/secrets/certs/intermediate_ca.crt")" + } >> "$dest" +} + +# Rebuilds the trust bundle from secrets/certs/ before every run. +# +# Rebuilt rather than reused: issuance merges the ACME response chain into +# this same file, so a bundle left over from an earlier run is output as +# much as input. Starting from the deployment's own two certificates keeps +# what the agent verifies against pinned to what `init` actually rendered. +ensure_compose_ca_bundle() { + local cert + for cert in root_ca.crt intermediate_ca.crt; do + [ -f "$ROOT_DIR/secrets/certs/$cert" ] || fail "Missing secrets/certs/$cert" + done + mkdir -p "$(dirname "$COMPOSE_CA_BUNDLE")" + cat "$ROOT_DIR/secrets/certs/root_ca.crt" \ + "$ROOT_DIR/secrets/certs/intermediate_ca.crt" > "$COMPOSE_CA_BUNDLE" +} + +# Prints a certificate's SHA-256 fingerprint as lowercase hex, which is +# the spelling trust.trusted_ca_sha256 compares against. +cert_sha256() { + openssl x509 -in "$1" -noout -fingerprint -sha256 \ + | cut -d= -f2 | tr -d ':' | tr 'A-Z' 'a-z' } run_agent_oneshot() { @@ -170,9 +244,9 @@ run_agent_oneshot() { ensure_agent_binary materialize_agent_config "$cfg" "$runtime" - # The compose stack uses a local self-signed CA, so verification is off for - # these runs exactly as the old container override had it. - (cd "$ROOT_DIR" && "$AGENT_BIN" --oneshot --insecure --config="$runtime") + # The compose stack uses a local self-signed CA, so the runtime config + # materialize_agent_config just wrote pins it; verification stays on. + (cd "$ROOT_DIR" && "$AGENT_BIN" --oneshot --config="$runtime") } run_agent_expect_fail() { @@ -269,6 +343,9 @@ wait_for_file() { check_prereqs() { require_cmd docker require_cmd grep + # Every run verifies step-ca, and the pins come from the deployment's + # own certificates. + require_cmd openssl # Only the build path needs cargo; an operator supplying their own binary # through BOOTROOT_AGENT_BIN does not. [ -n "${BOOTROOT_AGENT_BIN:-}" ] || require_cmd cargo @@ -284,6 +361,8 @@ check_prereqs() { [ -f "$ROOT_DIR/secrets/config/ca.json" ] || fail "Missing secrets/config/ca.json" [ -f "$ROOT_DIR/secrets/password.txt" ] || fail "Missing secrets/password.txt" [ -f "$ROOT_DIR/secrets/certs/root_ca.crt" ] || fail "Missing secrets/certs/root_ca.crt" + [ -f "$ROOT_DIR/secrets/certs/intermediate_ca.crt" ] \ + || fail "Missing secrets/certs/intermediate_ca.crt" # Build here, in the parent shell, before any scenario runs. The failure # scenarios invoke the agent inside `output="$(... 2>&1)"`, and a build @@ -381,7 +460,7 @@ TOML # Without it bash forks a child and the kill below would only reap the # subshell, leaving a daemon renewing into certs/ for the later scenarios. local agent_pid - (cd "$ROOT_DIR" && exec "$AGENT_BIN" --insecure --config="$runtime" \ + (cd "$ROOT_DIR" && exec "$AGENT_BIN" --config="$runtime" \ >"$TMP_DIR/agent.renewal.log" 2>&1) & agent_pid=$! diff --git a/src/acme/client.rs b/src/acme/client.rs index 244f4af3..9d6a06b1 100644 --- a/src/acme/client.rs +++ b/src/acme/client.rs @@ -118,9 +118,8 @@ impl AcmeClient { directory_url: String, settings: &AcmeSettings, trust: &TrustSettings, - insecure_mode: bool, ) -> Result { - Self::new_with_bootstrap(directory_url, settings, trust, insecure_mode, None) + Self::new_with_bootstrap(directory_url, settings, trust, None) } /// Creates an ACME client for the registrar bundle-repair path. @@ -131,7 +130,6 @@ impl AcmeClient { directory_url: String, settings: &AcmeSettings, trust: &TrustSettings, - insecure_mode: bool, bootstrap_pins: Option<&[String]>, ) -> Result { let rng = ring::rand::SystemRandom::new(); @@ -145,11 +143,11 @@ impl AcmeClient { let key_pair = EcdsaKeyPair::from_pkcs8(&ECDSA_P256_SHA256_FIXED_SIGNING, &pkcs8, &rng) .map_err(|_| anyhow::anyhow!("Failed to parse the ACME account key"))?; let client = match bootstrap_pins { - Some(pins) if !insecure_mode => Client::builder() + Some(pins) => Client::builder() .use_preconfigured_tls(build_bootstrap_client_config(pins)?) .build() .context("Failed to build bootstrap HTTP client")?, - _ => build_http_client(trust, insecure_mode)?, + None => build_http_client(trust)?, }; Ok(Self { @@ -636,7 +634,6 @@ mod tests { "http://example.com".to_string(), &test_settings(), &test_trust(), - false, ); assert!(client.is_ok()); } @@ -647,7 +644,6 @@ mod tests { "http://example.com".to_string(), &test_settings(), &test_trust(), - false, ) .unwrap(); let token = "test_token_123_xyz"; @@ -672,7 +668,6 @@ mod tests { "http://example.com".to_string(), &test_settings(), &test_trust(), - false, ) .unwrap(); let key = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(b"test-secret"); @@ -748,7 +743,6 @@ mod tests { format!("{}/directory", server.uri()), &test_settings(), &test_trust(), - false, ) .unwrap(); client @@ -832,7 +826,6 @@ mod tests { format!("{}/directory", server.uri()), &test_settings(), &test_trust(), - false, ) .unwrap(); client.fetch_directory().await.unwrap(); @@ -865,7 +858,6 @@ mod tests { format!("{}/directory", server.uri()), &test_settings(), &test_trust(), - false, ) .unwrap(); let nonce = client.get_nonce().await.unwrap(); @@ -914,7 +906,6 @@ mod tests { format!("{}/directory", server.uri()), &test_settings(), &test_trust(), - false, ) .unwrap(); let order = client @@ -943,7 +934,6 @@ mod tests { format!("{}/directory", server.uri()), &test_settings(), &test_trust(), - false, ) .unwrap(); let err = client.fetch_directory().await.unwrap_err(); @@ -977,7 +967,6 @@ mod tests { format!("{}/directory", server.uri()), &test_settings(), &test_trust(), - false, ) .unwrap(); let err = client.get_nonce().await.unwrap_err(); @@ -1016,7 +1005,6 @@ mod tests { format!("{}/directory", server.uri()), &test_settings(), &test_trust(), - false, ) .unwrap(); let err = client @@ -1211,21 +1199,6 @@ mod tests { Ok(path) } - #[tokio::test] - async fn allows_insecure_when_disabled() -> Result<()> { - let server = start_tls_server().await?; - let trust = TrustSettings::default(); - let mut client = AcmeClient::new( - format!("{}/directory", server.url()), - &trust_test_settings(), - &trust, - true, - )?; - client.fetch_directory().await?; - server.handle.abort(); - Ok(()) - } - #[tokio::test] async fn rejects_self_signed_without_trust() -> Result<()> { let server = start_tls_server().await?; @@ -1234,7 +1207,6 @@ mod tests { format!("{}/directory", server.url()), &trust_test_settings(), &trust, - false, )?; assert!(client.fetch_directory().await.is_err()); assert_eq!( @@ -1260,7 +1232,6 @@ mod tests { format!("{}/directory", server.url()), &trust_test_settings(), &trust, - false, )?; client.fetch_directory().await?; server.handle.abort(); @@ -1279,7 +1250,6 @@ mod tests { format!("{}/directory", server.url()), &trust_test_settings(), &trust, - false, Some(&trust.trusted_ca_sha256), )?; client.fetch_directory().await?; @@ -1297,7 +1267,6 @@ mod tests { format!("{}/directory", server.url()), &trust_test_settings(), &trust, - false, Some(&pins), )?; assert!(client.fetch_directory().await.is_err()); @@ -1312,7 +1281,6 @@ mod tests { "https://localhost/directory".to_string(), &trust_test_settings(), &trust, - false, Some(&[]), ); let Err(error) = result else { @@ -1339,7 +1307,6 @@ mod tests { format!("{}/directory", server.url()), &trust_test_settings(), &trust, - false, )?; assert!(client.fetch_directory().await.is_err()); server.handle.abort(); diff --git a/src/acme/flow.rs b/src/acme/flow.rs index 9fe666ed..20d2d327 100644 --- a/src/acme/flow.rs +++ b/src/acme/flow.rs @@ -466,16 +466,8 @@ pub async fn issue_certificate( settings: &crate::config::Settings, profile: &crate::config::DaemonProfileSettings, eab_creds: Option, - insecure_mode: bool, ) -> Result<()> { - issue_certificate_with( - settings, - profile, - eab_creds, - insecure_mode, - IssuanceOptions::default(), - ) - .await + issue_certificate_with(settings, profile, eab_creds, IssuanceOptions::default()).await } /// Issues a certificate via ACME protocol under `options`. @@ -500,11 +492,9 @@ pub async fn issue_certificate_with( settings: &crate::config::Settings, profile: &crate::config::DaemonProfileSettings, eab_creds: Option, - insecure_mode: bool, options: IssuanceOptions, ) -> Result<()> { - issue_certificate_with_bootstrap(settings, profile, eab_creds, insecure_mode, options, None) - .await + issue_certificate_with_bootstrap(settings, profile, eab_creds, options, None).await } /// Everything one ACME issuance produced, before a single byte of it has @@ -559,19 +549,10 @@ pub(crate) async fn issue_certificate_material( settings: &crate::config::Settings, profile: &crate::config::DaemonProfileSettings, eab_creds: Option, - insecure_mode: bool, options: IssuanceOptions, bootstrap_pins: Option<&[String]>, ) -> Result> { - run_issuance( - settings, - profile, - eab_creds, - insecure_mode, - options, - bootstrap_pins, - ) - .await + run_issuance(settings, profile, eab_creds, options, bootstrap_pins).await } /// Issues a registrar-surface certificate with optional pin-only bootstrap @@ -580,19 +561,11 @@ pub(crate) async fn issue_certificate_with_bootstrap( settings: &crate::config::Settings, profile: &crate::config::DaemonProfileSettings, eab_creds: Option, - insecure_mode: bool, options: IssuanceOptions, bootstrap_pins: Option<&[String]>, ) -> Result<()> { - let Some(material) = run_issuance( - settings, - profile, - eab_creds, - insecure_mode, - options, - bootstrap_pins, - ) - .await? + let Some(material) = + run_issuance(settings, profile, eab_creds, options, bootstrap_pins).await? else { return Ok(()); }; @@ -656,18 +629,13 @@ async fn run_issuance( settings: &crate::config::Settings, profile: &crate::config::DaemonProfileSettings, eab_creds: Option, - insecure_mode: bool, options: IssuanceOptions, bootstrap_pins: Option<&[String]>, ) -> Result> { - // `--insecure` remains its existing explicit override. Bootstrap mode - // never changes that mode's ACME or responder transport behavior. - let bootstrap_pins = (!insecure_mode).then_some(bootstrap_pins).flatten(); let mut client = AcmeClient::new_with_bootstrap( settings.server.clone(), &settings.acme, &settings.trust, - insecure_mode, bootstrap_pins, )?; diff --git a/src/agent_args.rs b/src/agent_args.rs index 19ae4f69..eca04a92 100644 --- a/src/agent_args.rs +++ b/src/agent_args.rs @@ -1,6 +1,6 @@ use std::path::PathBuf; -use clap::{ArgAction, Parser}; +use clap::Parser; use crate::secret::HmacSecret; @@ -47,10 +47,6 @@ pub struct Args { /// Run once and exit (disable daemon loop) #[arg(long)] pub oneshot: bool, - - /// Disable TLS certificate verification for this run only (INSECURE break-glass override) - #[arg(long, action = ArgAction::SetTrue)] - pub insecure: bool, } #[cfg(test)] @@ -59,16 +55,29 @@ mod tests { use super::*; + /// No supported runtime mode accepts an unverifiable certificate. + /// + /// The daemon's TLS trust comes from `[trust]` alone — the + /// configured CA bundle, its optional pins, or the system roots. + /// There is no flag that relaxes it, so neither the help text nor + /// the parser may offer one. #[test] - fn help_describes_insecure_override_semantics() { + fn no_flag_disables_certificate_verification() { let mut command = Args::command(); let mut help = Vec::new(); command.write_long_help(&mut help).expect("write help"); let help = String::from_utf8(help).expect("help is utf-8"); + assert!(!help.contains("--insecure")); assert!(!help.contains("--verify-certificates")); - assert!(help.contains("--insecure")); - assert!(help.contains("for this run only")); - assert!(help.contains("break-glass override")); + assert!(!help.to_lowercase().contains("break-glass")); + } + + #[test] + fn insecure_flag_is_rejected() { + let error = Args::command() + .try_get_matches_from(["bootroot-agent", "--oneshot", "--insecure"]) + .expect_err("--insecure must not parse"); + assert_eq!(error.kind(), clap::error::ErrorKind::UnknownArgument); } } diff --git a/src/bin/bootroot-agent.rs b/src/bin/bootroot-agent.rs index ebc30906..23b3c93d 100644 --- a/src/bin/bootroot-agent.rs +++ b/src/bin/bootroot-agent.rs @@ -20,14 +20,7 @@ async fn main() -> anyhow::Result<()> { if args.oneshot { let (settings, final_eab) = load_settings(&args).await?; - match run_oneshot( - Arc::new(settings), - final_eab, - args.config.clone(), - args.insecure, - ) - .await - { + match run_oneshot(Arc::new(settings), final_eab).await { Ok(()) => info!("Successfully issued certificate!"), Err(err) => { error!("Failed to issue certificate: {err:?}"); @@ -69,7 +62,7 @@ async fn main() -> anyhow::Result<()> { // has to be material this call has already ensured. On a host where // the endpoint is disabled this does nothing at all — no path // created, nothing asked of the CA or of OpenBao. - ensure_registrar_surface_certificates(&initial_settings, args.insecure).await?; + ensure_registrar_surface_certificates(&initial_settings).await?; let registrar_endpoint = RegistrarEndpoint::activate(&initial_settings)?; let mut pending = Some((initial_settings, initial_eab)); @@ -86,7 +79,6 @@ async fn main() -> anyhow::Result<()> { default_eab: final_eab, eab_refresh_path: eab_refresh_path.clone(), config_path: args.config.clone(), - insecure_mode: args.insecure, cli_overrides: cli_overrides.clone(), shutdown: shutdown.clone(), registrar_endpoint: registrar_endpoint.clone(), diff --git a/src/commands/init/steps/registrar_internal.rs b/src/commands/init/steps/registrar_internal.rs index 968e0c05..e9f1bd82 100644 --- a/src/commands/init/steps/registrar_internal.rs +++ b/src/commands/init/steps/registrar_internal.rs @@ -413,7 +413,7 @@ pub(crate) async fn issue_internal_material( kid: creds.kid.clone(), hmac: creds.hmac.clone(), }); - bootroot::acme::issue_certificate(&settings, profile, eab, false) + bootroot::acme::issue_certificate(&settings, profile, eab) .await .context("issuing the bootroot-internal leaf through step-ca's ACME endpoint")?; diff --git a/src/config.rs b/src/config.rs index c5e86a3b..19f1ce05 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1507,7 +1507,6 @@ mod tests { eab_hmac: None, eab_file: None, oneshot: false, - insecure: false, }; settings.merge_with_args(&args); diff --git a/src/daemon.rs b/src/daemon.rs index 00216aa1..bd146766 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -97,7 +97,6 @@ impl Default for DaemonShutdown { #[derive(Clone)] struct IssuanceRuntime { config_path: PathBuf, - insecure_mode: bool, cli_overrides: config::CliOverrides, } @@ -161,8 +160,6 @@ pub struct DaemonInvocation { pub eab_refresh_path: Option, /// The agent config path, for reload-driven rewrites. pub config_path: Option, - /// Whether certificate verification is relaxed for local testing. - pub insecure_mode: bool, /// CLI overrides that must survive a config reload. pub cli_overrides: config::CliOverrides, /// The stop signal, held by the caller across reloads. @@ -185,7 +182,6 @@ pub(crate) async fn run_daemon(invocation: DaemonInvocation) -> anyhow::Result<( default_eab, eab_refresh_path, config_path, - insecure_mode, cli_overrides, shutdown, registrar_endpoint, @@ -201,8 +197,7 @@ pub(crate) async fn run_daemon(invocation: DaemonInvocation) -> anyhow::Result<( // entries, so one that cannot arm them fails the invocation // rather than serving without them. Some((endpoint, built)) => { - let renewal = - prepare_registrar_cert_renewal(&settings, &endpoint, insecure_mode).await?; + let renewal = prepare_registrar_cert_renewal(&settings, &endpoint).await?; Some((endpoint, built, renewal)) } None => None, @@ -213,7 +208,6 @@ pub(crate) async fn run_daemon(invocation: DaemonInvocation) -> anyhow::Result<( let shutdown_rx = shutdown.receiver(); let runtime = IssuanceRuntime { config_path: resolve_config_path(config_path.as_deref()), - insecure_mode, cli_overrides, }; @@ -1106,12 +1100,10 @@ fn spawn_registrar_endpoint( async fn prepare_registrar_cert_renewal( settings: &Arc, endpoint: &Arc, - insecure_mode: bool, ) -> anyhow::Result { crate::registrar_renewal::RegistrarCertRenewal::prepare( Arc::clone(settings), Arc::clone(endpoint), - insecure_mode, ) .await .context("arming certificate renewal for the enabled registrar endpoint") @@ -1273,26 +1265,18 @@ async fn run_profile_daemon( pub(crate) async fn run_oneshot( settings: Arc, default_eab: Option, - config_path: Option, - insecure_mode: bool, ) -> anyhow::Result<()> { let max_concurrent = profile::max_concurrent_issuances(&settings)?; let semaphore = Arc::new(Semaphore::new(max_concurrent)); - let runtime = IssuanceRuntime { - config_path: resolve_config_path(config_path.as_deref()), - insecure_mode, - cli_overrides: config::CliOverrides::default(), - }; let mut handles = Vec::new(); for profile in settings.profiles.clone() { let settings = Arc::clone(&settings); let semaphore = Arc::clone(&semaphore); let default_eab = default_eab.clone(); - let runtime = runtime.clone(); handles.push(tokio::spawn(async move { - run_profile_oneshot(settings, profile, default_eab, semaphore, runtime).await + run_profile_oneshot(settings, profile, default_eab, semaphore).await })); } @@ -1362,7 +1346,6 @@ async fn run_profile_oneshot( profile: config::DaemonProfileSettings, default_eab: Option, semaphore: Arc, - runtime: IssuanceRuntime, ) -> anyhow::Result<()> { let profile_label = config::profile_domain(&settings, &profile); if let Some(err) = refuse_stale_internal_root(&settings, &profile, &profile_label).await { @@ -1372,8 +1355,7 @@ async fn run_profile_oneshot( let _permit = semaphore.acquire().await?; let profile_eab = profile::resolve_profile_eab(&profile, default_eab); - let result = - acme::issue_certificate(&settings, &profile, profile_eab, runtime.insecure_mode).await; + let result = acme::issue_certificate(&settings, &profile, profile_eab).await; handle_issuance_result(&result, &settings, &profile, &profile_label).await?; result } @@ -1426,7 +1408,6 @@ async fn issue_with_retry( let profile_domain = config::profile_domain(settings, profile); let config_path_owned = runtime.config_path.clone(); let cli_overrides = runtime.cli_overrides.clone(); - let insecure_mode = runtime.insecure_mode; let fallback = (settings.clone(), profile.clone()); issue_with_retry_inner( || { @@ -1439,7 +1420,7 @@ async fn issue_with_retry( let (fresh, fresh_profile) = reload_profile_or_fallback(&path, &overrides, &domain, fallback); let fresh_eab = profile::resolve_profile_eab(&fresh_profile, eab); - acme::issue_certificate(&fresh, &fresh_profile, fresh_eab, insecure_mode).await + acme::issue_certificate(&fresh, &fresh_profile, fresh_eab).await } }, |duration| tokio::time::sleep(duration), @@ -2220,7 +2201,6 @@ mod tests { lead, &IssuanceRuntime { config_path: paths.agent_config(), - insecure_mode: false, cli_overrides: config::CliOverrides::default(), }, ) diff --git a/src/daemon/registrar_handler_tests.rs b/src/daemon/registrar_handler_tests.rs index 77a101ca..833b4f4f 100644 --- a/src/daemon/registrar_handler_tests.rs +++ b/src/daemon/registrar_handler_tests.rs @@ -818,7 +818,6 @@ async fn an_unmounted_store_keeps_daemon_duties_running() { default_eab: None, eab_refresh_path: None, config_path: Some(deployment.path().join("agent.toml")), - insecure_mode: false, cli_overrides: crate::config::CliOverrides::default(), shutdown: shutdown.clone(), registrar_endpoint, @@ -938,7 +937,6 @@ async fn disabled_endpoint_configurations_do_not_enter_the_mount_refusal_path() default_eab: None, eab_refresh_path: None, config_path: Some(deployment.path().join("agent.toml")), - insecure_mode: false, cli_overrides: crate::config::CliOverrides::default(), shutdown: shutdown.clone(), registrar_endpoint: crate::registrar::RegistrarEndpoint::default(), @@ -1171,7 +1169,6 @@ async fn an_enabled_directory_endpoint_starts_the_ordinary_daemon_duties() { default_eab: None, eab_refresh_path: None, config_path: Some(deployment.path().join("agent.toml")), - insecure_mode: false, cli_overrides: crate::config::CliOverrides::default(), shutdown: shutdown.clone(), registrar_endpoint: endpoint.registrar_endpoint(), @@ -1275,7 +1272,6 @@ async fn an_unarmable_registrar_renewal_stops_the_daemon() { default_eab: None, eab_refresh_path: None, config_path: Some(deployment.path().join("agent.toml")), - insecure_mode: false, cli_overrides: crate::config::CliOverrides::default(), shutdown: DaemonShutdown::new(), registrar_endpoint: endpoint.registrar_endpoint(), @@ -1313,7 +1309,6 @@ async fn an_unopenable_production_audit_store_stops_the_daemon() { default_eab: None, eab_refresh_path: None, config_path: Some(deployment.path().join("agent.toml")), - insecure_mode: false, cli_overrides: crate::config::CliOverrides::default(), shutdown: DaemonShutdown::new(), registrar_endpoint: endpoint.registrar_endpoint(), diff --git a/src/lib.rs b/src/lib.rs index f2e9d404..e022525a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,3 @@ -use std::path::PathBuf; use std::sync::Arc; pub mod acme; @@ -60,8 +59,6 @@ pub async fn run_daemon(invocation: DaemonInvocation) -> anyhow::Result<()> { pub async fn run_oneshot( settings: Arc, default_eab: Option, - config_path: Option, - insecure_mode: bool, ) -> anyhow::Result<()> { - daemon::run_oneshot(settings, default_eab, config_path, insecure_mode).await + daemon::run_oneshot(settings, default_eab).await } diff --git a/src/registrar_certs.rs b/src/registrar_certs.rs index d0afcbe5..5755d15c 100644 --- a/src/registrar_certs.rs +++ b/src/registrar_certs.rs @@ -501,10 +501,7 @@ fn unset_path(setting: &str) -> anyhow::Error { /// composed from is known, names the four configured paths. /// There is no fallback on any of these: no self-signed leaf, no /// borrowed one, and no second source for either ACME input. -pub async fn ensure_registrar_surface_certificates( - settings: &Settings, - insecure_mode: bool, -) -> Result<()> { +pub async fn ensure_registrar_surface_certificates(settings: &Settings) -> Result<()> { if !settings.registrar_endpoint.enabled { return Ok(()); } @@ -533,7 +530,7 @@ pub async fn ensure_registrar_surface_certificates( ) })?; for pair in pending { - issue_surface_pair(settings, &pair, &plan.host, &inputs, insecure_mode).await?; + issue_surface_pair(settings, &pair, &plan.host, &inputs).await?; } Ok(()) } @@ -819,22 +816,17 @@ pub(crate) async fn issue_surface_pair( pair: &SurfacePairPaths, host: &str, inputs: &SurfaceAcmeInputs, - insecure_mode: bool, ) -> Result<()> { let issuance = issuance_settings(settings, pair, host, &inputs.responder_hmac); let profile = issuance .profiles .first() .ok_or_else(|| anyhow::anyhow!("the registrar surface issuance profile was not built"))?; - // `--insecure` is an explicit transport override. It must retain its - // established behavior and not inspect the output bundle before the - // existing post-issuance merge gate does. - let bootstrap_pins = bootstrap_pins_for_mode(&issuance.trust, insecure_mode); + let bootstrap_pins = bootstrap_pins(&issuance.trust); crate::acme::issue_certificate_with_bootstrap( &issuance, profile, inputs.eab.clone(), - insecure_mode, pair.leaf.issuance_options(), bootstrap_pins, ) @@ -878,19 +870,17 @@ pub(crate) async fn issue_surface_pair_material( pair: &SurfacePairPaths, host: &str, inputs: &SurfaceAcmeInputs, - insecure_mode: bool, ) -> Result { let issuance = issuance_settings(settings, pair, host, &inputs.responder_hmac); let profile = issuance .profiles .first() .ok_or_else(|| anyhow::anyhow!("the registrar surface issuance profile was not built"))?; - let bootstrap_pins = bootstrap_pins_for_mode(&issuance.trust, insecure_mode); + let bootstrap_pins = bootstrap_pins(&issuance.trust); let material = crate::acme::issue_certificate_material( &issuance, profile, inputs.eab.clone(), - insecure_mode, pair.leaf.issuance_options(), bootstrap_pins, ) @@ -937,17 +927,6 @@ fn bootstrap_pins(trust: &crate::config::TrustSettings) -> Option<&[String]> { } } -/// Selects bootstrap trust only for normal-mode issuance. -/// -/// `--insecure` deliberately does not inspect the configured output bundle -/// before the existing merge gate, preserving its established behavior. -fn bootstrap_pins_for_mode( - trust: &crate::config::TrustSettings, - insecure_mode: bool, -) -> Option<&[String]> { - (!insecure_mode).then(|| bootstrap_pins(trust)).flatten() -} - /// Builds the settings one surface issuance runs under. /// /// The daemon's own settings with two substitutions and nothing else: diff --git a/src/registrar_certs/tests.rs b/src/registrar_certs/tests.rs index 5a5e9ed6..68dcc96e 100644 --- a/src/registrar_certs/tests.rs +++ b/src/registrar_certs/tests.rs @@ -1356,7 +1356,7 @@ async fn the_client_pair_is_issued_with_every_name_invariant_and_the_client_auth .iter() .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) .expect("the client pair"); - issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), false) + issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs()) .await .expect("the client pair is issued"); @@ -1414,7 +1414,7 @@ async fn the_server_pair_is_issued_in_the_ordinary_shape_and_is_the_endpoint_ide .iter() .find(|pair| pair.leaf == SurfaceLeaf::EndpointServer) .expect("the server pair"); - issue_surface_pair(&host.settings, server, TEST_HOST, &openbao_inputs(), false) + issue_surface_pair(&host.settings, server, TEST_HOST, &openbao_inputs()) .await .expect("the server pair is issued"); @@ -1459,14 +1459,14 @@ async fn two_issuances_of_the_same_name_produce_different_keys() { .expect("the client pair"); let inputs = openbao_inputs(); - issue_surface_pair(&host.settings, client, TEST_HOST, &inputs, false) + issue_surface_pair(&host.settings, client, TEST_HOST, &inputs) .await .expect("first issuance"); let (cert_path, key_path) = host.client_pair(); let first_key = std::fs::read_to_string(&key_path).expect("read key"); let first_cert = std::fs::read_to_string(&cert_path).expect("read cert"); - issue_surface_pair(&host.settings, client, TEST_HOST, &inputs, false) + issue_surface_pair(&host.settings, client, TEST_HOST, &inputs) .await .expect("second issuance"); let second_key = std::fs::read_to_string(&key_path).expect("read key"); @@ -1490,7 +1490,7 @@ async fn the_published_key_is_never_group_or_world_readable() { let pairs = surface_pairs(host.endpoint(), TEST_HOST, TEST_DOMAIN).expect("pairs resolve"); let inputs = openbao_inputs(); for pair in &pairs { - issue_surface_pair(&host.settings, pair, TEST_HOST, &inputs, false) + issue_surface_pair(&host.settings, pair, TEST_HOST, &inputs) .await .expect("issued"); let key_mode = std::fs::metadata(&pair.key_path) @@ -1531,7 +1531,7 @@ async fn the_off_live_issuance_returns_material_and_publishes_nothing() { let before_cert = digest_of(&pair.cert_path); let before_key = digest_of(&pair.key_path); - let material = issue_surface_pair_material(&host.settings, pair, TEST_HOST, &inputs, false) + let material = issue_surface_pair_material(&host.settings, pair, TEST_HOST, &inputs) .await .expect("the off-live issuance produces material"); @@ -1580,10 +1580,10 @@ async fn two_off_live_issuances_of_the_same_name_produce_different_keys() { .expect("the client pair"); let inputs = openbao_inputs(); - let first = issue_surface_pair_material(&host.settings, client, TEST_HOST, &inputs, false) + let first = issue_surface_pair_material(&host.settings, client, TEST_HOST, &inputs) .await .expect("first candidate"); - let second = issue_surface_pair_material(&host.settings, client, TEST_HOST, &inputs, false) + let second = issue_surface_pair_material(&host.settings, client, TEST_HOST, &inputs) .await .expect("second candidate"); @@ -1614,7 +1614,7 @@ async fn issuance_never_writes_the_endpoint_pin_file() { let pairs = surface_pairs(host.endpoint(), TEST_HOST, TEST_DOMAIN).expect("pairs resolve"); let inputs = openbao_inputs(); for pair in &pairs { - issue_surface_pair(&host.settings, pair, TEST_HOST, &inputs, false) + issue_surface_pair(&host.settings, pair, TEST_HOST, &inputs) .await .expect("issued"); } @@ -1650,7 +1650,7 @@ async fn the_supplied_acme_inputs_reach_the_wire_and_the_local_ones_do_not() { .iter() .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) .expect("the client pair"); - issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), false) + issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs()) .await .expect("issued"); @@ -1723,57 +1723,18 @@ async fn the_supplied_acme_inputs_reach_the_wire_and_the_local_ones_do_not() { ); } -/// The chain is verified and the bundle merged **before** the leaf is -/// published, so a CA bundle the merge refuses to overwrite fails the -/// issuance with nothing on disk and the bundle byte-identical. +/// An unreadable output bundle is not bootstrap-eligible, so issuance +/// refuses while constructing its ordinary TLS client before it can send +/// ACME traffic or publish either material file. /// -/// Run with `insecure_mode`, which is the daemon's own `--insecure` -/// flag, so the outbound transport does not read the bundle and the -/// merge's own refusal is what is under test. The transport's separate, -/// pre-existing use of the same file is covered by -/// [`an_unreadable_ca_bundle_stops_normal_mode_before_acme_traffic`]. +/// There is no mode in which this file goes uninspected: the only +/// transports the daemon builds are the configured trust and the +/// pin-only bootstrap one, and neither accepts a certificate this +/// bundle cannot anchor. The merge gate's own refusal, on a bundle that +/// becomes unreadable after the client was built, is covered by +/// `acme::flow`'s `test_write_merged_ca_bundle_fails_when_existing_unreadable`. #[tokio::test] -async fn an_unreadable_ca_bundle_fails_at_merge_before_anything_is_published() { - let mut host = Host::new(); - let acme = start_acme(Arc::clone(&host.ca)).await; - aim_at(&mut host.settings, &acme); - - // A directory at the bundle path is a non-NotFound read error on - // every platform, without depending on chmod semantics that root in - // CI can bypass. - let bundle = host.dir.path().join("unreadable-bundle.pem"); - std::fs::create_dir_all(&bundle).expect("directory at the bundle path"); - host.settings.trust.ca_bundle_path = Some(bundle.clone()); - - let pairs = surface_pairs(host.endpoint(), TEST_HOST, TEST_DOMAIN).expect("pairs resolve"); - let client = pairs - .iter() - .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) - .expect("the client pair"); - let error = issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), true) - .await - .expect_err("an unreadable bundle must fail the issuance"); - let rendered = format!("{error:#}"); - assert!( - rendered.contains("refusing to overwrite unreadable CA bundle"), - "{rendered}" - ); - assert!( - rendered.contains(&bundle.display().to_string()), - "{rendered}" - ); - - let (cert_path, key_path) = host.client_pair(); - assert!(!cert_path.exists(), "no leaf may be published"); - assert!(!key_path.exists(), "no key may be published"); - assert!(bundle.is_dir(), "the bundle must be left exactly as it was"); -} - -/// An unreadable output bundle is not bootstrap-eligible, so normal-mode -/// issuance refuses while constructing its ordinary TLS client before it can -/// send ACME traffic or publish either material file. -#[tokio::test] -async fn an_unreadable_ca_bundle_stops_normal_mode_before_acme_traffic() { +async fn an_unreadable_ca_bundle_stops_issuance_before_acme_traffic() { let mut host = Host::new(); let acme = start_acme(Arc::clone(&host.ca)).await; aim_at(&mut host.settings, &acme); @@ -1790,9 +1751,9 @@ async fn an_unreadable_ca_bundle_stops_normal_mode_before_acme_traffic() { .iter() .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) .expect("the client pair"); - let error = issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), false) + let error = issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs()) .await - .expect_err("an unreadable bundle must stop normal-mode issuance"); + .expect_err("an unreadable bundle must stop issuance"); let rendered = format!("{error:#}"); assert!( rendered.contains("Failed to read CA bundle at"), @@ -1814,25 +1775,45 @@ async fn an_unreadable_ca_bundle_stops_normal_mode_before_acme_traffic() { assert!(bundle.is_dir(), "the bundle must be left exactly as it was"); } -/// `--insecure` retains its existing transport behavior: it bypasses TLS -/// verification without probing a bundle that only normal-mode bootstrap can -/// repair. +/// Pin-only bootstrap trust is the one alternative to the configured +/// bundle, and it is still verification: it is selected only for a +/// bundle that is absent or holds no parseable certificate, and it +/// carries the configured pins rather than accepting any certificate. #[test] -fn insecure_mode_does_not_select_bootstrap_pins() { +fn bootstrap_pins_are_selected_only_for_a_repairable_bundle() { let dir = tempfile::tempdir().expect("temporary directory"); - let trust = crate::config::TrustSettings { + let pin = "00".repeat(32); + let missing = crate::config::TrustSettings { ca_bundle_path: Some(dir.path().join("missing-bundle.pem")), - trusted_ca_sha256: vec!["00".repeat(32)], + trusted_ca_sha256: vec![pin.clone()], }; + assert_eq!( + bootstrap_pins(&missing), + Some(std::slice::from_ref(&pin)), + "a missing bundle is repaired over the configured pins" + ); + let usable_path = dir.path().join("usable-bundle.pem"); + let ca = TestCa::new("anchor.example"); + std::fs::write(&usable_path, &ca.root_pem).expect("write bundle"); + let usable = crate::config::TrustSettings { + ca_bundle_path: Some(usable_path), + trusted_ca_sha256: vec![pin.clone()], + }; assert!( - bootstrap_pins_for_mode(&trust, false).is_some(), - "normal mode selects bootstrap pins" + bootstrap_pins(&usable).is_none(), + "a usable bundle stays on the configured trust path" ); - let bootstrap_pins = bootstrap_pins_for_mode(&trust, true); + + let unreadable_path = dir.path().join("unreadable-bundle.pem"); + std::fs::create_dir_all(&unreadable_path).expect("directory at the bundle path"); + let unreadable = crate::config::TrustSettings { + ca_bundle_path: Some(unreadable_path), + trusted_ca_sha256: vec![pin], + }; assert!( - bootstrap_pins.is_none(), - "insecure mode must not select or inspect bootstrap trust" + bootstrap_pins(&unreadable).is_none(), + "an unreadable bundle is refused rather than repaired" ); } @@ -1856,7 +1837,7 @@ async fn a_missing_or_unparseable_bundle_still_publishes() { .iter() .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) .expect("the client pair"); - issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), false) + issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs()) .await .unwrap_or_else(|err| panic!("seed {seed:?} must still publish: {err:#}")); @@ -1892,7 +1873,7 @@ async fn tls_bootstrap_repairs_missing_or_unparseable_bundles() { .iter() .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) .expect("the client pair"); - issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), false) + issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs()) .await .unwrap_or_else(|error| { panic!("TLS bootstrap seed {seed:?} must complete issuance: {error:#}") @@ -1933,7 +1914,7 @@ async fn tls_bootstrap_rejects_empty_or_mismatched_pins_before_acme_traffic() { .iter() .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) .expect("the client pair"); - issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), false) + issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs()) .await .expect_err("untrusted bootstrap transport must fail the issuance"); @@ -1990,7 +1971,7 @@ async fn tls_bootstrap_uses_pins_for_an_https_responder() { .iter() .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) .expect("the client pair"); - issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), false) + issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs()) .await .unwrap_or_else(|error| { panic!( @@ -2046,7 +2027,7 @@ async fn tls_bootstrap_rejects_an_unpinned_https_responder() { .iter() .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) .expect("the client pair"); - issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), false) + issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs()) .await .expect_err("an unpinned HTTPS responder must fail bootstrap issuance"); @@ -2101,7 +2082,7 @@ async fn a_missing_bundle_uses_pinned_bootstrap_and_is_repaired() { .iter() .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) .expect("the client pair"); - issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), false) + issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs()) .await .expect("the pinned bootstrap transport repairs a missing bundle"); @@ -2131,7 +2112,7 @@ async fn a_write_that_cannot_land_is_a_failure_naming_the_path() { .iter() .find(|pair| pair.leaf == SurfaceLeaf::RegistrarClient) .expect("the client pair"); - let error = issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs(), false) + let error = issue_surface_pair(&host.settings, client, TEST_HOST, &openbao_inputs()) .await .expect_err("a certificate path that cannot be replaced must fail the start"); let rendered = format!("{error:#}"); @@ -2156,7 +2137,7 @@ async fn a_failed_acme_flow_refuses_and_names_the_material_paths() { .iter() .find(|pair| pair.leaf == SurfaceLeaf::EndpointServer) .expect("the server pair"); - let error = issue_surface_pair(&host.settings, server, TEST_HOST, &openbao_inputs(), false) + let error = issue_surface_pair(&host.settings, server, TEST_HOST, &openbao_inputs()) .await .expect_err("an unreachable CA must refuse the start"); let rendered = format!("{error:#}"); @@ -2200,7 +2181,7 @@ async fn a_disabled_endpoint_issues_nothing_and_reads_nothing() { // all would be a failure rather than a silent success. settings.server = "http://127.0.0.1:1/directory".to_string(); - ensure_registrar_surface_certificates(&settings, false) + ensure_registrar_surface_certificates(&settings) .await .expect("a disabled endpoint does nothing"); @@ -2233,7 +2214,7 @@ async fn both_pairs_usable_starts_with_openbao_unreachable_and_touches_nothing() // proof that none was made. settings.server = "http://127.0.0.1:1/directory".to_string(); - ensure_registrar_surface_certificates(&settings, false) + ensure_registrar_surface_certificates(&settings) .await .expect("usable material starts with OpenBao down"); @@ -2278,7 +2259,7 @@ async fn one_usable_pair_is_left_alone_while_the_other_is_issued() { let pending = pending_pairs(&plan, host.settings.trust.ca_bundle_path.as_deref()).await; assert_eq!(pending.len(), 1, "exactly one pair needs issuing"); for pair in &pending { - issue_surface_pair(&host.settings, pair, &plan.host, &openbao_inputs(), false) + issue_surface_pair(&host.settings, pair, &plan.host, &openbao_inputs()) .await .expect("the unusable pair is issued"); } @@ -2323,7 +2304,7 @@ async fn an_expired_leaf_on_either_pair_is_repaired_rather_than_refused() { let pending = pending_pairs(&plan, host.settings.trust.ca_bundle_path.as_deref()).await; assert_eq!(pending.len(), 2, "both expired pairs need issuing"); for pair in &pending { - issue_surface_pair(&host.settings, pair, &plan.host, &openbao_inputs(), false) + issue_surface_pair(&host.settings, pair, &plan.host, &openbao_inputs()) .await .expect("an expired pair is repaired by issuing"); assert_eq!( @@ -2809,7 +2790,7 @@ async fn a_failed_openbao_read_out_of_the_issuance_unit_names_the_pending_materi // its own — but the OpenBao read is reached first. settings.server = "http://127.0.0.1:1/directory".to_string(); - let error = ensure_registrar_surface_certificates(&settings, false) + let error = ensure_registrar_surface_certificates(&settings) .await .expect_err("an unreachable OpenBao must refuse the start"); let rendered = format!("{error:#}"); @@ -2857,7 +2838,7 @@ async fn a_resolution_failure_names_the_configured_material_paths() { .expect("a state file"); std::fs::remove_file(&state_file).expect("remove the state file"); - let error = ensure_registrar_surface_certificates(&host.settings, false) + let error = ensure_registrar_surface_certificates(&host.settings) .await .expect_err("an unreadable state file must refuse the start"); let rendered = format!("{error:#}"); @@ -2920,7 +2901,7 @@ async fn run_issuance(host: &Host) -> Vec { let plan = resolve_surface_plan(&host.settings).expect("the plan resolves"); let pending = pending_pairs(&plan, host.settings.trust.ca_bundle_path.as_deref()).await; for pair in &pending { - issue_surface_pair(&host.settings, pair, &plan.host, &openbao_inputs(), false) + issue_surface_pair(&host.settings, pair, &plan.host, &openbao_inputs()) .await .expect("the pending pair is issued"); } diff --git a/src/registrar_renewal.rs b/src/registrar_renewal.rs index 55bd4d27..549324d6 100644 --- a/src/registrar_renewal.rs +++ b/src/registrar_renewal.rs @@ -742,7 +742,6 @@ pub(crate) struct RegistrarCertRenewal { endpoint: Arc, state: RegistrarCertRenewalState, cadence: RenewalCadence, - insecure_mode: bool, live: Box, } @@ -773,12 +772,11 @@ impl RegistrarCertRenewal { pub(crate) async fn prepare( settings: Arc, endpoint: Arc, - insecure_mode: bool, ) -> Result { let plan = resolve_surface_plan(&settings) .context("resolving the registrar surface renewal plan")?; let cadence = RenewalCadence::from_internal_config(&plan.secrets_dir)?; - Self::assemble(settings, plan, endpoint, cadence, insecure_mode).await + Self::assemble(settings, plan, endpoint, cadence).await } /// Initializes the accessor and assembles the adapter around an @@ -799,7 +797,6 @@ impl RegistrarCertRenewal { plan: SurfacePlan, endpoint: Arc, cadence: RenewalCadence, - insecure_mode: bool, ) -> Result { let state = RegistrarCertRenewalState::default(); for pair in &plan.pairs { @@ -818,7 +815,6 @@ impl RegistrarCertRenewal { endpoint, state, cadence, - insecure_mode, live: Box::new(FilesystemPaths::new()), }) } @@ -849,7 +845,7 @@ impl RegistrarCertRenewal { endpoint: Arc, cadence: RenewalCadence, ) -> Result { - Self::assemble(settings, plan, endpoint, cadence, false).await + Self::assemble(settings, plan, endpoint, cadence).await } /// Returns the accessor this adapter writes. @@ -1052,14 +1048,9 @@ impl RegistrarCertRenewal { let produced: Mutex> = Mutex::new(None); utils::retry_with_backoff_and_sleep( || async { - let material = issue_surface_pair_material( - &self.settings, - pair, - &self.plan.host, - inputs, - self.insecure_mode, - ) - .await?; + let material = + issue_surface_pair_material(&self.settings, pair, &self.plan.host, inputs) + .await?; // The guard is taken and dropped inside this statement; // nothing holds it across an `.await`. *produced.lock().unwrap_or_else(PoisonError::into_inner) = Some(material); diff --git a/src/tls.rs b/src/tls.rs index 4e20c45c..39967c3b 100644 --- a/src/tls.rs +++ b/src/tls.rs @@ -32,32 +32,25 @@ pub(crate) const OPENBAO_CONNECT_TIMEOUT: Duration = Duration::from_secs(10); pub(crate) const OPENBAO_REQUEST_TIMEOUT: Duration = Duration::from_secs(30); /// Builds a [`reqwest::Client`] configured according to the given -/// [`TrustSettings`] and runtime TLS override. +/// [`TrustSettings`]. /// -/// Three modes: -/// - **Insecure override** (`--insecure`): accepts any certificate. +/// Two modes: /// - **System roots** (no `ca_bundle_path`): default webpki verification. /// - **Custom CA bundle** (with optional SHA-256 pinning): loads the bundle /// and optionally enforces certificate pins. /// +/// There is no third mode. Certificate verification is never disabled: +/// a peer this client cannot verify against the configured trust fails +/// the handshake, and the fix is the trust anchors, the SANs or the +/// clock rather than an override. +/// /// # Errors /// /// Returns an error if the CA bundle cannot be read or parsed, if /// certificate pins are specified without a CA bundle path, or if the /// HTTP client fails to build. -pub fn build_http_client(trust: &TrustSettings, insecure_mode: bool) -> Result { +pub fn build_http_client(trust: &TrustSettings) -> Result { install_crypto_provider(); - if insecure_mode { - // CodeQL flags `danger_accept_invalid_certs(true)` as - // rust/disabled-certificate-check. This is intentional: during - // break-glass recovery or explicit diagnostics the caller may opt in - // to an insecure ACME TLS client via `--insecure`. Dismiss the alert - // as a false positive because the override is explicit and temporary. - return Client::builder() - .danger_accept_invalid_certs(true) - .build() - .context("Failed to build insecure HTTP client"); - } let Some(bundle_path) = trust.ca_bundle_path.as_ref() else { if !trust.trusted_ca_sha256.is_empty() { diff --git a/tests/bootroot_agent_hardening.rs b/tests/bootroot_agent_hardening.rs index 627e578d..fc53ff3b 100644 --- a/tests/bootroot_agent_hardening.rs +++ b/tests/bootroot_agent_hardening.rs @@ -300,9 +300,14 @@ key = "{key_path}"{trust_block} Ok((config_path, cert_path, bundle_path)) } -async fn run_agent_oneshot(config_path: &Path, ca_url: &str, insecure: bool) -> Output { +async fn run_agent_oneshot(config_path: &Path, ca_url: &str) -> Output { + run_agent_oneshot_with(config_path, ca_url, &[]).await +} + +async fn run_agent_oneshot_with(config_path: &Path, ca_url: &str, extra: &[&str]) -> Output { let config_path = config_path.to_path_buf(); let ca_url = ca_url.to_string(); + let extra: Vec = extra.iter().map(|arg| (*arg).to_string()).collect(); tokio::task::spawn_blocking(move || { let mut cmd = Command::new(env!("CARGO_BIN_EXE_bootroot-agent")); cmd.args([ @@ -313,9 +318,7 @@ async fn run_agent_oneshot(config_path: &Path, ca_url: &str, insecure: bool) -> if !ca_url.is_empty() { cmd.args(["--ca-url", ca_url.as_str()]); } - if insecure { - cmd.arg("--insecure"); - } + cmd.args(&extra); let mut child = cmd .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -364,7 +367,7 @@ async fn oneshot_normal_run_uses_prestaged_trust_without_rewriting_config() -> R )?; let before = fs::read_to_string(&config_path).context("read config before run")?; - let output = run_agent_oneshot(&config_path, "", false).await; + let output = run_agent_oneshot(&config_path, "").await; assert!( output.status.success(), "stdout: {}\nstderr: {}", @@ -381,8 +384,15 @@ async fn oneshot_normal_run_uses_prestaged_trust_without_rewriting_config() -> R Ok(()) } +/// There is no runtime mode that accepts an untrusted server. +/// +/// `--insecure` used to be one, and a build that reintroduced it would +/// still pass every other assertion in this file: the untrusted-server +/// run below fails, and a flag that made it succeed would simply not be +/// exercised. So assert on the flag itself — the binary must refuse to +/// start rather than issue against a certificate it cannot verify. #[tokio::test] -async fn oneshot_insecure_override_allows_untrusted_server() -> Result<()> { +async fn oneshot_rejects_an_insecure_override_flag() -> Result<()> { let fixture = start_acme_tls_fixture().await?; assert_fixture_reachable(&fixture.directory_url(), fixture.ca_pem()).await; let tmp = tempdir().context("create tempdir")?; @@ -390,20 +400,21 @@ async fn oneshot_insecure_override_allows_untrusted_server() -> Result<()> { mount_responder_admin_mock(&responder).await; let (config_path, cert_path, bundle_path) = write_agent_config(tmp.path(), &fixture.directory_url(), &responder.uri(), None)?; - let before = fs::read_to_string(&config_path).context("read config before run")?; - let output = run_agent_oneshot(&config_path, "", true).await; + let output = run_agent_oneshot_with(&config_path, "", &["--insecure"]).await; assert!( - output.status.success(), + !output.status.success(), "stdout: {}\nstderr: {}", String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr) ); - - let after = fs::read_to_string(&config_path).context("read config after run")?; - assert_eq!(before, after); - assert!(cert_path.exists()); - assert!(!bundle_path.exists()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("unexpected argument '--insecure'"), + "{stderr}" + ); + assert!(!cert_path.exists(), "no leaf may be issued"); + assert!(!bundle_path.exists(), "no bundle may be written"); fixture.handle.abort(); Ok(()) @@ -419,7 +430,7 @@ async fn oneshot_normal_run_without_trust_fails() -> Result<()> { let (config_path, _cert_path, _bundle_path) = write_agent_config(tmp.path(), &fixture.directory_url(), &responder.uri(), None)?; - let output = run_agent_oneshot(&config_path, "", false).await; + let output = run_agent_oneshot(&config_path, "").await; assert!( !output.status.success(), "stdout: {}\nstderr: {}", From c85d605e60769f7406882eaf84d09357257e46ba Mon Sep 17 00:00:00 2001 From: Aco Piper Date: Wed, 2 Sep 2026 16:42:47 +0900 Subject: [PATCH 2/4] Say what the compose trust placeholders actually do The `[trust]` comment promised the placeholders would "fail the handshake closed", but they never reach a handshake: they are not 64 hex characters, so `validate_trust_settings` rejects the config outright with `trust.trusted_ca_sha256 must be 64 hex chars`. An operator who left them in would go looking for a TLS error and find a config error instead. The installation manuals ended the paragraph before the trust preparation with a colon, which now introduces prose rather than the block it used to. Part of #983 --- agent.toml.compose | 6 ++++-- docs/en/installation.md | 2 +- docs/ko/installation.md | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/agent.toml.compose b/agent.toml.compose index 963e46b9..5a8ae202 100644 --- a/agent.toml.compose +++ b/agent.toml.compose @@ -38,8 +38,10 @@ http_responder_token_ttl_secs = 300 # | cut -d= -f2 | tr -d ':' | tr 'A-Z' 'a-z' # done # -# trusted_ca_sha256 must hold those two values before deployment; the -# placeholders below match nothing and fail the handshake closed. +# trusted_ca_sha256 must hold those two values before deployment. The +# placeholders below are not 64 hex characters, so a run that leaves them +# in place is refused while the config is validated, before any connection +# is attempted. # scripts/preflight/extra/agent-scenarios.sh stamps both in for its own runs. [trust] ca_bundle_path = "certs/compose-ca-bundle.pem" diff --git a/docs/en/installation.md b/docs/en/installation.md index bf1b7116..9ed113de 100644 --- a/docs/en/installation.md +++ b/docs/en/installation.md @@ -612,7 +612,7 @@ service under the same user or group. bootroot-agent has no container image: it always runs as a host process, so there is nothing to `docker compose up`. To exercise a **one-shot** issuance against the compose stack, build the binary and point it at the ports the -stack publishes to the host: +stack publishes to the host. The compose stack's CA is self-signed and nothing skips verifying it, so prepare the trust material first from the deployment's own certificates: diff --git a/docs/ko/installation.md b/docs/ko/installation.md index 8bf6aedb..2be3091a 100644 --- a/docs/ko/installation.md +++ b/docs/ko/installation.md @@ -617,7 +617,7 @@ mTLS를 사용하는 서비스는 `trust.ca_bundle_path`에 저장되는 CA 번 bootroot-agent에는 컨테이너 이미지가 없습니다. 항상 호스트 프로세스로 실행되므로 `docker compose up` 대상이 존재하지 않습니다. compose 스택을 상대로 **1회 발급**(`--oneshot`)을 확인하려면 바이너리를 빌드한 뒤 스택이 -호스트에 게시한 포트로 연결합니다: +호스트에 게시한 포트로 연결합니다. compose 스택의 CA는 자체 서명이고 이를 건너뛰는 수단은 없으므로, 배포가 가진 인증서로 trust 자료를 먼저 준비합니다: From 2337f9b32a7a3362da6d98767916e1ab46429ec6 Mon Sep 17 00:00:00 2001 From: Aco Piper Date: Wed, 2 Sep 2026 16:45:48 +0900 Subject: [PATCH 3/4] Guard the shipped templates' trust keys Removing `--insecure` left `agent.toml.compose` as the one template that had to grow a `[trust]` section to stay runnable, and nothing asserts it is still there. Dropping it again would not fail a build or a test: the compose smoke path would simply fall back to the system CA store, which cannot anchor a self-signed step-ca, and the breakage would surface only when an operator ran the scenarios by hand. The neighbouring template test already stages both shipped configs and loads them, so assert the two trust keys there. The placeholder values are not checked -- `validate_trust_settings` rejects those, and `from_file` does not run it. Part of #983 --- src/config/validation.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/config/validation.rs b/src/config/validation.rs index 5468cf65..868401be 100644 --- a/src/config/validation.rs +++ b/src/config/validation.rs @@ -1093,4 +1093,39 @@ mod tests { } } } + + /// Every shipped template must carry both `[trust]` keys. + /// + /// Nothing in the agent skips verifying the ACME server, so a + /// template that ships without them hands an operator a config whose + /// only route to a handshake is the system CA store -- which cannot + /// anchor a self-signed step-ca. `agent.toml.compose` is the one + /// that used to be run with `--insecure` instead, so it is the one a + /// revert would quietly return to being unverifiable. + /// + /// The values are placeholders and are deliberately not checked + /// here: `validate_trust_settings` is what rejects them, and + /// `from_file` does not run it. What must not regress is the keys + /// being present at all. + #[test] + fn shipped_agent_config_templates_carry_both_trust_keys() { + for name in ["agent.toml.example", "agent.toml.compose"] { + let source = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join(name); + let contents = + std::fs::read_to_string(&source).unwrap_or_else(|err| panic!("read {name}: {err}")); + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("agent.toml"); + std::fs::write(&path, &contents).expect("stage template"); + let settings = crate::config::Settings::from_file(Some(path)) + .unwrap_or_else(|err| panic!("{name} must deserialize: {err}")); + assert!( + settings.trust.ca_bundle_path.is_some(), + "{name} must set trust.ca_bundle_path" + ); + assert!( + !settings.trust.trusted_ca_sha256.is_empty(), + "{name} must set trust.trusted_ca_sha256" + ); + } + } } From 2e68ef6c5e11fd48ad24ed3330dca665cb805f21 Mon Sep 17 00:00:00 2001 From: Aco Piper Date: Wed, 2 Sep 2026 17:24:27 +0900 Subject: [PATCH 4/4] Share the shipped-template staging in tests The trust-key assertion added beside the profile one repeated its whole setup: read the template out of the manifest directory, stage a copy under a `.toml` name the config loader can infer a format from, and load it. Two copies of that is two places to fix when a third template ships or the staging trick stops being needed. Name the template list once and put the staging behind a helper, so each test is the assertion it exists for. Part of #983 --- src/config/validation.rs | 45 ++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/config/validation.rs b/src/config/validation.rs index 868401be..7a862b27 100644 --- a/src/config/validation.rs +++ b/src/config/validation.rs @@ -1063,25 +1063,33 @@ mod tests { ); } + /// The configs an operator copies into place. + const SHIPPED_AGENT_CONFIG_TEMPLATES: [&str; 2] = ["agent.toml.example", "agent.toml.compose"]; + + /// Loads a shipped template the way an operator would. + /// + /// The templates ship under `.example` / `.compose` suffixes, which + /// the config loader cannot infer a format from, so stage a + /// byte-identical copy at a `.toml` name and read that. + fn load_shipped_template(name: &str) -> crate::config::Settings { + let source = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join(name); + let contents = + std::fs::read_to_string(&source).unwrap_or_else(|err| panic!("read {name}: {err}")); + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("agent.toml"); + std::fs::write(&path, &contents).expect("stage template"); + crate::config::Settings::from_file(Some(path)) + .unwrap_or_else(|err| panic!("{name} must deserialize: {err}")) + } + /// Every shipped `[[profiles]]` template must deserialize and pass /// `validate_profile`. The templates are what an operator copies, so /// one that omits the now-required `registration_id` would hand them /// a config the agent refuses to load. #[test] fn shipped_agent_config_templates_carry_a_valid_profile() { - for name in ["agent.toml.example", "agent.toml.compose"] { - let source = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join(name); - let contents = - std::fs::read_to_string(&source).unwrap_or_else(|err| panic!("read {name}: {err}")); - // The templates ship under `.example` / `.compose` suffixes, - // which the config loader cannot infer a format from; stage a - // byte-identical copy at a `.toml` name so this reads exactly - // what an operator would copy into place. - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("agent.toml"); - std::fs::write(&path, &contents).expect("stage template"); - let settings = crate::config::Settings::from_file(Some(path)) - .unwrap_or_else(|err| panic!("{name} must deserialize: {err}")); + for name in SHIPPED_AGENT_CONFIG_TEMPLATES { + let settings = load_shipped_template(name); assert!(!settings.profiles.is_empty(), "{name} must ship a profile"); for profile in &settings.profiles { assert!( @@ -1109,15 +1117,8 @@ mod tests { /// being present at all. #[test] fn shipped_agent_config_templates_carry_both_trust_keys() { - for name in ["agent.toml.example", "agent.toml.compose"] { - let source = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join(name); - let contents = - std::fs::read_to_string(&source).unwrap_or_else(|err| panic!("read {name}: {err}")); - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("agent.toml"); - std::fs::write(&path, &contents).expect("stage template"); - let settings = crate::config::Settings::from_file(Some(path)) - .unwrap_or_else(|err| panic!("{name} must deserialize: {err}")); + for name in SHIPPED_AGENT_CONFIG_TEMPLATES { + let settings = load_shipped_template(name); assert!( settings.trust.ca_bundle_path.is_some(), "{name} must set trust.ca_bundle_path"