use hostmatcher to replace matchlist, improve security

Author: wxiaoguang

integrations/mirror_pull_test.go

@@ -47,7 +47,7 @@ func TestMirrorPull(t *testing.T) {
 
 	ctx := context.Background()
 
-	mirror, err := repository.MigrateRepositoryGitData(ctx, user, mirrorRepo, opts)
+	mirror, err := repository.MigrateRepositoryGitData(ctx, user, mirrorRepo, opts, nil)
 	assert.NoError(t, err)
 
 	gitRepo, err := git.OpenRepository(repoPath)

models/error.go

@@ -909,7 +909,6 @@ type ErrInvalidCloneAddr struct {
 	IsPermissionDenied bool
 	LocalPath          bool
 	NotResolvedIP      bool
-	PrivateNet         string
 }
 
 // IsErrInvalidCloneAddr checks if an error is a ErrInvalidCloneAddr.
@@ -922,9 +921,6 @@ func (err *ErrInvalidCloneAddr) Error() string {
 	if err.NotResolvedIP {
 		return fmt.Sprintf("migration/cloning from '%s' is not allowed: unknown hostname", err.Host)
 	}
-	if len(err.PrivateNet) != 0 {
-		return fmt.Sprintf("migration/cloning from '%s' is not allowed: the host resolve to a private ip address '%s'", err.Host, err.PrivateNet)
-	}
 	if err.IsInvalidPath {
 		return fmt.Sprintf("migration/cloning from '%s' is not allowed: the provided path is invalid", err.Host)
 	}

modules/hostmatcher/hostmatcher.go

@@ -5,6 +5,7 @@
 package hostmatcher
 
 import (
+	"fmt"
 	"net"
 	"path/filepath"
 	"strings"
@@ -15,7 +16,11 @@ 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
+
+	// host name or built-in network name
+	names  []string
 	ipNets []*net.IPNet
 }
 
@@ -32,8 +37,8 @@ const MatchBuiltinPrivate = "private"
 const MatchBuiltinLoopback = "loopback"
 
 // 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 == "" {
@@ -43,52 +48,116 @@ func ParseHostMatchList(hostList string) *HostMatchList {
 		if err == nil {
 			hl.ipNets = append(hl.ipNets, ipNet)
 		} else {
-			hl.hosts = append(hl.hosts, s)
+			hl.names = append(hl.names, 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 {
+// ParseSimpleMatchList parse a simple matchlist (no built-in networks, no CIDR support)
+func ParseSimpleMatchList(settingKeyHint string, matchList string, includeLocalNetwork bool) *HostMatchList {
+	hl := &HostMatchList{
+		SettingKeyHint: settingKeyHint,
+		SettingValue:   matchList + fmt.Sprintf("(local-network:%v)", includeLocalNetwork),
+	}
+	for _, s := range strings.Split(matchList, ",") {
+		s = strings.ToLower(strings.TrimSpace(s))
+		if s == "" {
+			continue
+		}
+		if s == MatchBuiltinLoopback || s == MatchBuiltinPrivate || s == MatchBuiltinExternal {
+			// for built-in names, we convert it from "private" => "[p]rivate" for internal usage and keep the same result as `matchlist`
+			hl.names = append(hl.names, "["+s[:1]+"]"+s[1:])
+		} else {
+			// we keep the same result as `matchlist`, so no CIDR support here
+			hl.names = append(hl.names, s)
+		}
+	}
+	if includeLocalNetwork {
+		hl.names = append(hl.names, MatchBuiltinPrivate)
+	}
+	return hl
+}
+
+// IsEmpty checks if the check list is empty
+func (hl *HostMatchList) IsEmpty() bool {
+	return hl == nil || (len(hl.names) == 0 && len(hl.ipNets) == 0)
+}
+
+func (hl *HostMatchList) checkNames(host string) bool {
+	host = strings.ToLower(strings.TrimSpace(host))
+	for _, name := range hl.names {
+		switch name {
 		case "":
+		case MatchBuiltinExternal:
+		case MatchBuiltinPrivate:
+		case MatchBuiltinLoopback:
+			// ignore empty string or built-in network names
 			continue
 		case MatchBuiltinAll:
-			matched = true
-			break loop
+			return true
+		default:
+			if matched, _ := filepath.Match(name, host); matched {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+func (hl *HostMatchList) checkIP(ip net.IP) bool {
+	for _, name := range hl.names {
+		switch name {
+		case "":
+			continue
+		case MatchBuiltinAll:
+			return true
 		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.checkNames(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.checkNames(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)
 }

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,36 @@ 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", true)
+	cases = []tc{
+		{"loopback", nil, true},
+		{"", net.ParseIP("127.0.0.1"), false},
+		{"sub.domain.com", nil, true},
+		{"other.com", nil, false},
+		{"", net.ParseIP("192.168.1.1"), true},
+		{"", net.ParseIP("1.1.1.1"), false},
+	}
+	test(cases)
+
+	hl = ParseSimpleMatchList("", "external", false)
+	cases = []tc{
+		{"", net.ParseIP("192.168.1.1"), false},
+		{"", net.ParseIP("1.1.1.1"), false},
+		{"external", nil, true},
 	}
+	test(cases)
 }

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)
+	}
+}