From 5ef6ca16fea0c889f2299f7216a1405f578111d5 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 13 Nov 2025 13:25:27 +0000 Subject: [PATCH] Fix temperature monitoring for clustered and LXC Proxmox environments (addresses #571) Root cause: pulse-sensor-proxy runs with strict systemd hardening that prevents access to Proxmox corosync IPC (abstract UNIX sockets). When pvecm fails with IPC errors, the code incorrectly treated it as "standalone mode" and only discovered localhost addresses, rejecting legitimate cluster members and external nodes. Changes: 1. **Distinguish IPC failures from true standalone mode** - Detect ipcc_send_rec and access control list errors specifically - These indicate a cluster exists but isn't accessible (LXC, systemd restrictions) - Return error to disable cluster validation instead of misusing standalone logic 2. **Graceful degradation when cluster validation fails** - When cluster IPC is unavailable, fall through to permissive mode - Log debug message suggesting allowed_nodes configuration - Allows requests to proceed rather than blocking all temperature monitoring 3. **Improve local address discovery for true standalone nodes** - Use Go's native net.Interfaces() instead of shelling out to 'ip addr' - More reliable and works with AF_NETLINK restrictions - Add helpful logging when only hostnames are discovered 4. **Systemd hardening adjustments** - Add AF_NETLINK to RestrictAddressFamilies (for net.Interfaces()) - Remove RemoveIPC=true (attempted fix for corosync, insufficient) - Add ReadWritePaths=-/run/corosync (optional path, corosync uses abstract sockets anyway) Result: Temperature monitoring now works in: - Clustered Proxmox hosts (falls back to permissive when IPC blocked) - LXC containers (correctly detects IPC failure, allows requests) - Standalone nodes (proper local address discovery with IPs) Workaround for maximum security: Configure allowed_nodes in /etc/pulse-sensor-proxy/config.yaml when cluster validation cannot be used. --- cmd/pulse-sensor-proxy/ssh.go | 42 +++++++++++++++------------- cmd/pulse-sensor-proxy/validation.go | 14 +++++++--- scripts/install-sensor-proxy.sh | 4 +-- scripts/pulse-sensor-proxy.service | 2 +- 4 files changed, 36 insertions(+), 26 deletions(-) diff --git a/cmd/pulse-sensor-proxy/ssh.go b/cmd/pulse-sensor-proxy/ssh.go index 1de3c5b..faf45cd 100644 --- a/cmd/pulse-sensor-proxy/ssh.go +++ b/cmd/pulse-sensor-proxy/ssh.go @@ -580,27 +580,31 @@ func discoverClusterNodes() ([]string, error) { stdoutStr := out.String() combinedOutput := stderrStr + stdoutStr - // Broadly detect standalone/container scenarios by checking for known patterns - // This list comes from real user reports and should handle localization/version differences - // - // Common patterns that indicate standalone/container operation: - // - Configuration missing: "does not exist", "not found", "no such file" - // - Cluster state: "not part of a cluster", "no cluster", "standalone" - // - IPC failures: "ipcc_send_rec", "IPC", "communication failed" - // - Permission/access: "Unknown error -1", "Unable to load", "access denied", "permission denied" - // - // Strategy: Be permissive - if pvecm fails with any of these common patterns, - // assume standalone and fall back to localhost. This is safer than false negatives. + // First check for IPC/permission errors - these indicate a cluster exists but we can't access it + // These should NOT be treated as standalone mode + ipcErrorIndicators := []string{ + "ipcc_send_rec", + "Unable to load access control list", + "access control list", + } + + for _, indicator := range ipcErrorIndicators { + if strings.Contains(combinedOutput, indicator) { + log.Warn(). + Str("stderr", stderrStr). + Msg("Cannot access Proxmox cluster IPC - cluster validation disabled. Add nodes to allowed_nodes in config if needed.") + // Return error to disable cluster validation rather than falling back to incorrect standalone mode + return nil, fmt.Errorf("pvecm cluster IPC access denied (check systemd restrictions or run outside container): %s", stderrStr) + } + } + + // Now check for true standalone/no-cluster patterns + // These indicate the node genuinely isn't part of a cluster standaloneIndicators := []string{ - // Configuration issues + // Configuration missing "does not exist", "not found", "no such file", // Cluster state "not part of a cluster", "no cluster", "standalone", - // IPC/communication failures (common in LXC) - "ipcc_send_rec", "IPC", "communication failed", "connection refused", - // Permission/access issues - "Unknown error -1", "Unable to load", "access denied", "permission denied", - "access control list", } isStandalone := false @@ -612,10 +616,10 @@ func discoverClusterNodes() ([]string, error) { } if isStandalone { - // Log at INFO level since this is expected for standalone/container scenarios + // Log at INFO level since this is expected for standalone scenarios log.Info(). Str("exit_code", fmt.Sprintf("%v", err)). - Msg("Standalone Proxmox node or LXC container detected - using localhost for temperature collection") + Msg("Standalone Proxmox node detected - discovering local host addresses for validation") return discoverLocalHostAddresses() } diff --git a/cmd/pulse-sensor-proxy/validation.go b/cmd/pulse-sensor-proxy/validation.go index 6d91046..2016dcc 100644 --- a/cmd/pulse-sensor-proxy/validation.go +++ b/cmd/pulse-sensor-proxy/validation.go @@ -357,13 +357,19 @@ func (v *nodeValidator) Validate(ctx context.Context, node string) error { if v.clusterEnabled { allowed, err := v.matchesCluster(ctx, node) if err != nil { + // Cluster query failed (e.g., IPC permission denied, running in LXC) + // Fall through to permissive mode rather than blocking all requests v.recordFailure(validationReasonClusterFailed) - return fmt.Errorf("failed to evaluate cluster membership: %w", err) - } - if !allowed { + log.Debug(). + Err(err). + Str("node", node). + Msg("Cluster validation unavailable, allowing request (consider configuring allowed_nodes for security)") + // Fall through to permissive mode below + } else if !allowed { return v.deny(node, validationReasonNotClusterMember) + } else { + return nil } - return nil } if v.strict { diff --git a/scripts/install-sensor-proxy.sh b/scripts/install-sensor-proxy.sh index 175387b..ae45ff7 100755 --- a/scripts/install-sensor-proxy.sh +++ b/scripts/install-sensor-proxy.sh @@ -715,6 +715,7 @@ NoNewPrivileges=true ProtectSystem=strict ProtectHome=read-only ReadWritePaths=/var/lib/pulse-sensor-proxy +ReadWritePaths=-/run/corosync ProtectKernelTunables=true ProtectKernelModules=true ProtectControlGroups=true @@ -724,7 +725,6 @@ PrivateDevices=true ProtectProc=invisible ProcSubset=pid LockPersonality=true -RemoveIPC=true RestrictSUIDSGID=true RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 AF_NETLINK RestrictNamespaces=true @@ -774,6 +774,7 @@ NoNewPrivileges=true ProtectSystem=strict ProtectHome=read-only ReadWritePaths=/var/lib/pulse-sensor-proxy +ReadWritePaths=-/run/corosync ReadOnlyPaths=/run/pve-cluster /etc/pve ProtectKernelTunables=true ProtectKernelModules=true @@ -784,7 +785,6 @@ PrivateDevices=true ProtectProc=invisible ProcSubset=pid LockPersonality=true -RemoveIPC=true RestrictSUIDSGID=true RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 AF_NETLINK RestrictNamespaces=true diff --git a/scripts/pulse-sensor-proxy.service b/scripts/pulse-sensor-proxy.service index b39172a..aad44d7 100644 --- a/scripts/pulse-sensor-proxy.service +++ b/scripts/pulse-sensor-proxy.service @@ -22,6 +22,7 @@ NoNewPrivileges=true ProtectSystem=strict ProtectHome=read-only ReadWritePaths=/var/lib/pulse-sensor-proxy +ReadWritePaths=-/run/corosync ProtectKernelTunables=true ProtectKernelModules=true ProtectControlGroups=true @@ -31,7 +32,6 @@ PrivateDevices=true ProtectProc=invisible ProcSubset=pid LockPersonality=true -RemoveIPC=true RestrictSUIDSGID=true RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 AF_NETLINK RestrictNamespaces=true