diff --git a/cmd/review-bot/validateurl.go b/cmd/review-bot/validateurl.go index b235aa7..20d94b9 100644 --- a/cmd/review-bot/validateurl.go +++ b/cmd/review-bot/validateurl.go @@ -9,7 +9,7 @@ import ( "strings" "time" - "gitea.weiker.me/rodin/review-bot/gitea" + "gitea.weiker.me/rodin/review-bot/internal/netutil" ) // runValidateURL implements the `review-bot validate-url ` subcommand. @@ -114,7 +114,7 @@ func validateURL(rawURL string) error { } for _, a := range addrs { - if gitea.IsBlockedIP(a.IP) { + if netutil.IsBlockedIP(a.IP) { return &validateError{ code: 1, message: fmt.Sprintf("blocked: %q resolves to private/reserved IP %s", host, a.IP), diff --git a/gitea/ipcheck.go b/gitea/ipcheck.go index 186b2ef..d8a505e 100644 --- a/gitea/ipcheck.go +++ b/gitea/ipcheck.go @@ -1,91 +1,22 @@ // Package gitea provides a client for the Gitea API. -// ipcheck.go implements IP-level SSRF protection by checking resolved addresses -// against known blocked CIDR ranges (RFC1918, loopback, link-local, etc.). +// ipcheck.go re-exports the IsBlockedIP function from internal/netutil for use +// by this package's safe dialer (client.go) and for backward compatibility with +// any callers that previously imported it from here. +// +// The implementation has moved to internal/netutil so it can be shared with the +// validate-url subcommand (cmd/review-bot/validateurl.go) without creating a +// dependency from VCS-generic code on the Gitea-specific package. package gitea import ( - "fmt" "net" + + "gitea.weiker.me/rodin/review-bot/internal/netutil" ) -// blockedCIDRStrings is the canonical list of CIDR strings that should never -// be contacted by review-bot. See IsBlockedIP for the full list of covered -// address families. -// -// These are hard-coded literals: any parse failure is a programming error. -// Validity is verified by TestBlockedCIDRsValid in ipcheck_test.go. -var blockedCIDRStrings = []string{ - // IPv4 loopback - "127.0.0.0/8", - // IPv4 unspecified / "this network" - "0.0.0.0/8", - // RFC1918 private ranges - "10.0.0.0/8", - "172.16.0.0/12", - "192.168.0.0/16", - // IPv4 link-local (APIPA, also used by AWS instance metadata 169.254.169.254) - "169.254.0.0/16", - // IPv4 shared address space (RFC6598, carrier-grade NAT) - "100.64.0.0/10", - // IPv4 multicast - "224.0.0.0/4", - // IPv4 reserved / broadcast - "240.0.0.0/4", - // IPv6 loopback - "::1/128", - // IPv6 unspecified - "::/128", - // IPv6 link-local - "fe80::/10", - // IPv6 unique local (ULA) — RFC4193 - "fc00::/7", - // IPv6 multicast - "ff00::/8", -} - -// blockedCIDRs is the parsed form of blockedCIDRStrings. -// Any entry that fails to parse is recorded in blockedCIDRParseErrors instead -// of panicking; tests verify this slice is always empty via TestBlockedCIDRsValid. -var ( - blockedCIDRs []*net.IPNet - blockedCIDRParseErrors []string -) - -func init() { - blockedCIDRs = make([]*net.IPNet, 0, len(blockedCIDRStrings)) - for _, r := range blockedCIDRStrings { - _, cidr, err := net.ParseCIDR(r) - if err != nil { - // Record the error rather than panicking; TestBlockedCIDRsValid - // will catch this during tests, and the CI build will fail. - blockedCIDRParseErrors = append(blockedCIDRParseErrors, - fmt.Sprintf("ipcheck: invalid built-in CIDR %q: %v", r, err)) - continue - } - blockedCIDRs = append(blockedCIDRs, cidr) - } -} - // IsBlockedIP reports whether ip is in a blocked address range. -// It is exported for use by the validate-url subcommand and tests outside -// this package. -// -// IPv6-mapped IPv4 addresses (e.g. ::ffff:192.168.1.1) are normalized to their -// IPv4 form before checking so that IPv4 CIDRs catch them. -// -// Based on: -// - RFC1918 private ranges -// - RFC5735 / RFC4193 special-use IPv4/IPv6 ranges -// - RFC4291 IPv6 link-local / loopback +// It delegates to internal/netutil.IsBlockedIP; see that function for the full +// list of blocked ranges and IPv6-mapped IPv4 normalization behavior. func IsBlockedIP(ip net.IP) bool { - // Normalize IPv6-mapped IPv4 addresses (::ffff:x.x.x.x) to plain IPv4. - if v4 := ip.To4(); v4 != nil { - ip = v4 - } - for _, cidr := range blockedCIDRs { - if cidr.Contains(ip) { - return true - } - } - return false + return netutil.IsBlockedIP(ip) } diff --git a/gitea/ipcheck_test.go b/gitea/ipcheck_test.go index b9903e6..29308c4 100644 --- a/gitea/ipcheck_test.go +++ b/gitea/ipcheck_test.go @@ -3,142 +3,35 @@ package gitea import ( "net" "testing" + + "gitea.weiker.me/rodin/review-bot/internal/netutil" ) -func TestIsBlockedIP(t *testing.T) { - blocked := []struct { - name string - ip string +// TestIsBlockedIPForwarding verifies that gitea.IsBlockedIP correctly forwards +// to internal/netutil.IsBlockedIP. Full coverage of the blocking logic lives in +// internal/netutil/ipcheck_test.go. +func TestIsBlockedIPForwarding(t *testing.T) { + cases := []struct { + ip string + blocked bool }{ - // IPv4 loopback - {"loopback 127.0.0.1", "127.0.0.1"}, - {"loopback 127.0.0.2", "127.0.0.2"}, - {"loopback 127.255.255.255", "127.255.255.255"}, - // IPv4 unspecified - {"unspecified 0.0.0.0", "0.0.0.0"}, - {"unspecified 0.1.2.3", "0.1.2.3"}, - // RFC1918 - {"RFC1918 10.0.0.1", "10.0.0.1"}, - {"RFC1918 10.255.255.255", "10.255.255.255"}, - {"RFC1918 172.16.0.1", "172.16.0.1"}, - {"RFC1918 172.31.255.255", "172.31.255.255"}, - {"RFC1918 192.168.0.1", "192.168.0.1"}, - {"RFC1918 192.168.255.255", "192.168.255.255"}, - // Link-local (APIPA / AWS metadata) - {"link-local 169.254.0.1", "169.254.0.1"}, - {"link-local 169.254.169.254", "169.254.169.254"}, - // Shared address space (carrier-grade NAT) - {"CGN 100.64.0.1", "100.64.0.1"}, - {"CGN 100.127.255.255", "100.127.255.255"}, - // Multicast - {"multicast 224.0.0.1", "224.0.0.1"}, - {"multicast 239.255.255.255", "239.255.255.255"}, - // Reserved - {"reserved 240.0.0.1", "240.0.0.1"}, - {"broadcast 255.255.255.255", "255.255.255.255"}, - // IPv6 loopback - {"IPv6 loopback ::1", "::1"}, - // IPv6 unspecified - {"IPv6 unspecified ::", "::"}, - // IPv6 link-local - {"IPv6 link-local fe80::1", "fe80::1"}, - {"IPv6 link-local fe80::dead:beef", "fe80::dead:beef"}, - // IPv6 ULA - {"IPv6 ULA fc00::1", "fc00::1"}, - {"IPv6 ULA fd00::1", "fd00::1"}, - // IPv6 multicast - {"IPv6 multicast ff02::1", "ff02::1"}, + {"127.0.0.1", true}, // loopback — must be blocked + {"192.168.1.1", true}, // RFC1918 — must be blocked + {"8.8.8.8", false}, // public — must not be blocked + {"2001:4860:4860::8888", false}, // public IPv6 — must not be blocked } - - for _, tc := range blocked { - t.Run(tc.name, func(t *testing.T) { - ip := net.ParseIP(tc.ip) - if ip == nil { - t.Fatalf("failed to parse IP %q", tc.ip) - } - if !IsBlockedIP(ip) { - t.Errorf("IsBlockedIP(%q) = false, want true", tc.ip) - } - }) - } - - allowed := []struct { - name string - ip string - }{ - {"public 8.8.8.8", "8.8.8.8"}, - {"public 1.1.1.1", "1.1.1.1"}, - {"public 198.51.100.1", "198.51.100.1"}, // RFC5737 TEST-NET-2 — a documentation-only range; - // not assigned to any real host, but intentionally left unblocked here because - // it has no special routing treatment (unlike RFC1918/loopback/link-local) and - // blocking it would require tracking every RFC5737 range without meaningful - // security benefit (no server should ever listen on a TEST-NET address). - {"public 151.101.1.1", "151.101.1.1"}, // Fastly - {"public IPv6 2001:4860:4860::8888", "2001:4860:4860::8888"}, // Google DNS - {"public IPv6 2606:4700:4700::1111", "2606:4700:4700::1111"}, // Cloudflare DNS - } - - for _, tc := range allowed { - t.Run(tc.name, func(t *testing.T) { - ip := net.ParseIP(tc.ip) - if ip == nil { - t.Fatalf("failed to parse IP %q", tc.ip) - } - if IsBlockedIP(ip) { - t.Errorf("IsBlockedIP(%q) = true, want false", tc.ip) - } - }) - } -} - -func TestIsBlockedIPv6MappedIPv4(t *testing.T) { - // ::ffff:192.168.1.1 is an IPv6-mapped IPv4 address — should be blocked as RFC1918. - // Construct it manually as a 16-byte IP. - mapped := net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 192, 168, 1, 1} - if !IsBlockedIP(mapped) { - t.Errorf("IsBlockedIP(::ffff:192.168.1.1) = false, want true (IPv6-mapped IPv4 must be normalized)") - } - - // ::ffff:8.8.8.8 — IPv6-mapped public IP — should be allowed. - mappedPublic := net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 8, 8, 8, 8} - if IsBlockedIP(mappedPublic) { - t.Errorf("IsBlockedIP(::ffff:8.8.8.8) = true, want false") - } -} - -func TestIsBlockedIPEdgeCases(t *testing.T) { - // The boundary between RFC1918 and public ranges. - // 172.15.255.255 is NOT private (just below 172.16.0.0/12). - notPrivate := net.ParseIP("172.15.255.255") - if IsBlockedIP(notPrivate) { - t.Errorf("IsBlockedIP(172.15.255.255) = true, want false (outside 172.16.0.0/12)") - } - // 172.32.0.0 is NOT private (just above 172.31.255.255). - notPrivate2 := net.ParseIP("172.32.0.0") - if IsBlockedIP(notPrivate2) { - t.Errorf("IsBlockedIP(172.32.0.0) = true, want false (outside 172.16.0.0/12)") - } - // CGN: 100.63.255.255 is NOT in 100.64.0.0/10. - notCGN := net.ParseIP("100.63.255.255") - if IsBlockedIP(notCGN) { - t.Errorf("IsBlockedIP(100.63.255.255) = true, want false (outside 100.64.0.0/10)") - } - // CGN: 100.128.0.0 is NOT in 100.64.0.0/10. - notCGN2 := net.ParseIP("100.128.0.0") - if IsBlockedIP(notCGN2) { - t.Errorf("IsBlockedIP(100.128.0.0) = true, want false (outside 100.64.0.0/10)") - } -} - -// TestBlockedCIDRsValid verifies that all entries in blockedCIDRStrings parse -// successfully. This catches programming errors in the CIDR list without -// requiring a startup panic. The init() function records parse failures in -// blockedCIDRParseErrors rather than panicking; this test makes those failures -// visible as test failures during CI. -func TestBlockedCIDRsValid(t *testing.T) { - if len(blockedCIDRParseErrors) > 0 { - for _, msg := range blockedCIDRParseErrors { - t.Errorf("CIDR parse error: %s", msg) + for _, tc := range cases { + ip := net.ParseIP(tc.ip) + if ip == nil { + t.Fatalf("failed to parse IP %q", tc.ip) + } + got := IsBlockedIP(ip) + want := netutil.IsBlockedIP(ip) + if got != want { + t.Errorf("gitea.IsBlockedIP(%q) = %v, netutil.IsBlockedIP = %v: forwarding mismatch", tc.ip, got, want) + } + if got != tc.blocked { + t.Errorf("gitea.IsBlockedIP(%q) = %v, want %v", tc.ip, got, tc.blocked) } } } diff --git a/internal/netutil/ipcheck.go b/internal/netutil/ipcheck.go new file mode 100644 index 0000000..7940dfe --- /dev/null +++ b/internal/netutil/ipcheck.go @@ -0,0 +1,97 @@ +// Package netutil provides shared network utilities for review-bot. +// ipcheck.go implements IP-level SSRF protection by checking resolved addresses +// against known blocked CIDR ranges (RFC1918, loopback, link-local, etc.). +package netutil + +import ( + "fmt" + "net" +) + +// blockedCIDRStrings is the canonical list of CIDR strings that should never +// be contacted by review-bot. See IsBlockedIP for the full list of covered +// address families. +// +// These are hard-coded literals: any parse failure is a programming error. +// Validity is verified by TestBlockedCIDRsValid in ipcheck_test.go. +var blockedCIDRStrings = []string{ + // IPv4 loopback + "127.0.0.0/8", + // IPv4 unspecified / "this network" + "0.0.0.0/8", + // RFC1918 private ranges + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + // IPv4 link-local (APIPA, also used by AWS instance metadata 169.254.169.254) + "169.254.0.0/16", + // IPv4 shared address space (RFC6598, carrier-grade NAT) + "100.64.0.0/10", + // IPv4 multicast + "224.0.0.0/4", + // IPv4 reserved / broadcast + "240.0.0.0/4", + // IPv6 loopback + "::1/128", + // IPv6 unspecified + "::/128", + // IPv6 link-local + "fe80::/10", + // IPv6 unique local (ULA) — RFC4193 + "fc00::/7", + // IPv6 multicast + "ff00::/8", +} + +// blockedCIDRs is the parsed form of blockedCIDRStrings. +// Any entry that fails to parse is recorded in blockedCIDRParseErrors instead +// of panicking; tests verify this slice is always empty via TestBlockedCIDRsValid. +var ( + blockedCIDRs []*net.IPNet + blockedCIDRParseErrors []string +) + +func init() { + blockedCIDRs = make([]*net.IPNet, 0, len(blockedCIDRStrings)) + for _, r := range blockedCIDRStrings { + _, cidr, err := net.ParseCIDR(r) + if err != nil { + // Record the error rather than panicking; TestBlockedCIDRsValid + // will catch this during tests, and the CI build will fail. + blockedCIDRParseErrors = append(blockedCIDRParseErrors, + fmt.Sprintf("ipcheck: invalid built-in CIDR %q: %v", r, err)) + continue + } + blockedCIDRs = append(blockedCIDRs, cidr) + } +} + +// BlockedCIDRParseErrors returns any errors encountered parsing the built-in +// CIDR list. In correct code this will always be empty; tests assert it is. +func BlockedCIDRParseErrors() []string { + return blockedCIDRParseErrors +} + +// IsBlockedIP reports whether ip is in a blocked address range. +// It is exported for use by the gitea package's safe dialer, the validate-url +// subcommand, and tests outside this package. +// +// IPv6-mapped IPv4 addresses (e.g. ::ffff:192.168.1.1) are normalized to their +// IPv4 form before checking so that IPv4 CIDRs catch them. +// +// Based on: +// - RFC1918 private ranges +// - RFC5735 / RFC4193 special-use IPv4/IPv6 ranges +// - RFC4291 IPv6 link-local / loopback +func IsBlockedIP(ip net.IP) bool { + // Normalize IPv6-mapped IPv4 addresses (::ffff:x.x.x.x) to plain IPv4. + if v4 := ip.To4(); v4 != nil { + ip = v4 + } + for _, cidr := range blockedCIDRs { + if cidr.Contains(ip) { + return true + } + } + return false +} diff --git a/internal/netutil/ipcheck_test.go b/internal/netutil/ipcheck_test.go new file mode 100644 index 0000000..b4a62d0 --- /dev/null +++ b/internal/netutil/ipcheck_test.go @@ -0,0 +1,142 @@ +package netutil + +import ( + "net" + "testing" +) + +func TestIsBlockedIP(t *testing.T) { + blocked := []struct { + name string + ip string + }{ + // IPv4 loopback + {"loopback 127.0.0.1", "127.0.0.1"}, + {"loopback 127.0.0.2", "127.0.0.2"}, + {"loopback 127.255.255.255", "127.255.255.255"}, + // IPv4 unspecified + {"unspecified 0.0.0.0", "0.0.0.0"}, + {"unspecified 0.1.2.3", "0.1.2.3"}, + // RFC1918 + {"RFC1918 10.0.0.1", "10.0.0.1"}, + {"RFC1918 10.255.255.255", "10.255.255.255"}, + {"RFC1918 172.16.0.1", "172.16.0.1"}, + {"RFC1918 172.31.255.255", "172.31.255.255"}, + {"RFC1918 192.168.0.1", "192.168.0.1"}, + {"RFC1918 192.168.255.255", "192.168.255.255"}, + // Link-local (APIPA / AWS metadata) + {"link-local 169.254.0.1", "169.254.0.1"}, + {"link-local 169.254.169.254", "169.254.169.254"}, + // Shared address space (carrier-grade NAT) + {"CGN 100.64.0.1", "100.64.0.1"}, + {"CGN 100.127.255.255", "100.127.255.255"}, + // Multicast + {"multicast 224.0.0.1", "224.0.0.1"}, + {"multicast 239.255.255.255", "239.255.255.255"}, + // Reserved + {"reserved 240.0.0.1", "240.0.0.1"}, + {"broadcast 255.255.255.255", "255.255.255.255"}, + // IPv6 loopback + {"IPv6 loopback ::1", "::1"}, + // IPv6 unspecified + {"IPv6 unspecified ::", "::"}, + // IPv6 link-local + {"IPv6 link-local fe80::1", "fe80::1"}, + {"IPv6 link-local fe80::dead:beef", "fe80::dead:beef"}, + // IPv6 ULA + {"IPv6 ULA fc00::1", "fc00::1"}, + {"IPv6 ULA fd00::1", "fd00::1"}, + // IPv6 multicast + {"IPv6 multicast ff02::1", "ff02::1"}, + } + + for _, tc := range blocked { + t.Run(tc.name, func(t *testing.T) { + ip := net.ParseIP(tc.ip) + if ip == nil { + t.Fatalf("failed to parse IP %q", tc.ip) + } + if !IsBlockedIP(ip) { + t.Errorf("IsBlockedIP(%q) = false, want true", tc.ip) + } + }) + } + + allowed := []struct { + name string + ip string + }{ + {"public 8.8.8.8", "8.8.8.8"}, + {"public 1.1.1.1", "1.1.1.1"}, + {"public 198.51.100.1", "198.51.100.1"}, // RFC5737 TEST-NET-2 — a documentation-only range; + // not assigned to any real host, but intentionally left unblocked here because + // it has no special routing treatment (unlike RFC1918/loopback/link-local) and + // blocking it would require tracking every RFC5737 range without meaningful + // security benefit (no server should ever listen on a TEST-NET address). + {"public 151.101.1.1", "151.101.1.1"}, // Fastly + {"public IPv6 2001:4860:4860::8888", "2001:4860:4860::8888"}, // Google DNS + {"public IPv6 2606:4700:4700::1111", "2606:4700:4700::1111"}, // Cloudflare DNS + } + + for _, tc := range allowed { + t.Run(tc.name, func(t *testing.T) { + ip := net.ParseIP(tc.ip) + if ip == nil { + t.Fatalf("failed to parse IP %q", tc.ip) + } + if IsBlockedIP(ip) { + t.Errorf("IsBlockedIP(%q) = true, want false", tc.ip) + } + }) + } +} + +func TestIsBlockedIPv6MappedIPv4(t *testing.T) { + // ::ffff:192.168.1.1 is an IPv6-mapped IPv4 address — should be blocked as RFC1918. + // Construct it manually as a 16-byte IP. + mapped := net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 192, 168, 1, 1} + if !IsBlockedIP(mapped) { + t.Errorf("IsBlockedIP(::ffff:192.168.1.1) = false, want true (IPv6-mapped IPv4 must be normalized)") + } + + // ::ffff:8.8.8.8 — IPv6-mapped public IP — should be allowed. + mappedPublic := net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 8, 8, 8, 8} + if IsBlockedIP(mappedPublic) { + t.Errorf("IsBlockedIP(::ffff:8.8.8.8) = true, want false") + } +} + +func TestIsBlockedIPEdgeCases(t *testing.T) { + // The boundary between RFC1918 and public ranges. + // 172.15.255.255 is NOT private (just below 172.16.0.0/12). + notPrivate := net.ParseIP("172.15.255.255") + if IsBlockedIP(notPrivate) { + t.Errorf("IsBlockedIP(172.15.255.255) = true, want false (outside 172.16.0.0/12)") + } + // 172.32.0.0 is NOT private (just above 172.31.255.255). + notPrivate2 := net.ParseIP("172.32.0.0") + if IsBlockedIP(notPrivate2) { + t.Errorf("IsBlockedIP(172.32.0.0) = true, want false (outside 172.16.0.0/12)") + } + // CGN: 100.63.255.255 is NOT in 100.64.0.0/10. + notCGN := net.ParseIP("100.63.255.255") + if IsBlockedIP(notCGN) { + t.Errorf("IsBlockedIP(100.63.255.255) = true, want false (outside 100.64.0.0/10)") + } + // CGN: 100.128.0.0 is NOT in 100.64.0.0/10. + notCGN2 := net.ParseIP("100.128.0.0") + if IsBlockedIP(notCGN2) { + t.Errorf("IsBlockedIP(100.128.0.0) = true, want false (outside 100.64.0.0/10)") + } +} + +// TestBlockedCIDRsValid verifies that all entries in blockedCIDRStrings parse +// successfully. This catches programming errors in the CIDR list without +// requiring a startup panic. The init() function records parse failures in +// blockedCIDRParseErrors rather than panicking; this test makes those failures +// visible as test failures during CI. +func TestBlockedCIDRsValid(t *testing.T) { + for _, msg := range BlockedCIDRParseErrors() { + t.Errorf("CIDR parse error: %s", msg) + } +}