domi/forgejo-domi
1
0
Fork 0
forked from forgejo/forgejo
Code Pull requests Activity

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

Browse source 
Use hostmacher to replace matchlist.

And we introduce a better DialContext to do a full host/IP check, otherwise the attackers can still bypass the allow/block list by a 302 redirection.
This commit is contained in:
wxiaoguang 2021-11-20 17:34:05 +08:00 • committed by GitHub
parent c96be0cd98
commit 013fb73068
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
33 changed files with 377 additions and 293 deletions
Download patch file Download diff file Expand all files Collapse all files

73
services/migrations/migrate.go
View file

@ -15,8 +15,8 @@ import (
"code.gitea.io/gitea/models"
admin_model "code.gitea.io/gitea/models/admin"
"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"
@ -28,8 +28,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
@ -73,30 +73,35 @@ 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) {
hostName, _, err := net.SplitHostPort(u.Host)
if err != nil {
// u.Host can be "host" or "host:port"
err = nil //nolint
hostName = u.Host
}
addrList, err := net.LookupIP(hostName)
if err != nil {
return &models.ErrInvalidCloneAddr{Host: u.Host, NotResolvedIP: 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}
}
}
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}
}
}
}
return nil
// otherwise, we always follow the blocked list
return blockedError
}
// MigrateRepository migrate repository according MigrateOptions
@ -462,16 +467,18 @@ 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)
}
// TODO: maybe we can deprecate these legacy ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS/BLOCKED_DOMAINS, use ALLOWED_HOST_LIST/BLOCKED_HOST_LIST instead
blockList, err = matchlist.NewMatchlist(setting.Migrations.BlockedDomains...)
if err != nil {
return fmt.Errorf("init migration blockList domains failed: %v", err)
}
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.AppendBuiltin(hostmatcher.MatchBuiltinExternal)
}
if setting.Migrations.AllowLocalNetworks {
allowList.AppendBuiltin(hostmatcher.MatchBuiltinPrivate)
allowList.AppendBuiltin(hostmatcher.MatchBuiltinLoopback)
}
return nil
}