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.
This commit is contained in:
rcourtman
2026-10-16 23:22:42 +01:00
parent a93b5be1d3
commit 630bf897be
6 changed files with 139 additions and 23 deletions
+5
View File
@@ -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 <private-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,
@@ -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
@@ -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
@@ -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 <private-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,
+6 -6
View File
@@ -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" ||
+101 -14
View File
@@ -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)
}
})
}
}