Fix security regression: use localhost-only fallback instead of permissive mode
Codex independent review identified a critical security issue: when cluster validation fails, the previous fix fell back to permissive mode (allowing ALL nodes), making the proxy a potential SSRF/network scanner for any container that could reach the socket. NEW BEHAVIOR: When cluster validation is unavailable (IPC blocked), fall back to localhost-only validation instead of permissive mode. This maintains security while still allowing self-monitoring. Implementation: - Added validateAsLocalhost() method to nodeValidator - Calls discoverLocalHostAddresses() to get local IPs/hostnames - Only allows requests matching the local host - Blocks requests to other cluster members or arbitrary hosts Test results on delly (clustered node with IPC blocked): - Request to 192.168.0.5 (self): ALLOWED, temps fetched - Request to 192.168.0.134 (cluster peer): BLOCKED with node_not_localhost - No more "allowing all nodes" security regression Related to #571 - addresses Codex security audit feedback This prevents the proxy from being abused as a network scanner while still solving the original temperature monitoring issue.
This commit is contained in:
parent
848860a9a3
commit
7d2a2bd978
1 changed files with 39 additions and 3 deletions
|
|
@ -358,13 +358,14 @@ func (v *nodeValidator) Validate(ctx context.Context, node string) error {
|
||||||
allowed, err := v.matchesCluster(ctx, node)
|
allowed, err := v.matchesCluster(ctx, node)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// Cluster query failed (e.g., IPC permission denied, running in LXC)
|
// Cluster query failed (e.g., IPC permission denied, running in LXC)
|
||||||
// Fall through to permissive mode rather than blocking all requests
|
// Fall back to localhost-only validation instead of permissive mode
|
||||||
v.recordFailure(validationReasonClusterFailed)
|
v.recordFailure(validationReasonClusterFailed)
|
||||||
log.Warn().
|
log.Warn().
|
||||||
Err(err).
|
Err(err).
|
||||||
Str("node", node).
|
Str("node", node).
|
||||||
Msg("SECURITY: Cluster validation unavailable - allowing all nodes. Configure allowed_nodes in config to restrict access.")
|
Msg("SECURITY: Cluster validation unavailable - falling back to localhost-only validation. Configure allowed_nodes for cluster-wide access.")
|
||||||
// Fall through to permissive mode below
|
// Attempt to validate against localhost addresses
|
||||||
|
return v.validateAsLocalhost(ctx, node)
|
||||||
} else if !allowed {
|
} else if !allowed {
|
||||||
return v.deny(node, validationReasonNotClusterMember)
|
return v.deny(node, validationReasonNotClusterMember)
|
||||||
} else {
|
} else {
|
||||||
|
|
@ -379,6 +380,41 @@ func (v *nodeValidator) Validate(ctx context.Context, node string) error {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (v *nodeValidator) validateAsLocalhost(ctx context.Context, node string) error {
|
||||||
|
// When cluster validation is unavailable, only allow access to localhost
|
||||||
|
// This maintains security while allowing self-monitoring
|
||||||
|
if ctx == nil {
|
||||||
|
ctx = context.Background()
|
||||||
|
}
|
||||||
|
|
||||||
|
// Try to discover local host addresses
|
||||||
|
localAddrs, err := discoverLocalHostAddresses()
|
||||||
|
if err != nil {
|
||||||
|
log.Warn().Err(err).Msg("Failed to discover local host addresses for fallback validation")
|
||||||
|
// If we can't even discover localhost, deny access
|
||||||
|
return v.deny(node, validationReasonClusterFailed)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if the requested node matches any local address
|
||||||
|
normalized := normalizeAllowlistEntry(node)
|
||||||
|
if normalized == "" {
|
||||||
|
normalized = strings.ToLower(strings.TrimSpace(node))
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, localAddr := range localAddrs {
|
||||||
|
if strings.EqualFold(localAddr, normalized) {
|
||||||
|
log.Debug().
|
||||||
|
Str("node", node).
|
||||||
|
Str("matched_local", localAddr).
|
||||||
|
Msg("Node validated as localhost (cluster validation unavailable)")
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Node doesn't match any local address - deny
|
||||||
|
return v.deny(node, "node_not_localhost")
|
||||||
|
}
|
||||||
|
|
||||||
func (v *nodeValidator) matchesAllowlist(ctx context.Context, node string) (bool, error) {
|
func (v *nodeValidator) matchesAllowlist(ctx context.Context, node string) (bool, error) {
|
||||||
normalized := normalizeAllowlistEntry(node)
|
normalized := normalizeAllowlistEntry(node)
|
||||||
if normalized != "" {
|
if normalized != "" {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue