Skip to content

fix(security): C6 — extend SSRF block to RFC-1918 + IMDS ranges - #107

Closed
HongmingWang-Rabbit wants to merge 1 commit into
mainfrom
fix/c6-rfc1918-ssrf
Closed

fix(security): C6 — extend SSRF block to RFC-1918 + IMDS ranges#107
HongmingWang-Rabbit wants to merge 1 commit into
mainfrom
fix/c6-rfc1918-ssrf

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

  • Extends validateAgentURL (introduced in PR fix(security): C6 — block loopback IP literals in /registry/register #94) to block all private IP ranges, not just link-local:
    • 10.0.0.0/8 — RFC-1918 class A
    • 172.16.0.0/12 — RFC-1918 class B (includes Docker bridge 172.17–31.x.x)
    • 192.168.0.0/16 — RFC-1918 class C
    • ::1/128 — IPv6 loopback
    • fc00::/7 — IPv6 unique-local (covers fd00::/8 sub-range)
  • 169.254.0.0/16 (link-local / AWS IMDS) was already blocked; preserved.
  • CIDRs pre-parsed into a package-level ssrfBlockedNets slice — no per-request ParseCIDR cost.
  • Hostname-based URLs are resolved via DNS before the check; DNS failures are fail-open (don't block legitimate registrations).

What changed

File Change
platform/internal/handlers/registry.go Added ssrfBlockedNets var; rewrote validateAgentURL to iterate all blocked ranges + DNS resolve
platform/internal/handlers/registry_test.go Flipped 3 incorrect "RFC-1918 is valid" cases; added 15 new boundary/IPv6 sub-cases
platform/internal/handlers/handlers_test.go Changed http://localhost: fixture URLs to http://127.0.0.1: (DNS-safe for this environment)

Relation to PR #94

Test plan

  • CGO_ENABLED=0 go test ./... — all packages pass
  • 24 sub-cases in TestValidateAgentURL all pass (3 RFC-1918 ranges × 3 boundary IPs each + 3 IPv6 + existing link-local + scheme/input tests)
  • All pre-existing registry/handler tests pass (no regressions)

🤖 Generated with Claude Code

validateAgentURL previously blocked only 169.254.0.0/16 (link-local /
cloud metadata). This patch extends the blocked set to cover all private
and loopback ranges that can be used as SSRF vectors:

  10.0.0.0/8      RFC-1918 class A
  172.16.0.0/12   RFC-1918 class B (includes Docker bridge 172.17–31.x.x)
  192.168.0.0/16  RFC-1918 class C
  ::1/128         IPv6 loopback
  fc00::/7        IPv6 unique-local (fd00::/8 sub-range included)

CIDRs are pre-parsed into a package-level slice so net.ParseCIDR is not
called on every request. Hostname-based URLs are resolved via DNS before
the check; DNS failures are fail-open to avoid blocking legitimate
registrations on transient outages.

Test changes:
- Three existing test cases that incorrectly asserted RFC-1918 addresses
  were valid (wantErr: false) are corrected to wantErr: true.
- 15 new sub-cases cover range boundaries and all three new IPv6 ranges.
- Test fixtures that used http://localhost: (which DNS-resolves to ::1
  in this environment) are updated to http://127.0.0.1: so they remain
  valid until the sibling PR fix/c6-loopback-ssrf merges.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor Author

⚠️ Needs update before merge — PR #94 already landed.

This branch was created before PR #94 merged. Three issues that must be fixed:

  1. Missing 127.0.0.0/8 in ssrfBlockedNets — PR fix(security): C6 — block loopback IP literals in /registry/register #94 added loopback blocking. This PR's new function omits it, causing a regression. Add "127.0.0.0/8", // IPv4 loopback to the cidrs slice.

  2. Test fixtures revert to http://127.0.0.1: — Main now uses http://localhost: in the handler test fixtures (changed by PR fix(security): C6 — block loopback IP literals in /registry/register #94). The diff shows this PR switching them back to 127.0.0.1, which will fail once loopback is in the blocked list.

  3. Loopback test cases removed — Main now has wantErr: true for 127.0.0.x in TestValidateAgentURL. Those should be preserved.

Suggested path:

git rebase origin/main
# Resolve conflicts by keeping main's localhost fixtures
# Add "127.0.0.0/8" to ssrfBlockedNets
# Update wantErr for 127.0.0.1 test case to true

The DNS resolution + IPv6 + pre-parsed CIDR improvements are good and worth landing — they just need to build on top of the current main state.

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor Author

Superseded by #119 which takes the targeted approach: adds only the three IPv6 CIDRs (fe80::/10, ::1/128, fc00::/7) to the existing blockedRanges without removing the loopback and RFC-1918 IPv4 blocks that this PR's ssrfBlockedNets var omitted.

@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the fix/c6-rfc1918-ssrf branch April 16, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant