Use `hostmatcher` to replace `matchlist`, improve security (#17605)

Use hostmacher to replace matchlist.

And we introduce a better DialContext to do a full host/IP check, otherwise the attackers can still bypass the allow/block list by a 302 redirection.

3 files changed, 217 insertions, 58 deletions

diff --git a/modules/hostmatcher/hostmatcher.go b/modules/hostmatcher/hostmatcher.go
index f8a787c575..815b8c4e4a 100644
--- a/modules/hostmatcher/hostmatcher.go
+++ b/modules/hostmatcher/hostmatcher.go
@@ -13,15 +13,18 @@ import (
 )
 
 // HostMatchList is used to check if a host or IP is in a list.
-// If you only need to do wildcard matching, consider to use modules/matchlist
 type HostMatchList struct {
-	hosts  []string
+	SettingKeyHint string
+	SettingValue   string
+
+	// builtins networks
+	builtins []string
+	// patterns for host names (with wildcard support)
+	patterns []string
+	// ipNets is the CIDR network list
 	ipNets []*net.IPNet
 }
 
-// MatchBuiltinAll all hosts are matched
-const MatchBuiltinAll = "*"
-
 // MatchBuiltinExternal A valid non-private unicast IP, all hosts on public internet are matched
 const MatchBuiltinExternal = "external"
 
@@ -31,9 +34,13 @@ const MatchBuiltinPrivate = "private"
 // MatchBuiltinLoopback 127.0.0.0/8 for IPv4 and ::1/128 for IPv6, localhost is included.
 const MatchBuiltinLoopback = "loopback"
 
+func isBuiltin(s string) bool {
+	return s == MatchBuiltinExternal || s == MatchBuiltinPrivate || s == MatchBuiltinLoopback
+}
+
 // ParseHostMatchList parses the host list HostMatchList
-func ParseHostMatchList(hostList string) *HostMatchList {
-	hl := &HostMatchList{}
+func ParseHostMatchList(settingKeyHint string, hostList string) *HostMatchList {
+	hl := &HostMatchList{SettingKeyHint: settingKeyHint, SettingValue: hostList}
 	for _, s := range strings.Split(hostList, ",") {
 		s = strings.ToLower(strings.TrimSpace(s))
 		if s == "" {
@@ -42,53 +49,106 @@ func ParseHostMatchList(hostList string) *HostMatchList {
 		_, ipNet, err := net.ParseCIDR(s)
 		if err == nil {
 			hl.ipNets = append(hl.ipNets, ipNet)
+		} else if isBuiltin(s) {
+			hl.builtins = append(hl.builtins, s)
 		} else {
-			hl.hosts = append(hl.hosts, s)
+			hl.patterns = append(hl.patterns, s)
 		}
 	}
 	return hl
 }
 
-// MatchesHostOrIP checks if the host or IP matches an allow/deny(block) list
-func (hl *HostMatchList) MatchesHostOrIP(host string, ip net.IP) bool {
-	var matched bool
-	host = strings.ToLower(host)
-	ipStr := ip.String()
-loop:
-	for _, hostInList := range hl.hosts {
-		switch hostInList {
-		case "":
+// ParseSimpleMatchList parse a simple matchlist (no built-in networks, no CIDR support, only wildcard pattern match)
+func ParseSimpleMatchList(settingKeyHint string, matchList string) *HostMatchList {
+	hl := &HostMatchList{
+		SettingKeyHint: settingKeyHint,
+		SettingValue:   matchList,
+	}
+	for _, s := range strings.Split(matchList, ",") {
+		s = strings.ToLower(strings.TrimSpace(s))
+		if s == "" {
 			continue
-		case MatchBuiltinAll:
-			matched = true
-			break loop
+		}
+		// we keep the same result as old `matchlist`, so no builtin/CIDR support here, we only match wildcard patterns
+		hl.patterns = append(hl.patterns, s)
+	}
+	return hl
+}
+
+// AppendBuiltin appends more builtins to match
+func (hl *HostMatchList) AppendBuiltin(builtin string) {
+	hl.builtins = append(hl.builtins, builtin)
+}
+
+// IsEmpty checks if the checklist is empty
+func (hl *HostMatchList) IsEmpty() bool {
+	return hl == nil || (len(hl.builtins) == 0 && len(hl.patterns) == 0 && len(hl.ipNets) == 0)
+}
+
+func (hl *HostMatchList) checkPattern(host string) bool {
+	host = strings.ToLower(strings.TrimSpace(host))
+	for _, pattern := range hl.patterns {
+		if matched, _ := filepath.Match(pattern, host); matched {
+			return true
+		}
+	}
+	return false
+}
+
+func (hl *HostMatchList) checkIP(ip net.IP) bool {
+	for _, pattern := range hl.patterns {
+		if pattern == "*" {
+			return true
+		}
+	}
+	for _, builtin := range hl.builtins {
+		switch builtin {
 		case MatchBuiltinExternal:
-			if matched = ip.IsGlobalUnicast() && !util.IsIPPrivate(ip); matched {
-				break loop
+			if ip.IsGlobalUnicast() && !util.IsIPPrivate(ip) {
+				return true
 			}
 		case MatchBuiltinPrivate:
-			if matched = util.IsIPPrivate(ip); matched {
-				break loop
+			if util.IsIPPrivate(ip) {
+				return true
 			}
 		case MatchBuiltinLoopback:
-			if matched = ip.IsLoopback(); matched {
-				break loop
-			}
-		default:
-			if matched, _ = filepath.Match(hostInList, host); matched {
-				break loop
-			}
-			if matched, _ = filepath.Match(hostInList, ipStr); matched {
-				break loop
+			if ip.IsLoopback() {
+				return true
 			}
 		}
 	}
-	if !matched {
-		for _, ipNet := range hl.ipNets {
-			if matched = ipNet.Contains(ip); matched {
-				break
-			}
+	for _, ipNet := range hl.ipNets {
+		if ipNet.Contains(ip) {
+			return true
 		}
 	}
-	return matched
+	return false
+}
+
+// MatchHostName checks if the host matches an allow/deny(block) list
+func (hl *HostMatchList) MatchHostName(host string) bool {
+	if hl == nil {
+		return false
+	}
+	if hl.checkPattern(host) {
+		return true
+	}
+	if ip := net.ParseIP(host); ip != nil {
+		return hl.checkIP(ip)
+	}
+	return false
+}
+
+// MatchIPAddr checks if the IP matches an allow/deny(block) list, it's safe to pass `nil` to `ip`
+func (hl *HostMatchList) MatchIPAddr(ip net.IP) bool {
+	if hl == nil {
+		return false
+	}
+	host := ip.String() // nil-safe, we will get "<nil>" if ip is nil
+	return hl.checkPattern(host) || hl.checkIP(ip)
+}
+
+// MatchHostOrIP checks if the host or IP matches an allow/deny(block) list
+func (hl *HostMatchList) MatchHostOrIP(host string, ip net.IP) bool {
+	return hl.MatchHostName(host) || hl.MatchIPAddr(ip)
 }
diff --git a/modules/hostmatcher/hostmatcher_test.go b/modules/hostmatcher/hostmatcher_test.go
index 8eaafbdbc8..66030a32f1 100644
--- a/modules/hostmatcher/hostmatcher_test.go
+++ b/modules/hostmatcher/hostmatcher_test.go
@@ -20,17 +20,28 @@ func TestHostOrIPMatchesList(t *testing.T) {
 
 	// for IPv6: "::1" is loopback, "fd00::/8" is private
 
-	hl := ParseHostMatchList("private, External, *.myDomain.com, 169.254.1.0/24")
+	hl := ParseHostMatchList("", "private, External, *.myDomain.com, 169.254.1.0/24")
+
+	test := func(cases []tc) {
+		for _, c := range cases {
+			assert.Equalf(t, c.expected, hl.MatchHostOrIP(c.host, c.ip), "case domain=%s, ip=%v, expected=%v", c.host, c.ip, c.expected)
+		}
+	}
+
 	cases := []tc{
 		{"", net.IPv4zero, false},
 		{"", net.IPv6zero, false},
 
 		{"", net.ParseIP("127.0.0.1"), false},
+		{"127.0.0.1", nil, false},
 		{"", net.ParseIP("::1"), false},
 
 		{"", net.ParseIP("10.0.1.1"), true},
+		{"10.0.1.1", nil, true},
 		{"", net.ParseIP("192.168.1.1"), true},
+		{"192.168.1.1", nil, true},
 		{"", net.ParseIP("fd00::1"), true},
+		{"fd00::1", nil, true},
 
 		{"", net.ParseIP("8.8.8.8"), true},
 		{"", net.ParseIP("1001::1"), true},
@@ -39,13 +50,13 @@ func TestHostOrIPMatchesList(t *testing.T) {
 		{"sub.mydomain.com", net.IPv4zero, true},
 
 		{"", net.ParseIP("169.254.1.1"), true},
+		{"169.254.1.1", nil, true},
 		{"", net.ParseIP("169.254.2.2"), false},
+		{"169.254.2.2", nil, false},
 	}
-	for _, c := range cases {
-		assert.Equalf(t, c.expected, hl.MatchesHostOrIP(c.host, c.ip), "case %s(%v)", c.host, c.ip)
-	}
+	test(cases)
 
-	hl = ParseHostMatchList("loopback")
+	hl = ParseHostMatchList("", "loopback")
 	cases = []tc{
 		{"", net.IPv4zero, false},
 		{"", net.ParseIP("127.0.0.1"), true},
@@ -59,11 +70,9 @@ func TestHostOrIPMatchesList(t *testing.T) {
 
 		{"mydomain.com", net.IPv4zero, false},
 	}
-	for _, c := range cases {
-		assert.Equalf(t, c.expected, hl.MatchesHostOrIP(c.host, c.ip), "case %s(%v)", c.host, c.ip)
-	}
+	test(cases)
 
-	hl = ParseHostMatchList("private")
+	hl = ParseHostMatchList("", "private")
 	cases = []tc{
 		{"", net.IPv4zero, false},
 		{"", net.ParseIP("127.0.0.1"), false},
@@ -77,11 +86,9 @@ func TestHostOrIPMatchesList(t *testing.T) {
 
 		{"mydomain.com", net.IPv4zero, false},
 	}
-	for _, c := range cases {
-		assert.Equalf(t, c.expected, hl.MatchesHostOrIP(c.host, c.ip), "case %s(%v)", c.host, c.ip)
-	}
+	test(cases)
 
-	hl = ParseHostMatchList("external")
+	hl = ParseHostMatchList("", "external")
 	cases = []tc{
 		{"", net.IPv4zero, false},
 		{"", net.ParseIP("127.0.0.1"), false},
@@ -95,11 +102,9 @@ func TestHostOrIPMatchesList(t *testing.T) {
 
 		{"mydomain.com", net.IPv4zero, false},
 	}
-	for _, c := range cases {
-		assert.Equalf(t, c.expected, hl.MatchesHostOrIP(c.host, c.ip), "case %s(%v)", c.host, c.ip)
-	}
+	test(cases)
 
-	hl = ParseHostMatchList("*")
+	hl = ParseHostMatchList("", "*")
 	cases = []tc{
 		{"", net.IPv4zero, true},
 		{"", net.ParseIP("127.0.0.1"), true},
@@ -113,7 +118,43 @@ func TestHostOrIPMatchesList(t *testing.T) {
 
 		{"mydomain.com", net.IPv4zero, true},
 	}
-	for _, c := range cases {
-		assert.Equalf(t, c.expected, hl.MatchesHostOrIP(c.host, c.ip), "case %s(%v)", c.host, c.ip)
+	test(cases)
+
+	// built-in network names can be escaped (warping the first char with `[]`) to be used as a real host name
+	// this mechanism is reversed for internal usage only (maybe for some rare cases), it's not supposed to be used by end users
+	// a real user should never use loopback/private/external as their host names
+	hl = ParseHostMatchList("", "loopback, [p]rivate")
+	cases = []tc{
+		{"loopback", nil, false},
+		{"", net.ParseIP("127.0.0.1"), true},
+		{"private", nil, true},
+		{"", net.ParseIP("192.168.1.1"), false},
+	}
+	test(cases)
+
+	hl = ParseSimpleMatchList("", "loopback, *.domain.com")
+	cases = []tc{
+		{"loopback", nil, true},
+		{"", net.ParseIP("127.0.0.1"), false},
+		{"sub.domain.com", nil, true},
+		{"other.com", nil, false},
+		{"", net.ParseIP("1.1.1.1"), false},
+	}
+	test(cases)
+
+	hl = ParseSimpleMatchList("", "external")
+	cases = []tc{
+		{"", net.ParseIP("192.168.1.1"), false},
+		{"", net.ParseIP("1.1.1.1"), false},
+		{"external", nil, true},
+	}
+	test(cases)
+
+	hl = ParseSimpleMatchList("", "")
+	cases = []tc{
+		{"", net.ParseIP("192.168.1.1"), false},
+		{"", net.ParseIP("1.1.1.1"), false},
+		{"external", nil, false},
 	}
+	test(cases)
 }
diff --git a/modules/hostmatcher/http.go b/modules/hostmatcher/http.go
new file mode 100644
index 0000000000..9dae61a44c
--- /dev/null
+++ b/modules/hostmatcher/http.go
@@ -0,0 +1,58 @@
+// Copyright 2021 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package hostmatcher
+
+import (
+	"context"
+	"fmt"
+	"net"
+	"syscall"
+	"time"
+)
+
+// NewDialContext returns a DialContext for Transport, the DialContext will do allow/block list check
+func NewDialContext(usage string, allowList *HostMatchList, blockList *HostMatchList) func(ctx context.Context, network, addr string) (net.Conn, error) {
+	// How Go HTTP Client works with redirection:
+	//   transport.RoundTrip URL=http://domain.com, Host=domain.com
+	//   transport.DialContext addrOrHost=domain.com:80
+	//   dialer.Control tcp4:11.22.33.44:80
+	//   transport.RoundTrip URL=http://www.domain.com/, Host=(empty here, in the direction, HTTP client doesn't fill the Host field)
+	//   transport.DialContext addrOrHost=domain.com:80
+	//   dialer.Control tcp4:11.22.33.44:80
+	return func(ctx context.Context, network, addrOrHost string) (net.Conn, error) {
+		dialer := net.Dialer{
+			// default values comes from http.DefaultTransport
+			Timeout:   30 * time.Second,
+			KeepAlive: 30 * time.Second,
+
+			Control: func(network, ipAddr string, c syscall.RawConn) (err error) {
+				var host string
+				if host, _, err = net.SplitHostPort(addrOrHost); err != nil {
+					return err
+				}
+				// in Control func, the addr was already resolved to IP:PORT format, there is no cost to do ResolveTCPAddr here
+				tcpAddr, err := net.ResolveTCPAddr(network, ipAddr)
+				if err != nil {
+					return fmt.Errorf("%s can only call HTTP servers via TCP, deny '%s(%s:%s)', err=%v", usage, host, network, ipAddr, err)
+				}
+
+				var blockedError error
+				if blockList.MatchHostOrIP(host, tcpAddr.IP) {
+					blockedError = fmt.Errorf("%s can not call blocked HTTP servers (check your %s setting), deny '%s(%s)'", usage, blockList.SettingKeyHint, host, ipAddr)
+				}
+
+				// if we have an allow-list, check the allow-list first
+				if !allowList.IsEmpty() {
+					if !allowList.MatchHostOrIP(host, tcpAddr.IP) {
+						return fmt.Errorf("%s can only call allowed HTTP servers (check your %s setting), deny '%s(%s)'", usage, allowList.SettingKeyHint, host, ipAddr)
+					}
+				}
+				// otherwise, we always follow the blocked list
+				return blockedError
+			},
+		}
+		return dialer.DialContext(ctx, network, addrOrHost)
+	}
+}