diff --git a/app/controlplane/internal/conf/controlplane/config/v1/conf.pb.go b/app/controlplane/internal/conf/controlplane/config/v1/conf.pb.go index f83a219ba..09eefa43b 100644 --- a/app/controlplane/internal/conf/controlplane/config/v1/conf.pb.go +++ b/app/controlplane/internal/conf/controlplane/config/v1/conf.pb.go @@ -84,8 +84,12 @@ type Bootstrap struct { Attestations *Attestations `protobuf:"bytes,21,opt,name=attestations,proto3" json:"attestations,omitempty"` // Outbound network policy applied to the integration plugins PluginsNetworkPolicy *PluginsNetworkPolicy `protobuf:"bytes,22,opt,name=plugins_network_policy,json=pluginsNetworkPolicy,proto3" json:"plugins_network_policy,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // Additional origins (scheme://host[:port]) the post-login callback may redirect to, + // on top of the control plane external URL, the login URL and the UI dashboard URL. + // Matched exactly, wildcards, paths, queries and fragments are not allowed + AllowedCallbackOrigins []string `protobuf:"bytes,23,rep,name=allowed_callback_origins,json=allowedCallbackOrigins,proto3" json:"allowed_callback_origins,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *Bootstrap) Reset() { @@ -266,6 +270,13 @@ func (x *Bootstrap) GetPluginsNetworkPolicy() *PluginsNetworkPolicy { return nil } +func (x *Bootstrap) GetAllowedCallbackOrigins() []string { + if x != nil { + return x.AllowedCallbackOrigins + } + return nil +} + type PluginsNetworkPolicy struct { state protoimpl.MessageState `protogen:"open.v1"` // Refuse to connect to destinations that are not publicly routable @@ -1843,7 +1854,7 @@ var File_controlplane_config_v1_conf_proto protoreflect.FileDescriptor const file_controlplane_config_v1_conf_proto_rawDesc = "" + "\n" + - "!controlplane/config/v1/conf.proto\x12\x16controlplane.config.v1\x1a\x1bbuf/validate/validate.proto\x1a#controlplane/config/v1/config.proto\x1a\x1bcredentials/v1/config.proto\x1a\x1egoogle/protobuf/duration.proto\"\x95\x12\n" + + "!controlplane/config/v1/conf.proto\x12\x16controlplane.config.v1\x1a\x1bbuf/validate/validate.proto\x1a#controlplane/config/v1/config.proto\x1a\x1bcredentials/v1/config.proto\x1a\x1egoogle/protobuf/duration.proto\"\xcf\x12\n" + "\tBootstrap\x126\n" + "\x06server\x18\x01 \x01(\v2\x1e.controlplane.config.v1.ServerR\x06server\x120\n" + "\x04data\x18\x02 \x01(\v2\x1c.controlplane.config.v1.DataR\x04data\x120\n" + @@ -1871,7 +1882,8 @@ const file_controlplane_config_v1_conf_proto_rawDesc = "" + "\x10ui_dashboard_url\x18\x13 \x01(\tR\x0euiDashboardUrl\x12\x80\x01\n" + " operation_authorization_provider\x18\x14 \x01(\v26.controlplane.config.v1.OperationAuthorizationProviderR\x1eoperationAuthorizationProvider\x12H\n" + "\fattestations\x18\x15 \x01(\v2$.controlplane.config.v1.AttestationsR\fattestations\x12b\n" + - "\x16plugins_network_policy\x18\x16 \x01(\v2,.controlplane.config.v1.PluginsNetworkPolicyR\x14pluginsNetworkPolicy\x1a\x8d\x03\n" + + "\x16plugins_network_policy\x18\x16 \x01(\v2,.controlplane.config.v1.PluginsNetworkPolicyR\x14pluginsNetworkPolicy\x128\n" + + "\x18allowed_callback_origins\x18\x17 \x03(\tR\x16allowedCallbackOrigins\x1a\x8d\x03\n" + "\rObservability\x12N\n" + "\x06sentry\x18\x01 \x01(\v26.controlplane.config.v1.Bootstrap.Observability.SentryR\x06sentry\x12Q\n" + "\atracing\x18\x02 \x01(\v27.controlplane.config.v1.Bootstrap.Observability.TracingR\atracing\x1a<\n" + diff --git a/app/controlplane/internal/conf/controlplane/config/v1/conf.proto b/app/controlplane/internal/conf/controlplane/config/v1/conf.proto index 757856c67..f53e81de3 100644 --- a/app/controlplane/internal/conf/controlplane/config/v1/conf.proto +++ b/app/controlplane/internal/conf/controlplane/config/v1/conf.proto @@ -129,6 +129,11 @@ message Bootstrap { // Outbound network policy applied to the integration plugins PluginsNetworkPolicy plugins_network_policy = 22; + + // Additional origins (scheme://host[:port]) the post-login callback may redirect to, + // on top of the control plane external URL, the login URL and the UI dashboard URL. + // Matched exactly, wildcards, paths, queries and fragments are not allowed + repeated string allowed_callback_origins = 23; } message PluginsNetworkPolicy { diff --git a/app/controlplane/internal/service/auth.go b/app/controlplane/internal/service/auth.go index 0f8bfa14f..df459f1ea 100644 --- a/app/controlplane/internal/service/auth.go +++ b/app/controlplane/internal/service/auth.go @@ -139,6 +139,11 @@ func NewAuthService(userUC *biz.UserUseCase, orgUC *biz.OrganizationUseCase, mUC return nil, fmt.Errorf("failed to create OIDC authenticator: %w", err) } + extraOrigins, err := parseCallbackOrigins(bootstrapConfig.GetAllowedCallbackOrigins()) + if err != nil { + return nil, fmt.Errorf("invalid allowed_callback_origins: %w", err) + } + svc := newService(opts...) if authConfig.DevUser != "" { err := generateAndLogDevUser(userUC, svc.log, authConfig) @@ -157,11 +162,39 @@ func NewAuthService(userUC *biz.UserUseCase, orgUC *biz.OrganizationUseCase, mUC membershipUseCase: mUC, orgInvitesUseCase: inviteUC, auditorUseCase: auc, - allowedCallbackOrigins: originsOf(authURLs.Login, bootstrapConfig.GetServer().GetHttp().GetExternalUrl(), - bootstrapConfig.GetUiDashboardUrl()), + allowedCallbackOrigins: append(originsOf(authURLs.Login, bootstrapConfig.GetServer().GetHttp().GetExternalUrl(), + bootstrapConfig.GetUiDashboardUrl()), extraOrigins...), }, nil } +// parseCallbackOrigins validates the operator-provided origins. Unlike originsOf, a bad entry is an +// error, so a typo fails at start up instead of silently leaving the origin out +func parseCallbackOrigins(raw []string) ([]string, error) { + origins := make([]string, 0, len(raw)) + for _, o := range raw { + u, err := url.Parse(o) + if err != nil { + return nil, fmt.Errorf("origin %q: %w", o, err) + } + + if u.Scheme != "http" && u.Scheme != "https" { + return nil, fmt.Errorf("origin %q: scheme must be http or https", o) + } + + if u.Host == "" || strings.Contains(u.Host, "*") { + return nil, fmt.Errorf("origin %q: a host without wildcards is required", o) + } + + if u.User != nil || (u.Path != "" && u.Path != "/") || u.RawQuery != "" || u.Fragment != "" || u.ForceQuery { + return nil, fmt.Errorf("origin %q: must be scheme://host[:port] only", o) + } + + origins = append(origins, originOf(u)) + } + + return origins, nil +} + var errInvalidCallback = errors.New("invalid callback URL") // originsOf extracts the origin of the given URLs, skipping empty or malformed ones diff --git a/app/controlplane/internal/service/auth_test.go b/app/controlplane/internal/service/auth_test.go index a35bc20bf..95117da1b 100644 --- a/app/controlplane/internal/service/auth_test.go +++ b/app/controlplane/internal/service/auth_test.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -166,6 +166,86 @@ func TestCallbackAllowed(t *testing.T) { } } +const ( + testOrigin = "https://api.example.com" + testOriginWithPort = testOrigin + ":8443" +) + +func TestParseCallbackOrigins(t *testing.T) { + testCases := []struct { + name string + origins []string + want []string + wantErr bool + }{ + {name: "nil list", origins: nil, want: []string{}}, + {name: "empty list", origins: []string{}, want: []string{}}, + {name: "origin", origins: []string{testOrigin}, want: []string{testOrigin}}, + {name: "origin with port", origins: []string{testOriginWithPort}, want: []string{testOriginWithPort}}, + {name: "origin with a trailing slash", origins: []string{"https://api.example.com/"}, want: []string{testOrigin}}, + {name: "mixed case is normalized", origins: []string{"https://API.Example.com"}, want: []string{testOrigin}}, + {name: "upper case scheme is normalized", origins: []string{"HTTPS://api.example.com"}, want: []string{testOrigin}}, + {name: "several origins", origins: []string{"https://a.example.com", "http://b.example.com:8080"}, want: []string{"https://a.example.com", "http://b.example.com:8080"}}, + {name: "empty entry", origins: []string{""}, wantErr: true}, + {name: "malformed entry", origins: []string{testOrigin, "://nope"}, wantErr: true}, + {name: "missing scheme", origins: []string{"api.example.com"}, wantErr: true}, + {name: "non http scheme", origins: []string{"ftp://api.example.com"}, wantErr: true}, + {name: "wildcard host", origins: []string{"https://*.example.com"}, wantErr: true}, + {name: "wildcard only", origins: []string{"*"}, wantErr: true}, + {name: "with a path", origins: []string{"https://api.example.com/auth/callback"}, wantErr: true}, + {name: "with a query", origins: []string{"https://api.example.com?foo=bar"}, wantErr: true}, + {name: "with a fragment", origins: []string{"https://api.example.com#foo"}, wantErr: true}, + {name: "with user info", origins: []string{"https://user@api.example.com"}, wantErr: true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := parseCallbackOrigins(tc.origins) + if tc.wantErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestCallbackAllowedAdditionalOrigins(t *testing.T) { + extra, err := parseCallbackOrigins([]string{testOriginWithPort}) + assert.NoError(t, err) + + testCases := []struct { + name string + allowed []string + callback string + wantErr bool + }{ + {name: "exact match", allowed: extra, callback: "https://api.example.com:8443/auth/callback?state=foo"}, + {name: "different port", allowed: extra, callback: "https://api.example.com:9443/auth/callback", wantErr: true}, + {name: "default port", allowed: extra, callback: "https://api.example.com/auth/callback", wantErr: true}, + {name: "different scheme", allowed: extra, callback: "http://api.example.com:8443/auth/callback", wantErr: true}, + {name: "allowed origin as a host prefix", allowed: extra, callback: "https://api.example.com.evil.example:8443/collect", wantErr: true}, + {name: "allowed origin as a port prefix", allowed: extra, callback: "https://api.example.com:84431/collect", wantErr: true}, + {name: "allowed origin as user info", allowed: extra, callback: "https://api.example.com:8443@evil.example/collect", wantErr: true}, + {name: "empty list changes nothing", allowed: nil, callback: "https://api.example.com:8443/auth/callback", wantErr: true}, + {name: "empty list still allows loopback", allowed: nil, callback: "http://127.0.0.1:41337/auth/callback"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := callbackAllowed(tc.callback, tc.allowed) + if tc.wantErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + }) + } +} + // The callback cookie must not be set for a destination we would refuse to redirect to func TestLoginHandlerRejectsForeignCallback(t *testing.T) { svc := &AuthService{allowedCallbackOrigins: originsOf("https://app.chainloop.dev")} diff --git a/deployment/chainloop/Chart.yaml b/deployment/chainloop/Chart.yaml index db0f34be4..2d62c5bb8 100644 --- a/deployment/chainloop/Chart.yaml +++ b/deployment/chainloop/Chart.yaml @@ -7,7 +7,7 @@ description: Chainloop is an open source software supply chain control plane, a type: application # Bump the patch (not minor, not major) version on each change in the Chart Source code -version: 1.465.0 +version: 1.465.1 # Do not update appVersion, this is handled automatically by the release process appVersion: v1.122.2 diff --git a/deployment/chainloop/README.md b/deployment/chainloop/README.md index 25eb3392b..ccd8ddc37 100644 --- a/deployment/chainloop/README.md +++ b/deployment/chainloop/README.md @@ -578,6 +578,7 @@ Once done, you can access with [two predefined users](https://github.com/chainlo | `controlplane.federatedAuthentication` | Enable federated authentication during attestation process | | | `controlplane.federatedAuthentication.enabled` | Enable federated authentication | `false` | | `controlplane.federatedAuthentication.url` | URL of the federated authentication endpoint | `""` | +| `controlplane.allowedCallbackOrigins` | Additional HTTP(S) origins (http(s)://host[:port]) the OIDC login callback may redirect to, on top of the control plane external URL and the dashboard UI URL. Matched exactly, other schemes, wildcards and paths are rejected at startup | `[]` | | `controlplane.nats` | optional NATS configuration for events publishing. | | | `controlplane.nats.enabled` | Enable events publishing through a Nats stream | `false` | | `controlplane.nats.host` | NATS Host | `""` | diff --git a/deployment/chainloop/templates/controlplane/configmap.yaml b/deployment/chainloop/templates/controlplane/configmap.yaml index 018742f03..162d955ae 100644 --- a/deployment/chainloop/templates/controlplane/configmap.yaml +++ b/deployment/chainloop/templates/controlplane/configmap.yaml @@ -58,6 +58,10 @@ data: {{- if .Values.controlplane.uiDashboardURL }} ui_dashboard_url: {{ .Values.controlplane.uiDashboardURL | quote }} {{- end }} + {{- with .Values.controlplane.allowedCallbackOrigins }} + allowed_callback_origins: + {{- toYaml . | nindent 6 }} + {{- end }} {{ if .Values.controlplane.onboarding }} onboarding: {{- toYaml .Values.controlplane.onboarding | nindent 6 }} diff --git a/deployment/chainloop/values.yaml b/deployment/chainloop/values.yaml index f6f5089f3..23fb014d7 100644 --- a/deployment/chainloop/values.yaml +++ b/deployment/chainloop/values.yaml @@ -200,6 +200,9 @@ controlplane: ## @param controlplane.uiDashboardURL Optional external URL for the dashboard UI. If set, CLI will display attestation view links after push uiDashboardURL: "" + ## @param controlplane.allowedCallbackOrigins Additional HTTP(S) origins (http(s)://host[:port]) the OIDC login callback may redirect to, on top of the control plane external URL and the dashboard UI URL. Matched exactly, other schemes, wildcards and paths are rejected at startup + allowedCallbackOrigins: [] + ## @extra controlplane.nats optional NATS configuration for events publishing. ## @param controlplane.nats.enabled Enable events publishing through a Nats stream ## @param controlplane.nats.host NATS Host @@ -1755,7 +1758,7 @@ postgresql: auth: enablePostgresUser: false username: "chainloop" - password: "chainlooppwd" + password: "chainlooppwd" # checkov:skip=CKV_SECRET_6: default for the bundled development PostgreSQL, override in production database: "chainloop-cp" existingSecret: ""