mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2024-10-31 22:38:58 +00:00
Backport #27655 by @wolfogre When `webhook.PROXY_URL` has been set, the old code will check if the proxy host is in `ALLOWED_HOST_LIST` or reject requests through the proxy. It requires users to add the proxy host to `ALLOWED_HOST_LIST`. However, it actually allows all requests to any port on the host, when the proxy host is probably an internal address. But things may be even worse. `ALLOWED_HOST_LIST` doesn't really work when requests are sent to the allowed proxy, and the proxy could forward them to any hosts. This PR fixes it by: - If the proxy has been set, always allow connectioins to the host and port. - Check `ALLOWED_HOST_LIST` before forwarding. Co-authored-by: Jason Song <i@wolfogre.com>
This commit is contained in:
parent
5b80157aad
commit
dab40cd5f4
3 changed files with 72 additions and 20 deletions
|
@ -7,12 +7,17 @@ import (
|
|||
"context"
|
||||
"fmt"
|
||||
"net"
|
||||
"net/url"
|
||||
"syscall"
|
||||
"time"
|
||||
)
|
||||
|
||||
// NewDialContext returns a DialContext for Transport, the DialContext will do allow/block list check
|
||||
func NewDialContext(usage string, allowList, blockList *HostMatchList) func(ctx context.Context, network, addr string) (net.Conn, error) {
|
||||
return NewDialContextWithProxy(usage, allowList, blockList, nil)
|
||||
}
|
||||
|
||||
func NewDialContextWithProxy(usage string, allowList, blockList *HostMatchList, proxy *url.URL) 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
|
||||
|
@ -26,11 +31,18 @@ func NewDialContext(usage string, allowList, blockList *HostMatchList) func(ctx
|
|||
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 {
|
||||
Control: func(network, ipAddr string, c syscall.RawConn) error {
|
||||
host, port, err := net.SplitHostPort(addrOrHost)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if proxy != nil {
|
||||
// Always allow the host of the proxy, but only on the specified port.
|
||||
if host == proxy.Hostname() && port == proxy.Port() {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
// 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 {
|
||||
|
|
|
@ -239,7 +239,7 @@ var (
|
|||
hostMatchers []glob.Glob
|
||||
)
|
||||
|
||||
func webhookProxy() func(req *http.Request) (*url.URL, error) {
|
||||
func webhookProxy(allowList *hostmatcher.HostMatchList) func(req *http.Request) (*url.URL, error) {
|
||||
if setting.Webhook.ProxyURL == "" {
|
||||
return proxy.Proxy()
|
||||
}
|
||||
|
@ -257,6 +257,9 @@ func webhookProxy() func(req *http.Request) (*url.URL, error) {
|
|||
return func(req *http.Request) (*url.URL, error) {
|
||||
for _, v := range hostMatchers {
|
||||
if v.Match(req.URL.Host) {
|
||||
if !allowList.MatchHostName(req.URL.Host) {
|
||||
return nil, fmt.Errorf("webhook can only call allowed HTTP servers (check your %s setting), deny '%s'", allowList.SettingKeyHint, req.URL.Host)
|
||||
}
|
||||
return http.ProxyURL(setting.Webhook.ProxyURLFixed)(req)
|
||||
}
|
||||
}
|
||||
|
@ -278,8 +281,8 @@ func Init() error {
|
|||
Timeout: timeout,
|
||||
Transport: &http.Transport{
|
||||
TLSClientConfig: &tls.Config{InsecureSkipVerify: setting.Webhook.SkipTLSVerify},
|
||||
Proxy: webhookProxy(),
|
||||
DialContext: hostmatcher.NewDialContext("webhook", allowedHostMatcher, nil),
|
||||
Proxy: webhookProxy(allowedHostMatcher),
|
||||
DialContext: hostmatcher.NewDialContextWithProxy("webhook", allowedHostMatcher, nil, setting.Webhook.ProxyURLFixed),
|
||||
},
|
||||
}
|
||||
|
||||
|
|
|
@ -14,35 +14,72 @@ import (
|
|||
"code.gitea.io/gitea/models/db"
|
||||
"code.gitea.io/gitea/models/unittest"
|
||||
webhook_model "code.gitea.io/gitea/models/webhook"
|
||||
"code.gitea.io/gitea/modules/hostmatcher"
|
||||
"code.gitea.io/gitea/modules/setting"
|
||||
api "code.gitea.io/gitea/modules/structs"
|
||||
webhook_module "code.gitea.io/gitea/modules/webhook"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestWebhookProxy(t *testing.T) {
|
||||
oldWebhook := setting.Webhook
|
||||
t.Cleanup(func() {
|
||||
setting.Webhook = oldWebhook
|
||||
})
|
||||
|
||||
setting.Webhook.ProxyURL = "http://localhost:8080"
|
||||
setting.Webhook.ProxyURLFixed, _ = url.Parse(setting.Webhook.ProxyURL)
|
||||
setting.Webhook.ProxyHosts = []string{"*.discordapp.com", "discordapp.com"}
|
||||
|
||||
kases := map[string]string{
|
||||
"https://discordapp.com/api/webhooks/xxxxxxxxx/xxxxxxxxxxxxxxxxxxx": "http://localhost:8080",
|
||||
"http://s.discordapp.com/assets/xxxxxx": "http://localhost:8080",
|
||||
"http://github.com/a/b": "",
|
||||
allowedHostMatcher := hostmatcher.ParseHostMatchList("webhook.ALLOWED_HOST_LIST", "discordapp.com,s.discordapp.com")
|
||||
|
||||
tests := []struct {
|
||||
req string
|
||||
want string
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
req: "https://discordapp.com/api/webhooks/xxxxxxxxx/xxxxxxxxxxxxxxxxxxx",
|
||||
want: "http://localhost:8080",
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
req: "http://s.discordapp.com/assets/xxxxxx",
|
||||
want: "http://localhost:8080",
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
req: "http://github.com/a/b",
|
||||
want: "",
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
req: "http://www.discordapp.com/assets/xxxxxx",
|
||||
want: "",
|
||||
wantErr: true,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.req, func(t *testing.T) {
|
||||
req, err := http.NewRequest("POST", tt.req, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
for reqURL, proxyURL := range kases {
|
||||
req, err := http.NewRequest("POST", reqURL, nil)
|
||||
assert.NoError(t, err)
|
||||
u, err := webhookProxy(allowedHostMatcher)(req)
|
||||
if tt.wantErr {
|
||||
assert.Error(t, err)
|
||||
return
|
||||
}
|
||||
|
||||
u, err := webhookProxy()(req)
|
||||
assert.NoError(t, err)
|
||||
if proxyURL == "" {
|
||||
assert.Nil(t, u)
|
||||
} else {
|
||||
assert.EqualValues(t, proxyURL, u.String())
|
||||
}
|
||||
assert.NoError(t, err)
|
||||
|
||||
got := ""
|
||||
if u != nil {
|
||||
got = u.String()
|
||||
}
|
||||
assert.Equal(t, tt.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue