From e949447c2a547a646a1c3aad7dd28c1b217966c3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 10 Nov 2021 17:08:14 +0800 Subject: [PATCH 01/10] use hostmatcher to replace matchlist, improve security --- integrations/mirror_pull_test.go | 2 +- models/error.go | 4 - modules/hostmatcher/hostmatcher.go | 135 ++++++++++++++++++------ modules/hostmatcher/hostmatcher_test.go | 72 +++++++++---- modules/hostmatcher/http.go | 58 ++++++++++ modules/lfs/client.go | 5 +- modules/lfs/client_test.go | 4 +- modules/lfs/http_client.go | 14 +-- modules/matchlist/matchlist.go | 46 -------- modules/migrations/gitea_downloader.go | 17 +-- modules/migrations/gitea_uploader.go | 2 +- modules/migrations/github.go | 29 ++--- modules/migrations/gitlab.go | 17 +-- modules/migrations/gogs.go | 13 +-- modules/migrations/http_client.go | 30 ++++++ modules/migrations/migrate.go | 63 +++++------ modules/migrations/migrate_test.go | 6 +- modules/repository/repo.go | 19 ++-- modules/setting/migrations.go | 19 +--- modules/setting/webhook.go | 5 +- options/locale/locale_en-US.ini | 3 +- routers/api/v1/repo/migrate.go | 4 +- routers/web/repo/migrate.go | 4 +- routers/web/repo/setting.go | 4 +- services/mirror/mirror_pull.go | 5 +- services/mirror/mirror_push.go | 13 ++- services/webhook/deliver.go | 32 ++---- 27 files changed, 346 insertions(+), 279 deletions(-) create mode 100644 modules/hostmatcher/http.go delete mode 100644 modules/matchlist/matchlist.go create mode 100644 modules/migrations/http_client.go diff --git a/integrations/mirror_pull_test.go b/integrations/mirror_pull_test.go index f396aac4b445..ece8bd3fd76b 100644 --- a/integrations/mirror_pull_test.go +++ b/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) diff --git a/models/error.go b/models/error.go index 1179fa6eb751..028b349fc087 100644 --- a/models/error.go +++ b/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) } diff --git a/modules/hostmatcher/hostmatcher.go b/modules/hostmatcher/hostmatcher.go index f8a787c575e3..ceb9b8c081af 100644 --- a/modules/hostmatcher/hostmatcher.go +++ b/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 "" 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) } diff --git a/modules/hostmatcher/hostmatcher_test.go b/modules/hostmatcher/hostmatcher_test.go index 8eaafbdbc809..d5e45c5b37e4 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,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) } diff --git a/modules/hostmatcher/http.go b/modules/hostmatcher/http.go new file mode 100644 index 000000000000..9dae61a44cba --- /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) + } +} diff --git a/modules/lfs/client.go b/modules/lfs/client.go index 81b047c5bd99..aaf61aefcf36 100644 --- a/modules/lfs/client.go +++ b/modules/lfs/client.go @@ -7,6 +7,7 @@ package lfs import ( "context" "io" + "net/http" "net/url" ) @@ -24,9 +25,9 @@ type Client interface { } // NewClient creates a LFS client -func NewClient(endpoint *url.URL, skipTLSVerify bool) Client { +func NewClient(endpoint *url.URL, httpTransport *http.Transport) Client { if endpoint.Scheme == "file" { return newFilesystemClient(endpoint) } - return newHTTPClient(endpoint, skipTLSVerify) + return newHTTPClient(endpoint, httpTransport) } diff --git a/modules/lfs/client_test.go b/modules/lfs/client_test.go index ee6b7a59fc1e..88986f06d6b3 100644 --- a/modules/lfs/client_test.go +++ b/modules/lfs/client_test.go @@ -13,10 +13,10 @@ import ( func TestNewClient(t *testing.T) { u, _ := url.Parse("file:///test") - c := NewClient(u, true) + c := NewClient(u, nil) assert.IsType(t, &FilesystemClient{}, c) u, _ = url.Parse("https://test.com/lfs") - c = NewClient(u, true) + c = NewClient(u, nil) assert.IsType(t, &HTTPClient{}, c) } diff --git a/modules/lfs/http_client.go b/modules/lfs/http_client.go index 5df5ed33a9ff..a1a3e7f363e3 100644 --- a/modules/lfs/http_client.go +++ b/modules/lfs/http_client.go @@ -7,7 +7,6 @@ package lfs import ( "bytes" "context" - "crypto/tls" "errors" "fmt" "net/http" @@ -34,12 +33,15 @@ func (c *HTTPClient) BatchSize() int { return batchSize } -func newHTTPClient(endpoint *url.URL, skipTLSVerify bool) *HTTPClient { +func newHTTPClient(endpoint *url.URL, httpTransport *http.Transport) *HTTPClient { + if httpTransport == nil { + httpTransport = &http.Transport{ + Proxy: proxy.Proxy(), + } + } + hc := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: skipTLSVerify}, - Proxy: proxy.Proxy(), - }, + Transport: httpTransport, } client := &HTTPClient{ diff --git a/modules/matchlist/matchlist.go b/modules/matchlist/matchlist.go deleted file mode 100644 index b65ed909dc1c..000000000000 --- a/modules/matchlist/matchlist.go +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2019 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 matchlist - -import ( - "strings" - - "github.com/gobwas/glob" -) - -// Matchlist represents a block or allow list -type Matchlist struct { - ruleGlobs []glob.Glob -} - -// NewMatchlist creates a new block or allow list -func NewMatchlist(rules ...string) (*Matchlist, error) { - for i := range rules { - rules[i] = strings.ToLower(rules[i]) - } - list := Matchlist{ - ruleGlobs: make([]glob.Glob, 0, len(rules)), - } - - for _, rule := range rules { - rg, err := glob.Compile(rule) - if err != nil { - return nil, err - } - list.ruleGlobs = append(list.ruleGlobs, rg) - } - - return &list, nil -} - -// Match will matches -func (b *Matchlist) Match(u string) bool { - for _, r := range b.ruleGlobs { - if r.Match(u) { - return true - } - } - return false -} diff --git a/modules/migrations/gitea_downloader.go b/modules/migrations/gitea_downloader.go index d8a3c84678ef..677d4800d21f 100644 --- a/modules/migrations/gitea_downloader.go +++ b/modules/migrations/gitea_downloader.go @@ -6,7 +6,6 @@ package migrations import ( "context" - "crypto/tls" "errors" "fmt" "io" @@ -18,8 +17,6 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/migrations/base" - "code.gitea.io/gitea/modules/proxy" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" gitea_sdk "code.gitea.io/sdk/gitea" @@ -90,12 +87,7 @@ func NewGiteaDownloader(ctx context.Context, baseURL, repoPath, username, passwo gitea_sdk.SetToken(token), gitea_sdk.SetBasicAuth(username, password), gitea_sdk.SetContext(ctx), - gitea_sdk.SetHTTPClient(&http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Migrations.SkipTLSVerify}, - Proxy: proxy.Proxy(), - }, - }), + gitea_sdk.SetHTTPClient(NewMigrationHTTPClient()), ) if err != nil { log.Error(fmt.Sprintf("Failed to create NewGiteaDownloader for: %s. Error: %v", baseURL, err)) @@ -275,12 +267,7 @@ func (g *GiteaDownloader) convertGiteaRelease(rel *gitea_sdk.Release) *base.Rele Created: rel.CreatedAt, } - httpClient := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Migrations.SkipTLSVerify}, - Proxy: proxy.Proxy(), - }, - } + httpClient := NewMigrationHTTPClient() for _, asset := range rel.Attachments { size := int(asset.Size) diff --git a/modules/migrations/gitea_uploader.go b/modules/migrations/gitea_uploader.go index d62ce80c77f3..cf546afd4685 100644 --- a/modules/migrations/gitea_uploader.go +++ b/modules/migrations/gitea_uploader.go @@ -123,7 +123,7 @@ func (g *GiteaLocalUploader) CreateRepo(repo *base.Repository, opts base.Migrate Wiki: opts.Wiki, Releases: opts.Releases, // if didn't get releases, then sync them from tags MirrorInterval: opts.MirrorInterval, - }) + }, NewMigrationHTTPTransport()) g.repo = r if err != nil { diff --git a/modules/migrations/github.go b/modules/migrations/github.go index 874cd054394f..6a467b82dfbe 100644 --- a/modules/migrations/github.go +++ b/modules/migrations/github.go @@ -7,7 +7,6 @@ package migrations import ( "context" - "crypto/tls" "fmt" "io" "net/http" @@ -19,7 +18,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/migrations/base" "code.gitea.io/gitea/modules/proxy" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" @@ -99,12 +97,7 @@ func NewGithubDownloaderV3(ctx context.Context, baseURL, userName, password, tok ) var client = &http.Client{ Transport: &oauth2.Transport{ - Base: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Migrations.SkipTLSVerify}, - Proxy: func(req *http.Request) (*url.URL, error) { - return proxy.Proxy()(req) - }, - }, + Base: NewMigrationHTTPTransport(), Source: oauth2.ReuseTokenSource(nil, ts), }, } @@ -112,14 +105,13 @@ func NewGithubDownloaderV3(ctx context.Context, baseURL, userName, password, tok downloader.addClient(client, baseURL) } } else { + var transport = NewMigrationHTTPTransport() + transport.Proxy = func(req *http.Request) (*url.URL, error) { + req.SetBasicAuth(userName, password) + return proxy.Proxy()(req) + } var client = &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Migrations.SkipTLSVerify}, - Proxy: func(req *http.Request) (*url.URL, error) { - req.SetBasicAuth(userName, password) - return proxy.Proxy()(req) - }, - }, + Transport: transport, } downloader.addClient(client, baseURL) } @@ -315,12 +307,7 @@ func (g *GithubDownloaderV3) convertGithubRelease(rel *github.RepositoryRelease) r.Published = rel.PublishedAt.Time } - httpClient := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Migrations.SkipTLSVerify}, - Proxy: proxy.Proxy(), - }, - } + httpClient := NewMigrationHTTPClient() for _, asset := range rel.Assets { var assetID = *asset.ID // Don't optimize this, for closure we need a local variable diff --git a/modules/migrations/gitlab.go b/modules/migrations/gitlab.go index 91ba073d18f4..0a0b1e25b95b 100644 --- a/modules/migrations/gitlab.go +++ b/modules/migrations/gitlab.go @@ -6,7 +6,6 @@ package migrations import ( "context" - "crypto/tls" "errors" "fmt" "io" @@ -18,8 +17,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/migrations/base" - "code.gitea.io/gitea/modules/proxy" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" "github.com/xanzy/go-gitlab" @@ -77,12 +74,7 @@ type GitlabDownloader struct { // Use either a username/password, personal token entered into the username field, or anonymous/public access // Note: Public access only allows very basic access func NewGitlabDownloader(ctx context.Context, baseURL, repoPath, username, password, token string) (*GitlabDownloader, error) { - gitlabClient, err := gitlab.NewClient(token, gitlab.WithBaseURL(baseURL), gitlab.WithHTTPClient(&http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Migrations.SkipTLSVerify}, - Proxy: proxy.Proxy(), - }, - })) + gitlabClient, err := gitlab.NewClient(token, gitlab.WithBaseURL(baseURL), gitlab.WithHTTPClient(NewMigrationHTTPClient())) // Only use basic auth if token is blank and password is NOT // Basic auth will fail with empty strings, but empty token will allow anonymous public API usage if token == "" && password != "" { @@ -300,12 +292,7 @@ func (g *GitlabDownloader) convertGitlabRelease(rel *gitlab.Release) *base.Relea PublisherName: rel.Author.Username, } - httpClient := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Migrations.SkipTLSVerify}, - Proxy: proxy.Proxy(), - }, - } + httpClient := NewMigrationHTTPClient() for k, asset := range rel.Assets.Links { r.Assets = append(r.Assets, &base.ReleaseAsset{ diff --git a/modules/migrations/gogs.go b/modules/migrations/gogs.go index 06c944278b43..3425059fd160 100644 --- a/modules/migrations/gogs.go +++ b/modules/migrations/gogs.go @@ -6,7 +6,6 @@ package migrations import ( "context" - "crypto/tls" "fmt" "net/http" "net/url" @@ -16,7 +15,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/migrations/base" "code.gitea.io/gitea/modules/proxy" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" "github.com/gogs/go-gogs-client" @@ -97,13 +95,12 @@ func NewGogsDownloader(ctx context.Context, baseURL, userName, password, token, client = gogs.NewClient(baseURL, token) downloader.userName = token } else { - downloader.transport = &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Migrations.SkipTLSVerify}, - Proxy: func(req *http.Request) (*url.URL, error) { - req.SetBasicAuth(userName, password) - return proxy.Proxy()(req) - }, + var transport = NewMigrationHTTPTransport() + transport.Proxy = func(req *http.Request) (*url.URL, error) { + req.SetBasicAuth(userName, password) + return proxy.Proxy()(req) } + downloader.transport = transport client = gogs.NewClient(baseURL, "") client.SetHTTPClient(&http.Client{ diff --git a/modules/migrations/http_client.go b/modules/migrations/http_client.go new file mode 100644 index 000000000000..0d683693a1b6 --- /dev/null +++ b/modules/migrations/http_client.go @@ -0,0 +1,30 @@ +// 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 migrations + +import ( + "crypto/tls" + "net/http" + + "code.gitea.io/gitea/modules/hostmatcher" + "code.gitea.io/gitea/modules/proxy" + "code.gitea.io/gitea/modules/setting" +) + +// NewMigrationHTTPClient returns a HTTP client for migration +func NewMigrationHTTPClient() *http.Client { + return &http.Client{ + Transport: NewMigrationHTTPTransport(), + } +} + +// NewMigrationHTTPTransport returns a HTTP transport for migration +func NewMigrationHTTPTransport() *http.Transport { + return &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Migrations.SkipTLSVerify}, + Proxy: proxy.Proxy(), + DialContext: hostmatcher.NewDialContext("migration", allowList, blockList), + } +} diff --git a/modules/migrations/migrate.go b/modules/migrations/migrate.go index dbe69259f4ec..2dce12538631 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -14,8 +14,8 @@ import ( "strings" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/hostmatcher" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/matchlist" "code.gitea.io/gitea/modules/migrations/base" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -27,8 +27,8 @@ type MigrateOptions = base.MigrateOptions var ( factories []base.DownloaderFactory - allowList *matchlist.Matchlist - blockList *matchlist.Matchlist + allowList *hostmatcher.HostMatchList + blockList *hostmatcher.HostMatchList ) // RegisterDownloaderFactory registers a downloader factory @@ -72,30 +72,33 @@ func IsMigrateURLAllowed(remoteURL string, doer *models.User) error { return &models.ErrInvalidCloneAddr{Host: u.Host, IsProtocolInvalid: true, IsPermissionDenied: true, IsURLError: true} } - host := strings.ToLower(u.Host) - if len(setting.Migrations.AllowedDomains) > 0 { - if !allowList.Match(host) { - return &models.ErrInvalidCloneAddr{Host: u.Host, IsPermissionDenied: true} - } - } else { - if blockList.Match(host) { - return &models.ErrInvalidCloneAddr{Host: u.Host, IsPermissionDenied: true} - } + hostName, _, err := net.SplitHostPort(u.Host) + if err != nil { + return &models.ErrInvalidCloneAddr{Host: u.Host, IsURLError: true} + } + addrList, err := net.LookupIP(hostName) + if err != nil { + return &models.ErrInvalidCloneAddr{Host: u.Host, NotResolvedIP: true} } - if !setting.Migrations.AllowLocalNetworks { - addrList, err := net.LookupIP(strings.Split(u.Host, ":")[0]) - if err != nil { - return &models.ErrInvalidCloneAddr{Host: u.Host, NotResolvedIP: true} - } - for _, addr := range addrList { - if util.IsIPPrivate(addr) || !addr.IsGlobalUnicast() { - return &models.ErrInvalidCloneAddr{Host: u.Host, PrivateNet: addr.String(), IsPermissionDenied: true} - } + var ipAllowed bool + var ipBlocked bool + for _, addr := range addrList { + ipAllowed = ipAllowed || allowList.MatchIPAddr(addr) + ipBlocked = ipBlocked || blockList.MatchIPAddr(addr) + } + var blockedError error + if blockList.MatchHostName(hostName) || ipBlocked { + blockedError = &models.ErrInvalidCloneAddr{Host: u.Host, IsPermissionDenied: true} + } + // if we have an allow-list, check the allow-list first + if !allowList.IsEmpty() { + if !allowList.MatchHostName(hostName) && !ipAllowed { + return &models.ErrInvalidCloneAddr{Host: u.Host, IsPermissionDenied: true} } } - - return nil + // otherwise, we always follow the blocked list + return blockedError } // MigrateRepository migrate repository according MigrateOptions @@ -461,16 +464,8 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts // Init migrations service func Init() error { - var err error - allowList, err = matchlist.NewMatchlist(setting.Migrations.AllowedDomains...) - if err != nil { - return fmt.Errorf("init migration allowList domains failed: %v", err) - } - - blockList, err = matchlist.NewMatchlist(setting.Migrations.BlockedDomains...) - if err != nil { - return fmt.Errorf("init migration blockList domains failed: %v", err) - } - + // TODO: maybe we can deprecate these legacy ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS/BLOCKED_DOMAINS, use ALLOWED_HOST_LIST/BLOCKED_HOST_LIST instead + allowList = hostmatcher.ParseSimpleMatchList("migrations.ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS", setting.Migrations.AllowedDomains, setting.Migrations.AllowLocalNetworks) + blockList = hostmatcher.ParseSimpleMatchList("migrations.BLOCKED_DOMAINS", setting.Migrations.BlockedDomains, false) return nil } diff --git a/modules/migrations/migrate_test.go b/modules/migrations/migrate_test.go index c050a9abc086..3cd0938ef6af 100644 --- a/modules/migrations/migrate_test.go +++ b/modules/migrations/migrate_test.go @@ -21,7 +21,7 @@ func TestMigrateWhiteBlocklist(t *testing.T) { adminUser := db.AssertExistsAndLoadBean(t, &models.User{Name: "user1"}).(*models.User) nonAdminUser := db.AssertExistsAndLoadBean(t, &models.User{Name: "user2"}).(*models.User) - setting.Migrations.AllowedDomains = []string{"github.com"} + setting.Migrations.AllowedDomains = "github.com" assert.NoError(t, Init()) err := IsMigrateURLAllowed("https://gitlab.com/gitlab/gitlab.git", nonAdminUser) @@ -33,8 +33,8 @@ func TestMigrateWhiteBlocklist(t *testing.T) { err = IsMigrateURLAllowed("https://gITHUb.com/go-gitea/gitea.git", nonAdminUser) assert.NoError(t, err) - setting.Migrations.AllowedDomains = []string{} - setting.Migrations.BlockedDomains = []string{"github.com"} + setting.Migrations.AllowedDomains = "" + setting.Migrations.BlockedDomains = "github.com" assert.NoError(t, Init()) err = IsMigrateURLAllowed("https://gitlab.com/gitlab/gitlab.git", nonAdminUser) diff --git a/modules/repository/repo.go b/modules/repository/repo.go index 05306218de8b..dfdd1b6f8485 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -8,7 +8,7 @@ import ( "context" "fmt" "io" - "net/url" + "net/http" "path" "strings" "time" @@ -46,7 +46,10 @@ func WikiRemoteURL(remote string) string { } // MigrateRepositoryGitData starts migrating git related data after created migrating repository -func MigrateRepositoryGitData(ctx context.Context, u *models.User, repo *models.Repository, opts migration.MigrateOptions) (*models.Repository, error) { +func MigrateRepositoryGitData(ctx context.Context, u *models.User, + repo *models.Repository, opts migration.MigrateOptions, + httpTransport *http.Transport, +) (*models.Repository, error) { repoPath := models.RepoPath(u.Name, opts.RepoName) if u.IsOrganization() { @@ -141,8 +144,9 @@ func MigrateRepositoryGitData(ctx context.Context, u *models.User, repo *models. } if opts.LFS { - ep := lfs.DetermineEndpoint(opts.CloneAddr, opts.LFSEndpoint) - if err = StoreMissingLfsObjectsInRepository(ctx, repo, gitRepo, ep, setting.Migrations.SkipTLSVerify); err != nil { + endpoint := lfs.DetermineEndpoint(opts.CloneAddr, opts.LFSEndpoint) + lfsClient := lfs.NewClient(endpoint, httpTransport) + if err = StoreMissingLfsObjectsInRepository(ctx, repo, gitRepo, lfsClient); err != nil { log.Error("Failed to store missing LFS objects for repository: %v", err) } } @@ -336,8 +340,7 @@ func PushUpdateAddTag(repo *models.Repository, gitRepo *git.Repository, tagName } // StoreMissingLfsObjectsInRepository downloads missing LFS objects -func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *models.Repository, gitRepo *git.Repository, endpoint *url.URL, skipTLSVerify bool) error { - client := lfs.NewClient(endpoint, skipTLSVerify) +func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *models.Repository, gitRepo *git.Repository, lfsClient lfs.Client) error { contentStore := lfs.NewContentStore() pointerChan := make(chan lfs.PointerBlob) @@ -345,7 +348,7 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *models.Reposi go lfs.SearchPointerBlobs(ctx, gitRepo, pointerChan, errChan) downloadObjects := func(pointers []lfs.Pointer) error { - err := client.Download(ctx, pointers, func(p lfs.Pointer, content io.ReadCloser, objectError error) error { + err := lfsClient.Download(ctx, pointers, func(p lfs.Pointer, content io.ReadCloser, objectError error) error { if objectError != nil { return objectError } @@ -411,7 +414,7 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *models.Reposi } batch = append(batch, pointerBlob.Pointer) - if len(batch) >= client.BatchSize() { + if len(batch) >= lfsClient.BatchSize() { if err := downloadObjects(batch); err != nil { return err } diff --git a/modules/setting/migrations.go b/modules/setting/migrations.go index b663b52f896f..34d903727538 100644 --- a/modules/setting/migrations.go +++ b/modules/setting/migrations.go @@ -4,17 +4,13 @@ package setting -import ( - "strings" -) - var ( // Migrations settings Migrations = struct { MaxAttempts int RetryBackoff int - AllowedDomains []string - BlockedDomains []string + AllowedDomains string + BlockedDomains string AllowLocalNetworks bool SkipTLSVerify bool }{ @@ -28,15 +24,8 @@ func newMigrationsService() { Migrations.MaxAttempts = sec.Key("MAX_ATTEMPTS").MustInt(Migrations.MaxAttempts) Migrations.RetryBackoff = sec.Key("RETRY_BACKOFF").MustInt(Migrations.RetryBackoff) - Migrations.AllowedDomains = sec.Key("ALLOWED_DOMAINS").Strings(",") - for i := range Migrations.AllowedDomains { - Migrations.AllowedDomains[i] = strings.ToLower(Migrations.AllowedDomains[i]) - } - Migrations.BlockedDomains = sec.Key("BLOCKED_DOMAINS").Strings(",") - for i := range Migrations.BlockedDomains { - Migrations.BlockedDomains[i] = strings.ToLower(Migrations.BlockedDomains[i]) - } - + Migrations.AllowedDomains = sec.Key("ALLOWED_DOMAINS").MustString("") + Migrations.BlockedDomains = sec.Key("BLOCKED_DOMAINS").MustString("") Migrations.AllowLocalNetworks = sec.Key("ALLOW_LOCALNETWORKS").MustBool(false) Migrations.SkipTLSVerify = sec.Key("SKIP_TLS_VERIFY").MustBool(false) } diff --git a/modules/setting/webhook.go b/modules/setting/webhook.go index acd5bd045505..6284f397b171 100644 --- a/modules/setting/webhook.go +++ b/modules/setting/webhook.go @@ -7,7 +7,6 @@ package setting import ( "net/url" - "code.gitea.io/gitea/modules/hostmatcher" "code.gitea.io/gitea/modules/log" ) @@ -17,7 +16,7 @@ var ( QueueLength int DeliverTimeout int SkipTLSVerify bool - AllowedHostList *hostmatcher.HostMatchList + AllowedHostList string Types []string PagingNum int ProxyURL string @@ -38,7 +37,7 @@ func newWebhookService() { 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 = hostmatcher.ParseHostMatchList(sec.Key("ALLOWED_HOST_LIST").MustString(hostmatcher.MatchBuiltinExternal)) + Webhook.AllowedHostList = sec.Key("ALLOWED_HOST_LIST").MustString("") 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/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 2632531e2fb5..4680c72893db 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -892,8 +892,7 @@ migrate.clone_address_desc = The HTTP(S) or Git 'clone' URL of an existing repos migrate.github_token_desc = You can put one or more tokens with comma separated here to make migrating faster because of Github API rate limit. WARN: Abusing this feature may violate the service provider's policy and lead to account blocking. migrate.clone_local_path = or a local server path migrate.permission_denied = You are not allowed to import local repositories. -migrate.permission_denied_blocked = You are not allowed to import from blocked hosts. -migrate.permission_denied_private_ip = You are not allowed to import from private IPs. +migrate.permission_denied_blocked = You can not import from disallowed hosts, please check your ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS/BLOCKED_DOMAINS settings. migrate.invalid_local_path = "The local path is invalid. It does not exist or is not a directory." migrate.invalid_lfs_endpoint = The LFS endpoint is not valid. migrate.failed = Migration failed: %v diff --git a/routers/api/v1/repo/migrate.go b/routers/api/v1/repo/migrate.go index 87ceb547c60b..15b70f22ef0c 100644 --- a/routers/api/v1/repo/migrate.go +++ b/routers/api/v1/repo/migrate.go @@ -253,10 +253,8 @@ func handleRemoteAddrError(ctx *context.APIContext, err error) { case addrErr.IsPermissionDenied: if addrErr.LocalPath { ctx.Error(http.StatusUnprocessableEntity, "", "You are not allowed to import local repositories.") - } else if len(addrErr.PrivateNet) == 0 { - ctx.Error(http.StatusUnprocessableEntity, "", "You are not allowed to import from blocked hosts.") } else { - ctx.Error(http.StatusUnprocessableEntity, "", "You are not allowed to import from private IPs.") + ctx.Error(http.StatusUnprocessableEntity, "", "You can not import from disallowed hosts.") } case addrErr.IsInvalidPath: ctx.Error(http.StatusUnprocessableEntity, "", "Invalid local path, it does not exist or not a directory.") diff --git a/routers/web/repo/migrate.go b/routers/web/repo/migrate.go index b2e6fa890be0..40695cfc829e 100644 --- a/routers/web/repo/migrate.go +++ b/routers/web/repo/migrate.go @@ -127,10 +127,8 @@ func handleMigrateRemoteAddrError(ctx *context.Context, err error, tpl base.TplN case addrErr.IsPermissionDenied: if addrErr.LocalPath { ctx.RenderWithErr(ctx.Tr("repo.migrate.permission_denied"), tpl, form) - } else if len(addrErr.PrivateNet) == 0 { - ctx.RenderWithErr(ctx.Tr("repo.migrate.permission_denied_blocked"), tpl, form) } else { - ctx.RenderWithErr(ctx.Tr("repo.migrate.permission_denied_private_ip"), tpl, form) + ctx.RenderWithErr(ctx.Tr("repo.migrate.permission_denied_blocked"), tpl, form) } case addrErr.IsInvalidPath: ctx.RenderWithErr(ctx.Tr("repo.migrate.invalid_local_path"), tpl, form) diff --git a/routers/web/repo/setting.go b/routers/web/repo/setting.go index b104ede00522..f80ae175e63c 100644 --- a/routers/web/repo/setting.go +++ b/routers/web/repo/setting.go @@ -750,10 +750,8 @@ func handleSettingRemoteAddrError(ctx *context.Context, err error, form *forms.R case addrErr.IsPermissionDenied: if addrErr.LocalPath { ctx.RenderWithErr(ctx.Tr("repo.migrate.permission_denied"), tplSettingsOptions, form) - } else if len(addrErr.PrivateNet) == 0 { - ctx.RenderWithErr(ctx.Tr("repo.migrate.permission_denied_blocked"), tplSettingsOptions, form) } else { - ctx.RenderWithErr(ctx.Tr("repo.migrate.permission_denied_private_ip"), tplSettingsOptions, form) + ctx.RenderWithErr(ctx.Tr("repo.migrate.permission_denied_blocked"), tplSettingsOptions, form) } case addrErr.IsInvalidPath: ctx.RenderWithErr(ctx.Tr("repo.migrate.invalid_local_path"), tplSettingsOptions, form) diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 2dee49f71023..ed42d87039ae 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -260,8 +260,9 @@ func runSync(ctx context.Context, m *models.Mirror) ([]*mirrorSyncResult, bool) if m.LFS && setting.LFS.StartServer { log.Trace("SyncMirrors [repo: %-v]: syncing LFS objects...", m.Repo) - ep := lfs.DetermineEndpoint(remoteAddr.String(), m.LFSEndpoint) - if err = repo_module.StoreMissingLfsObjectsInRepository(ctx, m.Repo, gitRepo, ep, false); err != nil { + endpoint := lfs.DetermineEndpoint(remoteAddr.String(), m.LFSEndpoint) + lfsClient := lfs.NewClient(endpoint, nil) + if err = repo_module.StoreMissingLfsObjectsInRepository(ctx, m.Repo, gitRepo, lfsClient); err != nil { log.Error("Failed to synchronize LFS objects for repository: %v", err) } } diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index c1f53196e39e..7e33ffed3e7e 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -8,7 +8,6 @@ import ( "context" "errors" "io" - "net/url" "regexp" "time" @@ -133,8 +132,9 @@ func runPushSync(ctx context.Context, m *models.PushMirror) error { } defer gitRepo.Close() - ep := lfs.DetermineEndpoint(remoteAddr.String(), "") - if err := pushAllLFSObjects(ctx, gitRepo, ep, false); err != nil { + endpoint := lfs.DetermineEndpoint(remoteAddr.String(), "") + lfsClient := lfs.NewClient(endpoint, nil) + if err := pushAllLFSObjects(ctx, gitRepo, lfsClient); err != nil { return util.NewURLSanitizedError(err, remoteAddr, true) } } @@ -176,8 +176,7 @@ func runPushSync(ctx context.Context, m *models.PushMirror) error { return nil } -func pushAllLFSObjects(ctx context.Context, gitRepo *git.Repository, endpoint *url.URL, skipTLSVerify bool) error { - client := lfs.NewClient(endpoint, skipTLSVerify) +func pushAllLFSObjects(ctx context.Context, gitRepo *git.Repository, lfsClient lfs.Client) error { contentStore := lfs.NewContentStore() pointerChan := make(chan lfs.PointerBlob) @@ -185,7 +184,7 @@ func pushAllLFSObjects(ctx context.Context, gitRepo *git.Repository, endpoint *u go lfs.SearchPointerBlobs(ctx, gitRepo, pointerChan, errChan) uploadObjects := func(pointers []lfs.Pointer) error { - err := client.Upload(ctx, pointers, func(p lfs.Pointer, objectError error) (io.ReadCloser, error) { + err := lfsClient.Upload(ctx, pointers, func(p lfs.Pointer, objectError error) (io.ReadCloser, error) { if objectError != nil { return nil, objectError } @@ -219,7 +218,7 @@ func pushAllLFSObjects(ctx context.Context, gitRepo *git.Repository, endpoint *u } batch = append(batch, pointerBlob.Pointer) - if len(batch) >= client.BatchSize() { + if len(batch) >= lfsClient.BatchSize() { if err := uploadObjects(batch); err != nil { return err } diff --git a/services/webhook/deliver.go b/services/webhook/deliver.go index 04cec4c1c4ec..23bbfdbec9b0 100644 --- a/services/webhook/deliver.go +++ b/services/webhook/deliver.go @@ -13,25 +13,22 @@ import ( "encoding/hex" "fmt" "io" - "net" "net/http" "net/url" "strconv" "strings" "sync" - "syscall" "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/graceful" + "code.gitea.io/gitea/modules/hostmatcher" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/proxy" "code.gitea.io/gitea/modules/setting" "github.com/gobwas/glob" ) -var contextKeyWebhookRequest interface{} = "contextKeyWebhookRequest" - // Deliver deliver hook task func Deliver(t *models.HookTask) error { w, err := models.GetWebhookByID(t.HookID) @@ -174,7 +171,7 @@ func Deliver(t *models.HookTask) error { return fmt.Errorf("Webhook task skipped (webhooks disabled): [%d]", t.ID) } - resp, err := webhookHTTPClient.Do(req.WithContext(context.WithValue(req.Context(), contextKeyWebhookRequest, req))) + resp, err := webhookHTTPClient.Do(req) if err != nil { t.ResponseInfo.Body = fmt.Sprintf("Delivery: %v", err) return err @@ -295,29 +292,18 @@ func webhookProxy() func(req *http.Request) (*url.URL, error) { func InitDeliverHooks() { timeout := time.Duration(setting.Webhook.DeliverTimeout) * time.Second + allowedHostListValue := setting.Webhook.AllowedHostList + if allowedHostListValue == "" { + allowedHostListValue = hostmatcher.MatchBuiltinExternal + } + allowedHostMatcher := hostmatcher.ParseHostMatchList("webhook.ALLOWED_HOST_LIST", allowedHostListValue) + webhookHTTPClient = &http.Client{ Timeout: timeout, Transport: &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Webhook.SkipTLSVerify}, Proxy: webhookProxy(), - DialContext: func(ctx context.Context, network, addrOrHost string) (net.Conn, error) { - dialer := net.Dialer{ - Timeout: timeout, - Control: func(network, ipAddr string, c syscall.RawConn) error { - // 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) - req := ctx.Value(contextKeyWebhookRequest).(*http.Request) - 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 !setting.Webhook.AllowedHostList.MatchesHostOrIP(req.Host, tcpAddr.IP) { - 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 - }, - } - return dialer.DialContext(ctx, network, addrOrHost) - }, + DialContext: hostmatcher.NewDialContext("webhook", allowedHostMatcher, nil), }, } From c1c021fdd354f842d28613e06060de9c08a12634 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 10 Nov 2021 18:08:28 +0800 Subject: [PATCH 02/10] fix unit test --- modules/migrations/migrate.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/migrations/migrate.go b/modules/migrations/migrate.go index 2dce12538631..b3e88e42ff93 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -73,6 +73,10 @@ func IsMigrateURLAllowed(remoteURL string, doer *models.User) error { } hostName, _, err := net.SplitHostPort(u.Host) + if err != nil { + // u.Host can be "host" or "host:port" + hostName = u.Host + } if err != nil { return &models.ErrInvalidCloneAddr{Host: u.Host, IsURLError: true} } From ba3e16c4b4f9a6f73197f46805211cd1dd7b9bbb Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 10 Nov 2021 18:10:38 +0800 Subject: [PATCH 03/10] fix unit test --- integrations/api_repo_lfs_migrate_test.go | 3 ++ integrations/mirror_push_test.go | 2 + modules/hostmatcher/hostmatcher.go | 47 ++++++++++++----------- modules/hostmatcher/hostmatcher_test.go | 13 +++++-- modules/migrations/migrate.go | 17 +++++--- modules/migrations/migrate_test.go | 2 + 6 files changed, 53 insertions(+), 31 deletions(-) diff --git a/integrations/api_repo_lfs_migrate_test.go b/integrations/api_repo_lfs_migrate_test.go index 9acc96e4f473..01f26d782f91 100644 --- a/integrations/api_repo_lfs_migrate_test.go +++ b/integrations/api_repo_lfs_migrate_test.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/lfs" + "code.gitea.io/gitea/modules/migrations" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -25,6 +26,7 @@ func TestAPIRepoLFSMigrateLocal(t *testing.T) { oldAllowLocalNetworks := setting.Migrations.AllowLocalNetworks setting.ImportLocalPaths = true setting.Migrations.AllowLocalNetworks = true + _ = migrations.Init() user := db.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User) session := loginUser(t, user.Name) @@ -47,4 +49,5 @@ func TestAPIRepoLFSMigrateLocal(t *testing.T) { setting.ImportLocalPaths = oldImportLocalPaths setting.Migrations.AllowLocalNetworks = oldAllowLocalNetworks + _ = migrations.Init() } diff --git a/integrations/mirror_push_test.go b/integrations/mirror_push_test.go index 3d2966cfdcd4..740d013a42b2 100644 --- a/integrations/mirror_push_test.go +++ b/integrations/mirror_push_test.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/migrations" "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" mirror_service "code.gitea.io/gitea/services/mirror" @@ -29,6 +30,7 @@ func testMirrorPush(t *testing.T, u *url.URL) { defer prepareTestEnv(t)() setting.Migrations.AllowLocalNetworks = true + _ = migrations.Init() user := db.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) srcRepo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) diff --git a/modules/hostmatcher/hostmatcher.go b/modules/hostmatcher/hostmatcher.go index ceb9b8c081af..527c568d5ee8 100644 --- a/modules/hostmatcher/hostmatcher.go +++ b/modules/hostmatcher/hostmatcher.go @@ -5,7 +5,6 @@ package hostmatcher import ( - "fmt" "net" "path/filepath" "strings" @@ -19,9 +18,9 @@ type HostMatchList struct { SettingKeyHint string SettingValue string - // host name or built-in network name - names []string - ipNets []*net.IPNet + // patterns for host names or built-in networks + patterns []string + ipNets []*net.IPNet } // MatchBuiltinAll all hosts are matched @@ -48,17 +47,17 @@ func ParseHostMatchList(settingKeyHint string, hostList string) *HostMatchList { if err == nil { hl.ipNets = append(hl.ipNets, ipNet) } else { - hl.names = append(hl.names, s) + hl.patterns = append(hl.patterns, s) } } return hl } // ParseSimpleMatchList parse a simple matchlist (no built-in networks, no CIDR support) -func ParseSimpleMatchList(settingKeyHint string, matchList string, includeLocalNetwork bool) *HostMatchList { +func ParseSimpleMatchList(settingKeyHint string, matchList string) *HostMatchList { hl := &HostMatchList{ SettingKeyHint: settingKeyHint, - SettingValue: matchList + fmt.Sprintf("(local-network:%v)", includeLocalNetwork), + SettingValue: matchList, } for _, s := range strings.Split(matchList, ",") { s = strings.ToLower(strings.TrimSpace(s)) @@ -66,38 +65,40 @@ func ParseSimpleMatchList(settingKeyHint string, matchList string, includeLocalN 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:]) + // for built-in patterns, we convert it from "private" => "[p]rivate" for internal usage and keep the same result as `matchlist` + hl.patterns = append(hl.patterns, "["+s[:1]+"]"+s[1:]) } else { // we keep the same result as `matchlist`, so no CIDR support here - hl.names = append(hl.names, s) + hl.patterns = append(hl.patterns, s) } } - if includeLocalNetwork { - hl.names = append(hl.names, MatchBuiltinPrivate) - } return hl } +// AppendPattern appends more patterns to match +func (hl *HostMatchList) AppendPattern(pattern string) { + hl.patterns = append(hl.patterns, pattern) +} + // IsEmpty checks if the check list is empty func (hl *HostMatchList) IsEmpty() bool { - return hl == nil || (len(hl.names) == 0 && len(hl.ipNets) == 0) + return hl == nil || (len(hl.patterns) == 0 && len(hl.ipNets) == 0) } -func (hl *HostMatchList) checkNames(host string) bool { +func (hl *HostMatchList) checkPattern(host string) bool { host = strings.ToLower(strings.TrimSpace(host)) - for _, name := range hl.names { - switch name { + for _, pattern := range hl.patterns { + switch pattern { case "": case MatchBuiltinExternal: case MatchBuiltinPrivate: case MatchBuiltinLoopback: - // ignore empty string or built-in network names + // ignore empty string or built-in network patterns continue case MatchBuiltinAll: return true default: - if matched, _ := filepath.Match(name, host); matched { + if matched, _ := filepath.Match(pattern, host); matched { return true } } @@ -106,8 +107,8 @@ func (hl *HostMatchList) checkNames(host string) bool { } func (hl *HostMatchList) checkIP(ip net.IP) bool { - for _, name := range hl.names { - switch name { + for _, pattern := range hl.patterns { + switch pattern { case "": continue case MatchBuiltinAll: @@ -139,7 +140,7 @@ func (hl *HostMatchList) MatchHostName(host string) bool { if hl == nil { return false } - if hl.checkNames(host) { + if hl.checkPattern(host) { return true } if ip := net.ParseIP(host); ip != nil { @@ -154,7 +155,7 @@ func (hl *HostMatchList) MatchIPAddr(ip net.IP) bool { return false } host := ip.String() // nil-safe, we will get "" if ip is nil - return hl.checkNames(host) || hl.checkIP(ip) + return hl.checkPattern(host) || hl.checkIP(ip) } // MatchHostOrIP checks if the host or IP matches an allow/deny(block) list diff --git a/modules/hostmatcher/hostmatcher_test.go b/modules/hostmatcher/hostmatcher_test.go index d5e45c5b37e4..66030a32f1ed 100644 --- a/modules/hostmatcher/hostmatcher_test.go +++ b/modules/hostmatcher/hostmatcher_test.go @@ -132,22 +132,29 @@ func TestHostOrIPMatchesList(t *testing.T) { } test(cases) - hl = ParseSimpleMatchList("", "loopback, *.domain.com", true) + 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("192.168.1.1"), true}, {"", net.ParseIP("1.1.1.1"), false}, } test(cases) - hl = ParseSimpleMatchList("", "external", false) + 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/migrations/migrate.go b/modules/migrations/migrate.go index b3e88e42ff93..0b85d7a4c3b2 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -75,11 +75,9 @@ func IsMigrateURLAllowed(remoteURL string, doer *models.User) error { hostName, _, err := net.SplitHostPort(u.Host) if err != nil { // u.Host can be "host" or "host:port" + err = nil hostName = u.Host } - if err != nil { - return &models.ErrInvalidCloneAddr{Host: u.Host, IsURLError: true} - } addrList, err := net.LookupIP(hostName) if err != nil { return &models.ErrInvalidCloneAddr{Host: u.Host, NotResolvedIP: true} @@ -469,7 +467,16 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts // Init migrations service func Init() error { // TODO: maybe we can deprecate these legacy ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS/BLOCKED_DOMAINS, use ALLOWED_HOST_LIST/BLOCKED_HOST_LIST instead - allowList = hostmatcher.ParseSimpleMatchList("migrations.ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS", setting.Migrations.AllowedDomains, setting.Migrations.AllowLocalNetworks) - blockList = hostmatcher.ParseSimpleMatchList("migrations.BLOCKED_DOMAINS", setting.Migrations.BlockedDomains, false) + + blockList = hostmatcher.ParseSimpleMatchList("migrations.BLOCKED_DOMAINS", setting.Migrations.BlockedDomains) + + allowList = hostmatcher.ParseSimpleMatchList("migrations.ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS", setting.Migrations.AllowedDomains) + if allowList.IsEmpty() { + // the default policy is that migration module can access external hosts + allowList.AppendPattern(hostmatcher.MatchBuiltinExternal) + } + if setting.Migrations.AllowLocalNetworks { + allowList.AppendPattern(hostmatcher.MatchBuiltinPrivate) + } return nil } diff --git a/modules/migrations/migrate_test.go b/modules/migrations/migrate_test.go index 3cd0938ef6af..db472ce85994 100644 --- a/modules/migrations/migrate_test.go +++ b/modules/migrations/migrate_test.go @@ -22,6 +22,7 @@ func TestMigrateWhiteBlocklist(t *testing.T) { nonAdminUser := db.AssertExistsAndLoadBean(t, &models.User{Name: "user2"}).(*models.User) setting.Migrations.AllowedDomains = "github.com" + setting.Migrations.AllowLocalNetworks = false assert.NoError(t, Init()) err := IsMigrateURLAllowed("https://gitlab.com/gitlab/gitlab.git", nonAdminUser) @@ -47,6 +48,7 @@ func TestMigrateWhiteBlocklist(t *testing.T) { assert.Error(t, err) setting.Migrations.AllowLocalNetworks = true + assert.NoError(t, Init()) err = IsMigrateURLAllowed("https://10.0.0.1/go-gitea/gitea.git", nonAdminUser) assert.NoError(t, err) From af2cb2cd9415b037c27062ea4e38bc0237f98164 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 11 Nov 2021 01:36:07 +0800 Subject: [PATCH 04/10] fix unit test --- integrations/api_repo_test.go | 4 ++-- modules/migrations/migrate.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index b93319be338c..7f4c5129c42f 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -331,10 +331,10 @@ func TestAPIRepoMigrate(t *testing.T) { switch respJSON["message"] { case "Remote visit addressed rate limitation.": t.Log("test hit github rate limitation") - case "You are not allowed to import from private IPs.": + case "You can not import from disallowed hosts.": assert.EqualValues(t, "private-ip", testCase.repoName) default: - t.Errorf("unexpected error '%v' on url '%s'", respJSON["message"], testCase.cloneURL) + assert.Fail(t, "unexpected error '%v' on url '%s'", respJSON["message"], testCase.cloneURL) } } else { assert.EqualValues(t, testCase.expectedStatus, resp.Code) diff --git a/modules/migrations/migrate.go b/modules/migrations/migrate.go index 0b85d7a4c3b2..55c9fe1ac75a 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -477,6 +477,7 @@ func Init() error { } if setting.Migrations.AllowLocalNetworks { allowList.AppendPattern(hostmatcher.MatchBuiltinPrivate) + allowList.AppendPattern(hostmatcher.MatchBuiltinLoopback) } return nil } From 696a4e9b289b048d267e30cb5cba816c174ef9ad Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 12 Nov 2021 21:10:57 +0800 Subject: [PATCH 05/10] refactor --- modules/hostmatcher/hostmatcher.go | 54 ++++++++++++------------------ 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/modules/hostmatcher/hostmatcher.go b/modules/hostmatcher/hostmatcher.go index 527c568d5ee8..59bb8db5bbcf 100644 --- a/modules/hostmatcher/hostmatcher.go +++ b/modules/hostmatcher/hostmatcher.go @@ -13,19 +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 { SettingKeyHint string SettingValue string - // patterns for host names or built-in networks + // builtins networks + builtins []string + // patterns for host names (with wildcard support) patterns []string - ipNets []*net.IPNet + // 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" @@ -35,6 +34,10 @@ 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(settingKeyHint string, hostList string) *HostMatchList { hl := &HostMatchList{SettingKeyHint: settingKeyHint, SettingValue: hostList} @@ -46,6 +49,8 @@ func ParseHostMatchList(settingKeyHint string, 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.patterns = append(hl.patterns, s) } @@ -53,7 +58,7 @@ func ParseHostMatchList(settingKeyHint string, hostList string) *HostMatchList { return hl } -// ParseSimpleMatchList parse a simple matchlist (no built-in networks, no CIDR support) +// 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, @@ -64,13 +69,8 @@ func ParseSimpleMatchList(settingKeyHint string, matchList string) *HostMatchLis if s == "" { continue } - if s == MatchBuiltinLoopback || s == MatchBuiltinPrivate || s == MatchBuiltinExternal { - // for built-in patterns, we convert it from "private" => "[p]rivate" for internal usage and keep the same result as `matchlist` - hl.patterns = append(hl.patterns, "["+s[:1]+"]"+s[1:]) - } else { - // we keep the same result as `matchlist`, so no CIDR support here - hl.patterns = append(hl.patterns, s) - } + // 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 } @@ -80,27 +80,16 @@ func (hl *HostMatchList) AppendPattern(pattern string) { hl.patterns = append(hl.patterns, pattern) } -// IsEmpty checks if the check list is empty +// IsEmpty checks if the checklist is empty func (hl *HostMatchList) IsEmpty() bool { - return hl == nil || (len(hl.patterns) == 0 && len(hl.ipNets) == 0) + 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 { - switch pattern { - case "": - case MatchBuiltinExternal: - case MatchBuiltinPrivate: - case MatchBuiltinLoopback: - // ignore empty string or built-in network patterns - continue - case MatchBuiltinAll: + if matched, _ := filepath.Match(pattern, host); matched { return true - default: - if matched, _ := filepath.Match(pattern, host); matched { - return true - } } } return false @@ -108,11 +97,12 @@ func (hl *HostMatchList) checkPattern(host string) bool { func (hl *HostMatchList) checkIP(ip net.IP) bool { for _, pattern := range hl.patterns { - switch pattern { - case "": - continue - case MatchBuiltinAll: + if pattern == "*" { return true + } + } + for _, builtin := range hl.builtins { + switch builtin { case MatchBuiltinExternal: if ip.IsGlobalUnicast() && !util.IsIPPrivate(ip) { return true From 5b17f2320e8790e12efe0c389dbe30fb1a4fac9c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 12 Nov 2021 22:58:36 +0800 Subject: [PATCH 06/10] fix simple matchlist --- modules/hostmatcher/hostmatcher.go | 6 +++--- modules/migrations/migrate.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/hostmatcher/hostmatcher.go b/modules/hostmatcher/hostmatcher.go index 59bb8db5bbcf..815b8c4e4a92 100644 --- a/modules/hostmatcher/hostmatcher.go +++ b/modules/hostmatcher/hostmatcher.go @@ -75,9 +75,9 @@ func ParseSimpleMatchList(settingKeyHint string, matchList string) *HostMatchLis return hl } -// AppendPattern appends more patterns to match -func (hl *HostMatchList) AppendPattern(pattern string) { - hl.patterns = append(hl.patterns, pattern) +// 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 diff --git a/modules/migrations/migrate.go b/modules/migrations/migrate.go index 55c9fe1ac75a..f178b7f38eed 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -473,11 +473,11 @@ func Init() error { allowList = hostmatcher.ParseSimpleMatchList("migrations.ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS", setting.Migrations.AllowedDomains) if allowList.IsEmpty() { // the default policy is that migration module can access external hosts - allowList.AppendPattern(hostmatcher.MatchBuiltinExternal) + allowList.AppendBuiltin(hostmatcher.MatchBuiltinExternal) } if setting.Migrations.AllowLocalNetworks { - allowList.AppendPattern(hostmatcher.MatchBuiltinPrivate) - allowList.AppendPattern(hostmatcher.MatchBuiltinLoopback) + allowList.AppendBuiltin(hostmatcher.MatchBuiltinPrivate) + allowList.AppendBuiltin(hostmatcher.MatchBuiltinLoopback) } return nil } From 8a7ea658c743555aa18f6bc9ecfbd5587c44e8f4 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 17 Nov 2021 10:49:55 +0800 Subject: [PATCH 07/10] fix merge conflicts and unit test --- integrations/api_repo_lfs_migrate_test.go | 6 +++--- integrations/mirror_push_test.go | 4 ++-- services/migrations/gitea_downloader.go | 2 -- services/migrations/gitlab.go | 4 +--- {modules => services}/migrations/http_client.go | 0 services/migrations/migrate.go | 1 - 6 files changed, 6 insertions(+), 11 deletions(-) rename {modules => services}/migrations/http_client.go (100%) diff --git a/integrations/api_repo_lfs_migrate_test.go b/integrations/api_repo_lfs_migrate_test.go index 34043d99c83b..2873b425c5e1 100644 --- a/integrations/api_repo_lfs_migrate_test.go +++ b/integrations/api_repo_lfs_migrate_test.go @@ -12,9 +12,9 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/lfs" - "code.gitea.io/gitea/modules/migrations" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/services/migrations" "github.com/stretchr/testify/assert" ) @@ -26,7 +26,7 @@ func TestAPIRepoLFSMigrateLocal(t *testing.T) { oldAllowLocalNetworks := setting.Migrations.AllowLocalNetworks setting.ImportLocalPaths = true setting.Migrations.AllowLocalNetworks = true - _ = migrations.Init() + assert.NoError(t, migrations.Init()) user := unittest.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User) session := loginUser(t, user.Name) @@ -49,5 +49,5 @@ func TestAPIRepoLFSMigrateLocal(t *testing.T) { setting.ImportLocalPaths = oldImportLocalPaths setting.Migrations.AllowLocalNetworks = oldAllowLocalNetworks - _ = migrations.Init() + assert.NoError(t, migrations.Init()) // reset old migration settings } diff --git a/integrations/mirror_push_test.go b/integrations/mirror_push_test.go index d9869c3f7ab7..492e2c23eef6 100644 --- a/integrations/mirror_push_test.go +++ b/integrations/mirror_push_test.go @@ -14,9 +14,9 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/migrations" "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/services/migrations" mirror_service "code.gitea.io/gitea/services/mirror" "github.com/stretchr/testify/assert" @@ -30,7 +30,7 @@ func testMirrorPush(t *testing.T, u *url.URL) { defer prepareTestEnv(t)() setting.Migrations.AllowLocalNetworks = true - _ = migrations.Init() + assert.NoError(t, migrations.Init()) user := unittest.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) srcRepo := unittest.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) diff --git a/services/migrations/gitea_downloader.go b/services/migrations/gitea_downloader.go index 48ab41f8793f..163e592cf9cc 100644 --- a/services/migrations/gitea_downloader.go +++ b/services/migrations/gitea_downloader.go @@ -17,8 +17,6 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" base "code.gitea.io/gitea/modules/migration" - "code.gitea.io/gitea/modules/proxy" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" gitea_sdk "code.gitea.io/sdk/gitea" diff --git a/services/migrations/gitlab.go b/services/migrations/gitlab.go index 9ce412c0fd2a..4eb7e3e47c98 100644 --- a/services/migrations/gitlab.go +++ b/services/migrations/gitlab.go @@ -17,8 +17,6 @@ import ( "code.gitea.io/gitea/modules/log" base "code.gitea.io/gitea/modules/migration" - "code.gitea.io/gitea/modules/proxy" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" "github.com/xanzy/go-gitlab" @@ -80,7 +78,7 @@ func NewGitlabDownloader(ctx context.Context, baseURL, repoPath, username, passw // Only use basic auth if token is blank and password is NOT // Basic auth will fail with empty strings, but empty token will allow anonymous public API usage if token == "" && password != "" { - gitlabClient, err = gitlab.NewBasicAuthClient(username, password, gitlab.WithBaseURL(baseURL)) + gitlabClient, err = gitlab.NewBasicAuthClient(username, password, gitlab.WithBaseURL(baseURL), gitlab.WithHTTPClient(NewMigrationHTTPClient())) } if err != nil { diff --git a/modules/migrations/http_client.go b/services/migrations/http_client.go similarity index 100% rename from modules/migrations/http_client.go rename to services/migrations/http_client.go diff --git a/services/migrations/migrate.go b/services/migrations/migrate.go index 7758331aff8a..69d128b03d52 100644 --- a/services/migrations/migrate.go +++ b/services/migrations/migrate.go @@ -16,7 +16,6 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/hostmatcher" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/matchlist" base "code.gitea.io/gitea/modules/migration" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" From ff56fb62d5df55899d80807fcc8b9d5c18a614fb Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 18 Nov 2021 19:54:34 +0800 Subject: [PATCH 08/10] fix lint --- services/migrations/migrate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/migrations/migrate.go b/services/migrations/migrate.go index 7ee692e1f6d9..3e805f0b71eb 100644 --- a/services/migrations/migrate.go +++ b/services/migrations/migrate.go @@ -76,7 +76,7 @@ func IsMigrateURLAllowed(remoteURL string, doer *models.User) error { hostName, _, err := net.SplitHostPort(u.Host) if err != nil { // u.Host can be "host" or "host:port" - err = nil + err = nil //nolint hostName = u.Host } addrList, err := net.LookupIP(hostName) From 5ca9912e33694c38c470b822b8d7fe8c875563d5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 19 Nov 2021 10:18:38 +0800 Subject: [PATCH 09/10] use `webhookHTTPClient.Do(req.WithContext(graceful.GetManager().ShutdownContext()))` --- services/webhook/deliver.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/webhook/deliver.go b/services/webhook/deliver.go index cbd990186b88..36169baad469 100644 --- a/services/webhook/deliver.go +++ b/services/webhook/deliver.go @@ -95,10 +95,10 @@ func Deliver(t *webhook_model.HookTask) error { return err } default: - return fmt.Errorf("Invalid http method for webhook: [%d] %v", t.ID, w.HTTPMethod) + return fmt.Errorf("invalid http method for webhook: [%d] %v", t.ID, w.HTTPMethod) } default: - return fmt.Errorf("Invalid http method for webhook: [%d] %v", t.ID, w.HTTPMethod) + return fmt.Errorf("invalid http method for webhook: [%d] %v", t.ID, w.HTTPMethod) } var signatureSHA1 string @@ -169,10 +169,10 @@ func Deliver(t *webhook_model.HookTask) error { }() if setting.DisableWebhooks { - return fmt.Errorf("Webhook task skipped (webhooks disabled): [%d]", t.ID) + return fmt.Errorf("webhook task skipped (webhooks disabled): [%d]", t.ID) } - resp, err := webhookHTTPClient.Do(req) + resp, err := webhookHTTPClient.Do(req.WithContext(graceful.GetManager().ShutdownContext())) if err != nil { t.ResponseInfo.Body = fmt.Sprintf("Delivery: %v", err) return err From 692f121f644ff8f974dc3e1f19a84082ca2f2a52 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 19 Nov 2021 22:57:54 +0800 Subject: [PATCH 10/10] fix documents --- custom/conf/app.example.ini | 2 +- docs/content/doc/advanced/config-cheat-sheet.en-us.md | 2 +- docs/content/doc/advanced/config-cheat-sheet.zh-cn.md | 2 +- options/locale/locale_en-US.ini | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 9643e396b698..ac5dd3e23a47 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2101,7 +2101,7 @@ PATH = ;ALLOWED_DOMAINS = ;; ;; Blocklist for migrating, default is blank. Multiple domains could be separated by commas. -;; When ALLOWED_DOMAINS is not blank, this option will be ignored. +;; When ALLOWED_DOMAINS is not blank, this option has a higher priority to deny domains. ;BLOCKED_DOMAINS = ;; ;; Allow private addresses defined by RFC 1918, RFC 1122, RFC 4632 and RFC 4291 (false by default) 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 a087b253e99a..feb233d01b3b 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -1008,7 +1008,7 @@ Task queue configuration has been moved to `queue.task`. However, the below conf - `MAX_ATTEMPTS`: **3**: Max attempts per http/https request on migrations. - `RETRY_BACKOFF`: **3**: Backoff time per http/https request retry (seconds) - `ALLOWED_DOMAINS`: **\**: Domains allowlist for migrating repositories, default is blank. It means everything will be allowed. Multiple domains could be separated by commas. -- `BLOCKED_DOMAINS`: **\**: Domains blocklist for migrating repositories, default is blank. Multiple domains could be separated by commas. When `ALLOWED_DOMAINS` is not blank, this option will be ignored. +- `BLOCKED_DOMAINS`: **\**: Domains blocklist for migrating repositories, default is blank. Multiple domains could be separated by commas. When `ALLOWED_DOMAINS` is not blank, this option has a higher priority to deny domains. - `ALLOW_LOCALNETWORKS`: **false**: Allow private addresses defined by RFC 1918, RFC 1122, RFC 4632 and RFC 4291 - `SKIP_TLS_VERIFY`: **false**: Allow skip tls verify diff --git a/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md b/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md index fcbf49f1ab8b..7e02596f7bae 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md +++ b/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md @@ -335,7 +335,7 @@ IS_INPUT_FILE = false - `MAX_ATTEMPTS`: **3**: 在迁移过程中的 http/https 请求重试次数。 - `RETRY_BACKOFF`: **3**: 等待下一次重试的时间,单位秒。 - `ALLOWED_DOMAINS`: **\**: 迁移仓库的域名白名单,默认为空,表示允许从任意域名迁移仓库,多个域名用逗号分隔。 -- `BLOCKED_DOMAINS`: **\**: 迁移仓库的域名黑名单,默认为空,多个域名用逗号分隔。如果 `ALLOWED_DOMAINS` 不为空,此选项将会被忽略。 +- `BLOCKED_DOMAINS`: **\**: 迁移仓库的域名黑名单,默认为空,多个域名用逗号分隔。如果 `ALLOWED_DOMAINS` 不为空,此选项有更高的优先级拒绝这里的域名。 - `ALLOW_LOCALNETWORKS`: **false**: Allow private addresses defined by RFC 1918 - `SKIP_TLS_VERIFY`: **false**: 允许忽略 TLS 认证 diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 3864d298aac8..624f1b4d1b9f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -899,7 +899,7 @@ migrate.clone_address_desc = The HTTP(S) or Git 'clone' URL of an existing repos migrate.github_token_desc = You can put one or more tokens with comma separated here to make migrating faster because of Github API rate limit. WARN: Abusing this feature may violate the service provider's policy and lead to account blocking. migrate.clone_local_path = or a local server path migrate.permission_denied = You are not allowed to import local repositories. -migrate.permission_denied_blocked = You can not import from disallowed hosts, please check your ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS/BLOCKED_DOMAINS settings. +migrate.permission_denied_blocked = You can not import from disallowed hosts, please ask the admin to check ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS/BLOCKED_DOMAINS settings. migrate.invalid_local_path = "The local path is invalid. It does not exist or is not a directory." migrate.invalid_lfs_endpoint = The LFS endpoint is not valid. migrate.failed = Migration failed: %v