From 124ab78260e849c9d8cf7a0d4d607f858f0353be Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 19 Oct 2025 16:28:38 +0000 Subject: [PATCH] security: fix SSH command injection vulnerabilities in pulse-sensor-proxy CRITICAL security fixes for pulse-sensor-proxy: 1. Strengthened hostname validation regex: - Now requires hostnames to start with alphanumeric character - Prevents SSH option injection via hostnames starting with '-' - Pattern: ^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$ (1-64 chars total) - Added IPv4 and IPv6 validation regexes for future use 2. Added validation to vulnerable V1 RPC handlers: - handleGetTemperature: Now validates node parameter before SSH - handleRegisterNodes: Now validates discovered cluster nodes - Previously these handlers passed unsanitized input directly to SSH 3. Defense in depth: - V2 handlers already had validation (now using improved regex) - Multiple layers of protection against malicious node identifiers - Validation prevents container from passing SSH options as hostnames Without these fixes, a compromised container could potentially inject SSH options by providing malicious node names, though the 'root@' prefix provided some mitigation. Addresses high-severity finding from security audit. --- cmd/pulse-sensor-proxy/main.go | 35 ++++++++++++++++++++++++---- cmd/pulse-sensor-proxy/validation.go | 9 ++++++- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/cmd/pulse-sensor-proxy/main.go b/cmd/pulse-sensor-proxy/main.go index d2891c61f..089f948a1 100644 --- a/cmd/pulse-sensor-proxy/main.go +++ b/cmd/pulse-sensor-proxy/main.go @@ -39,6 +39,10 @@ func defaultWorkDir() string { return "/var/lib/pulse-sensor-proxy" } +var ( + configPath string +) + var rootCmd = &cobra.Command{ Use: "pulse-sensor-proxy", Short: "Pulse Sensor Proxy - Secure sensor data bridge for containerized Pulse", @@ -65,6 +69,7 @@ var versionCmd = &cobra.Command{ func init() { rootCmd.AddCommand(versionCmd) + rootCmd.PersistentFlags().StringVar(&configPath, "config", "", "Path to configuration file (default: /etc/pulse-sensor-proxy/config.yaml)") } func main() { @@ -135,12 +140,16 @@ func runProxy() { } // Load configuration - configPath := os.Getenv("PULSE_SENSOR_PROXY_CONFIG") - if configPath == "" { - configPath = defaultConfigPath + // Priority: --config flag > PULSE_SENSOR_PROXY_CONFIG env > default path + cfgPath := configPath // from flag + if cfgPath == "" { + cfgPath = os.Getenv("PULSE_SENSOR_PROXY_CONFIG") + } + if cfgPath == "" { + cfgPath = defaultConfigPath } - cfg, err := loadConfig(configPath) + cfg, err := loadConfig(cfgPath) if err != nil { log.Fatal().Err(err).Msg("Failed to load configuration") } @@ -151,7 +160,7 @@ func runProxy() { log.Info(). Str("socket", socketPath). Str("ssh_key_dir", sshKeyPath). - Str("config_path", configPath). + Str("config_path", cfgPath). Str("version", Version). Msg("Starting pulse-sensor-proxy") @@ -568,6 +577,13 @@ func (p *Proxy) handleRegisterNodes(req RPCRequest) RPCResponse { // Test SSH connectivity to each node nodeStatus := make([]map[string]interface{}, 0, len(nodes)) for _, node := range nodes { + // Validate node name to prevent SSH command injection + node = strings.TrimSpace(node) + if err := validateNodeName(node); err != nil { + log.Warn().Str("node", node).Msg("Invalid node name format from cluster discovery") + continue + } + status := map[string]interface{}{ "name": node, } @@ -609,6 +625,15 @@ func (p *Proxy) handleGetTemperature(req RPCRequest) RPCResponse { } } + // Validate node name to prevent SSH command injection + node = strings.TrimSpace(node) + if err := validateNodeName(node); err != nil { + return RPCResponse{ + Success: false, + Error: "invalid node name format", + } + } + // Fetch temperature data tempData, err := p.getTemperatureViaSSH(node) if err != nil { diff --git a/cmd/pulse-sensor-proxy/validation.go b/cmd/pulse-sensor-proxy/validation.go index b0fbe13aa..01ceee0f2 100644 --- a/cmd/pulse-sensor-proxy/validation.go +++ b/cmd/pulse-sensor-proxy/validation.go @@ -9,7 +9,14 @@ import ( var ( // nodeNameRegex validates node names (alphanumeric, dots, underscores, hyphens, 1-64 chars) - nodeNameRegex = regexp.MustCompile(`^[a-zA-Z0-9._-]{1,64}$`) + // Must not start with hyphen to prevent SSH option injection + nodeNameRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$`) + + // ipv4Regex validates IPv4 addresses + ipv4Regex = regexp.MustCompile(`^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$`) + + // ipv6Regex validates IPv6 addresses (simplified) + ipv6Regex = regexp.MustCompile(`^[0-9a-fA-F:]+$`) ) // sanitizeCorrelationID validates and sanitizes a correlation ID