diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 0395042d4e..c5423a709d 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -236,6 +236,18 @@ also preserve the unreported sentinel rather than collapsing an absent facet onto the struct zero value, because `0` is a real reading meaning no endurance remains. +Discovery suppression for configured connections is a fail-closed obligation, +not a best-effort optimisation. Every configured PVE, PBS and PMG host is +resolved into the discovery IP blocklist so the scanner never fingerprints a +connection Pulse already holds credentials for. Those fingerprint probes are +unauthenticated by construction and land in the operator's own server logs as +authentication failures, so a host that escapes suppression is visible damage +on someone else's system rather than a missed optimisation. Resolution must +cover every IPv4 address a configured host answers with, not the first, because +the scanner reaches whichever address it reaches. A host that cannot be resolved +yields no entry and must say so in the log rather than passing silently, and it +must never prevent the remaining configured hosts from being suppressed. + ## Canonical Files 1. `internal/monitoring/monitor.go` diff --git a/internal/monitoring/monitor_discovery_helpers.go b/internal/monitoring/monitor_discovery_helpers.go index f3ea8fc9da..d2d81140ad 100644 --- a/internal/monitoring/monitor_discovery_helpers.go +++ b/internal/monitoring/monitor_discovery_helpers.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rs/zerolog/log" ) var lookupConfiguredHostIP = net.LookupIP @@ -66,19 +67,38 @@ func configuredHostIPsFromConfig(cfg *config.Config) []string { } return } - // Try to resolve hostname to IP - if addrs, err := lookupConfiguredHostIP(host); err == nil && len(addrs) > 0 { - for _, addr := range addrs { - // Prefer IPv4 - if v4 := addr.To4(); v4 != nil { - ipStr := v4.String() - if _, exists := seen[ipStr]; !exists { - seen[ipStr] = struct{}{} - ips = append(ips, ipStr) - } - break - } + // Resolve the hostname and take every IPv4 it answers with. Stopping at + // the first address left a multi-homed host only partly suppressed, and + // discovery probes whichever address it reaches, not the one we picked. + addrs, err := lookupConfiguredHostIP(host) + if err != nil || len(addrs) == 0 { + // Suppression exists so Pulse does not write unauthenticated probe + // traffic into an operator's own PBS/PVE logs. Failing to resolve + // silently leaves the host scannable with no signal that the skip + // was missed, so say so (#1616). + log.Warn(). + Str("host", host). + Err(err). + Msg("Could not resolve configured host to an address; discovery may probe it as if it were unknown") + return + } + resolved := 0 + for _, addr := range addrs { + v4 := addr.To4() + if v4 == nil { + continue } + resolved++ + ipStr := v4.String() + if _, exists := seen[ipStr]; !exists { + seen[ipStr] = struct{}{} + ips = append(ips, ipStr) + } + } + if resolved == 0 { + log.Warn(). + Str("host", host). + Msg("Configured host resolved to no IPv4 address; discovery scans IPv4 only and may probe it as if it were unknown") } } diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index efa6b6329b..5d681fe2b1 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -2,6 +2,7 @@ package monitoring import ( "encoding/json" + "errors" "fmt" "net" "strings" @@ -4217,3 +4218,80 @@ func TestPhysicalDiskFromReadStateView_AbsentViewReportsUnknownWearout(t *testin t.Fatalf("absent view wearout = %d, want %d", disk.Wearout, unifiedresources.WearoutUnreported) } } + +// Discovery suppression exists so Pulse does not probe hosts it already holds +// credentials for and write unauthenticated requests into the operator's own +// PBS/PVE logs (#1616). Taking only the first resolved address left a +// multi-homed host partly suppressed, and discovery reaches whichever address +// it reaches, not the one we happened to keep. +func TestConfiguredHostIPs_CoversEveryResolvedIPv4(t *testing.T) { + originalLookup := lookupConfiguredHostIP + lookupConfiguredHostIP = func(host string) ([]net.IP, error) { + return []net.IP{ + net.ParseIP("2001:db8::1"), + net.ParseIP("192.0.2.10"), + net.ParseIP("192.0.2.11"), + }, nil + } + defer func() { lookupConfiguredHostIP = originalLookup }() + + ips := configuredHostIPsFromConfig(&config.Config{ + PBSInstances: []config.PBSInstance{{Host: "pbs.example.test"}}, + }) + + for _, want := range []string{"192.0.2.10", "192.0.2.11"} { + found := false + for _, got := range ips { + if got == want { + found = true + break + } + } + if !found { + t.Fatalf("configured host IPs %v missing resolved address %s", ips, want) + } + } +} + +// A host configured by literal IP needs no resolution and must be suppressed +// whatever DNS is doing. +func TestConfiguredHostIPs_LiteralAddressNeedsNoResolution(t *testing.T) { + originalLookup := lookupConfiguredHostIP + lookupConfiguredHostIP = func(host string) ([]net.IP, error) { + t.Fatalf("literal address must not be resolved, got lookup for %q", host) + return nil, nil + } + defer func() { lookupConfiguredHostIP = originalLookup }() + + ips := configuredHostIPsFromConfig(&config.Config{ + PBSInstances: []config.PBSInstance{{Host: "https://192.0.2.20:8007"}}, + }) + + if len(ips) != 1 || ips[0] != "192.0.2.20" { + t.Fatalf("configured host IPs = %v, want [192.0.2.20]", ips) + } +} + +// A name that does not resolve yields no suppression entry. That is +// unavoidable, but it must not take the other configured hosts down with it. +func TestConfiguredHostIPs_UnresolvableHostDoesNotDropOthers(t *testing.T) { + originalLookup := lookupConfiguredHostIP + lookupConfiguredHostIP = func(host string) ([]net.IP, error) { + if host == "broken.example.test" { + return nil, errors.New("no such host") + } + return []net.IP{net.ParseIP("192.0.2.30")}, nil + } + defer func() { lookupConfiguredHostIP = originalLookup }() + + ips := configuredHostIPsFromConfig(&config.Config{ + PBSInstances: []config.PBSInstance{ + {Host: "broken.example.test"}, + {Host: "good.example.test"}, + }, + }) + + if len(ips) != 1 || ips[0] != "192.0.2.30" { + t.Fatalf("configured host IPs = %v, want only the resolvable host [192.0.2.30]", ips) + } +}