diff --git a/apps/api/test-integration-api.yaml b/apps/api/test-integration-api.yaml index 919aaffa8..2f921f611 100644 --- a/apps/api/test-integration-api.yaml +++ b/apps/api/test-integration-api.yaml @@ -76,6 +76,9 @@ LogStore: Stdout: Enabled: true +Executions: + DenyList: # ZITADEL_EXECUTIONS_DENYLIST (comma separated list) + Projections: HandleActiveInstances: 30m RequeueEvery: 20s diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index 6695bf4c8..ee525ac2c 100644 --- a/cmd/defaults.yaml +++ b/cmd/defaults.yaml @@ -574,6 +574,14 @@ Executions: TransactionDuration: 10s # ZITADEL_EXECUTIONS_TRANSACTIONDURATION # Automatically cancel the notification if it cannot be handled within a specific time MaxTtl: 5m # ZITADEL_EXECUTIONS_MAXTTL + # List of domains and IPs that are not valid execution target's endpoints + # Wildcard sub domains are currently unsupported + DenyList: # ZITADEL_EXECUTIONS_DENYLIST (comma separated list) + - localhost + - "127.0.0.0/8" + - "::1" + - "0.0.0.0" + - "::" Auth: # See Projections.BulkLimit diff --git a/cmd/mirror/config.go b/cmd/mirror/config.go index 78560201a..331543477 100644 --- a/cmd/mirror/config.go +++ b/cmd/mirror/config.go @@ -18,6 +18,7 @@ import ( "github.com/zitadel/zitadel/internal/command" "github.com/zitadel/zitadel/internal/config/hook" "github.com/zitadel/zitadel/internal/database" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/id" ) @@ -100,6 +101,7 @@ func newConfig(v *viper.Viper, config any) error { mapstructure.StringToSliceHookFunc(","), database.DecodeHook(true), actions.HTTPConfigDecodeHook, + denylist.DenyListDecodeHook, hook.EnumHookFunc(internal_authz.MemberTypeString), mapstructure.TextUnmarshallerHookFunc(), )), diff --git a/cmd/mirror/projections.go b/cmd/mirror/projections.go index ab4393262..3b7f1219a 100644 --- a/cmd/mirror/projections.go +++ b/cmd/mirror/projections.go @@ -34,6 +34,7 @@ import ( "github.com/zitadel/zitadel/internal/config/systemdefaults" crypto_db "github.com/zitadel/zitadel/internal/crypto/database" "github.com/zitadel/zitadel/internal/database" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" old_es "github.com/zitadel/zitadel/internal/eventstore/repository/sql" @@ -215,6 +216,7 @@ func projections( config.OIDC.DefaultRefreshTokenIdleExpiration, config.DefaultInstance.SecretGenerators, config.Login.DefaultPaths, + []denylist.AddressChecker{}, ) logging.OnError(ctx, err).Fatal("unable to start commands") diff --git a/cmd/setup/03.go b/cmd/setup/03.go index 3037cb695..1497e9679 100644 --- a/cmd/setup/03.go +++ b/cmd/setup/03.go @@ -16,6 +16,7 @@ import ( "github.com/zitadel/zitadel/internal/crypto" crypto_db "github.com/zitadel/zitadel/internal/crypto/database" "github.com/zitadel/zitadel/internal/database" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" ) @@ -96,6 +97,7 @@ func (mig *FirstInstance) Execute(ctx context.Context, _ eventstore.Event) error 0, nil, mig.defaultPaths, + []denylist.AddressChecker{}, ) if err != nil { return err diff --git a/cmd/setup/config.go b/cmd/setup/config.go index 12f6b4e11..b431888f2 100644 --- a/cmd/setup/config.go +++ b/cmd/setup/config.go @@ -26,6 +26,7 @@ import ( "github.com/zitadel/zitadel/internal/config/hook" "github.com/zitadel/zitadel/internal/config/systemdefaults" "github.com/zitadel/zitadel/internal/database" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/execution" @@ -81,6 +82,7 @@ func NewConfig(cmd *cobra.Command, v *viper.Viper) (*Config, instrumentation.Shu hooks.MapTypeStringDecode[string, *authz.SystemAPIUser], hooks.MapHTTPHeaderStringDecode, database.DecodeHook(false), + denylist.DenyListDecodeHook, actions.HTTPConfigDecodeHook, hook.EnumHookFunc(authz.MemberTypeString), hook.Base64ToBytesHookFunc(), diff --git a/cmd/setup/config_change.go b/cmd/setup/config_change.go index 8e0bebe48..8096c9fb0 100644 --- a/cmd/setup/config_change.go +++ b/cmd/setup/config_change.go @@ -7,6 +7,7 @@ import ( "github.com/zitadel/zitadel/internal/cache/connector" "github.com/zitadel/zitadel/internal/command" "github.com/zitadel/zitadel/internal/config/systemdefaults" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/eventstore" ) @@ -60,6 +61,7 @@ func (mig *externalConfigChange) Execute(ctx context.Context, _ eventstore.Event 0, nil, &login.DefaultPaths{}, + []denylist.AddressChecker{}, ) if err != nil { diff --git a/cmd/setup/setup.go b/cmd/setup/setup.go index e6b78189d..0bec59862 100644 --- a/cmd/setup/setup.go +++ b/cmd/setup/setup.go @@ -575,6 +575,7 @@ func startCommandsQueries( config.OIDC.DefaultRefreshTokenIdleExpiration, config.DefaultInstance.SecretGenerators, config.Login.DefaultPaths, + config.Executions.DenyList, ) logging.OnError(ctx, err).Fatal("unable to start commands") diff --git a/cmd/start/config.go b/cmd/start/config.go index 054d28acc..95515c550 100644 --- a/cmd/start/config.go +++ b/cmd/start/config.go @@ -30,6 +30,7 @@ import ( "github.com/zitadel/zitadel/internal/config/network" "github.com/zitadel/zitadel/internal/config/systemdefaults" "github.com/zitadel/zitadel/internal/database" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/execution" @@ -146,6 +147,7 @@ func readConfig(v *viper.Viper) (*Config, error) { hooks.MapHTTPHeaderStringDecode, database.DecodeHook(false), actions.HTTPConfigDecodeHook, + denylist.DenyListDecodeHook, hook.EnumHookFunc(authz.MemberTypeString), hooks.MapTypeStringDecode[domain.Feature, any], hooks.SliceTypeStringDecode[*command.SetQuota], diff --git a/cmd/start/config_test.go b/cmd/start/config_test.go index 91d8b7274..36ac50571 100644 --- a/cmd/start/config_test.go +++ b/cmd/start/config_test.go @@ -14,9 +14,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/zitadel/zitadel/internal/actions" "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/command" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/feature" ) @@ -48,10 +48,10 @@ Log: Level: info `}, want: func(t *testing.T, config *Config) { - assert.Equal(t, config.Actions.HTTP.DenyList, []actions.AddressChecker{ - &actions.HostChecker{Domain: "localhost"}, - &actions.HostChecker{IP: net.ParseIP("127.0.0.1")}, - &actions.HostChecker{Domain: "foobar"}}) + assert.Equal(t, config.Actions.HTTP.DenyList, []denylist.AddressChecker{ + &denylist.HostChecker{Domain: "localhost"}, + &denylist.HostChecker{IP: net.ParseIP("127.0.0.1")}, + &denylist.HostChecker{Domain: "foobar"}}) }, }, { name: "actions deny list string ok", @@ -64,10 +64,10 @@ Log: Level: info `}, want: func(t *testing.T, config *Config) { - assert.Equal(t, config.Actions.HTTP.DenyList, []actions.AddressChecker{ - &actions.HostChecker{Domain: "localhost"}, - &actions.HostChecker{IP: net.ParseIP("127.0.0.1")}, - &actions.HostChecker{Domain: "foobar"}}) + assert.Equal(t, config.Actions.HTTP.DenyList, []denylist.AddressChecker{ + &denylist.HostChecker{Domain: "localhost"}, + &denylist.HostChecker{IP: net.ParseIP("127.0.0.1")}, + &denylist.HostChecker{Domain: "foobar"}}) }, }, { name: "features ok", diff --git a/cmd/start/start.go b/cmd/start/start.go index 74164cc5b..bb06ac8a1 100644 --- a/cmd/start/start.go +++ b/cmd/start/start.go @@ -289,6 +289,7 @@ func startZitadel(ctx context.Context, config *Config, masterKey string, server config.OIDC.DefaultRefreshTokenIdleExpiration, config.DefaultInstance.SecretGenerators, config.Login.DefaultPaths, + config.Executions.DenyList, ) if err != nil { return fmt.Errorf("cannot start commands: %w", err) @@ -478,6 +479,7 @@ func startAPIs( keys.Target, translator, config.Instrumentation.Trace.TrustRemoteSpans, + config.Executions.DenyList, ) if err != nil { return nil, fmt.Errorf("error creating api %w", err) diff --git a/internal/actions/http_module.go b/internal/actions/http_module.go index db7253428..b10c62e5e 100644 --- a/internal/actions/http_module.go +++ b/internal/actions/http_module.go @@ -7,13 +7,13 @@ import ( "io" "net" "net/http" - "net/url" "strings" "time" "github.com/dop251/goja" "github.com/zitadel/logging" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -179,32 +179,9 @@ func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) { if httpConfig == nil || len(httpConfig.DenyList) == 0 { return http.DefaultTransport.RoundTrip(req) } - if err := t.isHostBlocked(httpConfig.DenyList, req.URL); err != nil { + + if err := denylist.IsHostBlocked(httpConfig.DenyList, req.URL, t.lookup); err != nil { return nil, zerrors.ThrowInvalidArgument(err, "ACTIO-N72d0", "host is denied") } return http.DefaultTransport.RoundTrip(req) } - -func (t *transport) isHostBlocked(denyList []AddressChecker, address *url.URL) error { - 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 zerrors.ThrowInternal(err, "ACTIO-4m9s2", "lookup failed") - } - } - for _, denied := range denyList { - if err := denied.IsDenied(ips, host); err != nil { - return err - } - } - return nil -} - -type AddressChecker interface { - IsDenied([]net.IP, string) error -} diff --git a/internal/actions/http_module_config.go b/internal/actions/http_module_config.go index eaab9e754..2dd5adf2b 100644 --- a/internal/actions/http_module_config.go +++ b/internal/actions/http_module_config.go @@ -1,13 +1,11 @@ package actions import ( - "errors" - "fmt" - "net" "reflect" - "strings" "github.com/mitchellh/mapstructure" + + "github.com/zitadel/zitadel/internal/denylist" ) func SetHTTPConfig(config *HTTPConfig) { @@ -17,11 +15,11 @@ func SetHTTPConfig(config *HTTPConfig) { var httpConfig *HTTPConfig type HTTPConfig struct { - DenyList []AddressChecker + DenyList []denylist.AddressChecker } -func HTTPConfigDecodeHook(from, to reflect.Value) (interface{}, error) { - if to.Type() != reflect.TypeOf(HTTPConfig{}) { +func HTTPConfigDecodeHook(from, to reflect.Value) (any, error) { + if to.Type() != reflect.TypeFor[HTTPConfig]() { return from.Interface(), nil } @@ -30,7 +28,10 @@ func HTTPConfigDecodeHook(from, to reflect.Value) (interface{}, error) { }{} decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ - DecodeHook: mapstructure.StringToTimeDurationHookFunc(), + DecodeHook: mapstructure.ComposeDecodeHookFunc( + mapstructure.StringToTimeDurationHookFunc(), + mapstructure.StringToSliceHookFunc(","), + ), WeaklyTypedInput: true, Result: &config, }) @@ -43,77 +44,13 @@ func HTTPConfigDecodeHook(from, to reflect.Value) (interface{}, error) { } c := HTTPConfig{ - DenyList: make([]AddressChecker, 0), + DenyList: make([]denylist.AddressChecker, 0), } - for _, unsplit := range config.DenyList { - for _, split := range strings.Split(unsplit, ",") { - parsed, parseErr := NewHostChecker(split) - if parseErr != nil { - return nil, parseErr - } - if parsed != nil { - c.DenyList = append(c.DenyList, parsed) - } - } + c.DenyList, err = denylist.ParseDenyList(config.DenyList) + if err != nil { + return nil, err } return c, nil } - -func NewHostChecker(entry string) (AddressChecker, error) { - if entry == "" { - return nil, nil - } - _, network, err := net.ParseCIDR(entry) - if err == nil { - return &HostChecker{Net: network}, nil - } - if ip := net.ParseIP(entry); ip != nil { - return &HostChecker{IP: ip}, nil - } - return &HostChecker{Domain: entry}, nil -} - -type HostChecker struct { - Net *net.IPNet - IP net.IP - Domain string -} - -type AddressDeniedError struct { - deniedBy string -} - -func NewAddressDeniedError(deniedBy string) *AddressDeniedError { - return &AddressDeniedError{deniedBy: deniedBy} -} - -func (e *AddressDeniedError) Error() string { - return fmt.Sprintf("address is denied by '%s'", e.deniedBy) -} - -func (e *AddressDeniedError) Is(target error) bool { - var addressDeniedErr *AddressDeniedError - if !errors.As(target, &addressDeniedErr) { - return false - } - return e.deniedBy == addressDeniedErr.deniedBy -} - -func (c *HostChecker) IsDenied(ips []net.IP, address string) error { - // if the address matches the domain, no additional checks as needed - if c.Domain == address { - return NewAddressDeniedError(c.Domain) - } - // 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 NewAddressDeniedError(c.Net.String()) - } - if c.IP != nil && c.IP.Equal(ip) { - return NewAddressDeniedError(c.IP.String()) - } - } - return nil -} diff --git a/internal/actions/http_module_test.go b/internal/actions/http_module_test.go index 4021b593a..1c2e16f85 100644 --- a/internal/actions/http_module_test.go +++ b/internal/actions/http_module_test.go @@ -4,123 +4,16 @@ import ( "bytes" "context" "io" - "net" "net/http" "net/url" "reflect" "testing" "github.com/dop251/goja" - "github.com/stretchr/testify/assert" - "github.com/zitadel/zitadel/internal/logstore" - "github.com/zitadel/zitadel/internal/logstore/record" "github.com/zitadel/zitadel/internal/zerrors" ) -func Test_isHostBlocked(t *testing.T) { - SetLogstoreService(logstore.New[*record.ExecutionLog](nil, nil)) - var denyList = []AddressChecker{ - mustNewHostChecker(t, "192.168.5.0/24"), - mustNewHostChecker(t, "127.0.0.1"), - mustNewHostChecker(t, "test.com"), - } - type fields struct { - lookup func(host string) ([]net.IP, error) - } - type args struct { - address *url.URL - } - tests := []struct { - name string - fields fields - args args - want error - }{ - { - name: "in range", - args: args{ - address: mustNewURL(t, "https://192.168.5.4/hodor"), - }, - want: NewAddressDeniedError("192.168.5.0/24"), - }, - { - name: "exact ip", - args: args{ - address: mustNewURL(t, "http://127.0.0.1:8080/hodor"), - }, - want: NewAddressDeniedError("127.0.0.1"), - }, - { - name: "address match", - fields: fields{ - lookup: func(host string) ([]net.IP, error) { - return []net.IP{net.ParseIP("194.264.52.4")}, nil - }, - }, - args: args{ - address: mustNewURL(t, "https://test.com:42/hodor"), - }, - want: NewAddressDeniedError("test.com"), - }, - { - 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{ - address: mustNewURL(t, "https://test2.com/hodor"), - }, - want: nil, - }, - { - 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: NewAddressDeniedError("127.0.0.1"), - }, - { - name: "looked up failure", - fields: fields{ - lookup: func(host string) ([]net.IP, error) { - return nil, io.EOF - }, - }, - args: args{ - address: mustNewURL(t, "https://test2.com/hodor"), - }, - want: zerrors.ThrowInternal(io.EOF, "ACTIO-4m9s2", "lookup failed"), - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - trans := &transport{ - lookup: tt.fields.lookup, - } - got := trans.isHostBlocked(denyList, tt.args.address) - assert.ErrorIs(t, got, tt.want) - }) - } -} - -func mustNewHostChecker(t *testing.T, ip string) AddressChecker { - t.Helper() - checker, err := NewHostChecker(ip) - if err != nil { - t.Errorf("unable to parse cidr of %q because: %v", ip, err) - t.FailNow() - } - return checker -} - func mustNewURL(t *testing.T, raw string) *url.URL { u, err := url.Parse(raw) if err != nil { diff --git a/internal/api/api.go b/internal/api/api.go index c4b2b4a71..4d135b43b 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -26,6 +26,7 @@ import ( http_mw "github.com/zitadel/zitadel/internal/api/http/middleware" "github.com/zitadel/zitadel/internal/api/ui/login" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/i18n" "github.com/zitadel/zitadel/internal/query" "github.com/zitadel/zitadel/internal/telemetry/tracing" @@ -57,6 +58,7 @@ type API struct { targetEncryptionAlgorithm crypto.EncryptionAlgorithm translator *i18n.Translator connectOTELInterceptor *otelconnect.Interceptor + actionV2DenyList []denylist.AddressChecker } func (a *API) ListGrpcServices() []string { @@ -111,6 +113,7 @@ func New( targetEncryptionAlgorithm crypto.EncryptionAlgorithm, translator *i18n.Translator, trustRemoteSpans bool, + deniedIPList []denylist.AddressChecker, ) (_ *API, err error) { api := &API{ port: port, @@ -126,9 +129,10 @@ func New( connectServices: make(map[string][]string), targetEncryptionAlgorithm: targetEncryptionAlgorithm, translator: translator, + actionV2DenyList: deniedIPList, } - api.grpcServer = server.CreateServer(api.verifier, systemAuthz, authZ, queries, externalDomain, tlsConfig, accessInterceptor.AccessService(), targetEncryptionAlgorithm, api.translator) + api.grpcServer = server.CreateServer(api.verifier, systemAuthz, authZ, queries, externalDomain, tlsConfig, accessInterceptor.AccessService(), targetEncryptionAlgorithm, api.translator, deniedIPList) api.grpcGateway, err = server.CreateGateway(ctx, port, hostHeaders, accessInterceptor, tlsConfig) if err != nil { return nil, err @@ -219,7 +223,7 @@ func (a *API) registerConnectServer(service server.ConnectServer) { connect_middleware.AuthorizationInterceptor(a.verifier, a.systemAuthZ, a.authConfig), connect_middleware.TranslationHandler(), connect_middleware.QuotaExhaustedInterceptor(a.accessInterceptor.AccessService(), system_pb.SystemService_ServiceDesc.ServiceName), - connect_middleware.ExecutionHandler(a.targetEncryptionAlgorithm, a.queries.GetActiveSigningWebKey), + connect_middleware.ExecutionHandler(a.targetEncryptionAlgorithm, a.queries.GetActiveSigningWebKey, a.actionV2DenyList), connect_middleware.ValidationHandler(), connect_middleware.ServiceHandler(), connect_middleware.ActivityInterceptor(), diff --git a/internal/api/grpc/action/v2/integration_test/execution_target_test.go b/internal/api/grpc/action/v2/integration_test/execution_target_test.go index dc6b9150f..4fe7448aa 100644 --- a/internal/api/grpc/action/v2/integration_test/execution_target_test.go +++ b/internal/api/grpc/action/v2/integration_test/execution_target_test.go @@ -71,7 +71,7 @@ func TestServer_ExecutionTarget(t *testing.T) { // create target for target changes targetCreatedName := integration.TargetName() - targetCreatedURL := "https://nonexistent" + targetCreatedURL := "https://example.com" targetCreatedPayloadType := action.PayloadType_PAYLOAD_TYPE_JSON targetCreated := instance.CreateTarget(ctx, t, targetCreatedName, targetCreatedURL, target_domain.TargetTypeCall, false, targetCreatedPayloadType) @@ -225,7 +225,7 @@ func TestServer_ExecutionTarget(t *testing.T) { // create target for target changes targetCreatedName := integration.TargetName() - targetCreatedURL := "https://nonexistent" + targetCreatedURL := "https://example.com" targetCreatedPayloadType := action.PayloadType_PAYLOAD_TYPE_JSON targetCreated := instance.CreateTarget(ctx, t, targetCreatedName, targetCreatedURL, target_domain.TargetTypeCall, false, targetCreatedPayloadType) diff --git a/internal/api/grpc/action/v2/integration_test/execution_test.go b/internal/api/grpc/action/v2/integration_test/execution_test.go index e6a78a674..398bf6649 100644 --- a/internal/api/grpc/action/v2/integration_test/execution_test.go +++ b/internal/api/grpc/action/v2/integration_test/execution_test.go @@ -18,7 +18,7 @@ import ( func TestServer_SetExecution_Request(t *testing.T) { instance := integration.NewInstance(CTX) isolatedIAMOwnerCTX := instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner) - targetResp := instance.CreateTarget(isolatedIAMOwnerCTX, t, "", "https://notexisting", target_domain.TargetTypeWebhook, false, action.PayloadType_PAYLOAD_TYPE_JSON) + targetResp := instance.CreateTarget(isolatedIAMOwnerCTX, t, "", "https://example.com", target_domain.TargetTypeWebhook, false, action.PayloadType_PAYLOAD_TYPE_JSON) tests := []struct { name string @@ -175,7 +175,7 @@ func assertSetExecutionResponse(t *testing.T, creationDate, setDate time.Time, e func TestServer_SetExecution_Response(t *testing.T) { instance := integration.NewInstance(CTX) isolatedIAMOwnerCTX := instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner) - targetResp := instance.CreateTarget(isolatedIAMOwnerCTX, t, "", "https://notexisting", target_domain.TargetTypeWebhook, false, action.PayloadType_PAYLOAD_TYPE_JSON) + targetResp := instance.CreateTarget(isolatedIAMOwnerCTX, t, "", "https://example.com", target_domain.TargetTypeWebhook, false, action.PayloadType_PAYLOAD_TYPE_JSON) tests := []struct { name string @@ -319,7 +319,7 @@ func TestServer_SetExecution_Response(t *testing.T) { func TestServer_SetExecution_Event(t *testing.T) { instance := integration.NewInstance(CTX) isolatedIAMOwnerCTX := instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner) - targetResp := instance.CreateTarget(isolatedIAMOwnerCTX, t, "", "https://notexisting", target_domain.TargetTypeWebhook, false, action.PayloadType_PAYLOAD_TYPE_JSON) + targetResp := instance.CreateTarget(isolatedIAMOwnerCTX, t, "", "https://example.com", target_domain.TargetTypeWebhook, false, action.PayloadType_PAYLOAD_TYPE_JSON) tests := []struct { name string @@ -482,7 +482,7 @@ func TestServer_SetExecution_Event(t *testing.T) { func TestServer_SetExecution_Function(t *testing.T) { instance := integration.NewInstance(CTX) isolatedIAMOwnerCTX := instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner) - targetResp := instance.CreateTarget(isolatedIAMOwnerCTX, t, "", "https://notexisting", target_domain.TargetTypeWebhook, false, action.PayloadType_PAYLOAD_TYPE_JSON) + targetResp := instance.CreateTarget(isolatedIAMOwnerCTX, t, "", "https://example.com", target_domain.TargetTypeWebhook, false, action.PayloadType_PAYLOAD_TYPE_JSON) tests := []struct { name string diff --git a/internal/api/grpc/action/v2beta/integration_test/execution_target_test.go b/internal/api/grpc/action/v2beta/integration_test/execution_target_test.go index d463263b4..c9b6a82ee 100644 --- a/internal/api/grpc/action/v2beta/integration_test/execution_target_test.go +++ b/internal/api/grpc/action/v2beta/integration_test/execution_target_test.go @@ -71,7 +71,7 @@ func TestServer_ExecutionTarget(t *testing.T) { // create target for target changes targetCreatedName := integration.TargetName() - targetCreatedURL := "https://nonexistent" + targetCreatedURL := "https://example.com" targetCreated := instance.CreateTargetWithoutPayloadType(ctx, t, targetCreatedName, targetCreatedURL, target_domain.TargetTypeCall, false) @@ -221,7 +221,7 @@ func TestServer_ExecutionTarget(t *testing.T) { // create target for target changes targetCreatedName := integration.TargetName() - targetCreatedURL := "https://nonexistent" + targetCreatedURL := "https://example.com" targetCreated := instance.CreateTargetWithoutPayloadType(ctx, t, targetCreatedName, targetCreatedURL, target_domain.TargetTypeCall, false) diff --git a/internal/api/grpc/action/v2beta/integration_test/execution_test.go b/internal/api/grpc/action/v2beta/integration_test/execution_test.go index 18433c305..c6cabe207 100644 --- a/internal/api/grpc/action/v2beta/integration_test/execution_test.go +++ b/internal/api/grpc/action/v2beta/integration_test/execution_test.go @@ -18,7 +18,7 @@ import ( func TestServer_SetExecution_Request(t *testing.T) { instance := integration.NewInstance(CTX) isolatedIAMOwnerCTX := instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner) - targetResp := instance.CreateTargetWithoutPayloadType(isolatedIAMOwnerCTX, t, "", "https://notexisting", target_domain.TargetTypeWebhook, false) + targetResp := instance.CreateTargetWithoutPayloadType(isolatedIAMOwnerCTX, t, "", "https://example.com", target_domain.TargetTypeWebhook, false) tests := []struct { name string @@ -175,7 +175,7 @@ func assertSetExecutionResponse(t *testing.T, creationDate, setDate time.Time, e func TestServer_SetExecution_Response(t *testing.T) { instance := integration.NewInstance(CTX) isolatedIAMOwnerCTX := instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner) - targetResp := instance.CreateTargetWithoutPayloadType(isolatedIAMOwnerCTX, t, "", "https://notexisting", target_domain.TargetTypeWebhook, false) + targetResp := instance.CreateTargetWithoutPayloadType(isolatedIAMOwnerCTX, t, "", "https://example.com", target_domain.TargetTypeWebhook, false) tests := []struct { name string @@ -319,7 +319,7 @@ func TestServer_SetExecution_Response(t *testing.T) { func TestServer_SetExecution_Event(t *testing.T) { instance := integration.NewInstance(CTX) isolatedIAMOwnerCTX := instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner) - targetResp := instance.CreateTargetWithoutPayloadType(isolatedIAMOwnerCTX, t, "", "https://notexisting", target_domain.TargetTypeWebhook, false) + targetResp := instance.CreateTargetWithoutPayloadType(isolatedIAMOwnerCTX, t, "", "https://example.com", target_domain.TargetTypeWebhook, false) tests := []struct { name string @@ -482,7 +482,7 @@ func TestServer_SetExecution_Event(t *testing.T) { func TestServer_SetExecution_Function(t *testing.T) { instance := integration.NewInstance(CTX) isolatedIAMOwnerCTX := instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner) - targetResp := instance.CreateTargetWithoutPayloadType(isolatedIAMOwnerCTX, t, "", "https://notexisting", target_domain.TargetTypeWebhook, false) + targetResp := instance.CreateTargetWithoutPayloadType(isolatedIAMOwnerCTX, t, "", "https://example.com", target_domain.TargetTypeWebhook, false) tests := []struct { name string diff --git a/internal/api/grpc/server/connect_middleware/execution_interceptor.go b/internal/api/grpc/server/connect_middleware/execution_interceptor.go index 111ac14fb..be57ab8f2 100644 --- a/internal/api/grpc/server/connect_middleware/execution_interceptor.go +++ b/internal/api/grpc/server/connect_middleware/execution_interceptor.go @@ -13,6 +13,7 @@ import ( "github.com/zitadel/zitadel/internal/api/authz" http_utils "github.com/zitadel/zitadel/internal/api/http" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/execution" target_domain "github.com/zitadel/zitadel/internal/execution/target" "github.com/zitadel/zitadel/internal/telemetry/tracing" @@ -26,12 +27,12 @@ var headersToForward = map[string]bool{ strings.ToLower(http_utils.Origin): true, } -func ExecutionHandler(alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey) connect.UnaryInterceptorFunc { +func ExecutionHandler(alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey, deniedIPList []denylist.AddressChecker) connect.UnaryInterceptorFunc { return func(handler connect.UnaryFunc) connect.UnaryFunc { return func(ctx context.Context, req connect.AnyRequest) (_ connect.AnyResponse, err error) { requestTargets := execution.QueryExecutionTargetsForRequest(ctx, req.Spec().Procedure) - handledReq, err := executeTargetsForRequest(ctx, requestTargets, req.Spec().Procedure, req, alg, activeSigningKey) + handledReq, err := executeTargetsForRequest(ctx, requestTargets, req.Spec().Procedure, req, alg, activeSigningKey, deniedIPList) if err != nil { return nil, err } @@ -42,12 +43,12 @@ func ExecutionHandler(alg crypto.EncryptionAlgorithm, activeSigningKey execution } responseTargets := execution.QueryExecutionTargetsForResponse(ctx, req.Spec().Procedure) - return executeTargetsForResponse(ctx, responseTargets, req.Spec().Procedure, handledReq, response, alg, activeSigningKey) + return executeTargetsForResponse(ctx, responseTargets, req.Spec().Procedure, handledReq, response, alg, activeSigningKey, deniedIPList) } } } -func executeTargetsForRequest(ctx context.Context, targets []target_domain.Target, fullMethod string, req connect.AnyRequest, alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey) (_ connect.AnyRequest, err error) { +func executeTargetsForRequest(ctx context.Context, targets []target_domain.Target, fullMethod string, req connect.AnyRequest, alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey, deniedIPList []denylist.AddressChecker) (_ connect.AnyRequest, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -67,14 +68,14 @@ func executeTargetsForRequest(ctx context.Context, targets []target_domain.Targe Headers: SetRequestHeaders(req.Header()), } - _, err = execution.CallTargets(ctx, targets, info, alg, activeSigningKey) + _, err = execution.CallTargets(ctx, targets, info, alg, activeSigningKey, deniedIPList) if err != nil { return nil, err } return req, nil } -func executeTargetsForResponse(ctx context.Context, targets []target_domain.Target, fullMethod string, req connect.AnyRequest, resp connect.AnyResponse, alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey) (_ connect.AnyResponse, err error) { +func executeTargetsForResponse(ctx context.Context, targets []target_domain.Target, fullMethod string, req connect.AnyRequest, resp connect.AnyResponse, alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey, deniedIPList []denylist.AddressChecker) (_ connect.AnyResponse, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -95,7 +96,7 @@ func executeTargetsForResponse(ctx context.Context, targets []target_domain.Targ Headers: SetRequestHeaders(req.Header()), } - _, err = execution.CallTargets(ctx, targets, info, alg, activeSigningKey) + _, err = execution.CallTargets(ctx, targets, info, alg, activeSigningKey, deniedIPList) if err != nil { return nil, err } diff --git a/internal/api/grpc/server/connect_middleware/execution_interceptor_test.go b/internal/api/grpc/server/connect_middleware/execution_interceptor_test.go index a11c3b433..2d979cd43 100644 --- a/internal/api/grpc/server/connect_middleware/execution_interceptor_test.go +++ b/internal/api/grpc/server/connect_middleware/execution_interceptor_test.go @@ -21,6 +21,7 @@ import ( "google.golang.org/protobuf/types/known/structpb" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/execution" target_domain "github.com/zitadel/zitadel/internal/execution/target" ) @@ -74,6 +75,10 @@ func newMockContextInfoResponse(fullMethod, request, response string) *ContextIn } func Test_executeTargetsForGRPCFullMethod_request(t *testing.T) { + deniedLocalhost, err := denylist.NewHostChecker("127.0.0.1") + require.NoError(t, err) + deniedIPs := []denylist.AddressChecker{deniedLocalhost} + type target struct { reqBody execution.ContextInfo sleep time.Duration @@ -89,9 +94,10 @@ func Test_executeTargetsForGRPCFullMethod_request(t *testing.T) { fullMethod string req connect.AnyRequest getActiveSigningWebKey execution.GetActiveSigningWebKey + deniedIPs []denylist.AddressChecker } type res struct { - want interface{} + want any wantErr bool } tests := []struct { @@ -286,6 +292,37 @@ func Test_executeTargetsForGRPCFullMethod_request(t *testing.T) { want: newMockContentRequest("content1"), }, }, + { + "when target endpoint is in deny list should return error", + args{ + ctx: context.Background(), + fullMethod: "/service/method", + executionTargets: []target_domain.Target{ + { + ExecutionID: "request./zitadel.session.v2.SessionService/SetSession", + TargetID: "target", + TargetType: target_domain.TargetTypeCall, + Timeout: time.Minute, + InterruptOnError: true, + Endpoint: "127.0.0.1", + }, + }, + targets: []target{ + { + reqBody: newMockContextInfoRequest("/service/method", "content"), + respBody: newMockContentResponse("content1"), + sleep: 0, + statusCode: http.StatusOK, + requestVerification: validateJSONPayload, + }, + }, + req: newMockContentRequest("content"), + deniedIPs: deniedIPs, + }, + res{ + wantErr: true, + }, + }, { "target async, timeout", args{ @@ -631,6 +668,7 @@ func Test_executeTargetsForGRPCFullMethod_request(t *testing.T) { tt.args.req, nil, tt.args.getActiveSigningWebKey, + tt.args.deniedIPs, ) if tt.res.wantErr { @@ -736,6 +774,9 @@ func validateJWEPayload(t *testing.T) func(expected, sent []byte) bool { } func Test_executeTargetsForGRPCFullMethod_response(t *testing.T) { + deniedLocalhost, err := denylist.NewHostChecker("127.0.0.1") + require.NoError(t, err) + deniedIPs := []denylist.AddressChecker{deniedLocalhost} type target struct { reqBody execution.ContextInfo sleep time.Duration @@ -750,6 +791,7 @@ func Test_executeTargetsForGRPCFullMethod_response(t *testing.T) { fullMethod string req connect.AnyRequest resp connect.AnyResponse + deniedIPs []denylist.AddressChecker } type res struct { want interface{} @@ -844,6 +886,37 @@ func Test_executeTargetsForGRPCFullMethod_response(t *testing.T) { want: newMockContentResponse("response1"), }, }, + { + "when target endpoint is in deny list should return error", + args{ + ctx: context.Background(), + fullMethod: "/service/method", + executionTargets: []target_domain.Target{ + { + ExecutionID: "response./zitadel.session.v2.SessionService/SetSession", + TargetID: "target", + TargetType: target_domain.TargetTypeCall, + Timeout: time.Minute, + InterruptOnError: true, + Endpoint: "127.0.0.1", + }, + }, + targets: []target{ + { + reqBody: newMockContextInfoResponse("/service/method", "request", "response"), + respBody: newMockContentResponse("response1"), + sleep: 0, + statusCode: http.StatusOK, + }, + }, + req: newMockContentRequest("request"), + resp: newMockContentResponse("response"), + deniedIPs: deniedIPs, + }, + res{ + wantErr: true, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -869,6 +942,7 @@ func Test_executeTargetsForGRPCFullMethod_response(t *testing.T) { tt.args.resp, nil, mockGetActiveSigningWebKey(), + tt.args.deniedIPs, ) if tt.res.wantErr { diff --git a/internal/api/grpc/server/middleware/execution_interceptor.go b/internal/api/grpc/server/middleware/execution_interceptor.go index 265380406..6aacadd08 100644 --- a/internal/api/grpc/server/middleware/execution_interceptor.go +++ b/internal/api/grpc/server/middleware/execution_interceptor.go @@ -13,16 +13,17 @@ import ( "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/api/grpc/server/connect_middleware" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/execution" target_domain "github.com/zitadel/zitadel/internal/execution/target" "github.com/zitadel/zitadel/internal/telemetry/tracing" ) -func ExecutionHandler(alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey) grpc.UnaryServerInterceptor { +func ExecutionHandler(alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey, deniedIPList []denylist.AddressChecker) grpc.UnaryServerInterceptor { return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { requestTargets := execution.QueryExecutionTargetsForRequest(ctx, info.FullMethod) // call targets otherwise return req - handledReq, err := executeTargetsForRequest(ctx, requestTargets, info.FullMethod, req, alg, activeSigningKey) + handledReq, err := executeTargetsForRequest(ctx, requestTargets, info.FullMethod, req, alg, activeSigningKey, deniedIPList) if err != nil { return nil, err } @@ -33,11 +34,11 @@ func ExecutionHandler(alg crypto.EncryptionAlgorithm, activeSigningKey execution } responseTargets := execution.QueryExecutionTargetsForResponse(ctx, info.FullMethod) - return executeTargetsForResponse(ctx, responseTargets, info.FullMethod, handledReq, response, alg, activeSigningKey) + return executeTargetsForResponse(ctx, responseTargets, info.FullMethod, handledReq, response, alg, activeSigningKey, deniedIPList) } } -func executeTargetsForRequest(ctx context.Context, targets []target_domain.Target, fullMethod string, req interface{}, alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey) (_ interface{}, err error) { +func executeTargetsForRequest(ctx context.Context, targets []target_domain.Target, fullMethod string, req any, alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey, deniedIPList []denylist.AddressChecker) (_ interface{}, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -58,10 +59,10 @@ func executeTargetsForRequest(ctx context.Context, targets []target_domain.Targe Headers: connect_middleware.SetRequestHeaders(md), } - return execution.CallTargets(ctx, targets, info, alg, activeSigningKey) + return execution.CallTargets(ctx, targets, info, alg, activeSigningKey, deniedIPList) } -func executeTargetsForResponse(ctx context.Context, targets []target_domain.Target, fullMethod string, req, resp interface{}, alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey) (_ interface{}, err error) { +func executeTargetsForResponse(ctx context.Context, targets []target_domain.Target, fullMethod string, req, resp interface{}, alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey, deniedIPList []denylist.AddressChecker) (_ interface{}, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -83,7 +84,7 @@ func executeTargetsForResponse(ctx context.Context, targets []target_domain.Targ Headers: connect_middleware.SetRequestHeaders(md), } - return execution.CallTargets(ctx, targets, info, alg, activeSigningKey) + return execution.CallTargets(ctx, targets, info, alg, activeSigningKey, deniedIPList) } var _ execution.ContextInfo = &ContextInfoRequest{} diff --git a/internal/api/grpc/server/middleware/execution_interceptor_test.go b/internal/api/grpc/server/middleware/execution_interceptor_test.go index bd80d3714..439c24144 100644 --- a/internal/api/grpc/server/middleware/execution_interceptor_test.go +++ b/internal/api/grpc/server/middleware/execution_interceptor_test.go @@ -20,6 +20,7 @@ import ( "google.golang.org/protobuf/types/known/structpb" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/execution" target_domain "github.com/zitadel/zitadel/internal/execution/target" ) @@ -63,6 +64,10 @@ func newMockContextInfoResponse(fullMethod, request, response string) *ContextIn } func Test_executeTargetsForGRPCFullMethod_request(t *testing.T) { + deniedLocalhost, err := denylist.NewHostChecker("127.0.0.1") + require.NoError(t, err) + deniedIPs := []denylist.AddressChecker{deniedLocalhost} + type target struct { reqBody execution.ContextInfo sleep time.Duration @@ -78,6 +83,7 @@ func Test_executeTargetsForGRPCFullMethod_request(t *testing.T) { fullMethod string req interface{} getActiveSigningWebKey execution.GetActiveSigningWebKey + deniedIPs []denylist.AddressChecker } type res struct { want interface{} @@ -275,6 +281,36 @@ func Test_executeTargetsForGRPCFullMethod_request(t *testing.T) { want: newMockContentRequest("content1"), }, }, + { + "when target endpoint is in denylist should return error", + args{ + ctx: context.Background(), + fullMethod: "/service/method", + executionTargets: []target_domain.Target{ + { + ExecutionID: "request./zitadel.session.v2.SessionService/SetSession", + TargetID: "target", + TargetType: target_domain.TargetTypeCall, + Timeout: time.Minute, + InterruptOnError: true, + }, + }, + targets: []target{ + { + reqBody: newMockContextInfoRequest("/service/method", "content"), + respBody: newMockContentRequest("content1"), + sleep: 0, + statusCode: http.StatusOK, + requestVerification: validateJSONPayload, + }, + }, + req: newMockContentRequest("content"), + deniedIPs: deniedIPs, + }, + res{ + wantErr: true, + }, + }, { "target async, timeout", args{ @@ -620,6 +656,7 @@ func Test_executeTargetsForGRPCFullMethod_request(t *testing.T) { tt.args.req, nil, tt.args.getActiveSigningWebKey, + tt.args.deniedIPs, ) if tt.res.wantErr { @@ -725,6 +762,10 @@ func validateJWEPayload(t *testing.T) func(expected, sent []byte) bool { } func Test_executeTargetsForGRPCFullMethod_response(t *testing.T) { + deniedLocalhost, err := denylist.NewHostChecker("127.0.0.1") + require.NoError(t, err) + deniedIPs := []denylist.AddressChecker{deniedLocalhost} + type target struct { reqBody execution.ContextInfo sleep time.Duration @@ -739,6 +780,7 @@ func Test_executeTargetsForGRPCFullMethod_response(t *testing.T) { fullMethod string req interface{} resp interface{} + deniedIPs []denylist.AddressChecker } type res struct { want interface{} @@ -833,6 +875,36 @@ func Test_executeTargetsForGRPCFullMethod_response(t *testing.T) { want: newMockContentRequest("response1"), }, }, + { + "when target endpoint is in deny list should return error", + args{ + ctx: context.Background(), + fullMethod: "/service/method", + executionTargets: []target_domain.Target{ + { + ExecutionID: "response./zitadel.session.v2.SessionService/SetSession", + TargetID: "target", + TargetType: target_domain.TargetTypeCall, + Timeout: time.Minute, + InterruptOnError: true, + }, + }, + targets: []target{ + { + reqBody: newMockContextInfoResponse("/service/method", "request", "response"), + respBody: newMockContentRequest("response1"), + sleep: 0, + statusCode: http.StatusOK, + }, + }, + req: newMockContentRequest("request"), + resp: newMockContentRequest("response"), + deniedIPs: deniedIPs, + }, + res{ + wantErr: true, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -858,6 +930,7 @@ func Test_executeTargetsForGRPCFullMethod_response(t *testing.T) { tt.args.resp, nil, mockGetActiveSigningWebKey(), + tt.args.deniedIPs, ) if tt.res.wantErr { diff --git a/internal/api/grpc/server/server.go b/internal/api/grpc/server/server.go index 4134cf5fb..ef61a5fe2 100644 --- a/internal/api/grpc/server/server.go +++ b/internal/api/grpc/server/server.go @@ -16,6 +16,7 @@ import ( grpc_api "github.com/zitadel/zitadel/internal/api/grpc" "github.com/zitadel/zitadel/internal/api/grpc/server/middleware" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/i18n" "github.com/zitadel/zitadel/internal/logstore" "github.com/zitadel/zitadel/internal/logstore/record" @@ -64,6 +65,7 @@ func CreateServer( accessSvc *logstore.Service[*record.AccessLog], targetEncAlg crypto.EncryptionAlgorithm, translator *i18n.Translator, + deniedIPList []denylist.AddressChecker, ) *grpc.Server { metricTypes := []metrics.MetricType{metrics.MetricTypeTotalCount, metrics.MetricTypeRequestCount, metrics.MetricTypeStatusCode} serverOptions := []grpc.ServerOption{ @@ -80,7 +82,7 @@ func CreateServer( middleware.AuthorizationInterceptor(verifier, systemAuthz, authConfig), middleware.TranslationHandler(), middleware.QuotaExhaustedInterceptor(accessSvc, system_pb.SystemService_ServiceDesc.ServiceName), - middleware.ExecutionHandler(targetEncAlg, queries.GetActiveSigningWebKey), + middleware.ExecutionHandler(targetEncAlg, queries.GetActiveSigningWebKey, deniedIPList), middleware.ValidationHandler(), middleware.ServiceHandler(), middleware.ActivityInterceptor(), diff --git a/internal/api/oidc/userinfo.go b/internal/api/oidc/userinfo.go index 50c15ca5c..754b3a521 100644 --- a/internal/api/oidc/userinfo.go +++ b/internal/api/oidc/userinfo.go @@ -461,7 +461,7 @@ func (s *Server) userinfoFlows(ctx context.Context, qu *query.OIDCUserInfo, user UserGrants: qu.UserGrants, } - resp, err := execution.CallTargets(ctx, executionTargets, info, s.targetEncryptionAlgorithm, s.query.GetActiveSigningWebKey) + resp, err := execution.CallTargets(ctx, executionTargets, info, s.targetEncryptionAlgorithm, s.query.GetActiveSigningWebKey, s.command.ActionsV2DenyList) if err != nil { return err } diff --git a/internal/api/saml/storage.go b/internal/api/saml/storage.go index 1c81b3d5c..53418ea2f 100644 --- a/internal/api/saml/storage.go +++ b/internal/api/saml/storage.go @@ -401,7 +401,7 @@ func (p *Storage) getCustomAttributes(ctx context.Context, user *query.User, use UserGrants: userGrants.UserGrants, } - resp, err := execution.CallTargets(ctx, executionTargets, info, p.targetEncAlg, p.query.GetActiveSigningWebKey) + resp, err := execution.CallTargets(ctx, executionTargets, info, p.targetEncAlg, p.query.GetActiveSigningWebKey, p.command.ActionsV2DenyList) if err != nil { return nil, err } diff --git a/internal/command/action_v2_target.go b/internal/command/action_v2_target.go index 5317c53c2..38f55f04e 100644 --- a/internal/command/action_v2_target.go +++ b/internal/command/action_v2_target.go @@ -7,9 +7,11 @@ import ( "github.com/zitadel/zitadel/internal/command/preparation" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore/v1/models" target_domain "github.com/zitadel/zitadel/internal/execution/target" + internal_net "github.com/zitadel/zitadel/internal/net" "github.com/zitadel/zitadel/internal/repository/target" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -27,18 +29,22 @@ type AddTarget struct { SigningKey string } -func (a *AddTarget) IsValid() error { +func (a *AddTarget) isValid(inputDenyList []denylist.AddressChecker, lookupFunc internal_net.IPLookupFunc) error { if a.Name == "" { return zerrors.ThrowInvalidArgument(nil, "COMMAND-ddqbm9us5p", "Errors.Target.Invalid") } if a.Timeout == 0 { return zerrors.ThrowInvalidArgument(nil, "COMMAND-39f35d8uri", "Errors.Target.NoTimeout") } - _, err := url.Parse(a.Endpoint) + parsedURL, err := url.Parse(a.Endpoint) if err != nil || a.Endpoint == "" { return zerrors.ThrowInvalidArgument(err, "COMMAND-1r2k6qo6wg", "Errors.Target.InvalidURL") } + if err := denylist.IsHostBlocked(inputDenyList, parsedURL, lookupFunc); err != nil { + return zerrors.ThrowInvalidArgument(err, "COMMAND-NcJUKo", "Errors.Target.DeniedURL") + } + return nil } @@ -47,7 +53,7 @@ func (c *Commands) AddTarget(ctx context.Context, add *AddTarget, resourceOwner return time.Time{}, zerrors.ThrowInvalidArgument(nil, "COMMAND-brml926e2d", "Errors.IDMissing") } - if err := add.IsValid(); err != nil { + if err := add.isValid(c.ActionsV2DenyList, c.IPLookupFunction); err != nil { return time.Time{}, err } @@ -103,7 +109,7 @@ type ChangeTarget struct { SigningKey *string } -func (a *ChangeTarget) IsValid() error { +func (a *ChangeTarget) isValid(inputDenyList []denylist.AddressChecker, lookupFunc internal_net.IPLookupFunc) error { if a.AggregateID == "" { return zerrors.ThrowInvalidArgument(nil, "COMMAND-1l6ympeagp", "Errors.IDMissing") } @@ -114,11 +120,15 @@ func (a *ChangeTarget) IsValid() error { return zerrors.ThrowInvalidArgument(nil, "COMMAND-08b39vdi57", "Errors.Target.NoTimeout") } if a.Endpoint != nil { - _, err := url.Parse(*a.Endpoint) + parsedURL, err := url.Parse(*a.Endpoint) if err != nil || *a.Endpoint == "" { return zerrors.ThrowInvalidArgument(err, "COMMAND-jsbaera7b6", "Errors.Target.InvalidURL") } + if err := denylist.IsHostBlocked(inputDenyList, parsedURL, lookupFunc); err != nil { + return zerrors.ThrowInvalidArgument(err, "COMMAND-jKbbu2", "Errors.Target.DeniedURL") + } } + return nil } @@ -126,7 +136,7 @@ func (c *Commands) ChangeTarget(ctx context.Context, change *ChangeTarget, resou if resourceOwner == "" { return time.Time{}, zerrors.ThrowInvalidArgument(nil, "COMMAND-zqibgg0wwh", "Errors.IDMissing") } - if err := change.IsValid(); err != nil { + if err := change.isValid(c.ActionsV2DenyList, c.IPLookupFunction); err != nil { return time.Time{}, err } existing, err := c.getTargetWriteModelByID(ctx, change.AggregateID, resourceOwner) diff --git a/internal/command/action_v2_target_test.go b/internal/command/action_v2_target_test.go index 57bea2085..b9fa28dc5 100644 --- a/internal/command/action_v2_target_test.go +++ b/internal/command/action_v2_target_test.go @@ -2,28 +2,37 @@ package command import ( "context" + "net" "testing" "time" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/eventstore/v1/models" target_domain "github.com/zitadel/zitadel/internal/execution/target" "github.com/zitadel/zitadel/internal/id" "github.com/zitadel/zitadel/internal/id/mock" + internal_net "github.com/zitadel/zitadel/internal/net" "github.com/zitadel/zitadel/internal/repository/target" "github.com/zitadel/zitadel/internal/zerrors" ) func TestCommands_AddTarget(t *testing.T) { + localhostAddrChecker, err := denylist.NewHostChecker("localhost") + require.NoError(t, err) + type fields struct { eventstore func(t *testing.T) *eventstore.Eventstore idGenerator id.Generator newEncryptedCodeWithDefault encryptedCodeWithDefaultFunc defaultSecretGenerators *SecretGenerators + denyList []denylist.AddressChecker + lookupFunc internal_net.IPLookupFunc } type args struct { ctx context.Context @@ -120,6 +129,28 @@ func TestCommands_AddTarget(t *testing.T) { err: zerrors.IsErrorInvalidArgument, }, }, + { + "ip not allowed, error", + fields{ + eventstore: expectEventstore(), + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("127.0.0.1")}, nil + }, + }, + args{ + ctx: context.Background(), + add: &AddTarget{ + Name: "name", + Timeout: time.Second, + Endpoint: "http://localhost", + }, + resourceOwner: "instance", + }, + res{ + err: zerrors.IsErrorInvalidArgument, + }, + }, { "unique constraint failed, error", fields{ @@ -147,6 +178,10 @@ func TestCommands_AddTarget(t *testing.T) { idGenerator: mock.ExpectID(t, "id1"), newEncryptedCodeWithDefault: mockEncryptedCodeWithDefault("12345678", time.Hour), defaultSecretGenerators: &SecretGenerators{}, + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("192.168.1.1")}, nil + }, }, args{ ctx: context.Background(), @@ -174,6 +209,10 @@ func TestCommands_AddTarget(t *testing.T) { ), ), idGenerator: mock.ExpectID(t, "id1"), + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("192.168.1.1")}, nil + }, }, args{ ctx: context.Background(), @@ -201,6 +240,10 @@ func TestCommands_AddTarget(t *testing.T) { idGenerator: mock.ExpectID(t, "id1"), newEncryptedCodeWithDefault: mockEncryptedCodeWithDefault("12345678", time.Hour), defaultSecretGenerators: &SecretGenerators{}, + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("192.168.1.1")}, nil + }, }, args{ ctx: context.Background(), @@ -233,6 +276,10 @@ func TestCommands_AddTarget(t *testing.T) { idGenerator: mock.ExpectID(t, "id1"), newEncryptedCodeWithDefault: mockEncryptedCodeWithDefault("12345678", time.Hour), defaultSecretGenerators: &SecretGenerators{}, + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("192.168.1.1")}, nil + }, }, args{ ctx: context.Background(), @@ -258,6 +305,7 @@ func TestCommands_AddTarget(t *testing.T) { idGenerator: tt.fields.idGenerator, newEncryptedCodeWithDefault: tt.fields.newEncryptedCodeWithDefault, defaultSecretGenerators: tt.fields.defaultSecretGenerators, + ActionsV2DenyList: tt.fields.denyList, } _, err := c.AddTarget(tt.args.ctx, tt.args.add, tt.args.resourceOwner) if tt.res.err == nil { @@ -274,10 +322,15 @@ func TestCommands_AddTarget(t *testing.T) { } func TestCommands_ChangeTarget(t *testing.T) { + localhostAddrChecker, err := denylist.NewHostChecker("localhost") + require.NoError(t, err) + type fields struct { eventstore func(t *testing.T) *eventstore.Eventstore newEncryptedCodeWithDefault encryptedCodeWithDefaultFunc defaultSecretGenerators *SecretGenerators + denyList []denylist.AddressChecker + lookupFunc internal_net.IPLookupFunc } type args struct { ctx context.Context @@ -385,12 +438,39 @@ func TestCommands_ChangeTarget(t *testing.T) { err: zerrors.IsErrorInvalidArgument, }, }, + { + "ip not allowed, error", + fields{ + eventstore: expectEventstore(), + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("127.0.0.1")}, nil + }, + }, + args{ + ctx: context.Background(), + change: &ChangeTarget{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "id1", + }, + Endpoint: gu.Ptr("http://localhost"), + }, + resourceOwner: "instance", + }, + res{ + err: zerrors.IsErrorInvalidArgument, + }, + }, { "not found, error", fields{ eventstore: expectEventstore( expectFilter(), ), + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("192.168.1.1")}, nil + }, }, args{ ctx: context.Background(), @@ -416,6 +496,10 @@ func TestCommands_ChangeTarget(t *testing.T) { ), ), ), + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("192.168.1.1")}, nil + }, }, args{ ctx: context.Background(), @@ -448,6 +532,10 @@ func TestCommands_ChangeTarget(t *testing.T) { ), ), ), + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("192.168.1.1")}, nil + }, }, args{ ctx: context.Background(), @@ -481,6 +569,10 @@ func TestCommands_ChangeTarget(t *testing.T) { ), ), ), + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("192.168.1.1")}, nil + }, }, args{ ctx: context.Background(), @@ -525,6 +617,10 @@ func TestCommands_ChangeTarget(t *testing.T) { ), newEncryptedCodeWithDefault: mockEncryptedCodeWithDefault("12345678", time.Hour), defaultSecretGenerators: &SecretGenerators{}, + denyList: []denylist.AddressChecker{localhostAddrChecker}, + lookupFunc: func(_ string) ([]net.IP, error) { + return []net.IP{[]byte("192.168.1.1")}, nil + }, }, args{ ctx: context.Background(), @@ -551,6 +647,8 @@ func TestCommands_ChangeTarget(t *testing.T) { eventstore: tt.fields.eventstore(t), newEncryptedCodeWithDefault: tt.fields.newEncryptedCodeWithDefault, defaultSecretGenerators: tt.fields.defaultSecretGenerators, + ActionsV2DenyList: tt.fields.denyList, + IPLookupFunction: tt.fields.lookupFunc, } _, err := c.ChangeTarget(tt.args.ctx, tt.args.change, tt.args.resourceOwner) if tt.res.err == nil { diff --git a/internal/command/command.go b/internal/command/command.go index 104589139..3a03016c0 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -8,6 +8,7 @@ import ( "encoding/pem" "fmt" "math/big" + "net" "net/http" "slices" "strconv" @@ -24,9 +25,11 @@ import ( "github.com/zitadel/zitadel/internal/command/preparation" sd "github.com/zitadel/zitadel/internal/config/systemdefaults" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/id" + internal_net "github.com/zitadel/zitadel/internal/net" "github.com/zitadel/zitadel/internal/notification/senders" "github.com/zitadel/zitadel/internal/static" "github.com/zitadel/zitadel/internal/telemetry/tracing" @@ -101,7 +104,9 @@ type Commands struct { // so the query and cache overhead can be completely eliminated. milestonesCompleted sync.Map - loginPaths LoginPaths + loginPaths LoginPaths + ActionsV2DenyList []denylist.AddressChecker + IPLookupFunction internal_net.IPLookupFunc } //go:generate mockgen -package command -destination ./mock_login_paths.go . LoginPaths @@ -130,6 +135,7 @@ func StartCommands( defaultAccessTokenLifetime, defaultRefreshTokenLifetime, defaultRefreshTokenIdleLifetime time.Duration, defaultSecretGenerators *SecretGenerators, loginPaths LoginPaths, + actionsDeniedHostList []denylist.AddressChecker, ) (repo *Commands, err error) { if externalDomain == "" { return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-Df21s", "no external domain specified") @@ -215,9 +221,11 @@ func StartCommands( WithHyphen: defaults.Multifactors.RecoveryCodes.WithHyphen, }, }, - GenerateDomain: domain.NewGeneratedInstanceDomain, - caches: caches, - loginPaths: loginPaths, + GenerateDomain: domain.NewGeneratedInstanceDomain, + caches: caches, + loginPaths: loginPaths, + ActionsV2DenyList: actionsDeniedHostList, + IPLookupFunction: net.LookupIP, } if defaultSecretGenerators != nil && defaultSecretGenerators.ClientSecret != nil { diff --git a/internal/denylist/denylist.go b/internal/denylist/denylist.go new file mode 100644 index 000000000..a469ad145 --- /dev/null +++ b/internal/denylist/denylist.go @@ -0,0 +1,47 @@ +package denylist + +import ( + "net" + "reflect" + + "github.com/mitchellh/mapstructure" +) + +type AddressChecker interface { + IsDenied([]net.IP, string) error +} + +func ParseDenyList(inputList []string) ([]AddressChecker, error) { + var toReturn []AddressChecker + for _, input := range inputList { + parsed, parseErr := NewHostChecker(input) + if parseErr != nil { + return nil, parseErr + } + if parsed != nil { + toReturn = append(toReturn, parsed) + } + } + return toReturn, nil +} + +func DenyListDecodeHook(from, to reflect.Value) (interface{}, error) { + if to.Type() != reflect.TypeFor[[]AddressChecker]() { + return from.Interface(), nil + } + var inputDenyList []string + decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: mapstructure.ComposeDecodeHookFunc( + mapstructure.StringToSliceHookFunc(","), + ), + WeaklyTypedInput: true, + Result: &inputDenyList, + }) + if err != nil { + return nil, err + } + if err = decoder.Decode(from.Interface()); err != nil { + return nil, err + } + return ParseDenyList(inputDenyList) +} diff --git a/internal/denylist/error.go b/internal/denylist/error.go new file mode 100644 index 000000000..dad0428c3 --- /dev/null +++ b/internal/denylist/error.go @@ -0,0 +1,26 @@ +package denylist + +import ( + "errors" + "fmt" +) + +type AddressDeniedError struct { + deniedBy string +} + +func NewAddressDeniedError(deniedBy string) *AddressDeniedError { + return &AddressDeniedError{deniedBy: deniedBy} +} + +func (e *AddressDeniedError) Error() string { + return fmt.Sprintf("address is denied by '%s'", e.deniedBy) +} + +func (e *AddressDeniedError) Is(target error) bool { + var addressDeniedErr *AddressDeniedError + if !errors.As(target, &addressDeniedErr) { + return false + } + return e.deniedBy == addressDeniedErr.deniedBy +} diff --git a/internal/denylist/host_checker.go b/internal/denylist/host_checker.go new file mode 100644 index 000000000..708ad1479 --- /dev/null +++ b/internal/denylist/host_checker.go @@ -0,0 +1,69 @@ +package denylist + +import ( + "net" + "net/url" + + internal_net "github.com/zitadel/zitadel/internal/net" +) + +var _ AddressChecker = (*HostChecker)(nil) + +type HostChecker struct { + Net *net.IPNet + IP net.IP + Domain string +} + +func NewHostChecker(entry string) (AddressChecker, error) { + if entry == "" { + return nil, nil + } + _, network, err := net.ParseCIDR(entry) + if err == nil { + return &HostChecker{Net: network}, nil + } + if ip := net.ParseIP(entry); ip != nil { + return &HostChecker{IP: ip}, nil + } + return &HostChecker{Domain: entry}, nil +} + +func (c *HostChecker) IsDenied(ips []net.IP, address string) error { + // if the address matches the domain, no additional checks as needed + if c.Domain == address { + return NewAddressDeniedError(c.Domain) + } + // 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 NewAddressDeniedError(c.Net.String()) + } + if c.IP != nil && c.IP.Equal(ip) { + return NewAddressDeniedError(c.IP.String()) + } + } + return nil +} + +// IsHostBlocked checks address against denyList. If a match is found, an [AddressDeniedError] will be returned +// Takes an input lookupFunc to convert address to a list of IP addresses. +// If lookupFunc is nil, it defaults to [net.LookupIP] +func IsHostBlocked(denyList []AddressChecker, address *url.URL, lookupFunc internal_net.IPLookupFunc) error { + if lookupFunc == nil { + lookupFunc = net.LookupIP + } + + host := address.Hostname() + ips, err := internal_net.HostnameToIPList(host, lookupFunc) + if err != nil { + return err + } + + for _, denied := range denyList { + if err := denied.IsDenied(ips, host); err != nil { + return err + } + } + return nil +} diff --git a/internal/denylist/host_checker_test.go b/internal/denylist/host_checker_test.go new file mode 100644 index 000000000..e6ed5ba69 --- /dev/null +++ b/internal/denylist/host_checker_test.go @@ -0,0 +1,122 @@ +package denylist + +import ( + "io" + "net" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/zitadel/zitadel/internal/zerrors" +) + +func TestIsHostBlocked(t *testing.T) { + t.Parallel() + var denyList = []AddressChecker{ + mustNewHostChecker(t, "192.168.5.0/24"), + mustNewHostChecker(t, "127.0.0.1"), + mustNewHostChecker(t, "test.com"), + } + type fields struct { + lookup func(host string) ([]net.IP, error) + } + type args struct { + address *url.URL + } + tests := []struct { + name string + fields fields + args args + want error + }{ + { + name: "in range", + args: args{ + address: mustNewURL(t, "https://192.168.5.4/hodor"), + }, + want: NewAddressDeniedError("192.168.5.0/24"), + }, + { + name: "exact ip", + args: args{ + address: mustNewURL(t, "http://127.0.0.1:8080/hodor"), + }, + want: NewAddressDeniedError("127.0.0.1"), + }, + { + name: "address match", + fields: fields{ + lookup: func(host string) ([]net.IP, error) { + return []net.IP{net.ParseIP("194.264.52.4")}, nil + }, + }, + args: args{ + address: mustNewURL(t, "https://test.com:42/hodor"), + }, + want: NewAddressDeniedError("test.com"), + }, + { + 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{ + address: mustNewURL(t, "https://test2.com/hodor"), + }, + want: nil, + }, + { + 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: NewAddressDeniedError("127.0.0.1"), + }, + { + name: "lookup failure", + fields: fields{ + lookup: func(host string) ([]net.IP, error) { + return nil, io.EOF + }, + }, + args: args{ + address: mustNewURL(t, "https://test2.com/hodor"), + }, + want: zerrors.ThrowInternal(io.EOF, "NET-4m9s2", "lookup failed"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := IsHostBlocked(denyList, tt.args.address, tt.fields.lookup) + assert.ErrorIs(t, got, tt.want) + }) + } +} + +func mustNewHostChecker(t *testing.T, ip string) AddressChecker { + t.Helper() + checker, err := NewHostChecker(ip) + if err != nil { + t.Errorf("unable to parse cidr of %q because: %v", ip, err) + t.FailNow() + } + return checker +} + +func mustNewURL(t *testing.T, raw string) *url.URL { + u, err := url.Parse(raw) + if err != nil { + t.Errorf("unable to parse address of %q because: %v", raw, err) + t.FailNow() + } + return u +} diff --git a/internal/execution/execution.go b/internal/execution/execution.go index ffe39fa37..58e79064c 100644 --- a/internal/execution/execution.go +++ b/internal/execution/execution.go @@ -9,7 +9,9 @@ import ( "encoding/json" "encoding/pem" "io" + "net" "net/http" + "net/url" "sync" "time" @@ -20,6 +22,7 @@ import ( zhttp "github.com/zitadel/zitadel/internal/api/http" "github.com/zitadel/zitadel/internal/api/oidc/sign" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/domain" target_domain "github.com/zitadel/zitadel/internal/execution/target" "github.com/zitadel/zitadel/internal/repository/execution" @@ -43,7 +46,8 @@ func CallTargets( info ContextInfo, alg crypto.EncryptionAlgorithm, activeSigningKey GetActiveSigningWebKey, -) (_ interface{}, err error) { + deniedIPList []denylist.AddressChecker, +) (_ any, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -54,7 +58,7 @@ func CallTargets( for _, target := range targets { // call the type of target - resp, err := CallTarget(ctx, target, info, alg, signerOnce, encrypters) + resp, err := CallTarget(ctx, target, info, alg, signerOnce, encrypters, deniedIPList) // handle error if interrupt is set logging.WithFields("instanceID", authz.GetInstance(ctx).InstanceID(), "target", target.GetTargetID()).OnError(err).Error("error calling target") if err != nil && target.IsInterruptOnError() { @@ -82,6 +86,7 @@ func CallTarget( alg crypto.EncryptionAlgorithm, signerOnce sign.SignerFunc, encrypters *sync.Map, + deniedIPList []denylist.AddressChecker, ) (res []byte, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -90,6 +95,17 @@ func CallTarget( if err != nil { return nil, zerrors.ThrowInternal(err, "EXEC-thiiCh5b", "Errors.Internal") } + + if target.GetEndpoint() != "" { + endpointURL, err := url.Parse(target.GetEndpoint()) + if err != nil { + return nil, zerrors.ThrowInvalidArgument(err, "EXEC-N5lu09", "Errors.Endpoint.Invalid") + } + if err := denylist.IsHostBlocked(deniedIPList, endpointURL, net.LookupIP); err != nil { + return nil, zerrors.ThrowInvalidArgument(err, "EXEC-N5lu09", "Errors.Endpoint.Denied") + } + } + body, err := payload(ctx, info.GetHTTPRequestBody(), target, signerOnce, encrypters) if err != nil { return nil, err diff --git a/internal/execution/execution_test.go b/internal/execution/execution_test.go index 7174e2b2c..f41e7a9d5 100644 --- a/internal/execution/execution_test.go +++ b/internal/execution/execution_test.go @@ -24,6 +24,7 @@ import ( "github.com/zitadel/zitadel/internal/api/grpc/server/middleware" "github.com/zitadel/zitadel/internal/api/oidc/sign" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" "github.com/zitadel/zitadel/internal/execution" target_domain "github.com/zitadel/zitadel/internal/execution/target" "github.com/zitadel/zitadel/internal/zerrors" @@ -405,7 +406,11 @@ func Test_CallTarget(t *testing.T) { respBody, err := testServer( t, tt.args.server, - testCallTarget(tt.args.ctx, tt.args.info, tt.args.target, crypto.CreateMockEncryptionAlg(gomock.NewController(t)), tt.args.signer, &sync.Map{}), + testCallTarget( + tt.args.ctx, tt.args.info, tt.args.target, + crypto.CreateMockEncryptionAlg(gomock.NewController(t)), tt.args.signer, &sync.Map{}, + []denylist.AddressChecker{}, + ), ) if tt.res.wantErr { assert.Error(t, err) @@ -418,6 +423,9 @@ func Test_CallTarget(t *testing.T) { } func Test_CallTargets(t *testing.T) { + deniedLocalhost, err := denylist.NewHostChecker("127.0.0.1") + require.NoError(t, err) + deniedIPs := []denylist.AddressChecker{deniedLocalhost} type args struct { ctx context.Context info *middleware.ContextInfoRequest @@ -426,17 +434,18 @@ func Test_CallTargets(t *testing.T) { getActiveSigningWebKey func(*int32) execution.GetActiveSigningWebKey } type res struct { - ret interface{} + ret any wantErr bool } tests := []struct { - name string - args args - res res + name string + denyList []denylist.AddressChecker + args args + res res }{ { - "interrupt on status", - args{ + name: "interrupt on status", + args: args{ ctx: context.Background(), info: requestContextInfo1, servers: []*callTestServer{{ @@ -457,13 +466,13 @@ func Test_CallTargets(t *testing.T) { {InterruptOnError: true}, }, }, - res{ + res: res{ wantErr: true, }, }, { - "continue on status", - args{ + name: "continue on status", + args: args{ ctx: context.Background(), info: requestContextInfo1, servers: []*callTestServer{{ @@ -484,13 +493,13 @@ func Test_CallTargets(t *testing.T) { {InterruptOnError: false}, }, }, - res{ + res: res{ ret: requestContextInfo1.GetContent(), }, }, { - "interrupt on json error", - args{ + name: "interrupt on json error", + args: args{ ctx: context.Background(), info: requestContextInfo1, servers: []*callTestServer{{ @@ -511,13 +520,13 @@ func Test_CallTargets(t *testing.T) { {InterruptOnError: true}, }, }, - res{ + res: res{ wantErr: true, }, }, { - "continue on json error", - args{ + name: "continue on json error", + args: args{ ctx: context.Background(), info: requestContextInfo1, servers: []*callTestServer{{ @@ -537,13 +546,13 @@ func Test_CallTargets(t *testing.T) { {InterruptOnError: false}, {InterruptOnError: false}, }}, - res{ + res: res{ ret: requestContextInfo1.GetContent(), }, }, { - "multiple JWT/JWE targets, ok", - args{ + name: "multiple JWT/JWE targets, ok", + args: args{ ctx: context.Background(), info: requestContextInfo1, servers: []*callTestServer{{ @@ -573,10 +582,38 @@ func Test_CallTargets(t *testing.T) { }, getActiveSigningWebKey: testActiveSingingWebKey, }, - res{ + res: res{ ret: requestContextInfo1.GetContent(), }, }, + { + name: "block request when target in denylist", + denyList: deniedIPs, + args: args{ + ctx: context.Background(), + info: requestContextInfo1, + servers: []*callTestServer{{ + timeout: time.Second, + method: http.MethodPost, + expectBody: validateJSONPayload(requestContextInfoBody1), + respondBody: requestContextInfoBody2, + statusCode: http.StatusOK, + }, { + timeout: time.Second, + method: http.MethodPost, + expectBody: validateJSONPayload(requestContextInfoBody1), + respondBody: []byte("just a string, not json"), + statusCode: http.StatusOK, + }}, + targets: []target_domain.Target{ + {InterruptOnError: false}, + {InterruptOnError: true}, + }, + }, + res: res{ + wantErr: true, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -587,7 +624,9 @@ func Test_CallTargets(t *testing.T) { } respBody, err := testServers(t, tt.args.servers, - testCallTargets(tt.args.ctx, tt.args.info, tt.args.targets, crypto.CreateMockEncryptionAlg(gomock.NewController(t)), getActiveSigningWebKey), + testCallTargets( + tt.args.ctx, tt.args.info, tt.args.targets, + crypto.CreateMockEncryptionAlg(gomock.NewController(t)), getActiveSigningWebKey, tt.denyList), ) if tt.res.wantErr { assert.Error(t, err) @@ -691,10 +730,11 @@ func testCallTarget(ctx context.Context, alg crypto.EncryptionAlgorithm, signerOnce sign.SignerFunc, encrypters *sync.Map, + actionsDenyList []denylist.AddressChecker, ) func(string) ([]byte, error) { return func(url string) (r []byte, err error) { target.Endpoint = url - return execution.CallTarget(ctx, target, info, alg, signerOnce, encrypters) + return execution.CallTarget(ctx, target, info, alg, signerOnce, encrypters, actionsDenyList) } } @@ -703,14 +743,15 @@ func testCallTargets(ctx context.Context, target []target_domain.Target, alg crypto.EncryptionAlgorithm, activeSigningKey execution.GetActiveSigningWebKey, -) func([]string) (interface{}, error) { - return func(urls []string) (interface{}, error) { + actionsDenyList []denylist.AddressChecker, +) func([]string) (any, error) { + return func(urls []string) (any, error) { targets := make([]target_domain.Target, len(target)) for i, t := range target { t.Endpoint = urls[i] targets[i] = t } - return execution.CallTargets(ctx, targets, info, alg, activeSigningKey) + return execution.CallTargets(ctx, targets, info, alg, activeSigningKey, actionsDenyList) } } @@ -723,10 +764,6 @@ var requestContextInfo1 = &middleware.ContextInfoRequest{ var requestContextInfoBody1 = []byte("{\"request\":{\"content\":\"request1\"}}") var requestContextInfoBody2 = []byte("{\"request\":{\"content\":\"request2\"}}") -type request struct { - Request string `json:"request"` -} - func testErrorBody(code int, message string) []byte { body := &execution.ErrorBody{ForwardedStatusCode: code, ForwardedErrorMessage: message} data, _ := json.Marshal(body) diff --git a/internal/execution/worker.go b/internal/execution/worker.go index 40a58ae80..a1b3d46d1 100644 --- a/internal/execution/worker.go +++ b/internal/execution/worker.go @@ -10,6 +10,7 @@ import ( "github.com/riverqueue/river" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/denylist" target_domain "github.com/zitadel/zitadel/internal/execution/target" exec_repo "github.com/zitadel/zitadel/internal/repository/execution" ) @@ -46,7 +47,7 @@ func (w *Worker) Work(ctx context.Context, job *river.Job[*exec_repo.Request]) e return river.JobCancel(fmt.Errorf("unable to unmarshal targets because %w", err)) } - _, err = CallTargets(ctx, targets, exec_repo.ContextInfoFromRequest(job.Args), w.targetEncAlg, w.activeSigningKey) + _, err = CallTargets(ctx, targets, exec_repo.ContextInfoFromRequest(job.Args), w.targetEncAlg, w.activeSigningKey, w.config.DenyList) if err != nil { // If there is an error returned from the targets, it means that the execution was interrupted return river.JobCancel(fmt.Errorf("interruption during call of targets because %w", err)) @@ -61,6 +62,7 @@ type WorkerConfig struct { Workers uint8 TransactionDuration time.Duration MaxTtl time.Duration + DenyList []denylist.AddressChecker } func NewWorker( diff --git a/internal/net/net.go b/internal/net/net.go new file mode 100644 index 000000000..638e51ca3 --- /dev/null +++ b/internal/net/net.go @@ -0,0 +1,30 @@ +package net + +import ( + builtin_net "net" + + "github.com/zitadel/zitadel/internal/zerrors" +) + +type IPLookupFunc func(string) ([]builtin_net.IP, error) + +// HostnameToIPList converts the input URL to a list [net.IP]. +// +// Returns an internal error if the lookup fails. +// Returns an invalid argument if the lookup function is nil +func HostnameToIPList(hostname string, lookupFunc IPLookupFunc) ([]builtin_net.IP, error) { + ip := builtin_net.ParseIP(hostname) + ips := []builtin_net.IP{ip} + if ip != nil { + return ips, nil + } + // if the hostname is a domain, we need to check resolve the ip(s), since it might be denied + if lookupFunc == nil { + return nil, zerrors.ThrowInvalidArgument(nil, "NET-naSn77", "lookup function must not be nil") + } + ips, err := lookupFunc(hostname) + if err != nil { + return nil, zerrors.ThrowInternal(err, "NET-4m9s2", "lookup failed") + } + return ips, nil +} diff --git a/internal/net/net_test.go b/internal/net/net_test.go new file mode 100644 index 000000000..2cf73bd44 --- /dev/null +++ b/internal/net/net_test.go @@ -0,0 +1,78 @@ +package net + +import ( + builtin_net "net" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zitadel/zitadel/internal/zerrors" +) + +func TestHostnameToIPList(t *testing.T) { + t.Parallel() + addrErr := &builtin_net.AddrError{Addr: "invalid.local", Err: "Invalid address"} + tests := []struct { + name string + hostname string + lookupFunc IPLookupFunc + want []builtin_net.IP + expectedErr error + }{ + { + name: "valid IP address", + hostname: "192.168.1.1", + lookupFunc: nil, + want: []builtin_net.IP{builtin_net.ParseIP("192.168.1.1")}, + }, + { + name: "domain with lookup function", + hostname: "example.com", + lookupFunc: func(s string) ([]builtin_net.IP, error) { + return []builtin_net.IP{builtin_net.ParseIP("127.0.0.1")}, nil + }, + want: []builtin_net.IP{builtin_net.ParseIP("127.0.0.1")}, + }, + { + name: "domain without lookup function", + hostname: "example.com", + lookupFunc: nil, + want: nil, + expectedErr: zerrors.ThrowInvalidArgument(nil, "NET-naSn77", "lookup function must not be nil"), + }, + { + name: "domain with lookup error", + hostname: "invalid.local", + lookupFunc: func(s string) ([]builtin_net.IP, error) { + return nil, addrErr + }, + want: nil, + expectedErr: zerrors.ThrowInternal(addrErr, "NET-4m9s2", "lookup failed"), + }, + { + name: "domain with multiple IPs", + hostname: "multi.example.com", + lookupFunc: func(s string) ([]builtin_net.IP, error) { + return []builtin_net.IP{ + builtin_net.ParseIP("127.0.0.1"), + builtin_net.ParseIP("127.0.0.2"), + }, nil + }, + want: []builtin_net.IP{ + builtin_net.ParseIP("127.0.0.1"), + builtin_net.ParseIP("127.0.0.2"), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := HostnameToIPList(tt.hostname, tt.lookupFunc) + + require.ErrorIs(t, err, tt.expectedErr) + assert.ElementsMatch(t, got, tt.want) + }) + } +}