fix: correctly check denied domains and ips for actions (#8810)

# Which Problems Are Solved

System administrators can block hosts and IPs for HTTP calls in actions.
Using DNS, blocked IPs could be bypassed.

# How the Problems Are Solved

- Hosts are resolved (DNS lookup) to check whether their corresponding
IP is blocked.

# Additional Changes

- Added complete lookup ip address range and "unspecified" address to
the default `DenyList`

(cherry picked from commit 79fb4cc1cc6ebba91f9af917807e0e3651516acd)
This commit is contained in:
Livio Spring 2024-10-22 16:16:44 +02:00
parent 27ab1a22e7
commit 7508e6c9f3
No known key found for this signature in database
GPG Key ID: 26BB1C2FA5952CF0
5 changed files with 95 additions and 60 deletions

View File

@ -600,7 +600,10 @@ Actions:
# Wildcard sub domains are currently unsupported # Wildcard sub domains are currently unsupported
DenyList: # ZITADEL_ACTIONS_HTTP_DENYLIST (comma separated list) DenyList: # ZITADEL_ACTIONS_HTTP_DENYLIST (comma separated list)
- localhost - localhost
- "127.0.0.1" - "127.0.0.0/8"
- "::1"
- "0.0.0.0"
- "::"
LogStore: LogStore:
Access: Access:

View File

@ -47,9 +47,9 @@ Log:
`}, `},
want: func(t *testing.T, config *Config) { want: func(t *testing.T, config *Config) {
assert.Equal(t, config.Actions.HTTP.DenyList, []actions.AddressChecker{ assert.Equal(t, config.Actions.HTTP.DenyList, []actions.AddressChecker{
&actions.DomainChecker{Domain: "localhost"}, &actions.HostChecker{Domain: "localhost"},
&actions.IPChecker{IP: net.ParseIP("127.0.0.1")}, &actions.HostChecker{IP: net.ParseIP("127.0.0.1")},
&actions.DomainChecker{Domain: "foobar"}}) &actions.HostChecker{Domain: "foobar"}})
}, },
}, { }, {
name: "actions deny list string ok", name: "actions deny list string ok",
@ -63,9 +63,9 @@ Log:
`}, `},
want: func(t *testing.T, config *Config) { want: func(t *testing.T, config *Config) {
assert.Equal(t, config.Actions.HTTP.DenyList, []actions.AddressChecker{ assert.Equal(t, config.Actions.HTTP.DenyList, []actions.AddressChecker{
&actions.DomainChecker{Domain: "localhost"}, &actions.HostChecker{Domain: "localhost"},
&actions.IPChecker{IP: net.ParseIP("127.0.0.1")}, &actions.HostChecker{IP: net.ParseIP("127.0.0.1")},
&actions.DomainChecker{Domain: "foobar"}}) &actions.HostChecker{Domain: "foobar"}})
}, },
}, { }, {
name: "features ok", name: "features ok",

View File

@ -5,6 +5,7 @@ import (
"context" "context"
"encoding/json" "encoding/json"
"io" "io"
"net"
"net/http" "net/http"
"net/url" "net/url"
"strings" "strings"
@ -19,7 +20,7 @@ import (
func WithHTTP(ctx context.Context) Option { func WithHTTP(ctx context.Context) Option {
return func(c *runConfig) { return func(c *runConfig) {
c.modules["zitadel/http"] = func(runtime *goja.Runtime, module *goja.Object) { c.modules["zitadel/http"] = func(runtime *goja.Runtime, module *goja.Object) {
requireHTTP(ctx, &http.Client{Transport: new(transport)}, runtime, module) requireHTTP(ctx, &http.Client{Transport: &transport{lookup: net.LookupIP}}, runtime, module)
} }
} }
} }
@ -170,21 +171,34 @@ func parseHeaders(headers *goja.Object) http.Header {
return h return h
} }
type transport struct{} type transport struct {
lookup func(string) ([]net.IP, error)
}
func (*transport) RoundTrip(req *http.Request) (*http.Response, error) { func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) {
if httpConfig == nil { if httpConfig == nil {
return http.DefaultTransport.RoundTrip(req) return http.DefaultTransport.RoundTrip(req)
} }
if isHostBlocked(httpConfig.DenyList, req.URL) { if t.isHostBlocked(httpConfig.DenyList, req.URL) {
return nil, zerrors.ThrowInvalidArgument(nil, "ACTIO-N72d0", "host is denied") return nil, zerrors.ThrowInvalidArgument(nil, "ACTIO-N72d0", "host is denied")
} }
return http.DefaultTransport.RoundTrip(req) return http.DefaultTransport.RoundTrip(req)
} }
func isHostBlocked(denyList []AddressChecker, address *url.URL) bool { func (t *transport) isHostBlocked(denyList []AddressChecker, address *url.URL) bool {
host := address.Hostname()
ip := net.ParseIP(host)
ips := []net.IP{ip}
// if the hostname is a domain, we need to check resolve the ip(s), since it might be denied
if ip == nil {
var err error
ips, err = t.lookup(host)
if err != nil {
return true
}
}
for _, blocked := range denyList { for _, blocked := range denyList {
if blocked.Matches(address.Hostname()) { if blocked.Matches(ips, host) {
return true return true
} }
} }
@ -192,5 +206,5 @@ func isHostBlocked(denyList []AddressChecker, address *url.URL) bool {
} }
type AddressChecker interface { type AddressChecker interface {
Matches(string) bool Matches([]net.IP, string) bool
} }

View File

@ -6,8 +6,6 @@ import (
"strings" "strings"
"github.com/mitchellh/mapstructure" "github.com/mitchellh/mapstructure"
"github.com/zitadel/zitadel/internal/zerrors"
) )
func SetHTTPConfig(config *HTTPConfig) { func SetHTTPConfig(config *HTTPConfig) {
@ -48,7 +46,7 @@ func HTTPConfigDecodeHook(from, to reflect.Value) (interface{}, error) {
for _, unsplit := range config.DenyList { for _, unsplit := range config.DenyList {
for _, split := range strings.Split(unsplit, ",") { for _, split := range strings.Split(unsplit, ",") {
parsed, parseErr := parseDenyListEntry(split) parsed, parseErr := NewHostChecker(split)
if parseErr != nil { if parseErr != nil {
return nil, parseErr return nil, parseErr
} }
@ -61,46 +59,36 @@ func HTTPConfigDecodeHook(from, to reflect.Value) (interface{}, error) {
return c, nil return c, nil
} }
func parseDenyListEntry(entry string) (AddressChecker, error) { func NewHostChecker(entry string) (AddressChecker, error) {
if checker, err := NewIPChecker(entry); err == nil { _, network, err := net.ParseCIDR(entry)
return checker, nil
}
return &DomainChecker{Domain: entry}, nil
}
func NewIPChecker(i string) (AddressChecker, error) {
_, network, err := net.ParseCIDR(i)
if err == nil { if err == nil {
return &IPChecker{Net: network}, nil return &HostChecker{Net: network}, nil
} }
if ip := net.ParseIP(i); ip != nil { if ip := net.ParseIP(entry); ip != nil {
return &IPChecker{IP: ip}, nil return &HostChecker{IP: ip}, nil
} }
return nil, zerrors.ThrowInvalidArgument(nil, "ACTIO-ddJ7h", "invalid ip") return &HostChecker{Domain: entry}, nil
} }
type IPChecker struct { type HostChecker struct {
Net *net.IPNet Net *net.IPNet
IP net.IP IP net.IP
}
func (c *IPChecker) Matches(address string) bool {
ip := net.ParseIP(address)
if ip == nil {
return false
}
if c.IP != nil {
return c.IP.Equal(ip)
}
return c.Net.Contains(ip)
}
type DomainChecker struct {
Domain string Domain string
} }
func (c *DomainChecker) Matches(domain string) bool { func (c *HostChecker) Matches(ips []net.IP, address string) bool {
//TODO: allow wild cards // if the address matches the domain, no additional checks as needed
return c.Domain == domain if c.Domain == address {
return true
}
// otherwise we need to check on ips (incl. the resolved ips of the host)
for _, ip := range ips {
if c.Net != nil && c.Net.Contains(ip) {
return true
}
if c.IP != nil && c.IP.Equal(ip) {
return true
}
}
return false
} }

View File

@ -4,6 +4,7 @@ import (
"bytes" "bytes"
"context" "context"
"io" "io"
"net"
"net/http" "net/http"
"net/url" "net/url"
"reflect" "reflect"
@ -19,17 +20,21 @@ import (
func Test_isHostBlocked(t *testing.T) { func Test_isHostBlocked(t *testing.T) {
SetLogstoreService(logstore.New[*record.ExecutionLog](nil, nil)) SetLogstoreService(logstore.New[*record.ExecutionLog](nil, nil))
var denyList = []AddressChecker{ var denyList = []AddressChecker{
mustNewIPChecker(t, "192.168.5.0/24"), mustNewHostChecker(t, "192.168.5.0/24"),
mustNewIPChecker(t, "127.0.0.1"), mustNewHostChecker(t, "127.0.0.1"),
&DomainChecker{Domain: "test.com"}, mustNewHostChecker(t, "test.com"),
}
type fields struct {
lookup func(host string) ([]net.IP, error)
} }
type args struct { type args struct {
address *url.URL address *url.URL
} }
tests := []struct { tests := []struct {
name string name string
args args fields fields
want bool args args
want bool
}{ }{
{ {
name: "in range", name: "in range",
@ -47,6 +52,11 @@ func Test_isHostBlocked(t *testing.T) {
}, },
{ {
name: "address match", name: "address match",
fields: fields{
lookup: func(host string) ([]net.IP, error) {
return []net.IP{net.ParseIP("194.264.52.4")}, nil
},
},
args: args{ args: args{
address: mustNewURL(t, "https://test.com:42/hodor"), address: mustNewURL(t, "https://test.com:42/hodor"),
}, },
@ -54,24 +64,44 @@ func Test_isHostBlocked(t *testing.T) {
}, },
{ {
name: "address not match", name: "address not match",
fields: fields{
lookup: func(host string) ([]net.IP, error) {
return []net.IP{net.ParseIP("194.264.52.4")}, nil
},
},
args: args{ args: args{
address: mustNewURL(t, "https://test2.com/hodor"), address: mustNewURL(t, "https://test2.com/hodor"),
}, },
want: false, want: false,
}, },
{
name: "looked up ip matches",
fields: fields{
lookup: func(host string) ([]net.IP, error) {
return []net.IP{net.ParseIP("127.0.0.1")}, nil
},
},
args: args{
address: mustNewURL(t, "https://test2.com/hodor"),
},
want: true,
},
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
if got := isHostBlocked(denyList, tt.args.address); got != tt.want { trans := &transport{
lookup: tt.fields.lookup,
}
if got := trans.isHostBlocked(denyList, tt.args.address); got != tt.want {
t.Errorf("isHostBlocked() = %v, want %v", got, tt.want) t.Errorf("isHostBlocked() = %v, want %v", got, tt.want)
} }
}) })
} }
} }
func mustNewIPChecker(t *testing.T, ip string) AddressChecker { func mustNewHostChecker(t *testing.T, ip string) AddressChecker {
t.Helper() t.Helper()
checker, err := NewIPChecker(ip) checker, err := NewHostChecker(ip)
if err != nil { if err != nil {
t.Errorf("unable to parse cidr of %q because: %v", ip, err) t.Errorf("unable to parse cidr of %q because: %v", ip, err)
t.FailNow() t.FailNow()