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.
This commit is contained in:
parent
67862e6f11
commit
3a6a4fd362
2 changed files with 38 additions and 6 deletions
|
|
@ -39,6 +39,10 @@ func defaultWorkDir() string {
|
||||||
return "/var/lib/pulse-sensor-proxy"
|
return "/var/lib/pulse-sensor-proxy"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var (
|
||||||
|
configPath string
|
||||||
|
)
|
||||||
|
|
||||||
var rootCmd = &cobra.Command{
|
var rootCmd = &cobra.Command{
|
||||||
Use: "pulse-sensor-proxy",
|
Use: "pulse-sensor-proxy",
|
||||||
Short: "Pulse Sensor Proxy - Secure sensor data bridge for containerized Pulse",
|
Short: "Pulse Sensor Proxy - Secure sensor data bridge for containerized Pulse",
|
||||||
|
|
@ -65,6 +69,7 @@ var versionCmd = &cobra.Command{
|
||||||
|
|
||||||
func init() {
|
func init() {
|
||||||
rootCmd.AddCommand(versionCmd)
|
rootCmd.AddCommand(versionCmd)
|
||||||
|
rootCmd.PersistentFlags().StringVar(&configPath, "config", "", "Path to configuration file (default: /etc/pulse-sensor-proxy/config.yaml)")
|
||||||
}
|
}
|
||||||
|
|
||||||
func main() {
|
func main() {
|
||||||
|
|
@ -135,12 +140,16 @@ func runProxy() {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Load configuration
|
// Load configuration
|
||||||
configPath := os.Getenv("PULSE_SENSOR_PROXY_CONFIG")
|
// Priority: --config flag > PULSE_SENSOR_PROXY_CONFIG env > default path
|
||||||
if configPath == "" {
|
cfgPath := configPath // from flag
|
||||||
configPath = defaultConfigPath
|
if cfgPath == "" {
|
||||||
|
cfgPath = os.Getenv("PULSE_SENSOR_PROXY_CONFIG")
|
||||||
|
}
|
||||||
|
if cfgPath == "" {
|
||||||
|
cfgPath = defaultConfigPath
|
||||||
}
|
}
|
||||||
|
|
||||||
cfg, err := loadConfig(configPath)
|
cfg, err := loadConfig(cfgPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Fatal().Err(err).Msg("Failed to load configuration")
|
log.Fatal().Err(err).Msg("Failed to load configuration")
|
||||||
}
|
}
|
||||||
|
|
@ -151,7 +160,7 @@ func runProxy() {
|
||||||
log.Info().
|
log.Info().
|
||||||
Str("socket", socketPath).
|
Str("socket", socketPath).
|
||||||
Str("ssh_key_dir", sshKeyPath).
|
Str("ssh_key_dir", sshKeyPath).
|
||||||
Str("config_path", configPath).
|
Str("config_path", cfgPath).
|
||||||
Str("version", Version).
|
Str("version", Version).
|
||||||
Msg("Starting pulse-sensor-proxy")
|
Msg("Starting pulse-sensor-proxy")
|
||||||
|
|
||||||
|
|
@ -568,6 +577,13 @@ func (p *Proxy) handleRegisterNodes(req RPCRequest) RPCResponse {
|
||||||
// Test SSH connectivity to each node
|
// Test SSH connectivity to each node
|
||||||
nodeStatus := make([]map[string]interface{}, 0, len(nodes))
|
nodeStatus := make([]map[string]interface{}, 0, len(nodes))
|
||||||
for _, node := range 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{}{
|
status := map[string]interface{}{
|
||||||
"name": node,
|
"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
|
// Fetch temperature data
|
||||||
tempData, err := p.getTemperatureViaSSH(node)
|
tempData, err := p.getTemperatureViaSSH(node)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
|
||||||
|
|
@ -9,7 +9,14 @@ import (
|
||||||
|
|
||||||
var (
|
var (
|
||||||
// nodeNameRegex validates node names (alphanumeric, dots, underscores, hyphens, 1-64 chars)
|
// 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
|
// sanitizeCorrelationID validates and sanitizes a correlation ID
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue