Address Codex security review feedback
Changes based on independent Codex review:
1. Elevated log level from Debug to Warn for permissive mode fallback
- Operators now see "SECURITY: Cluster validation unavailable" in
journalctl at default log level
- Added similar warning on startup when running in permissive mode
- Makes it obvious when node validation is bypassed
2. Added runtime fallback for AF_NETLINK restrictions
- New discoverLocalHostAddressesFallback() shells out to 'ip addr'
- Triggered when net.Interfaces() fails with netlinkrib error
- Ensures existing installations work even without systemd unit update
- Logs recommendation to update systemd unit for better performance
3. Improved security awareness
- Changed message to explicitly state "allowing all nodes"
- Recommends configuring allowed_nodes for security
- Makes permissive fallback behavior transparent to operators
Related to #571 - temperature monitoring on standalone nodes
These changes ensure the fix works for existing installations that
haven't updated their systemd units, while clearly communicating when
the proxy is running in an insecure permissive mode.
This commit is contained in:
parent
5ef6ca16fe
commit
848860a9a3
2 changed files with 91 additions and 3 deletions
|
|
@ -690,6 +690,13 @@ func discoverLocalHostAddresses() ([]string, error) {
|
|||
ipCount := 0
|
||||
interfaces, err := net.Interfaces()
|
||||
if err != nil {
|
||||
// Check if this is an AF_NETLINK restriction error from systemd
|
||||
if strings.Contains(err.Error(), "netlinkrib") || strings.Contains(err.Error(), "address family not supported") {
|
||||
log.Warn().
|
||||
Err(err).
|
||||
Msg("AF_NETLINK restricted by systemd - falling back to 'ip addr' command. Update systemd unit to add AF_NETLINK to RestrictAddressFamilies.")
|
||||
return discoverLocalHostAddressesFallback()
|
||||
}
|
||||
log.Warn().
|
||||
Err(err).
|
||||
Msg("Failed to enumerate network interfaces - temperature monitoring may require manual allowed_nodes configuration")
|
||||
|
|
@ -771,6 +778,87 @@ func discoverLocalHostAddresses() ([]string, error) {
|
|||
return result, nil
|
||||
}
|
||||
|
||||
// discoverLocalHostAddressesFallback uses 'ip addr' command when AF_NETLINK is restricted
|
||||
func discoverLocalHostAddressesFallback() ([]string, error) {
|
||||
addresses := make(map[string]struct{})
|
||||
|
||||
// Get hostname and FQDN (same as native version)
|
||||
if hostname, err := os.Hostname(); err == nil && hostname != "" {
|
||||
addresses[strings.ToLower(hostname)] = struct{}{}
|
||||
cmd := exec.Command("hostname", "-f")
|
||||
if out, err := cmd.Output(); err == nil {
|
||||
fqdn := strings.TrimSpace(string(out))
|
||||
if fqdn != "" && fqdn != hostname {
|
||||
addresses[strings.ToLower(fqdn)] = struct{}{}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Use 'ip addr' to get IP addresses
|
||||
cmd := exec.Command("ip", "addr", "show")
|
||||
out, err := cmd.Output()
|
||||
if err != nil {
|
||||
log.Warn().Err(err).Msg("Failed to run 'ip addr' command")
|
||||
// Return at least the hostname
|
||||
result := make([]string, 0, len(addresses))
|
||||
for addr := range addresses {
|
||||
result = append(result, addr)
|
||||
}
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// Parse 'ip addr' output for inet/inet6 lines
|
||||
// Example: " inet 192.168.0.5/24 brd 192.168.0.255 scope global eno1"
|
||||
ipCount := 0
|
||||
lines := strings.Split(string(out), "\n")
|
||||
for _, line := range lines {
|
||||
line = strings.TrimSpace(line)
|
||||
if !strings.HasPrefix(line, "inet ") && !strings.HasPrefix(line, "inet6 ") {
|
||||
continue
|
||||
}
|
||||
|
||||
fields := strings.Fields(line)
|
||||
if len(fields) < 2 {
|
||||
continue
|
||||
}
|
||||
|
||||
// Second field is the IP/CIDR (e.g., "192.168.0.5/24")
|
||||
ipCIDR := fields[1]
|
||||
ipStr, _, _ := strings.Cut(ipCIDR, "/")
|
||||
|
||||
ip := net.ParseIP(ipStr)
|
||||
if ip == nil || ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsUnspecified() {
|
||||
continue
|
||||
}
|
||||
|
||||
addresses[ip.String()] = struct{}{}
|
||||
ipCount++
|
||||
}
|
||||
|
||||
result := make([]string, 0, len(addresses))
|
||||
for addr := range addresses {
|
||||
result = append(result, addr)
|
||||
}
|
||||
|
||||
if len(result) == 0 {
|
||||
return nil, fmt.Errorf("no local host addresses found")
|
||||
}
|
||||
|
||||
// Log helpful info about discovered addresses
|
||||
logger := log.Info().
|
||||
Strs("addresses", result).
|
||||
Int("ip_count", ipCount).
|
||||
Int("hostname_count", len(result)-ipCount)
|
||||
|
||||
if ipCount == 0 {
|
||||
logger.Msg("WARNING: No IP addresses discovered via 'ip addr' fallback - only hostnames available. Temperature monitoring may require manual allowed_nodes configuration.")
|
||||
} else {
|
||||
logger.Msg("Discovered local host addresses via 'ip addr' fallback (systemd unit needs AF_NETLINK update)")
|
||||
}
|
||||
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// isProxmoxHost checks if we're running on a Proxmox host
|
||||
func isProxmoxHost() bool {
|
||||
// Check for pvecm command
|
||||
|
|
|
|||
|
|
@ -324,7 +324,7 @@ func newNodeValidator(cfg *Config, metrics *ProxyMetrics) (*nodeValidator, error
|
|||
if v.strict {
|
||||
log.Warn().Msg("strict_node_validation enabled but no allowlist or cluster context is available")
|
||||
} else {
|
||||
log.Info().Msg("Node validator running in permissive mode (no allowlist or cluster context)")
|
||||
log.Warn().Msg("SECURITY: Node validator running in permissive mode (no allowlist or cluster context) - all nodes allowed. Configure allowed_nodes to restrict access.")
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -360,10 +360,10 @@ func (v *nodeValidator) Validate(ctx context.Context, node string) error {
|
|||
// Cluster query failed (e.g., IPC permission denied, running in LXC)
|
||||
// Fall through to permissive mode rather than blocking all requests
|
||||
v.recordFailure(validationReasonClusterFailed)
|
||||
log.Debug().
|
||||
log.Warn().
|
||||
Err(err).
|
||||
Str("node", node).
|
||||
Msg("Cluster validation unavailable, allowing request (consider configuring allowed_nodes for security)")
|
||||
Msg("SECURITY: Cluster validation unavailable - allowing all nodes. Configure allowed_nodes in config to restrict access.")
|
||||
// Fall through to permissive mode below
|
||||
} else if !allowed {
|
||||
return v.deny(node, validationReasonNotClusterMember)
|
||||
|
|
|
|||
Loading…
Reference in a new issue