From 74b822eef9e2d3f5d70d0e1fe1c5e2d22cc46d7c Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 24 Jul 2026 22:34:54 +0100 Subject: [PATCH] Stop discovery suppression failing open on configured hosts Discovery fingerprints hosts on the scanned subnet, and its PBS fallback probes api2/json/status and api2/json/config/datastore with no credentials, which PBS records as authentication failures. Configured connections are meant to be skipped for exactly that reason, so every gap in the skip writes noise into an operator's own server log on every scan (#1616, split from #1606). Two paths failed open. Hostname resolution kept only the first IPv4 and broke out of the loop, leaving a multi-homed host partly suppressed while discovery reaches whichever address it reaches. Resolution failure returned nothing at all with no record that suppression had been skipped, so the host stayed scannable and looked identical to one that was never configured. Resolution now covers every IPv4 a configured host answers with, and both a lookup failure and an IPv4-less answer are logged as the suppression gaps they are. An unresolvable host still yields no entry, which is unavoidable, but it no longer does so silently and cannot affect the other configured hosts. This does not by itself explain the reporter's case in #1616. Their sanitized export rewrites the PBS host to a generated placeholder, so it cannot show whether the connection was configured by address or by name. Verified: internal/monitoring, gofmt, canonical completion guard, status, control-plane, registry and contract audits, active-target automated and hybrid readiness proofs, and all thirteen release-control unit test modules. --- .../v6/internal/subsystems/monitoring.md | 12 +++ .../monitoring/monitor_discovery_helpers.go | 44 ++++++++--- .../monitoring/monitor_host_agents_test.go | 78 +++++++++++++++++++ 3 files changed, 122 insertions(+), 12 deletions(-) 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) + } +}