diff --git a/chart/Chart.yaml b/chart/Chart.yaml index 0bbc9778..0aeb0636 100644 --- a/chart/Chart.yaml +++ b/chart/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: versitygw description: A Helm chart for deploying the Versity S3 Gateway on Kubernetes type: application -version: 0.4.2 +version: 0.4.3 sources: - https://github.com/versity/versitygw icon: https://raw.githubusercontent.com/versity/versitygw/main/webui/web/assets/images/Versity-logo-blue-horizontal.png diff --git a/chart/README.md b/chart/README.md index 7a2ac9c6..504a0e76 100644 --- a/chart/README.md +++ b/chart/README.md @@ -146,7 +146,7 @@ Key points: - **Private mTLS endpoint**: gateways reach the standalone IAM service over a private endpoint (`iamServer.private.port`, default `7443`) that always requires mutual TLS on TCP. Provide certificates either via `existingSecret` (bring your own `tls.crt`/`tls.key`/`ca.crt`) or `certificate.create=true` to auto-provision via cert-manager. - **Shared CA requirement**: when using cert-manager auto-provisioning, `iamServer.private.certificate.issuerRef` and `iam.standalone.certificate.issuerRef` **must reference the same CA-type issuer** (an `Issuer`/`ClusterIssuer` of kind `CA`, or a Vault issuer) — one that populates `ca.crt` in the resulting Secret. Both sides verify their peer using their own certificate's `ca.crt`, which only works when both certificates share the same issuing CA. - **External IAM service**: to point a gateway at a standalone IAM service deployed outside this chart (or by a separate chart release), set `iam.standalone.endpoint` to its `host:port` and provide the mTLS material via `iam.standalone.certificate.existingSecret`. -- **OIDC identity providers**: `AssumeRoleWithWebIdentity` only trusts an OIDC provider reachable over verified https at a publicly routable address on the implicit `:443`. An in-cluster provider satisfies none of that, so `iamServer.oidc.allowPrivateEndpoints` permits private/loopback addresses and an explicit port (a SPIRE OIDC discovery provider on a ClusterIP `Service`), and `iamServer.oidc.allowInsecureTransport` additionally permits plaintext `http://` providers and drops TLS verification (the same provider bound to `127.0.0.1` as a sidecar). Each relaxes real protections — see the comments in `values.yaml` — so enable only the one your topology needs. +- **OIDC identity providers**: `AssumeRoleWithWebIdentity` only trusts an OIDC provider reachable over verified https at a publicly routable address on the implicit `:443`. An in-cluster provider satisfies none of that, so `iamServer.oidc.allowPrivateEndpoints` permits private/loopback addresses and an explicit port (a SPIRE OIDC discovery provider on a ClusterIP `Service`), and `iamServer.oidc.allowInsecureTransport` additionally permits plaintext `http://` providers and drops TLS verification (the same provider bound to `127.0.0.1` as a sidecar). Each relaxes real protections — see the comments in `values.yaml` — so enable only the one your topology needs. `iamServer.oidc.discoveryUrls` is the narrower alternative for a provider that is public but reachable in-cluster: it moves only one provider's discovery fetch to an address you name (private addresses included, without `allowPrivateEndpoints`), while tokens keep naming their public issuer. - **WebUI access**: to manage IAM users from the WebUI, set `webui.iamGateways` to the URL a browser can reach `iamServer` on, and `iamServer.corsAllowOrigin` to the WebUI's own origin. Every WebUI call to the IAM API is cross-origin, so without `corsAllowOrigin` the browser blocks it and the WebUI's IAM navigation silently never appears. - **Secret rotation**: the processes load mTLS material and environment-based credentials at startup. After a referenced Secret rotates, restart both Deployments or configure a Secret-reloader controller through `deploymentAnnotations` and `iamServer.deploymentAnnotations`. diff --git a/chart/templates/iam-deployment.yaml b/chart/templates/iam-deployment.yaml index b3f06540..8a439578 100644 --- a/chart/templates/iam-deployment.yaml +++ b/chart/templates/iam-deployment.yaml @@ -124,6 +124,10 @@ spec: - name: VGW_IAM_OIDC_ALLOW_INSECURE_TRANSPORT value: "true" {{- end }} + {{- if $iamServerOidc.discoveryUrls }} + - name: VGW_IAM_OIDC_DISCOVERY_URLS + value: {{ join "," $iamServerOidc.discoveryUrls | quote }} + {{- end }} {{- if .Values.iamServer.corsAllowOrigin }} - name: VGW_CORS_ALLOW_ORIGIN value: {{ .Values.iamServer.corsAllowOrigin | quote }} diff --git a/chart/values.yaml b/chart/values.yaml index 89b2f389..1a6f2587 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -435,6 +435,19 @@ iamServer: # own pod. Needs allowPrivateEndpoints as well for a loopback or # cluster-internal address. allowInsecureTransport: false + # Fetch individual providers' discovery documents from somewhere other + # than the provider URL itself, as "=" + # entries. The discovery URL is fetched exactly as written, so include the + # "/.well-known/openid-configuration" path, and it may be a private + # in-cluster address without allowPrivateEndpoints above -- it is named + # here rather than by a request. Only the fetch moves: tokens are still + # matched against the provider URL, the fetched document's own issuer must + # still equal it, and keys still come from the jwks_uri that document + # publishes. This is how an identity provider hands out tokens naming its + # public issuer while the gateway reads its keys over a cluster-internal + # Service. + discoveryUrls: [] + # - "https://oidc.example.com=https://oidc.oidc-ns/.well-known/openid-configuration" # Access-Control-Allow-Origin for the control-plane API. Required before a # browser can reach this service: the WebUI is served from another origin, so # every call it makes is cross-origin and is blocked without this. Set it to diff --git a/cmd/internal/gwcli/iam.go b/cmd/internal/gwcli/iam.go index b49bce3c..60ae6e0e 100644 --- a/cmd/internal/gwcli/iam.go +++ b/cmd/internal/gwcli/iam.go @@ -124,6 +124,11 @@ func IAMCommand() *cli.Command { Usage: "allow plaintext http OIDC provider URLs and skip TLS certificate verification (thumbprint pinning included) for https ones; only for an identity provider reached over an already-trusted path, such as a loopback-bound sidecar", EnvVars: []string{"VGW_IAM_OIDC_ALLOW_INSECURE_TRANSPORT"}, }, + &cli.StringSliceFlag{ + Name: "oidc-discovery-url", + Usage: "fetch one OIDC provider's discovery document from somewhere other than the provider URL itself, as '=' (can be specified multiple times); the discovery URL is fetched exactly as given, so include the '/.well-known/openid-configuration' path, and it may be a private in-cluster address without --oidc-allow-private-endpoints. Tokens are still matched against the provider URL, and keys are still read from the jwks_uri the fetched document publishes", + EnvVars: []string{"VGW_IAM_OIDC_DISCOVERY_URLS"}, + }, &cli.StringSliceFlag{ Name: "private-ports", Usage: "private endpoint listen address: a unix socket path, or :/: when mTLS (--private-cert/--private-cert-key/--private-client-ca) is also configured — refuses to start otherwise (can be specified multiple times)", diff --git a/cmd/versitygw/iam.go b/cmd/versitygw/iam.go index 93254f4a..610dc331 100644 --- a/cmd/versitygw/iam.go +++ b/cmd/versitygw/iam.go @@ -73,6 +73,7 @@ func runIAM(ctx *cli.Context) error { DisableOIDCThumbprintAutoFetch: ctx.Bool("disable-oidc-thumbprint-autofetch"), OIDCAllowPrivateEndpoints: ctx.Bool("oidc-allow-private-endpoints"), OIDCAllowInsecureTransport: ctx.Bool("oidc-allow-insecure-transport"), + OIDCDiscoveryURLs: ctx.StringSlice("oidc-discovery-url"), CORSAllowOrigin: corsAllowOrigin, Region: region, WebuiPorts: webuiPorts, diff --git a/embedgw/iam.go b/embedgw/iam.go index 9bbf1431..00006406 100644 --- a/embedgw/iam.go +++ b/embedgw/iam.go @@ -208,6 +208,15 @@ type IAMConfig struct { // itself trusted, such as a discovery provider bound to loopback as a // sidecar in this process's own pod. OIDCAllowInsecureTransport bool + + // OIDCDiscoveryURLs redirects individual providers' discovery-document + // fetches, as "=" pairs. The discovery URL + // is fetched exactly as given, path included. Only the fetch moves: the + // provider Url stays what a token's iss claim and the fetched document's + // own issuer field must match, so an IdP can hand out tokens naming its + // public issuer while this process reads its keys over a private, + // in-cluster path. + OIDCDiscoveryURLs []string } // privateAPIServer is the standalone IAM service's private endpoint set @@ -441,6 +450,9 @@ func RunIAMAPI(ctx context.Context, cfg *IAMConfig) error { if cfg.OIDCAllowInsecureTransport { opts = append(opts, iamapi.WithOIDCAllowInsecureTransport()) } + if len(cfg.OIDCDiscoveryURLs) > 0 { + opts = append(opts, iamapi.WithOIDCDiscoveryURLs(cfg.OIDCDiscoveryURLs)) + } corsAllowOrigin := strings.TrimSpace(cfg.CORSAllowOrigin) if len(cfg.WebuiPorts) > 0 && corsAllowOrigin == "" { // Every WebUI call to this API is cross-origin, so without an allowed diff --git a/extra/example-iam.conf b/extra/example-iam.conf index 5ae70692..d6bb7e42 100644 --- a/extra/example-iam.conf +++ b/extra/example-iam.conf @@ -153,4 +153,13 @@ ROOT_SECRET_ACCESS_KEY= # verification (thumbprint pinning included) for https ones; only for an # identity provider reached over an already-trusted path, such as a # loopback-bound sidecar. -#VGW_IAM_OIDC_ALLOW_INSECURE_TRANSPORT=false \ No newline at end of file +#VGW_IAM_OIDC_ALLOW_INSECURE_TRANSPORT=false + +# Fetch one OIDC provider's discovery document from somewhere other than the +# provider URL itself, as '='. Specify one or +# more comma-separated entries. The discovery URL is fetched exactly as +# given, so include the '/.well-known/openid-configuration' path, and it may +# be a private in-cluster address without VGW_IAM_OIDC_ALLOW_PRIVATE_ENDPOINTS. +# Tokens are still matched against the provider URL, and keys are still read +# from the jwks_uri the fetched document publishes. +#VGW_IAM_OIDC_DISCOVERY_URLS= \ No newline at end of file diff --git a/iamapi/controller.go b/iamapi/controller.go index b6bcdc66..8e3a59c6 100644 --- a/iamapi/controller.go +++ b/iamapi/controller.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "slices" + "strings" "time" "github.com/gofiber/fiber/v3" @@ -1054,13 +1055,14 @@ func (c IAMApiController) CreateOpenIDConnectProvider(ctx fiber.Ctx) (*Response, thumbprints := iamutil.ParseStringList(ctx, "ThumbprintList") if len(thumbprints) == 0 { + endpoint, _ := c.oidcPolicy.ResolveDiscovery(url) switch { - case iamutil.IsInsecureOIDCProviderURL(url): - // A plaintext http provider never presents a certificate, so + case !strings.HasPrefix(endpoint, "https://"): + // A plaintext http endpoint never presents a certificate, so // there is nothing to auto-fetch and nothing for a later JWKS // fetch to pin against: an empty ThumbprintList is the accurate // record of that, not a missing one. - debuglogger.Logf("CreateOpenIDConnectProvider: %q is a plaintext http provider; storing an empty ThumbprintList", url) + debuglogger.Logf("CreateOpenIDConnectProvider: %q is reached over plaintext http; storing an empty ThumbprintList", endpoint) case c.oidc.ThumbprintAutoFetchDisabled: debuglogger.Logf("CreateOpenIDConnectProvider: ThumbprintList omitted and auto-fetch is disabled") return nil, iamerr.MissingValue("thumbprintList") diff --git a/iamapi/controller_test.go b/iamapi/controller_test.go index a2527a72..aa990495 100644 --- a/iamapi/controller_test.go +++ b/iamapi/controller_test.go @@ -3864,6 +3864,102 @@ func TestIAMApiControllerAssumeRoleWithWebIdentityLoopbackIdP(t *testing.T) { }) } +// TestIAMApiControllerAssumeRoleWithWebIdentityDiscoveryURL exercises a +// configured discovery URL: the token's issuer is a public URL that is never +// contacted, while the discovery document and the JWKS it points to are +// served from loopback. Only the discovery override makes that address +// reachable — insecure transport is about schemes, not addresses — and the +// document's own issuer must still equal the provider Url. +func TestIAMApiControllerAssumeRoleWithWebIdentityDiscoveryURL(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa.GenerateKey: %v", err) + } + + const goodIssuer = "https://oidc.discovery.example" + const badIssuer = "https://oidc.mismatch.example" + + var keysURI string + idp := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/good/.well-known/openid-configuration": + json.NewEncoder(w).Encode(map[string]any{"issuer": goodIssuer, "jwks_uri": keysURI}) + case "/mismatch/.well-known/openid-configuration": + // Served for badIssuer, but naming its own private location. + json.NewEncoder(w).Encode(map[string]any{"issuer": r.Host, "jwks_uri": keysURI}) + case "/keys": + json.NewEncoder(w).Encode(map[string]any{"keys": []any{map[string]any{ + "kty": "RSA", + "kid": "k1", + "n": base64.RawURLEncoding.EncodeToString(key.N.Bytes()), + "e": base64.RawURLEncoding.EncodeToString(big.NewInt(int64(key.E)).Bytes()), + }}}) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer idp.Close() + keysURI = idp.URL + "/keys" + + server := newIAMControllerTestServerWith(t, + WithOIDCAllowInsecureTransport(), + WithOIDCDiscoveryURLs([]string{ + goodIssuer + "=" + idp.URL + "/good/.well-known/openid-configuration", + badIssuer + "=" + idp.URL + "/mismatch/.well-known/openid-configuration", + })) + + for _, issuer := range []string{goodIssuer, badIssuer} { + providerArn := createTestOIDCProviderForTrust(t, server, issuer, "versitygw") + roleName := "role-for-" + iamutil.CanonicalOIDCProviderURL(issuer) + createTestRoleForTrust(t, server, roleName, + `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Federated":"`+providerArn+`"},`+ + `"Action":"sts:AssumeRoleWithWebIdentity"}]}`) + } + + assume := func(t *testing.T, issuer, roleName string) *http.Response { + t.Helper() + token := jwt.NewWithClaims(jwt.SigningMethodRS256, jwt.MapClaims{ + "iss": issuer, + "aud": "versitygw", + "sub": "spiffe://example.org/ns/default/sa/versitygw", + "iat": time.Now().Unix(), + "exp": time.Now().Add(time.Hour).Unix(), + }) + token.Header["kid"] = "k1" + signed, err := token.SignedString(key) + if err != nil { + t.Fatalf("sign token: %v", err) + } + return doSTSAction(t, server, url.Values{ + "Action": {"AssumeRoleWithWebIdentity"}, + "RoleArn": {"arn:aws:iam::000000000000:role/" + roleName}, + "RoleSessionName": {"discovery-session"}, + "WebIdentityToken": {signed}, + }) + } + + t.Run("keys read over the private path", func(t *testing.T) { + resp := assume(t, goodIssuer, "role-for-oidc.discovery.example") + if resp.StatusCode != http.StatusOK { + t.Fatalf("AssumeRoleWithWebIdentity status = %d, body=%s", resp.StatusCode, readBody(t, resp)) + } + var out iamtypes.AssumeRoleWithWebIdentityResponse + unmarshalXML(t, readBody(t, resp), &out) + if out.Result.Provider != goodIssuer { + t.Errorf("Provider = %q, want the public issuer %q", out.Result.Provider, goodIssuer) + } + if out.Result.Credentials.SessionToken == "" { + t.Errorf("no session credentials returned: %+v", out.Result.Credentials) + } + }) + + t.Run("discovery document naming another issuer is rejected", func(t *testing.T) { + resp := assume(t, badIssuer, "role-for-oidc.mismatch.example") + requireSTSError(t, resp, http.StatusBadRequest, "Sender", "InvalidIdentityToken", + "Couldn't retrieve verification key from your identity provider, please reference AssumeRoleWithWebIdentity documentation for requirements") + }) +} + // accessKeyImplicitUserNameActions are the four access-key actions that // accept an omitted UserName and infer it from the calling access key. var accessKeyImplicitUserNameActions = []string{"CreateAccessKey", "UpdateAccessKey", "DeleteAccessKey", "ListAccessKeys"} diff --git a/iamapi/internal/iamutil/oidc.go b/iamapi/internal/iamutil/oidc.go index b0643bda..43f47277 100644 --- a/iamapi/internal/iamutil/oidc.go +++ b/iamapi/internal/iamutil/oidc.go @@ -79,6 +79,12 @@ type OIDCEndpointPolicy struct { // path is trustworthy on its own, such as a sidecar bound to loopback // inside the gateway's own pod. AllowInsecureTransport bool + + // DiscoveryURLs maps a stored provider Url to the exact URL its + // discovery document is fetched from, so an IdP's keys can be read over + // a private in-cluster path while the tokens it issues keep naming their + // public issuer. Only the fetch moves; see ResolveDiscovery. + DiscoveryURLs map[string]string } // IsInsecureOIDCProviderURL reports whether providerURL, a stored provider @@ -103,6 +109,32 @@ func OIDCEndpointURL(providerURL string) string { return "https://" + providerURL } +// CanonicalOIDCProviderURL reduces a full provider URL to the form providers +// are stored under, the inverse of OIDCEndpointURL. +func CanonicalOIDCProviderURL(rawURL string) string { + if IsInsecureOIDCProviderURL(rawURL) { + return rawURL + } + return strings.TrimPrefix(rawURL, "https://") +} + +// ResolveDiscovery returns where providerURL's discovery document is fetched +// from, and the policy governing that fetch and the jwks_uri the document +// publishes: the well-known path under the provider's own endpoint under an +// unchanged policy, or a configured DiscoveryURLs entry under one whose +// private-address check is waived. That entry is named by the operator at +// startup rather than by a request, so it needs no AllowPrivateEndpoints to +// be private. Transport is unaffected either way. +func (p OIDCEndpointPolicy) ResolveDiscovery(providerURL string) (string, OIDCEndpointPolicy) { + endpoint, ok := p.DiscoveryURLs[providerURL] + if !ok { + base := strings.TrimRight(OIDCEndpointURL(providerURL), "/") + return base + "/.well-known/openid-configuration", p + } + p.AllowPrivateEndpoints = true + return endpoint, p +} + // ParseStringList reads flat indexed list members ".member.1", // ".member.2", ... — the AWS Query-protocol wire form for a bare // []string (distinct from ParseTags's Key/Value-pair member form, used by @@ -256,10 +288,7 @@ func ValidateOIDCProviderURL(rawURL string, policy OIDCEndpointPolicy) (string, return "", iamerr.InvalidInput("Invalid Open ID Connect Provider URL.") } - if insecure { - return rawURL, nil - } - return strings.TrimPrefix(rawURL, "https://"), nil + return CanonicalOIDCProviderURL(rawURL), nil } func isValidOIDCHostname(host string) bool { @@ -269,7 +298,7 @@ func isValidOIDCHostname(host string) bool { if host == "" || len(host) > 253 { return false } - for _, label := range strings.Split(host, ".") { + for label := range strings.SplitSeq(host, ".") { if !oidcHostLabelPattern.MatchString(label) { return false } diff --git a/iamapi/internal/iamutil/oidc_test.go b/iamapi/internal/iamutil/oidc_test.go index bb77f7e0..150dee5a 100644 --- a/iamapi/internal/iamutil/oidc_test.go +++ b/iamapi/internal/iamutil/oidc_test.go @@ -25,6 +25,44 @@ var ( insecureOIDCPolicy = OIDCEndpointPolicy{AllowPrivateEndpoints: true, AllowInsecureTransport: true} ) +// TestResolveDiscovery covers where a provider's discovery document is +// fetched from, and the address-check waiver a configured discovery URL +// carries: the endpoint is named by the operator at startup, not by a +// request, so it needs no AllowPrivateEndpoints to be private. +func TestResolveDiscovery(t *testing.T) { + const clusterURL = "https://oidc.oidc-ns/.well-known/openid-configuration" + policy := OIDCEndpointPolicy{DiscoveryURLs: map[string]string{"oidc.example.com": clusterURL}} + + tests := []struct { + name string + providerURL string + want string + wantPrivate bool + }{ + {"override", "oidc.example.com", clusterURL, true}, + {"other provider unaffected", "other.example.com", "https://other.example.com/.well-known/openid-configuration", false}, + {"trailing slash", "other.example.com/", "https://other.example.com/.well-known/openid-configuration", false}, + {"http provider keeps its scheme", "http://127.0.0.1:8080", "http://127.0.0.1:8080/.well-known/openid-configuration", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, gotPolicy := policy.ResolveDiscovery(tt.providerURL) + if got != tt.want { + t.Errorf("ResolveDiscovery(%q) = %q, want %q", tt.providerURL, got, tt.want) + } + if gotPolicy.AllowPrivateEndpoints != tt.wantPrivate { + t.Errorf("AllowPrivateEndpoints = %v, want %v", gotPolicy.AllowPrivateEndpoints, tt.wantPrivate) + } + }) + } + + // The waiver is scoped to the returned copy: the policy the rest of the + // request is validated against keeps its address check. + if policy.AllowPrivateEndpoints { + t.Error("ResolveDiscovery mutated the receiver's AllowPrivateEndpoints") + } +} + func TestValidateOIDCProviderURL(t *testing.T) { tests := []struct { name string diff --git a/iamapi/internal/iamutil/oidc_thumbprint.go b/iamapi/internal/iamutil/oidc_thumbprint.go index 0157e2f7..3cd935d0 100644 --- a/iamapi/internal/iamutil/oidc_thumbprint.go +++ b/iamapi/internal/iamutil/oidc_thumbprint.go @@ -38,7 +38,9 @@ const oidcThumbprintFetchTimeout = 8 * time.Second // the presented chain against the system trust store and the provider's own // hostname like any normal TLS client, and returns the SHA-1 thumbprint of // the last (top-most/intermediate CA) certificate in the peer's presented -// chain. +// chain. A configured discovery URL moves the handshake to that endpoint's +// host, since that is the host every later fetch pins this thumbprint +// against. // // SSRF hardening: the hostname is resolved once via // net.DefaultResolver.LookupIP; if any resolved address is @@ -70,14 +72,15 @@ const oidcThumbprintFetchTimeout = 8 * time.Second // verification for this handshake entirely and pins whatever is presented. func FetchThumbprint(ctx context.Context, providerURL string, policy OIDCEndpointPolicy) (string, error) { displayURL := OIDCEndpointURL(providerURL) - if IsInsecureOIDCProviderURL(providerURL) { - // A plaintext http provider performs no handshake, so there is no + endpoint, policy := policy.ResolveDiscovery(providerURL) + if !strings.HasPrefix(endpoint, "https://") { + // A plaintext http endpoint performs no handshake, so there is no // certificate to observe. Callers skip auto-fetch for these // entirely; this is the guard for the ones that don't. - debuglogger.Logf("oidc thumbprint fetch: %q is a plaintext http provider and presents no certificate", displayURL) + debuglogger.Logf("oidc thumbprint fetch: %q is reached over plaintext http and presents no certificate", endpoint) return "", iamerr.OpenIdIdpCommunicationError(displayURL) } - host, port := splitOIDCHostPort(hostFromOIDCUrl(providerURL)) + host, port := splitOIDCHostPort(hostFromOIDCUrl(CanonicalOIDCProviderURL(endpoint))) ctx, cancel := context.WithTimeout(ctx, oidcThumbprintFetchTimeout) defer cancel() diff --git a/iamapi/internal/iamutil/oidc_thumbprint_test.go b/iamapi/internal/iamutil/oidc_thumbprint_test.go index 677164ec..10e1b618 100644 --- a/iamapi/internal/iamutil/oidc_thumbprint_test.go +++ b/iamapi/internal/iamutil/oidc_thumbprint_test.go @@ -180,6 +180,31 @@ func TestFetchThumbprintAllowPrivateEndpoints(t *testing.T) { } } +// TestFetchThumbprintDiscoveryURL confirms auto-fetch pins the endpoint it +// will actually talk to: with a discovery URL configured, the handshake goes +// to that endpoint's host — a loopback one here, which the default address +// check would refuse — rather than to the provider's own unreachable host. +func TestFetchThumbprintDiscoveryURL(t *testing.T) { + srv := httptest.NewTLSServer(nil) + defer srv.Close() + + policy := OIDCEndpointPolicy{ + AllowInsecureTransport: true, + DiscoveryURLs: map[string]string{ + "idp.example": "https://" + srv.Listener.Addr().String() + "/.well-known/openid-configuration", + }, + } + got, err := FetchThumbprint(context.Background(), "idp.example", policy) + if err != nil { + t.Fatalf("FetchThumbprint(discovery url): %v", err) + } + + sum := sha1.Sum(srv.Certificate().Raw) + if want := hex.EncodeToString(sum[:]); got != want { + t.Fatalf("FetchThumbprint thumbprint = %q, want the discovery endpoint's %q", got, want) + } +} + // TestFetchThumbprintRejectsPlaintextProvider covers the defensive branch // for an http provider Url: there is no handshake to observe a certificate // in, so auto-fetch must report a failure rather than dial anything. diff --git a/iamapi/internal/iamutil/webidentity.go b/iamapi/internal/iamutil/webidentity.go index 721d8e6e..7e12daa4 100644 --- a/iamapi/internal/iamutil/webidentity.go +++ b/iamapi/internal/iamutil/webidentity.go @@ -617,18 +617,22 @@ var ( // current ThumbprintList (freshly read from storage for the request being // verified), so a changed configuration always maps to a different key here; // thumbprints are sorted first since storage doesn't guarantee list order is -// stable across reads of an unchanged provider. -func jwksCacheKey(issuerURL string, thumbprints []string) string { +// stable across reads of an unchanged provider. The discovery endpoint +// policy resolves issuerURL to is bound in as well, so an in-process restart +// with a different discovery override never reuses key material fetched +// from the previous endpoint. +func jwksCacheKey(issuerURL string, thumbprints []string, policy OIDCEndpointPolicy) string { sorted := slices.Clone(thumbprints) slices.Sort(sorted) - return issuerURL + "|" + strings.Join(sorted, ",") + discoveryURL, _ := policy.ResolveDiscovery(issuerURL) + return issuerURL + "|" + discoveryURL + "|" + strings.Join(sorted, ",") } // cachedJWKS returns issuerURL's key set from cache if a fresh-enough entry // exists for the current thumbprints, otherwise fetches and caches a fresh // one. func cachedJWKS(ctx context.Context, issuerURL string, thumbprints []string, policy OIDCEndpointPolicy) (*jwkSet, error) { - key := jwksCacheKey(issuerURL, thumbprints) + key := jwksCacheKey(issuerURL, thumbprints, policy) jwksCacheMu.Lock() entry, ok := jwksCache[key] jwksCacheMu.Unlock() @@ -656,7 +660,7 @@ func cachedJWKS(ctx context.Context, issuerURL string, thumbprints []string, pol // gated the next one. Recording the attempt up front bounds retries to one // per jwksMinForcedRefreshInterval regardless of whether the fetch succeeds. func forceRefreshJWKSCache(ctx context.Context, issuerURL string, thumbprints []string, policy OIDCEndpointPolicy) (*jwkSet, error) { - key := jwksCacheKey(issuerURL, thumbprints) + key := jwksCacheKey(issuerURL, thumbprints, policy) jwksCacheMu.Lock() entry, ok := jwksCache[key] if ok && time.Since(entry.lastForcedRefresh) < jwksMinForcedRefreshInterval { @@ -679,12 +683,12 @@ func forceRefreshJWKSCache(ctx context.Context, issuerURL string, thumbprints [] } // fetchAndCacheJWKS fetches issuerURL's key set and, on success, replaces -// its cache entry, coalescing concurrent callers for the same issuerURL AND -// thumbprints via jwksFetchGroup (keyed identically to jwksCache, so a +// its cache entry, coalescing concurrent callers for the same issuerURL, +// thumbprints AND discovery endpoint via jwksFetchGroup (keyed identically to jwksCache, so a // caller mid-fetch for one thumbprint configuration never receives a result // coalesced from a differently-configured concurrent caller). func fetchAndCacheJWKS(ctx context.Context, issuerURL string, thumbprints []string, policy OIDCEndpointPolicy) (*jwkSet, error) { - key := jwksCacheKey(issuerURL, thumbprints) + key := jwksCacheKey(issuerURL, thumbprints, policy) v, err, _ := jwksFetchGroup.Do(key, func() (any, error) { keys, err := fetchJWKS(ctx, issuerURL, thumbprints, policy) if err != nil { @@ -704,17 +708,18 @@ func fetchAndCacheJWKS(ctx context.Context, issuerURL string, thumbprints []stri return v.(*jwkSet), nil } -// fetchJWKS retrieves issuerURL's OIDC discovery document, then the JWKS it -// points to. issuerURL is the provider's stored Url. thumbprints, if -// non-empty, lets the fetch's TLS connections succeed against a -// self-signed/private-CA certificate whose chain matches one of them, the -// same trust-pinning fallback real AWS documents for OIDC providers. +// fetchJWKS retrieves issuerURL's OIDC discovery document — from wherever +// ResolveDiscovery places it — then the JWKS that document points to. +// issuerURL is the provider's stored Url. thumbprints, if non-empty, lets +// the fetch's TLS connections succeed against a self-signed/private-CA +// certificate whose chain matches one of them, the same trust-pinning +// fallback real AWS documents for OIDC providers. func fetchJWKS(ctx context.Context, issuerURL string, thumbprints []string, policy OIDCEndpointPolicy) (*jwkSet, error) { + discoveryURL, policy := policy.ResolveDiscovery(issuerURL) client := ssrfSafeHTTPClient(thumbprints, policy) - base := OIDCEndpointURL(issuerURL) var doc oidcDiscoveryDoc - if err := fetchJSON(ctx, client, strings.TrimRight(base, "/")+"/.well-known/openid-configuration", &doc); err != nil { + if err := fetchJSON(ctx, client, discoveryURL, &doc); err != nil { return nil, err } if err := validateDiscoveryIssuer(doc, issuerURL); err != nil { diff --git a/iamapi/internal/iamutil/webidentity_test.go b/iamapi/internal/iamutil/webidentity_test.go index 4147222d..ffccedb1 100644 --- a/iamapi/internal/iamutil/webidentity_test.go +++ b/iamapi/internal/iamutil/webidentity_test.go @@ -572,27 +572,31 @@ func TestValidateDiscoveryIssuer(t *testing.T) { } func TestJWKSCacheKeyBindsThumbprints(t *testing.T) { - base := jwksCacheKey("example.com", []string{"aaaa"}) + base := jwksCacheKey("example.com", []string{"aaaa"}, OIDCEndpointPolicy{}) - if got := jwksCacheKey("example.com", []string{"bbbb"}); got == base { + if got := jwksCacheKey("example.com", []string{"bbbb"}, OIDCEndpointPolicy{}); got == base { t.Errorf("jwksCacheKey did not change when thumbprint changed: %q", got) } - if got := jwksCacheKey("example.com", nil); got == base { + if got := jwksCacheKey("example.com", nil, OIDCEndpointPolicy{}); got == base { t.Errorf("jwksCacheKey did not change when thumbprint was removed: %q", got) } - if got := jwksCacheKey("other.example.com", []string{"aaaa"}); got == base { + if got := jwksCacheKey("other.example.com", []string{"aaaa"}, OIDCEndpointPolicy{}); got == base { t.Errorf("jwksCacheKey did not change when issuer changed: %q", got) } + overridden := OIDCEndpointPolicy{DiscoveryURLs: map[string]string{"example.com": "https://oidc.internal/.well-known/openid-configuration"}} + if got := jwksCacheKey("example.com", []string{"aaaa"}, overridden); got == base { + t.Errorf("jwksCacheKey did not change when the discovery endpoint changed: %q", got) + } // Storage doesn't guarantee ThumbprintList order is stable across reads // of an unchanged provider, so the key must not depend on input order. - if got := jwksCacheKey("example.com", []string{"bbbb", "aaaa"}); got != jwksCacheKey("example.com", []string{"aaaa", "bbbb"}) { + if got := jwksCacheKey("example.com", []string{"bbbb", "aaaa"}, OIDCEndpointPolicy{}); got != jwksCacheKey("example.com", []string{"aaaa", "bbbb"}, OIDCEndpointPolicy{}) { t.Errorf("jwksCacheKey is sensitive to thumbprint order: %q", got) } } func TestForceRefreshJWKSCacheGatesFailedAttempts(t *testing.T) { issuer := "localhost" - key := jwksCacheKey(issuer, nil) + key := jwksCacheKey(issuer, nil, OIDCEndpointPolicy{}) jwksCacheMu.Lock() delete(jwksCache, key) jwksCacheMu.Unlock() diff --git a/iamapi/server.go b/iamapi/server.go index 023e8a81..575c923b 100644 --- a/iamapi/server.go +++ b/iamapi/server.go @@ -18,6 +18,7 @@ import ( "fmt" "net" "net/http" + "net/url" "os" "strings" "time" @@ -84,17 +85,74 @@ type OIDCConfig struct { // drops TLS verification for https ones; see // WithOIDCAllowInsecureTransport. AllowInsecureTransport bool + // DiscoveryURLs holds "=" pairs, each + // redirecting one provider's discovery-document fetch; see + // WithOIDCDiscoveryURLs. + DiscoveryURLs []string + // discovery is DiscoveryURLs parsed and keyed by stored provider Url, + // built by New. + discovery map[string]string } -// endpointPolicy projects the two endpoint relaxations into the form -// iamutil's URL-validation and fetch helpers take. +// endpointPolicy projects the endpoint relaxations into the form iamutil's +// URL-validation and fetch helpers take. func (c OIDCConfig) endpointPolicy() iamutil.OIDCEndpointPolicy { return iamutil.OIDCEndpointPolicy{ AllowPrivateEndpoints: c.AllowPrivateEndpoints, AllowInsecureTransport: c.AllowInsecureTransport, + DiscoveryURLs: c.discovery, } } +// parseDiscoveryURLs turns DiscoveryURLs' pairs into the map the endpoint +// policy takes, keyed by stored provider Url so a lookup by a provider's +// stored form hits directly. The provider Url is held to the same rules +// CreateOpenIDConnectProvider applies, so a pair naming a provider that could +// never be registered fails here; the discovery URL must be an absolute +// http or https URL, since it is fetched exactly as written. +func (c *OIDCConfig) parseDiscoveryURLs() error { + if len(c.DiscoveryURLs) == 0 { + return nil + } + c.discovery = make(map[string]string, len(c.DiscoveryURLs)) + for _, pair := range c.DiscoveryURLs { + providerURL, discoveryURL, ok := cutDiscoveryURLPair(pair) + providerURL, discoveryURL = strings.TrimSpace(providerURL), strings.TrimSpace(discoveryURL) + if !ok || providerURL == "" { + return fmt.Errorf("iamapi: oidc discovery url %q must be in = form, with an http:// or https:// discovery url", pair) + } + stored, err := iamutil.ValidateOIDCProviderURL(providerURL, c.endpointPolicy()) + if err != nil { + return fmt.Errorf("iamapi: oidc discovery url %q: invalid provider url %q: %w", pair, providerURL, err) + } + parsed, err := url.Parse(discoveryURL) + if err != nil || (parsed.Scheme != "https" && parsed.Scheme != "http") || parsed.Host == "" { + return fmt.Errorf("iamapi: oidc discovery url %q: invalid discovery url %q", pair, discoveryURL) + } + if parsed.Scheme == "http" && !c.AllowInsecureTransport { + return fmt.Errorf("iamapi: plaintext oidc discovery url %q requires insecure transport to be allowed", discoveryURL) + } + c.discovery[stored] = discoveryURL + } + return nil +} + +// cutDiscoveryURLPair splits a "=" pair at the +// "=" immediately preceding the discovery URL's scheme rather than at the +// first "=", since a provider Url's path may itself contain "=". +func cutDiscoveryURLPair(pair string) (providerURL, discoveryURL string, ok bool) { + i := -1 + for _, sep := range []string{"=https://", "=http://"} { + if j := strings.Index(pair, sep); j >= 0 && (i < 0 || j < i) { + i = j + } + } + if i < 0 { + return "", "", false + } + return pair[:i], pair[i+1:], true +} + func New(store storage.Storer, root RootCredentials, opts ...Option) (*IAMApiServer, error) { if store == nil { return nil, fmt.Errorf("iamapi: storer is required") @@ -112,6 +170,10 @@ func New(store storage.Storer, root RootCredentials, opts ...Option) (*IAMApiSer opt(server) } + if err := server.oidc.parseDiscoveryURLs(); err != nil { + return nil, err + } + app := fiber.New(fiber.Config{ AppName: "versitygw-iam", ServerHeader: "VERSITYGW", @@ -236,6 +298,22 @@ func WithOIDCAllowInsecureTransport() Option { return func(s *IAMApiServer) { s.oidc.AllowInsecureTransport = true } } +// WithOIDCDiscoveryURLs redirects the discovery-document fetch of individual +// providers, taking "=" pairs. The discovery URL +// is fetched exactly as given, so it must include the +// "/.well-known/openid-configuration" path when the IdP serves it there. +// +// Only the fetch moves: the provider Url is still what a token's iss claim +// and the fetched document's own issuer field must match, and the JWKS is +// still fetched from the jwks_uri that document publishes. That is what lets +// an IdP hand out tokens naming a public issuer while this gateway reads its +// keys over a cluster-internal path — the endpoints being private is the +// point, so a configured discovery URL and the jwks_uri it publishes are +// exempt from the private-address check without WithOIDCAllowPrivateEndpoints. +func WithOIDCDiscoveryURLs(pairs []string) Option { + return func(s *IAMApiServer) { s.oidc.DiscoveryURLs = pairs } +} + func (s *IAMApiServer) ServeMultiPort(ports []string) error { if len(ports) == 0 { return fmt.Errorf("no ports specified") diff --git a/iamapi/server_test.go b/iamapi/server_test.go new file mode 100644 index 00000000..5f286c26 --- /dev/null +++ b/iamapi/server_test.go @@ -0,0 +1,149 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package iamapi + +import ( + "testing" + + "github.com/versity/versitygw/iamapi/storage" +) + +// TestOIDCConfigParseDiscoveryURLs covers the "=" pairs the CLI and Helm chart pass through: a malformed pair fails at +// startup rather than at the first AssumeRoleWithWebIdentity, and a valid one +// is keyed by the provider's stored Url so the lookup at fetch time hits. +func TestOIDCConfigParseDiscoveryURLs(t *testing.T) { + const discoveryURL = "https://oidc.oidc-ns/.well-known/openid-configuration" + + tests := []struct { + name string + pairs []string + insecure bool + private bool + want map[string]string + }{ + { + name: "https provider keyed scheme-stripped", + pairs: []string{"https://oidc.example.com=" + discoveryURL}, + want: map[string]string{"oidc.example.com": discoveryURL}, + }, + { + name: "provider port survives into the key", + pairs: []string{"https://oidc.example.com:8443=" + discoveryURL}, + private: true, + want: map[string]string{"oidc.example.com:8443": discoveryURL}, + }, + { + name: "provider port without private endpoints", + pairs: []string{"https://oidc.example.com:8443=" + discoveryURL}, + }, + { + name: "provider path containing =", + pairs: []string{"https://oidc.example.com/tenant=abc=" + discoveryURL}, + want: map[string]string{"oidc.example.com/tenant=abc": discoveryURL}, + }, + { + name: "discovery url query containing =", + pairs: []string{"https://oidc.example.com=" + discoveryURL + "?tenant=abc"}, + want: map[string]string{"oidc.example.com": discoveryURL + "?tenant=abc"}, + }, + { + name: "http provider keeps its scheme", + pairs: []string{"http://127.0.0.1:8080=" + discoveryURL}, + insecure: true, + private: true, + want: map[string]string{"http://127.0.0.1:8080": discoveryURL}, + }, + { + name: "plaintext discovery url with insecure transport", + pairs: []string{"https://oidc.example.com=http://127.0.0.1:8080/.well-known/openid-configuration"}, + insecure: true, + want: map[string]string{"oidc.example.com": "http://127.0.0.1:8080/.well-known/openid-configuration"}, + }, + { + name: "plaintext discovery url without insecure transport", + pairs: []string{"https://oidc.example.com=http://127.0.0.1:8080/.well-known/openid-configuration"}, + }, + { + name: "no separator", + pairs: []string{"https://oidc.example.com"}, + }, + { + name: "empty discovery url", + pairs: []string{"https://oidc.example.com="}, + }, + { + name: "provider url without a scheme", + pairs: []string{"oidc.example.com=" + discoveryURL}, + }, + { + name: "discovery url without a scheme", + pairs: []string{"https://oidc.example.com=oidc.oidc-ns"}, + }, + { + name: "provider url without a host", + pairs: []string{"https://=" + discoveryURL}, + }, + { + name: "provider url with an invalid host", + pairs: []string{"https://bad host=" + discoveryURL}, + }, + { + name: "discovery url without a host", + pairs: []string{"https://oidc.example.com=https://"}, + }, + { + name: "discovery url with an invalid host", + pairs: []string{"https://oidc.example.com=https://bad host/.well-known/openid-configuration"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := OIDCConfig{DiscoveryURLs: tt.pairs, AllowInsecureTransport: tt.insecure, AllowPrivateEndpoints: tt.private} + err := cfg.parseDiscoveryURLs() + if tt.want == nil { + if err == nil { + t.Fatalf("parseDiscoveryURLs(%q) = nil, want an error", tt.pairs) + } + return + } + if err != nil { + t.Fatalf("parseDiscoveryURLs(%q): %v", tt.pairs, err) + } + got := cfg.endpointPolicy().DiscoveryURLs + if len(got) != len(tt.want) { + t.Fatalf("DiscoveryURLs = %v, want %v", got, tt.want) + } + for provider, want := range tt.want { + if got[provider] != want { + t.Errorf("DiscoveryURLs[%q] = %q, want %q", provider, got[provider], want) + } + } + }) + } +} + +// TestNewRejectsInvalidDiscoveryURLs confirms the parse runs at construction +// time, so a bad flag value never reaches a running server. +func TestNewRejectsInvalidDiscoveryURLs(t *testing.T) { + store, err := storage.New(storage.Config{Dir: t.TempDir()}) + if err != nil { + t.Fatalf("storage.New: %v", err) + } + if _, err := New(store, testRoot, WithQuiet(), WithOIDCDiscoveryURLs([]string{"not-a-pair"})); err == nil { + t.Fatal("New with a malformed discovery url = nil error, want a failure") + } +}