geoprobe: route LocationOffsets to result destination

[ben-dz]

Summary

• Route outbound LocationOffsets to the user's ResultDestination when set on GeolocationUser, instead of sending to the measurement target address
• Skip ICMP targets that have no result destination (nothing is listening at the measurement IP)
• Add DNS resolution with caching (5min TTL) for domain-based result destinations
• Validate delivery addresses against public-IP scope checks to prevent DNS rebinding/SSRF attacks
• Validate ResultDestination format at discovery time to surface errors early

Details

The ResultDestination field was added to GeolocationUser in #3480/#3501. This PR implements the geoprobe-agent side: reading the field during target discovery, carrying it through as DeliveryAddrs on TargetUpdate/ICMPTargetUpdate, and routing offsets to the specified address in sendCompositeOffsets.

Key design decisions:

• Delivery addresses are carried as a separate map[ProbeAddress]string rather than modifying ProbeAddress itself, to preserve Pinger map key semantics
• DNSCache.Resolve() validates resolved IPs against ValidateScope() to prevent DNS rebinding
• Change detection for delivery addresses triggers target updates even when the target list is unchanged

Depends on: #3468

Testing Verification

• Unit tests for outbound target with result destination → delivery override populated
• Unit tests for user without result destination → no delivery overrides
• Unit tests for ICMP target with result destination → delivery override in ICMP update
• Unit tests for mixed users (with/without result destination)
• Unit tests for domain-based result destination passthrough
• Unit tests for delivery address change detection → triggers update
• Unit tests for invalid result destination format → ignored with warning
• DNS cache: IP passthrough, domain resolution and caching, TTL expiry, concurrent access
• DNS cache security: private IP rejection, DNS rebinding to private IP rejected

Fixes #3469

[nikw9944]

Code Review: geoprobe: route LocationOffsets to result destination

RFC Reference: RFC-16: Geolocation Verification — specifically the SetResultDestination instruction and GeolocationUser.result_destination field.

Overall Assessment

This is a well-structured PR that implements the geoprobe-agent side of result destination routing. The design decisions are sound — carrying delivery addresses as a separate map rather than modifying ProbeAddress is the right call, and the DNS caching with SSRF protection is a good security measure. The test coverage is comprehensive. A few items worth discussing:

Issues

1. ICMP target skip logs at Warn level (main.go:963-965)

log.Warn("ICMP target has no result destination, skipping offset delivery",
    "target", addr.Host)

Once ICMP targets exist in production, this will fire on every measurement cycle for any ICMP target without a result destination. That's potentially every 5 seconds per target. This should likely be Debug level, since the absence of a result destination on an ICMP target is an expected (non-exceptional) configuration — it just means the user hasn't set up a listener yet. If you want visibility for the initial rollout, at minimum consider rate-limiting or logging only on first occurrence per target.

2. sendCompositeOffsets signature is getting long (main.go:934)

The function now takes 11 parameters. This is starting to strain readability — especially the three new trailing parameters (deliveryAddrs, icmpTargets, dnsCache). Consider grouping the new delivery-related params into a small struct (e.g., deliveryContext) in a follow-up. Not blocking.

3. Merged delivery map in runCycle() could shadow on key collision (main.go:903-916)

mergedDelivery := make(map[geoprobe.ProbeAddress]string, len(ml.deliveryAddrs)+len(ml.icmpDeliveryAddrs))
for k, v := range ml.deliveryAddrs {
    mergedDelivery[k] = v
}
for k, v := range ml.icmpDeliveryAddrs {
    mergedDelivery[k] = v
}

If a ProbeAddress somehow appears in both deliveryAddrs and icmpDeliveryAddrs, the ICMP entry wins silently. In practice this shouldn't happen because outbound targets use Validate() (requiring TWAMPPort) while ICMP targets use ValidateICMP() (TWAMPPort=0), so the ProbeAddress keys should be disjoint. But the code doesn't enforce or assert this. A comment noting this invariant would help future readers.

4. discover() returns a single flat deliveryAddrs map, then discoverAndSend() re-splits it (target_discovery.go:102-115)

The discover() function merges all delivery addresses into one map, then discoverAndSend() immediately splits them back into outboundDelivery and icmpDelivery by looking up against an outboundSet. This round-trip is slightly wasteful. Consider returning two maps from discover() directly — it would align the return values with how they're consumed and remove the splitting logic. Low priority.

Positives

• Security: DNS rebinding protection via ValidateScope() on resolved IPs is exactly right. Validating at resolution time (not just at discovery time) prevents time-of-check/time-of-use issues on the DNS→IP boundary.
• Validation at discovery time: Checking ResultDestination format (host:port) during discover() rather than on every send cycle is a good pattern — log once, don't spam.
• Change detection: The deliveryAddrsEqual() check triggers updates even when only the delivery address changes (not the target list). This is important because SetResultDestination bumps target_update_count on probes, and without this check the agent would see new targets but the delivery override wouldn't propagate.
• E2E test: The TestE2E_GeoprobeResultDestination test is thorough — it sets up two separate containers (measurement target vs result destination) and verifies the offset arrives at the result destination. This is the right way to test this feature.
• ICMP fix in existing test: The TestE2E_GeoprobeIcmpTargets fix adding setGeolocationUserResultDestination is a good catch — ICMP targets have no listener, so without a result destination the offsets have nowhere to go.
• Test coverage: Unit tests cover all the important permutations — outbound with/without result dest, ICMP with result dest, mixed users, domain names, change detection, and invalid format handling.