From 630bf897bedeaba63a5446b32191ff59d00a3982 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Fri, 16 Oct 2026 23:22:42 +0100 Subject: [PATCH] fix(agent): separate runner trust from collector TLS bypass Legacy updates recover the collector's insecure setting even after the host trusts the server certificate. Do not reject verified runner system trust because of that unrelated setting. Keep HTTPS runner verification enabled and preserve custom CA and exact fingerprint trust. Exercise recovered configuration and the real credential lifecycle client against TLS, including untrusted certificates and mismatched pins that must receive no credential-bearing request. --- docs/UNIFIED_AGENT.md | 5 + .../v6/internal/subsystems/agent-lifecycle.md | 17 ++- .../subsystems/deployment-installability.md | 8 ++ frontend-modern/public/docs/UNIFIED_AGENT.md | 5 + scripts/install.sh | 12 +- scripts/installtests/install_sh_test.go | 115 +++++++++++++++--- 6 files changed, 139 insertions(+), 23 deletions(-) diff --git a/docs/UNIFIED_AGENT.md b/docs/UNIFIED_AGENT.md index 6833ce0997..bbfe9a086b 100644 --- a/docs/UNIFIED_AGENT.md +++ b/docs/UNIFIED_AGENT.md @@ -278,6 +278,11 @@ separate install choices. `--least-privilege --enable-privileged-helper` selects the opt-in unprivileged collector and no-network helper. Adding `--enable-action-runner --action-token-file ` installs the root-owned runner with a separately issued, host-bound action credential. +The runner verifies HTTPS using the host's system trust store, an explicit +`--cacert` file, or a separately verified `--server-fingerprint`. A collector's +saved `--insecure` setting does not disable runner verification or prevent +enrollment once the host trusts the server certificate. Adding the internal CA +to the host trust store also lets subsequent runner updates use normal trust. `--disable-action-runner` and `--uninstall-action-runner` remove remediation without removing the collector. The runner accepts only the documented typed host, Proxmox guest, and container operations; shell, generic exec, diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 18ceb43eab..33fec096ad 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -1071,9 +1071,20 @@ Runner WS registration and activation, cancellation, and self-revocation HTTP calls require HTTPS/WSS with normal trust, a configured CA, or an enforced certificate-DER fingerprint. Plaintext HTTP/WS accepts only exact `localhost`, `127/8`, or `::1` literals (not `*.localhost`) and never -inherits generic installer insecure or curl `-k` behavior. Bearer-bearing -activation, cancellation, and self-revocation clients connect directly and do -not inherit ambient HTTP proxy variables. +inherits generic installer insecure or curl `-k` behavior. + +The installer must also keep these trust scopes separate during enrollment: +a recovered collector bypass cannot reject otherwise valid system-root trust. +Generated HTTPS runner environments omit the bypass even when collector +continuity retains it. `TestInstallSHActionRunnerRecoveredCollectorTrust` +executes legacy environment recovery, real configuration persistence and the +actual runner cancellation client against TLS. Process-confined system roots, +custom CA and exact-pin controls admit the credential, while untrusted +certificates and mismatched pins receive no HTTP request. This is source-level +TLS/credential proof, not native installation or customer-host acceptance. + +Bearer-bearing activation, cancellation, and self-revocation clients connect +directly and do not inherit ambient HTTP proxy variables. Runner readiness is exposed through a bounded, secret-free health marker that is replaced atomically only after its contents reach stable storage. The marker distinguishes registered/pending from activated and carries only the diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index a4a60e9c6c..fe053f1883 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -1400,6 +1400,14 @@ credential stops and disables the runner but retains every artifact for retry or manual server-side revocation. Runner WS and lifecycle HTTP require HTTPS/WSS with system/custom CA trust or exact certificate-DER pinning; plaintext is loopback-only and generic installer insecure/curl `-k` is never inherited. +Collector update recovery may retain its legacy insecure flag without blocking +normal system-root trust for the independently configured runner. Enrollment +must omit the HTTPS runner bypass rather than treating that collector flag as +runner policy. `TestInstallSHActionRunnerRecoveredCollectorTrust` executes +recovery, private config writes and the actual lifecycle binary with confined +system roots, a custom CA and an exact pin. Untrusted and wrong-pin controls +send no credential-bearing request. This does not establish native service +installation, release inclusion or recovery on a reporter's host. The runner must retain a writable host filesystem because its closed protocol performs real package, storage, guest, and container mutations; `ProtectSystem=strict` is therefore forbidden for that unit. This exception diff --git a/frontend-modern/public/docs/UNIFIED_AGENT.md b/frontend-modern/public/docs/UNIFIED_AGENT.md index 6833ce0997..bbfe9a086b 100644 --- a/frontend-modern/public/docs/UNIFIED_AGENT.md +++ b/frontend-modern/public/docs/UNIFIED_AGENT.md @@ -278,6 +278,11 @@ separate install choices. `--least-privilege --enable-privileged-helper` selects the opt-in unprivileged collector and no-network helper. Adding `--enable-action-runner --action-token-file ` installs the root-owned runner with a separately issued, host-bound action credential. +The runner verifies HTTPS using the host's system trust store, an explicit +`--cacert` file, or a separately verified `--server-fingerprint`. A collector's +saved `--insecure` setting does not disable runner verification or prevent +enrollment once the host trusts the server certificate. Adding the internal CA +to the host trust store also lets subsequent runner updates use normal trust. `--disable-action-runner` and `--uninstall-action-runner` remove remediation without removing the collector. The runner accepts only the documented typed host, Proxmox guest, and container operations; shell, generic exec, diff --git a/scripts/install.sh b/scripts/install.sh index 91bf5cb06e..a655081c18 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -2034,14 +2034,14 @@ write_action_runner_config() { -L "$ACTION_RUNNER_TOKEN_FILE" || -L "$ACTION_RUNNER_ENV_FILE" ]]; then fail "Refusing unsafe symlink in the action-runner config or state boundary" "$EXIT_GENERAL" fi - install -d -o root -g root -m 0700 "$ACTION_RUNNER_CONFIG_DIR" - install -d -o root -g root -m 0700 "$ACTION_RUNNER_STATE_DIR" action_runner_url_transport_allowed "$PULSE_URL" || fail "Action runner requires HTTPS/WSS; plaintext HTTP/WS is allowed only for loopback local use" "$EXIT_MISSING_ARGS" - if [[ "$INSECURE" == "true" && "$PULSE_URL" =~ ^[Hh][Tt][Tt][Pp][Ss]:// && - -z "$CURL_CA_BUNDLE" && -z "$SERVER_FINGERPRINT" ]]; then - fail "Action runner refuses generic insecure HTTPS; configure a trusted CA bundle or exact server fingerprint" "$EXIT_MISSING_ARGS" - fi + # INSECURE belongs to collector/download continuity and may be recovered + # from an older install even after the host trusts the server certificate. + # Runner HTTPS always verifies system/custom roots or the exact pin. Never + # turn the collector's bypass into runner authority or reject system trust. + install -d -o root -g root -m 0700 "$ACTION_RUNNER_CONFIG_DIR" + install -d -o root -g root -m 0700 "$ACTION_RUNNER_STATE_DIR" if [[ -n "$ACTION_TOKEN" ]]; then persist_action_runner_replacement_token "$ACTION_TOKEN" || diff --git a/scripts/installtests/install_sh_test.go b/scripts/installtests/install_sh_test.go index 635f494f41..e10bc59385 100644 --- a/scripts/installtests/install_sh_test.go +++ b/scripts/installtests/install_sh_test.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha256" "encoding/base64" + "encoding/hex" "encoding/json" "encoding/pem" "errors" @@ -20,6 +21,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "syscall" "testing" "time" @@ -7308,42 +7310,127 @@ write_action_runner_config } } -func TestInstallSHActionRunnerRejectsGenericInsecureHTTPSBeforeCredentialWrite(t *testing.T) { +func TestInstallSHActionRunnerRecoveredCollectorTrust(t *testing.T) { root := t.TempDir() - tokenPath := filepath.Join(root, "config", "token") - script := ` -set -u + runnerBinary := filepath.Join(root, "pulse-agent-runner") + build := exec.Command("go", "build", "-o", runnerBinary, "./cmd/pulse-agent-runner") + build.Dir = filepath.Dir(repoFile("go.mod")) + if output, err := build.CombinedOutput(); err != nil { + t.Fatalf("build action runner: %v\n%s", err, output) + } + const token = "fixture-runner-credential" + var requests atomic.Int32 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + if r.Header.Get("Authorization") != "Bearer "+token || r.Method != http.MethodDelete { + t.Errorf("unexpected credential lifecycle request") + } + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + caFile := filepath.Join(root, "ca.pem") + if err := os.WriteFile(caFile, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: server.Certificate().Raw}), 0o600); err != nil { + t.Fatal(err) + } + fingerprint := sha256.Sum256(server.Certificate().Raw) + for _, tc := range []struct { + name, ca, pin string + systemTrust, wantOK bool + }{ + {name: "system roots", systemTrust: true, wantOK: true}, + {name: "recovered custom CA", ca: caFile, wantOK: true}, + {name: "recovered exact pin", pin: hex.EncodeToString(fingerprint[:]), wantOK: true}, + {name: "untrusted certificate"}, + {name: "mismatched pin", pin: strings.Repeat("00", 32), systemTrust: true}, + } { + t.Run(tc.name, func(t *testing.T) { + if tc.name == "system roots" && runtime.GOOS != "linux" { + t.Skip("process-confined system roots require the Linux root loader") + } + root := t.TempDir() + tokenPath := filepath.Join(root, "config", "token") + envFile := filepath.Join(root, "config", "runner.env") + script := ` +set -euo pipefail ACTION_RUNNER_CONFIG_DIR="` + filepath.Join(root, "config") + `" ACTION_RUNNER_STATE_DIR="` + filepath.Join(root, "state") + `" ACTION_RUNNER_TOKEN_FILE="` + tokenPath + `" -ACTION_RUNNER_ENV_FILE="` + filepath.Join(root, "config", "runner.env") + `" +ACTION_RUNNER_ENV_FILE="` + envFile + `" +ACTION_RUNNER_BINARY_PATH="` + runnerBinary + `" ACTION_RUNNER_HEALTH_FILE="` + filepath.Join(root, "state", "health.json") + `" ACTION_RUNNER_ACTIVATION_NONCE="` + strings.Repeat("a", 64) + `" STATE_DIR="` + filepath.Join(root, "collector") + `" HOSTNAME_OVERRIDE="host.local" AGENT_ID="agent-1" -ACTION_TOKEN="must-not-be-written" -PULSE_URL="https://pulse.example" +ACTION_TOKEN="` + token + `" +PULSE_URL="` + server.URL + `" +PULSE_TOKEN="fixture-collector-credential" SERVER_FINGERPRINT="" CURL_CA_BUNDLE="" -INSECURE="true" +INSECURE="false" EXIT_GENERAL=1 EXIT_MISSING_ARGS=2 fail() { printf '%s\n' "$1" >&2; exit "$2"; } install() { local destination="${!#}"; mkdir -p "$destination"; chmod 0700 "$destination"; } +chown() { return 0; } +stat() { + case "$1:$2" in + '-c:%u') printf '0\n' ;; + '-c:%a') printf '600\n' ;; + *) command stat "$@" ;; + esac +} +` + extractInstallShellFunction(t, "recovered_connection_state_ready") + ` +` + extractInstallShellFunction(t, "recover_connection_state_from_env_stream") + ` ` + extractInstallShellFunction(t, "write_action_runner_env_value") + ` ` + extractInstallShellFunction(t, "action_runner_url_uses_loopback_http") + ` ` + extractInstallShellFunction(t, "action_runner_url_transport_allowed") + ` +` + extractInstallShellFunction(t, "resolve_action_runner_agent_id") + ` ` + extractInstallShellFunction(t, "persist_action_runner_replacement_token") + ` ` + extractInstallShellFunction(t, "write_action_runner_config") + ` +` + extractInstallShellFunction(t, "cancel_pending_action_runner_credential") + ` +recover_connection_state_from_env_stream <<'SAVED_STATE' +PULSE_INSECURE_SKIP_VERIFY=true +PULSE_CACERT=` + tc.ca + ` +PULSE_SERVER_FINGERPRINT=` + tc.pin + ` +SAVED_STATE +[[ "$INSECURE" == "true" ]] write_action_runner_config +[[ "$INSECURE" == "true" && -z "$ACTION_TOKEN" ]] +# Load the installed environment as systemd would, preserving machine trust +# when no explicit CA is configured. The collector's bypass must stay absent. +set -a +. "$ACTION_RUNNER_ENV_FILE" +set +a +[[ "${PULSE_INSECURE:-false}" == "false" ]] +cancel_pending_action_runner_credential "agent-1" "host.local" "` + token + `" ` - out, err := exec.Command("bash", "-c", script).CombinedOutput() - if err == nil || !strings.Contains(string(out), "refuses generic insecure HTTPS") { - t.Fatalf("generic insecure HTTPS was not rejected: %v\n%s", err, out) - } - if _, statErr := os.Stat(tokenPath); !os.IsNotExist(statErr) { - t.Fatalf("runner credential was written before transport rejection: %v", statErr) + cmd := exec.Command("bash", "-c", script) + // Go's system root loader uses these paths. Confine the fixture's + // machine trust to this child, without changing the host trust store. + roots := filepath.Join(root, "empty-roots.pem") + mustWrite(t, roots, "") + if tc.systemTrust { + roots = caFile + } + cmd.Env = append(os.Environ(), "SSL_CERT_FILE="+roots, "SSL_CERT_DIR="+root, "PULSE_INSECURE=") + before := requests.Load() + out, err := cmd.CombinedOutput() + if (err == nil) != tc.wantOK { + t.Fatalf("recovered collector trust: error=%v, want success=%v\n%s", err, tc.wantOK, out) + } + if got := requests.Load() - before; (got == 1) != tc.wantOK || got > 1 { + t.Fatalf("credential-bearing requests = %d, trusted=%v", got, tc.wantOK) + } + content, readErr := os.ReadFile(envFile) + if readErr != nil || strings.Contains(string(content), "PULSE_INSECURE") || strings.Contains(string(content), token) { + t.Fatalf("runner environment contains collector bypass/credential or is missing: %v", readErr) + } + persisted, readErr := os.ReadFile(tokenPath) + if readErr != nil || string(persisted) != token+"\n" || strings.Contains(string(out), token) { + t.Fatalf("private runner credential was lost or exposed: %v", readErr) + } + }) } }