From d007a940d338700fd7b6edaa0854dfead9f99872 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 30 Oct 2021 17:15:42 +0800 Subject: [PATCH] Improve documents, unit tests and comments. --- cmd/web.go | 5 +- custom/conf/app.example.ini | 7 +- .../doc/advanced/config-cheat-sheet.en-us.md | 9 +- modules/setting/webhook.go | 23 +--- modules/util/net.go | 93 ++++++++++++++ modules/util/net_test.go | 119 ++++++++++++++++++ modules/util/util.go | 11 -- services/webhook/deliver.go | 48 +------ services/webhook/deliver_test.go | 82 ------------ 9 files changed, 233 insertions(+), 164 deletions(-) create mode 100644 modules/util/net.go create mode 100644 modules/util/net_test.go diff --git a/cmd/web.go b/cmd/web.go index 6d8808824d0a9..8d9387e06f7ab 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -194,7 +194,10 @@ func listen(m http.Handler, handleRedirector bool) error { listenAddr = net.JoinHostPort(listenAddr, setting.HTTPPort) } log.Info("Listen: %v://%s%s", setting.Protocol, listenAddr, setting.AppSubURL) - log.Info("AppURL: %s", setting.AppURL) + // This can be useful for users, many users do wrong to their config and get strange behaviors behind a reverse-proxy. + // A user may fix the configuration mistake when he sees this log. + // And this is also very helpful to maintainers to provide help to users to resolve their configuration problems. + log.Info("AppURL(ROOT_URL): %s", setting.AppURL) if setting.LFS.StartServer { log.Info("LFS server enabled") diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index b601499480cb8..b3afead2cbcc9 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1396,8 +1396,11 @@ PATH = ;; Deliver timeout in seconds ;DELIVER_TIMEOUT = 5 ;; -;; Webhook can only call allowed hosts for security reasons. Comma separated list: loopback, private, global, or all, or CIDR list (1.2.3.0/8), or wildcard hosts (*.mydomain.com) -; ALLOWED_HOST_LIST = global +;; Webhook can only call allowed hosts for security reasons. Comma separated list, eg: external, 192.168.1.0/24, *.mydomain.com +;; Built-in: loopback (for localhost), private (for LAN/intranet), external (for public hosts on internet), all (for all hosts) +;; CIDR list: 1.2.3.0/8, 2001:db8::/32 +;; Wildcard hosts: *.mydomain.com, 192.168.100.* +; ALLOWED_HOST_LIST = external ;; ;; Allow insecure certification ;SKIP_TLS_VERIFY = false diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 0a47635482422..ca35074d23f94 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -581,7 +581,14 @@ Define allowed algorithms and their minimum key length (use -1 to disable a type - `QUEUE_LENGTH`: **1000**: Hook task queue length. Use caution when editing this value. - `DELIVER_TIMEOUT`: **5**: Delivery timeout (sec) for shooting webhooks. -- `ALLOWED_HOST_LIST`: **global**: Webhook can only call allowed hosts for security reasons. Comma separated list: `loopback`, `private`, `global`, or `all`, or CIDR list (1.2.3.0/8), or wildcard hosts (*.mydomain.com) +- `ALLOWED_HOST_LIST`: **external**: Webhook can only call allowed hosts for security reasons. Comma separated list. + - Built-in networks: + - `loopback`: 127.0.0.0/8 for IPv4 and ::1/128 for IPv6, localhost is included. + - `private`: RFC 1918 (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16) and RFC 4193 (FC00::/7). Also called LAN/Intranet. + - `external`: A valid non-private unicast IP, you can access all hosts on public internet. + - `all`: All hosts are allowed. + - CIDR list: `1.2.3.0/8` for IPv4 and `2001:db8::/32` for IPv6 + - Wildcard hosts: `*.mydomain.com`, `192.168.100.*` - `SKIP_TLS_VERIFY`: **false**: Allow insecure certification. - `PAGING_NUM`: **10**: Number of webhook history events that are shown in one page. - `PROXY_URL`: **\**: Proxy server URL, support http://, https//, socks://, blank will follow environment http_proxy/https_proxy. If not given, will use global proxy setting. diff --git a/modules/setting/webhook.go b/modules/setting/webhook.go index dd3f90c8b2012..1daa258232f34 100644 --- a/modules/setting/webhook.go +++ b/modules/setting/webhook.go @@ -7,9 +7,9 @@ package setting import ( "net" "net/url" - "strings" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ) var ( @@ -18,7 +18,7 @@ var ( QueueLength int DeliverTimeout int SkipTLSVerify bool - AllowedHostList []string // loopback,private,global, or all, or CIDR list, or wildcard hosts + AllowedHostList []string AllowedHostIPNets []*net.IPNet Types []string PagingNum int @@ -35,29 +35,12 @@ var ( } ) -// ParseWebhookAllowedHostList parses the ALLOWED_HOST_LIST value -func ParseWebhookAllowedHostList(allowedHostListStr string) (allowedHostList []string, allowedHostIPNets []*net.IPNet) { - for _, s := range strings.Split(allowedHostListStr, ",") { - s = strings.TrimSpace(s) - if s == "" { - continue - } - _, ipNet, err := net.ParseCIDR(s) - if err == nil { - allowedHostIPNets = append(allowedHostIPNets, ipNet) - } else { - allowedHostList = append(allowedHostList, s) - } - } - return -} - func newWebhookService() { sec := Cfg.Section("webhook") Webhook.QueueLength = sec.Key("QUEUE_LENGTH").MustInt(1000) Webhook.DeliverTimeout = sec.Key("DELIVER_TIMEOUT").MustInt(5) Webhook.SkipTLSVerify = sec.Key("SKIP_TLS_VERIFY").MustBool() - Webhook.AllowedHostList, Webhook.AllowedHostIPNets = ParseWebhookAllowedHostList(sec.Key("ALLOWED_HOST_LIST").MustString("global")) + Webhook.AllowedHostList, Webhook.AllowedHostIPNets = util.ParseHostMatchList(sec.Key("ALLOWED_HOST_LIST").MustString(util.HostListBuiltinExternal)) Webhook.Types = []string{"gitea", "gogs", "slack", "discord", "dingtalk", "telegram", "msteams", "feishu", "matrix", "wechatwork"} Webhook.PagingNum = sec.Key("PAGING_NUM").MustInt(10) Webhook.ProxyURL = sec.Key("PROXY_URL").MustString("") diff --git a/modules/util/net.go b/modules/util/net.go new file mode 100644 index 0000000000000..c8fe24c095d24 --- /dev/null +++ b/modules/util/net.go @@ -0,0 +1,93 @@ +// 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 util + +import ( + "net" + "path/filepath" + "strings" +) + +//HostListBuiltinAll all hosts are matched +const HostListBuiltinAll = "all" + +//HostListBuiltinExternal A valid non-private unicast IP, all hosts on public internet are matched +const HostListBuiltinExternal = "external" + +//HostListBuiltinPrivate RFC 1918 (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16) and RFC 4193 (FC00::/7). Also called LAN/Intranet. +const HostListBuiltinPrivate = "private" + +//HostListBuiltinLoopback 127.0.0.0/8 for IPv4 and ::1/128 for IPv6, localhost is included. +const HostListBuiltinLoopback = "loopback" + +// IsIPPrivate for net.IP.IsPrivate. TODO: replace with `ip.IsPrivate()` if min go version is bumped to 1.17 +func IsIPPrivate(ip net.IP) bool { + if ip4 := ip.To4(); ip4 != nil { + return ip4[0] == 10 || + (ip4[0] == 172 && ip4[1]&0xf0 == 16) || + (ip4[0] == 192 && ip4[1] == 168) + } + return len(ip) == net.IPv6len && ip[0]&0xfe == 0xfc +} + +// ParseHostMatchList parses the host list for HostOrIPMatchesList +func ParseHostMatchList(hostListStr string) (hostList []string, ipNets []*net.IPNet) { + for _, s := range strings.Split(hostListStr, ",") { + s = strings.TrimSpace(s) + if s == "" { + continue + } + _, ipNet, err := net.ParseCIDR(s) + if err == nil { + ipNets = append(ipNets, ipNet) + } else { + hostList = append(hostList, s) + } + } + return +} + +// HostOrIPMatchesList checks if the host or IP matches an allow/deny(block) list +func HostOrIPMatchesList(host string, ip net.IP, hostList []string, ipNets []*net.IPNet) bool { + var matched bool + ipStr := ip.String() +loop: + for _, hostInList := range hostList { + switch hostInList { + case "": + continue + case HostListBuiltinAll: + matched = true + break loop + case HostListBuiltinExternal: + if matched = ip.IsGlobalUnicast() && !IsIPPrivate(ip); matched { + break loop + } + case HostListBuiltinPrivate: + if matched = IsIPPrivate(ip); matched { + break loop + } + case HostListBuiltinLoopback: + 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 !matched { + for _, ipNet := range ipNets { + if matched = ipNet.Contains(ip); matched { + break + } + } + } + return matched +} diff --git a/modules/util/net_test.go b/modules/util/net_test.go new file mode 100644 index 0000000000000..f3bc59db966a5 --- /dev/null +++ b/modules/util/net_test.go @@ -0,0 +1,119 @@ +// 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 util + +import ( + "net" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestHostOrIPMatchesList(t *testing.T) { + type tc struct { + host string + ip net.IP + expected bool + } + + // for IPv6: "::1" is loopback, "fd00::/8" is private + + ah, an := ParseHostMatchList("private, external, *.mydomain.com, 169.254.1.0/24") + cases := []tc{ + {"", net.IPv4zero, false}, + {"", net.IPv6zero, false}, + + {"", net.ParseIP("127.0.0.1"), false}, + {"", net.ParseIP("::1"), false}, + + {"", net.ParseIP("10.0.1.1"), true}, + {"", net.ParseIP("192.168.1.1"), true}, + {"", net.ParseIP("fd00::1"), true}, + + {"", net.ParseIP("8.8.8.8"), true}, + {"", net.ParseIP("1001::1"), true}, + + {"mydomain.com", net.IPv4zero, false}, + {"sub.mydomain.com", net.IPv4zero, true}, + + {"", net.ParseIP("169.254.1.1"), true}, + {"", net.ParseIP("169.254.2.2"), false}, + } + for _, c := range cases { + assert.Equalf(t, c.expected, HostOrIPMatchesList(c.host, c.ip, ah, an), "case %s(%v)", c.host, c.ip) + } + + ah, an = ParseHostMatchList("loopback") + cases = []tc{ + {"", net.IPv4zero, false}, + {"", net.ParseIP("127.0.0.1"), true}, + {"", net.ParseIP("10.0.1.1"), false}, + {"", net.ParseIP("192.168.1.1"), false}, + {"", net.ParseIP("8.8.8.8"), false}, + + {"", net.ParseIP("::1"), true}, + {"", net.ParseIP("fd00::1"), false}, + {"", net.ParseIP("1000::1"), false}, + + {"mydomain.com", net.IPv4zero, false}, + } + for _, c := range cases { + assert.Equalf(t, c.expected, HostOrIPMatchesList(c.host, c.ip, ah, an), "case %s(%v)", c.host, c.ip) + } + + ah, an = ParseHostMatchList("private") + cases = []tc{ + {"", net.IPv4zero, false}, + {"", net.ParseIP("127.0.0.1"), false}, + {"", net.ParseIP("10.0.1.1"), true}, + {"", net.ParseIP("192.168.1.1"), true}, + {"", net.ParseIP("8.8.8.8"), false}, + + {"", net.ParseIP("::1"), false}, + {"", net.ParseIP("fd00::1"), true}, + {"", net.ParseIP("1000::1"), false}, + + {"mydomain.com", net.IPv4zero, false}, + } + for _, c := range cases { + assert.Equalf(t, c.expected, HostOrIPMatchesList(c.host, c.ip, ah, an), "case %s(%v)", c.host, c.ip) + } + + ah, an = ParseHostMatchList("external") + cases = []tc{ + {"", net.IPv4zero, false}, + {"", net.ParseIP("127.0.0.1"), false}, + {"", net.ParseIP("10.0.1.1"), false}, + {"", net.ParseIP("192.168.1.1"), false}, + {"", net.ParseIP("8.8.8.8"), true}, + + {"", net.ParseIP("::1"), false}, + {"", net.ParseIP("fd00::1"), false}, + {"", net.ParseIP("1000::1"), true}, + + {"mydomain.com", net.IPv4zero, false}, + } + for _, c := range cases { + assert.Equalf(t, c.expected, HostOrIPMatchesList(c.host, c.ip, ah, an), "case %s(%v)", c.host, c.ip) + } + + ah, an = ParseHostMatchList("all") + cases = []tc{ + {"", net.IPv4zero, true}, + {"", net.ParseIP("127.0.0.1"), true}, + {"", net.ParseIP("10.0.1.1"), true}, + {"", net.ParseIP("192.168.1.1"), true}, + {"", net.ParseIP("8.8.8.8"), true}, + + {"", net.ParseIP("::1"), true}, + {"", net.ParseIP("fd00::1"), true}, + {"", net.ParseIP("1000::1"), true}, + + {"mydomain.com", net.IPv4zero, true}, + } + for _, c := range cases { + assert.Equalf(t, c.expected, HostOrIPMatchesList(c.host, c.ip, ah, an), "case %s(%v)", c.host, c.ip) + } +} diff --git a/modules/util/util.go b/modules/util/util.go index 4f48b14f82363..cbc6eb4f8a01c 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -9,7 +9,6 @@ import ( "crypto/rand" "errors" "math/big" - "net" "strconv" "strings" ) @@ -162,13 +161,3 @@ func RandomString(length int64) (string, error) { } return string(bytes), nil } - -// IsIPPrivate for net.IP.IsPrivate. TODO: replace with `ip.IsPrivate()` if min go version is bumped to 1.17 -func IsIPPrivate(ip net.IP) bool { - if ip4 := ip.To4(); ip4 != nil { - return ip4[0] == 10 || - (ip4[0] == 172 && ip4[1]&0xf0 == 16) || - (ip4[0] == 192 && ip4[1] == 168) - } - return len(ip) == net.IPv6len && ip[0]&0xfe == 0xfc -} diff --git a/services/webhook/deliver.go b/services/webhook/deliver.go index b18ede21013f2..a0df7a67d9ee4 100644 --- a/services/webhook/deliver.go +++ b/services/webhook/deliver.go @@ -16,7 +16,6 @@ import ( "net" "net/http" "net/url" - "path/filepath" "strconv" "strings" "sync" @@ -294,51 +293,6 @@ func webhookProxy() func(req *http.Request) (*url.URL, error) { } } -func isWebhookRequestAllowed(allowedHostList []string, allowedHostIPNets []*net.IPNet, host string, ip net.IP) bool { - var allowed bool - ipStr := ip.String() -loop: - for _, allowedHost := range allowedHostList { - switch allowedHost { - case "": - continue - case "all": - allowed = true - break loop - case "global": - if allowed = ip.IsGlobalUnicast() && !util.IsIPPrivate(ip); allowed { - break loop - } - case "private": - if allowed = util.IsIPPrivate(ip); allowed { - break loop - } - case "loopback": - if allowed = ip.IsLoopback(); allowed { - break loop - } - default: - if ok, _ := filepath.Match(allowedHost, host); ok { - allowed = true - break loop - } - if ok, _ := filepath.Match(allowedHost, ipStr); ok { - allowed = true - break loop - } - } - } - if !allowed { - for _, allowIPNet := range allowedHostIPNets { - if allowIPNet.Contains(ip) { - allowed = true - break - } - } - } - return allowed -} - // InitDeliverHooks starts the hooks delivery thread func InitDeliverHooks() { timeout := time.Duration(setting.Webhook.DeliverTimeout) * time.Second @@ -358,7 +312,7 @@ func InitDeliverHooks() { if err != nil { return fmt.Errorf("webhook can only call HTTP servers via TCP, deny '%s(%s:%s)', err=%v", req.Host, network, ipAddr, err) } - if !isWebhookRequestAllowed(setting.Webhook.AllowedHostList, setting.Webhook.AllowedHostIPNets, req.Host, tcpAddr.IP) { + if !util.HostOrIPMatchesList(req.Host, tcpAddr.IP, setting.Webhook.AllowedHostList, setting.Webhook.AllowedHostIPNets) { return fmt.Errorf("webhook can only call allowed HTTP servers (check your webhook.ALLOWED_HOST_LIST setting), deny '%s(%s)'", req.Host, ipAddr) } return nil diff --git a/services/webhook/deliver_test.go b/services/webhook/deliver_test.go index 52efc04ee511a..cfc99d796a536 100644 --- a/services/webhook/deliver_test.go +++ b/services/webhook/deliver_test.go @@ -5,7 +5,6 @@ package webhook import ( - "net" "net/http" "net/url" "testing" @@ -38,84 +37,3 @@ func TestWebhookProxy(t *testing.T) { } } } - -func TestIsWebhookRequestAllowed(t *testing.T) { - type tc struct { - host string - ip net.IP - expected bool - } - - ah, an := setting.ParseWebhookAllowedHostList("private, global, *.google.com, 169.254.1.0/24") - cases := []tc{ - {"", net.IPv4zero, false}, - - {"", net.ParseIP("127.0.0.1"), false}, - - {"", net.ParseIP("10.0.1.1"), true}, - {"", net.ParseIP("192.168.1.1"), true}, - - {"", net.ParseIP("8.8.8.8"), true}, - - {"google.com", net.IPv4zero, false}, - {"sub.google.com", net.IPv4zero, true}, - - {"", net.ParseIP("169.254.1.1"), true}, - {"", net.ParseIP("169.254.2.2"), false}, - } - for _, c := range cases { - assert.Equalf(t, c.expected, isWebhookRequestAllowed(ah, an, c.host, c.ip), "case %s(%v)", c.host, c.ip) - } - - ah, an = setting.ParseWebhookAllowedHostList("loopback") - cases = []tc{ - {"", net.IPv4zero, false}, - {"", net.ParseIP("127.0.0.1"), true}, - {"", net.ParseIP("10.0.1.1"), false}, - {"", net.ParseIP("192.168.1.1"), false}, - {"", net.ParseIP("8.8.8.8"), false}, - {"google.com", net.IPv4zero, false}, - } - for _, c := range cases { - assert.Equalf(t, c.expected, isWebhookRequestAllowed(ah, an, c.host, c.ip), "case %s(%v)", c.host, c.ip) - } - - ah, an = setting.ParseWebhookAllowedHostList("private") - cases = []tc{ - {"", net.IPv4zero, false}, - {"", net.ParseIP("127.0.0.1"), false}, - {"", net.ParseIP("10.0.1.1"), true}, - {"", net.ParseIP("192.168.1.1"), true}, - {"", net.ParseIP("8.8.8.8"), false}, - {"google.com", net.IPv4zero, false}, - } - for _, c := range cases { - assert.Equalf(t, c.expected, isWebhookRequestAllowed(ah, an, c.host, c.ip), "case %s(%v)", c.host, c.ip) - } - - ah, an = setting.ParseWebhookAllowedHostList("global") - cases = []tc{ - {"", net.IPv4zero, false}, - {"", net.ParseIP("127.0.0.1"), false}, - {"", net.ParseIP("10.0.1.1"), false}, - {"", net.ParseIP("192.168.1.1"), false}, - {"", net.ParseIP("8.8.8.8"), true}, - {"google.com", net.IPv4zero, false}, - } - for _, c := range cases { - assert.Equalf(t, c.expected, isWebhookRequestAllowed(ah, an, c.host, c.ip), "case %s(%v)", c.host, c.ip) - } - - ah, an = setting.ParseWebhookAllowedHostList("all") - cases = []tc{ - {"", net.IPv4zero, true}, - {"", net.ParseIP("127.0.0.1"), true}, - {"", net.ParseIP("10.0.1.1"), true}, - {"", net.ParseIP("192.168.1.1"), true}, - {"", net.ParseIP("8.8.8.8"), true}, - {"google.com", net.IPv4zero, true}, - } - for _, c := range cases { - assert.Equalf(t, c.expected, isWebhookRequestAllowed(ah, an, c.host, c.ip), "case %s(%v)", c.host, c.ip) - } -}