fix(egress): expose more ranges as restricted rather than reserved (#39560)

Introduce second list of addresses which are classified as dialable if
explicitly allowed when in Lax mode.
Restricted pool now includes: link-local, site local, private (including
ULA), CGNAT, discard, dummy, documentation and test addreses.
Reserved pool shrinks to: this network, wireserver embedding/translation
ranges and multicasts

Rationale for the choice is that while items in restricted pool can be
dangerous to allow they could be a legitimate target in some
deployments. Ranges left in reserved list are ranges which make no sense
to dial, are public (wireserver) or are 6to4 embedding which cannot be
reasonably verified to be safe. To unlock those a proxy should be used
instead

fixes: https://github.com/go-gitea/gitea/issues/39557

---------

Signed-off-by: TheFox0x7 <thefox0x7@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

custom/conf/app.example.ini

@@ -536,7 +536,7 @@ INTERNAL_TOKEN =
 ;CONTENT_SECURITY_POLICY_GENERAL =
 ;;
 ;; Egress mode toggles between strictness of outgoing requests:
-;; Lax requires addresses to be allowed only if they are in private ranges, it allows all public ones
+;; Lax requires non-public targets (private, loopback, link-local, CGNAT and special-use ranges) to be allowed, it allows all public ones
 ;; Strict requires an explicit allow of all addresses
 ; EGRESS_MODE = lax
 ;;
@@ -551,10 +551,12 @@ INTERNAL_TOKEN =
 ;; a bracketed set of ports and ranges, | separated: *.mydomain.com:[80|443|3000-3010]
 ;; all ports: *.mydomain.com:*
 ;; A portless entry covers all ports in Lax mode, only 80 and 443 in Strict mode
-;; Port specs apply only where the list is consulted: in Lax mode that is private, loopback and CGNAT
+;; Port specs apply only where the list is consulted: in Lax mode that is non-public targets alone,
-;; targets alone, public targets are allowed on every port whatever the list says. In Strict mode every
+;; public targets are allowed on every port whatever the list says. In Strict mode every
 ;; target is checked, so ports restrict public hosts too.
-;; Reserved addresses like link-local and cloud metadata are denied
+;; Non-public targets need an IP or built-in entry, a host name entry alone never covers them.
+;; Reserved addresses (the IPv4-embedding NAT64, Teredo and 6to4 ranges, this-network, multicast and
+;; broadcast) are denied whatever the list says. To reach them configure an HTTP proxy
 ;; This list is enforced on direct connections only. When an HTTP proxy is configured, restricting the proxied target is the proxy server's responsibility.
 ;ALLOWED_HOST_LIST =
 

modules/egress/policies.go

@@ -98,6 +98,7 @@ func NewWebhookPolicy() *policy.Policy {
 	}
 	p = policy.NewPolicy("webhook", policyMode(setting.Security.EgressMode),
 		policy.WithAllow(setting.Webhook.AllowedHostList, "security.ALLOWED_HOST_LIST"),
+		policy.WithLocalNeedsIPAllow(),
 		policy.WithProxy(selectProxy))
 
 	return p
@@ -106,6 +107,7 @@ func NewWebhookPolicy() *policy.Policy {
 func NewSecurityPolicy(usage string) *policy.Policy {
 	return policy.NewPolicy(usage, policyMode(setting.Security.EgressMode),
 		policy.WithAllow(setting.Security.AllowedHostList, "security.ALLOWED_HOST_LIST"),
+		policy.WithLocalNeedsIPAllow(),
 		policy.WithProxy(proxy.Proxy()))
 }
 

modules/egress/policies_test.go

@@ -4,10 +4,13 @@
 package egress
 
 import (
+	"net"
 	"net/http"
 	"net/url"
+	"strconv"
 	"testing"
 
+	"gitea.dev/modules/egress/policy"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/test"
 
@@ -70,6 +73,27 @@ func TestWebhookPolicyProxy(t *testing.T) {
 	}
 }
 
+func TestWebhookPolicyNeedsIPAllow(t *testing.T) {
+	defer test.MockVariableValue(&setting.Webhook.AllowedHostList, "localhost")()
+	defer test.MockVariableValue(&setting.Security.EgressMode, "lax")()
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = ln.Close() })
+	dial := func() error {
+		tcpAddr, ok := ln.Addr().(*net.TCPAddr)
+		require.True(t, ok)
+		target := net.JoinHostPort("localhost", strconv.Itoa(tcpAddr.Port))
+		conn, err := NewWebhookPolicy().NewDialContext()(t.Context(), "tcp", target)
+		if err == nil {
+			_ = conn.Close()
+		}
+		return err
+	}
+	assert.ErrorIs(t, dial(), policy.ErrDenied) // a host name entry doesn't cover the loopback address
+	setting.Webhook.AllowedHostList = "loopback"
+	assert.NoError(t, dial()) // an IP entry does
+}
+
 func TestSecurityPolicy(t *testing.T) {
 	defer test.MockVariableValue(&setting.Security.AllowedHostList, "avatars.example.com")()
 	defer test.MockVariableValue(&setting.Security.EgressMode, "lax")()

modules/egress/policy/matchlist.go

@@ -411,37 +411,43 @@ var cgnatRange = netip.MustParsePrefix("100.64.0.0/10") // RFC 6598
 // reservedRanges are never dialable, based on https://microsoft.github.io/AntiSSRF/ipaddressranges.html
 var reservedRanges = func() (ranges []netip.Prefix) {
 	for _, cidr := range []string{
-		"0.0.0.0/8",          // "this network"
+		"0.0.0.0/8",        // "this network"
-		"100.100.100.200/32", // Alibaba Cloud metadata
+		"168.63.129.16/32", // Azure WireServer
-		"168.63.129.16/32",   // Azure WireServer
+		"192.88.99.0/24",   // 6to4 relay anycast
-		"169.254.0.0/16",     // link-local, cloud metadata endpoints
+		"224.0.0.0/4",      // multicast
-		"192.0.0.0/24",       // IETF protocol assignments
+		"240.0.0.0/4",      // reserved, incl. limited broadcast
-		"192.0.2.0/24",       // TEST-NET-1
+		"::/96",            // IPv4-compatible, embeds IPv4
-		"192.31.196.0/24",    // AS112
+		"::ffff:0:0:0/96",  // IPv4-translated, embeds IPv4
-		"192.52.193.0/24",    // AMT
+		"64:ff9b::/96",     // wkp NAT64
-		"192.88.99.0/24",     // 6to4 relay anycast
+		"64:ff9b:1::/48",   // local-use NAT64
-		"192.175.48.0/24",    // AS112
+		"2001::/32",        // Teredo, embeds IPv4
-		"198.18.0.0/15",      // benchmarking
+		"2002::/16",        // 6to4, embeds IPv4
-		"198.51.100.0/24",    // TEST-NET-2
+		"ff00::/8",         // multicast
-		"203.0.113.0/24",     // TEST-NET-3
+	} {
-		"224.0.0.0/4",        // multicast
+		ranges = append(ranges, netip.MustParsePrefix(cidr))
-		"240.0.0.0/4",        // reserved, incl. limited broadcast
+	}
-		"::/96",              // IPv4-compatible, embeds IPv4
+	return ranges
-		"::ffff:0:0:0/96",    // IPv4-translated, embeds IPv4
+}()
-		"64:ff9b::/96",       // wkp NAT64
+
-		"64:ff9b:1::/48",     // local-use NAT64
+// restrictedRanges are dialable if they have been explicitly allowed.
-		"100::/64",           // discard-only
+var restrictedRanges = func() (ranges []netip.Prefix) {
-		"100:0:0:1::/64",     // dummy
+	for _, cidr := range []string{
-		"2001::/23",          // IETF protocol assignments, incl. Teredo and ORCHID
+		"192.0.0.0/24",      // IETF protocol assignments
-		"2001:db8::/32",      // documentation
+		"192.0.2.0/24",      // TEST-NET-1
-		"2002::/16",          // 6to4, embeds IPv4
+		"192.31.196.0/24",   // AS112
-		"2620:4f:8000::/48",  // AS112
+		"192.52.193.0/24",   // AMT
-		"3fff::/20",          // documentation
+		"192.175.48.0/24",   // AS112
-		"5f00::/16",          // SRv6 SIDs
+		"198.18.0.0/15",     // benchmarking
-		"fd00:ec2::254/128",  // AWS IMDS
+		"198.51.100.0/24",   // TEST-NET-2
-		"fe80::/10",          // link-local
+		"203.0.113.0/24",    // TEST-NET-3
-		"fec0::/10",          // site-local
+		"100::/64",          // discard-only
-		"ff00::/8",           // multicast
+		"100:0:0:1::/64",    // dummy
+		"2001::/23",         // IETF protocol assignments
+		"2001:db8::/32",     // documentation
+		"2620:4f:8000::/48", // AS112
+		"3fff::/20",         // documentation
+		"5f00::/16",         // SRv6 SIDs
+		"fec0::/10",         // site-local
 	} {
 		ranges = append(ranges, netip.MustParsePrefix(cidr))
 	}
@@ -451,10 +457,14 @@ var reservedRanges = func() (ranges []netip.Prefix) {
 // classifyAddr reports the class of a canonical address.
 func classifyAddr(ip netip.Addr) addrClass {
 	switch {
-	case ip.Zone() != "" || !ip.IsLoopback() && slices.ContainsFunc(reservedRanges, func(p netip.Prefix) bool { return p.Contains(ip) }):
+	case ip.Zone() != "" || !ip.IsLoopback() && inRange(reservedRanges, ip):
 		return classReserved
-	case ip.IsPrivate() || ip.IsLoopback() || cgnatRange.Contains(ip):
+	case ip.IsPrivate() || ip.IsLoopback() || ip.IsLinkLocalUnicast() || cgnatRange.Contains(ip) || inRange(restrictedRanges, ip):
 		return classRestricted
 	}
 	return classPublic
 }
+
+func inRange(p []netip.Prefix, ip netip.Addr) bool {
+	return slices.ContainsFunc(p, func(p netip.Prefix) bool { return p.Contains(ip) })
+}

modules/egress/policy/policy.go

@@ -56,7 +56,7 @@ func WithBlock(hostList, key string) Option {
 	}
 }
 
-// WithLocalNeedsIPAllow requires private, loopback and CGNAT targets to match an IP allow entry (CIDR or named range), a host name match is not enough.
+// WithLocalNeedsIPAllow requires non-public targets (private, loopback, link-local, CGNAT and special-use ranges) to match an IP allow entry (CIDR or named range), a host name match is not enough.
 func WithLocalNeedsIPAllow() Option {
 	return func(p *Policy) {
 		p.localNeedsIPAllow = true
@@ -140,9 +140,9 @@ func (p *Policy) allowCheck(host string, ip netip.AddrPort, class addrClass) err
 		return p.notAllowedError(denyTarget(host, ip))
 	}
 	if !hostnameOk {
-		return fmt.Errorf("%s needs an explicit IP allow entry (private/loopback/CGNAT)", denyTarget(host, ip))
+		return fmt.Errorf("%s needs an explicit IP allow entry (non-public address)", denyTarget(host, ip))
 	}
-	return fmt.Errorf("%s needs an explicit allow entry (private/loopback/CGNAT)", denyTarget(host, ip))
+	return fmt.Errorf("%s needs an explicit allow entry (non-public address)", denyTarget(host, ip))
 }
 
 func (p *Policy) blockReason(host string, ip netip.AddrPort) error {

modules/egress/policy/policy_test.go

@@ -33,8 +33,16 @@ func TestCheckAddr(t *testing.T) {
 		{name: "allow host", allow: "example.com", host: "example.com", ip: "8.8.8.8", want: true},
 		{name: "allow cidr", allow: "10.0.0.0/8", ip: "10.0.0.5", want: true},
 		{name: "block overrides allow", allow: "10.0.0.0/8", block: "10.0.0.5/32", ip: "10.0.0.5"},
-		{name: "reserved denied by cidr", allow: "169.254.0.0/16", ip: "169.254.169.254"},
+		{name: "non cloud link-local is default denied", ip: "::ffff:169.254.1.2"},
-		{name: "reserved denied ipv4-mapped", allow: "169.254.0.0/16", ip: "::ffff:169.254.169.254"},
+		{name: "link-local allowed by cidr", allow: "169.254.0.0/16", ip: "169.254.169.254", want: true},
+		{name: "link-local allowed ipv4-mapped", allow: "169.254.0.0/16", ip: "::ffff:169.254.169.254", want: true},
+		{name: "restricted range allowed by cidr", allow: "192.0.2.0/24", ip: "192.0.2.1", want: true},
+		{name: "ula metadata allowed by private", allow: "private", ip: "fd00:ec2::254", want: true},
+		{name: "reserved denied despite allow", allow: "168.63.129.16/32", ip: "168.63.129.16"},
+		{name: "reserved denied ipv4-mapped", allow: "168.63.129.16/32", ip: "::ffff:168.63.129.16"},
+		{name: "nat64 reserved denied despite allow", allow: "64:ff9b::/96", ip: "64:ff9b::a9fe:a9fe"},
+		{name: "teredo reserved denied despite allow", allow: "2001::/23", ip: "2001::1"},
+		{name: "protocol assignment allowed by cidr", allow: "2001::/23", ip: "2001:3::1", want: true},
 		{name: "local gate ignores host", allow: "example.com", host: "example.com", ip: "10.0.0.5", localNeedsIPAllow: true},
 		{name: "local gate accepts builtin", allow: "private", ip: "100.64.0.1", localNeedsIPAllow: true, want: true},
 		{name: "local gate accepts cidr", allow: "10.0.0.0/24", ip: "10.0.0.5", localNeedsIPAllow: true, want: true},
@@ -46,7 +54,8 @@ func TestCheckAddr(t *testing.T) {
 		{name: "strict rejects unmatched host", allow: "example.com", host: "other.com", ip: "8.8.8.8", strict: true},
 		{name: "strict allows matched host", allow: "example.com", host: "example.com", ip: "8.8.8.8", strict: true, want: true},
 		{name: "strict block overrides allow", allow: "10.0.0.0/8", block: "10.0.0.5/32", ip: "10.0.0.5", strict: true},
-		{name: "strict reserved denied by cidr", allow: "169.254.0.0/16", ip: "169.254.169.254", strict: true},
+		{name: "strict reserved denied despite allow", allow: "168.63.129.16/32", ip: "168.63.129.16", strict: true},
+		{name: "strict link-local allowed by cidr", allow: "169.254.0.0/16", ip: "169.254.169.254", strict: true, want: true},
 		{name: "strict local gate ignores host", allow: "example.com", host: "example.com", ip: "10.0.0.5", localNeedsIPAllow: true, strict: true},
 		{name: "strict local gate accepts builtin", allow: "private", ip: "100.64.0.1", localNeedsIPAllow: true, strict: true, want: true},
 	} {
@@ -63,7 +72,7 @@ func TestCheckAddr(t *testing.T) {
 			mode = Strict
 		}
 		err := NewPolicy("test", mode, opts...).checkAddr(tc.host, netip.AddrPortFrom(addr, 80))
-		assert.Equal(t, tc.want, err == nil, "%s: %v", tc.name, err)
+		assert.Equal(t, tc.want, err == nil, "%s (%s): %v", tc.name, tc.ip, err)
 	}
 }
 
@@ -115,8 +124,9 @@ func TestCheckHostIPs(t *testing.T) {
 
 	builtins := NewPolicy("test", Lax, WithAllow("private, loopback", ""))
 	assert.NoError(t, builtins.checkHostIPs(hostURL(t, "http://example.com"), ips("8.8.8.8", "100.64.0.1", "::1")))
+	assert.NoError(t, builtins.checkHostIPs(hostURL(t, "http://example.com"), ips("100.100.100.200"))) // cloud metadata is opt-in with its containing range
 	for _, ip := range []string{
-		"0.1.2.3", "100.100.100.200", "168.63.129.16", "169.254.169.254", "192.0.2.1", "192.88.99.1", "198.18.0.1",
+		"0.1.2.3", "168.63.129.16", "169.254.169.254", "192.0.2.1", "192.88.99.1", "198.18.0.1",
 		"198.51.100.1", "203.0.113.1", "::7f00:1", "::ffff:0:a00:5", "64:ff9b::a9fe:a9fe", "64:ff9b::808:808", "2001::1", "2001:db8::1",
 		"2002::1", "fe80::1",
 	} {