Require TLS for non-loopback agent transport

This commit is contained in:
rcourtman 2026-04-21 23:56:07 +01:00
parent 3ec2c0779e
commit c49176d700
9 changed files with 83 additions and 33 deletions

View file

@ -1087,6 +1087,13 @@ keys through the managed `ssh_known_hosts` store before any automated deploy
fan-out writes a bootstrap token or runs the installer on a remote node, keep
`StrictHostKeyChecking=yes`, and fail closed on key mismatch or missing-host-
key state instead of downgrading to unauthenticated SSH during install.
That same transport boundary also keeps plaintext Pulse URLs loopback-only.
`internal/hostagent/agent.go`, `internal/hostagent/commands.go`, and
`internal/agentupdate/update.go` may keep local-development `http://` or
`ws://` only for loopback hosts, but private-network and remote Pulse URLs
must still use HTTPS/WSS. `InsecureSkipVerify` may relax certificate
verification on TLS transport; it must not reopen plaintext HTTP for
private-network updater, websocket, or reporting paths.
That same shared `internal/api/` lifecycle boundary also assumes tenant-scoped
resource helpers stay on canonical unified-resource seeds: adjacent fleet and
install surfaces may not revive raw tenant `StateSnapshot` fallback through

View file

@ -212,11 +212,18 @@ func TestValidatePulseURL(t *testing.T) {
}
})
t.Run("AllowRemoteHTTPInInsecureMode", func(t *testing.T) {
t.Run("RejectRemoteHTTPInInsecureMode", func(t *testing.T) {
u := newUpdaterForTest("http://pulse.example.com")
u.cfg.InsecureSkipVerify = true
if err := u.validatePulseURL(); err != nil {
t.Fatalf("expected remote http URL in insecure mode to be valid, got %v", err)
if err := u.validatePulseURL(); err == nil {
t.Fatalf("expected remote http URL in insecure mode to be rejected")
}
})
t.Run("RejectPrivateNetworkHTTP", func(t *testing.T) {
u := newUpdaterForTest("http://10.0.0.5:7655")
if err := u.validatePulseURL(); err == nil {
t.Fatalf("expected private-network http URL to be rejected")
}
})
@ -804,8 +811,7 @@ func TestPerformUpdateDownloadErrorAndHeaders(t *testing.T) {
func TestPerformUpdateDownloadRetriesTransientError(t *testing.T) {
_, execPath := writeTempExec(t)
u := newUpdaterForTest("http://example")
u.cfg.InsecureSkipVerify = true
u := newUpdaterForTest("https://example")
u.cfg.APIToken = "token"
data := testBinary()

View file

@ -568,14 +568,6 @@ func isLoopbackHost(host string) bool {
return ip != nil && ip.IsLoopback()
}
func isPrivateNetworkHost(host string) bool {
if host == "" {
return false
}
ip := net.ParseIP(strings.Trim(host, "[]"))
return ip != nil && ip.IsPrivate()
}
func (u *Updater) validatePulseURL() error {
pulseURL := strings.TrimSpace(u.cfg.PulseURL)
if pulseURL == "" {
@ -601,10 +593,10 @@ func (u *Updater) validatePulseURL() error {
case "https":
return nil
case "http":
if u.cfg.InsecureSkipVerify || isLoopbackHost(parsed.Hostname()) || isPrivateNetworkHost(parsed.Hostname()) {
if isLoopbackHost(parsed.Hostname()) {
return nil
}
return fmt.Errorf("HTTP Pulse URL is only allowed for localhost/loopback, private networks, or when insecure mode is enabled")
return fmt.Errorf("HTTP Pulse URL is only allowed for localhost/loopback")
default:
return fmt.Errorf("unsupported Pulse URL scheme %q", parsed.Scheme)
}

View file

@ -867,8 +867,8 @@ func normalizePulseURL(rawURL string) (string, error) {
case "https":
// Always allowed.
case "http":
if !isLoopbackOrPrivateHost(parsed.Hostname()) {
return "", fmt.Errorf("pulse URL %q must use https unless host is loopback or private network", rawURL)
if !isLoopbackHost(parsed.Hostname()) {
return "", fmt.Errorf("pulse URL %q must use https unless host is loopback", rawURL)
}
default:
return "", fmt.Errorf("pulse URL %q has unsupported scheme %q", rawURL, parsed.Scheme)
@ -881,19 +881,19 @@ func normalizePulseURL(rawURL string) (string, error) {
return parsed.String(), nil
}
// isLoopbackOrPrivateHost returns true for loopback and RFC1918 private
// network addresses. HTTP (non-TLS) is safe over a local/private network;
// the scheme guard only needs to prevent plaintext over the public internet.
func isLoopbackOrPrivateHost(host string) bool {
if strings.EqualFold(host, "localhost") {
// isLoopbackHost returns true for localhost and loopback IPs only. Plain HTTP
// is never allowed for non-loopback Pulse URLs, even on private networks.
func isLoopbackHost(host string) bool {
normalized := strings.ToLower(strings.Trim(host, "[]"))
if normalized == "localhost" || strings.HasSuffix(normalized, ".localhost") {
return true
}
ip := net.ParseIP(host)
ip := net.ParseIP(normalized)
if ip == nil {
return false
}
return ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast()
return ip.IsLoopback()
}
// collectTemperatures attempts to collect temperature data from the local system.

View file

@ -390,7 +390,12 @@ func TestNew_RejectsInvalidPulseURL(t *testing.T) {
{
name: "non-loopback http rejected",
url: "http://example.com",
want: "must use https unless host is loopback or private network",
want: "must use https unless host is loopback",
},
{
name: "private-network http rejected",
url: "http://10.0.0.5:7655",
want: "must use https unless host is loopback",
},
{
name: "query rejected",

View file

@ -98,11 +98,21 @@ func TestCommandClientBuildWebSocketURL(t *testing.T) {
pulseURL: "http://example.invalid",
wantErr: true,
},
{
name: "private-network http rejected",
pulseURL: "http://10.0.0.5:7655",
wantErr: true,
},
{
name: "non-loopback ws rejected",
pulseURL: "ws://example.invalid",
wantErr: true,
},
{
name: "private-network ws rejected",
pulseURL: "ws://10.0.0.5:7655",
wantErr: true,
},
{
name: "query rejected",
pulseURL: "https://example.invalid?x=1",

View file

@ -374,15 +374,15 @@ func (c *CommandClient) buildWebSocketURL() (string, error) {
case "https":
parsed.Scheme = "wss"
case "http":
if !isLoopbackOrPrivateHost(parsed.Hostname()) {
return "", fmt.Errorf("pulse URL %q must use https/wss unless host is loopback or private network", c.pulseURL)
if !isLoopbackHost(parsed.Hostname()) {
return "", fmt.Errorf("pulse URL %q must use https/wss unless host is loopback", c.pulseURL)
}
parsed.Scheme = "ws"
case "wss":
parsed.Scheme = "wss"
case "ws":
if !isLoopbackOrPrivateHost(parsed.Hostname()) {
return "", fmt.Errorf("pulse URL %q must use https/wss unless host is loopback or private network", c.pulseURL)
if !isLoopbackHost(parsed.Hostname()) {
return "", fmt.Errorf("pulse URL %q must use https/wss unless host is loopback", c.pulseURL)
}
parsed.Scheme = "ws"
default:

View file

@ -256,6 +256,13 @@ func (c *CommandClient) preflightTarget(
"preflight_complete", "failed", fmt.Sprintf("Invalid node IP: %s", target.NodeIP), data, false)
return false
}
normalizedPulseURL, err := normalizePulseURL(pulseURL)
if err != nil {
data := marshalPreflightResult(false, false, false, "", err.Error())
c.sendDeployProgress(conn, requestID, jobID, target.TargetID,
"preflight_complete", "failed", fmt.Sprintf("Invalid Pulse URL: %v", err), data, false)
return false
}
// 1. SSH reachability check.
c.sendDeployProgress(conn, requestID, jobID, target.TargetID,
@ -303,7 +310,7 @@ func (c *CommandClient) preflightTarget(
c.sendDeployProgress(conn, requestID, jobID, target.TargetID,
"preflight_reachability", "started", "Checking Pulse URL reachability", "", false)
reachable := c.checkPulseReachabilitySSH(tctx, target.NodeIP, pulseURL)
reachable := c.checkPulseReachabilitySSH(tctx, target.NodeIP, normalizedPulseURL)
reachStatus := "ok"
reachMsg := "Pulse URL reachable"
if !reachable {
@ -348,6 +355,13 @@ func (c *CommandClient) installTarget(
"install_complete", "failed", fmt.Sprintf("Invalid node IP: %s", target.NodeIP), data, false)
return false
}
normalizedPulseURL, err := normalizePulseURL(pulseURL)
if err != nil {
data := marshalInstallResult(-1, err.Error())
c.sendDeployProgress(conn, requestID, jobID, target.TargetID,
"install_complete", "failed", fmt.Sprintf("Invalid Pulse URL: %v", err), data, false)
return false
}
// 1. Write bootstrap token to target via SSH stdin.
c.sendDeployProgress(conn, requestID, jobID, target.TargetID,
@ -366,7 +380,7 @@ func (c *CommandClient) installTarget(
c.sendDeployProgress(conn, requestID, jobID, target.TargetID,
"install_execute", "started", "Running install script", "", false)
exitCode, output, err := c.runInstallSSH(tctx, target.NodeIP, pulseURL)
exitCode, output, err := c.runInstallSSH(tctx, target.NodeIP, normalizedPulseURL)
if err != nil || exitCode != 0 {
msg := fmt.Sprintf("Install failed (exit %d)", exitCode)
if err != nil {
@ -484,13 +498,18 @@ func (c *CommandClient) writeTokenSSH(ctx context.Context, nodeIP, token string)
// runInstallSSH runs the Pulse install script on a remote node via SSH.
func (c *CommandClient) runInstallSSH(ctx context.Context, nodeIP, pulseURL string) (int, string, error) {
normalizedPulseURL, err := normalizePulseURL(pulseURL)
if err != nil {
return -1, "", err
}
// SSH concatenates all command arguments into a single string passed to the
// remote shell. We use shellescape (single quotes) for the URL, which prevents
// all shell expansion ($(), backticks, $VAR). We invoke bash explicitly so
// pipefail works regardless of the remote user's default shell (e.g. dash/ash).
// The single-quoted URL is embedded in the outer single-quoted bash -c argument
// using the '\'' escape pattern (end quote, literal quote, resume quote).
escapedURL := shellescape(pulseURL)
escapedURL := shellescape(normalizedPulseURL)
innerCmd := fmt.Sprintf(
"set -o pipefail; curl -sfL -- %s/install.sh | bash -s -- --non-interactive --token-file /run/pulse-agent/bootstrap.token --pulse-url %s --enroll --enable-commands --enable-proxmox",
escapedURL, escapedURL,

View file

@ -150,7 +150,7 @@ func TestRunInstallSSH_IncludesEnrollAndEnableCommands(t *testing.T) {
sshKnownHosts: stubKnownHostsManager{path: "/tmp/pulse-test-known-hosts"},
}
exitCode, _, err := c.runInstallSSH(context.Background(), "10.0.0.1", "http://10.0.0.1:7655")
exitCode, _, err := c.runInstallSSH(context.Background(), "10.0.0.1", "https://10.0.0.1:7655")
if err != nil {
t.Fatalf("runInstallSSH error: %v", err)
}
@ -179,3 +179,14 @@ func TestRunInstallSSH_IncludesEnrollAndEnableCommands(t *testing.T) {
t.Error("SSH install command does not isolate global known_hosts state")
}
}
func TestRunInstallSSH_RejectsNonLoopbackPlainHTTP(t *testing.T) {
c := &CommandClient{
logger: zerolog.New(zerolog.NewTestWriter(t)),
sshKnownHosts: stubKnownHostsManager{path: "/tmp/pulse-test-known-hosts"},
}
if _, _, err := c.runInstallSSH(context.Background(), "10.0.0.1", "http://10.0.0.1:7655"); err == nil {
t.Fatal("expected non-loopback plain HTTP Pulse URL to be rejected")
}
}