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..5a8ae202 100644 --- a/agent.toml.compose +++ b/agent.toml.compose @@ -26,6 +26,27 @@ 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 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" +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..9ed113de 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 @@ -611,20 +612,35 @@ 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: + +```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..2be3091a 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 번들 소비 서비스 권한 @@ -618,20 +617,35 @@ mTLS를 사용하는 서비스는 `trust.ca_bundle_path`에 저장되는 CA 번 bootroot-agent에는 컨테이너 이미지가 없습니다. 항상 호스트 프로세스로 실행되므로 `docker compose up` 대상이 존재하지 않습니다. compose 스택을 상대로 **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/config/validation.rs b/src/config/validation.rs index 5468cf65..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!( @@ -1093,4 +1101,32 @@ 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 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" + ); + assert!( + !settings.trust.trusted_ca_sha256.is_empty(), + "{name} must set trust.trusted_ca_sha256" + ); + } + } } 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: {}",