diff --git a/chart/Chart.yaml b/chart/Chart.yaml index e78dc197..0bbc9778 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.1 +version: 0.4.2 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 7693ccfe..7a2ac9c6 100644 --- a/chart/README.md +++ b/chart/README.md @@ -146,6 +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. - **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 6a1f9d43..b3f06540 100644 --- a/chart/templates/iam-deployment.yaml +++ b/chart/templates/iam-deployment.yaml @@ -1,6 +1,7 @@ {{- $iamServer := .Values.iamServer | default dict -}} {{- if ($iamServer.enabled | default false) }} {{- $iamServerAuth := .Values.iamServer.auth | default dict -}} +{{- $iamServerOidc := .Values.iamServer.oidc | default dict -}} {{- if not (or (eq .Values.iamServer.storage.type "internal") (eq .Values.iamServer.storage.type "vault")) }} {{- fail "iamServer.storage.type must be either internal or vault" }} {{- end }} @@ -111,10 +112,18 @@ spec: - name: VGW_LOG_LEVEL value: {{ .Values.iamServer.logLevel | quote }} {{- end }} - {{- if .Values.iamServer.disableOidcThumbprintAutofetch }} + {{- if or $iamServerOidc.disableThumbprintAutofetch .Values.iamServer.disableOidcThumbprintAutofetch }} - name: VGW_IAM_DISABLE_OIDC_THUMBPRINT_AUTOFETCH value: "true" {{- end }} + {{- if $iamServerOidc.allowPrivateEndpoints }} + - name: VGW_IAM_OIDC_ALLOW_PRIVATE_ENDPOINTS + value: "true" + {{- end }} + {{- if $iamServerOidc.allowInsecureTransport }} + - name: VGW_IAM_OIDC_ALLOW_INSECURE_TRANSPORT + value: "true" + {{- 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 14d05c9c..89b2f389 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -407,10 +407,34 @@ iamServer: # Debug logger verbosity: "silent" (default), "debug", or "unsafe" -- see # gateway.logLevel for details. logLevel: silent - # Reject CreateOpenIDConnectProvider requests that omit ThumbprintList - # instead of auto-fetching it over an outbound TLS connection to the - # caller-supplied URL. Recommended for restricted/air-gapped clusters. + # Deprecated alias for oidc.disableThumbprintAutofetch below; still honored. disableOidcThumbprintAutofetch: false + + # -- OIDC identity providers (AssumeRoleWithWebIdentity) -- + oidc: + # Reject CreateOpenIDConnectProvider requests that omit ThumbprintList + # instead of auto-fetching it over an outbound TLS connection to the + # caller-supplied URL. Recommended for restricted/air-gapped clusters. + disableThumbprintAutofetch: false + # Allow OIDC provider URLs that resolve to loopback/private/link-local + # addresses and that carry an explicit port. Both are refused by default, + # which makes an in-cluster identity provider -- a SPIFFE/SPIRE OIDC + # discovery provider on a ClusterIP Service, say -- impossible to register + # or to verify tokens against. Transport is unaffected: still https, still + # fully verified. + # + # This also re-permits cloud metadata endpoints (169.254.169.254) as fetch + # targets, so enable it only where CreateOpenIDConnectProvider is already + # an administrator-only operation. + allowPrivateEndpoints: false + # Allow plaintext http OIDC provider URLs and skip TLS certificate + # verification (thumbprint pinning included) for https ones, leaving the + # network path as the only thing authenticating the identity provider. + # Intended for a provider reached over an already-trusted path -- a + # discovery provider bound to 127.0.0.1 as a sidecar in the IAM server's + # own pod. Needs allowPrivateEndpoints as well for a loopback or + # cluster-internal address. + allowInsecureTransport: false # 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 496dd8bf..b49bce3c 100644 --- a/cmd/internal/gwcli/iam.go +++ b/cmd/internal/gwcli/iam.go @@ -114,6 +114,16 @@ func IAMCommand() *cli.Command { Usage: "reject CreateOpenIDConnectProvider requests that omit ThumbprintList instead of auto-fetching it over an outbound TLS connection", EnvVars: []string{"VGW_IAM_DISABLE_OIDC_THUMBPRINT_AUTOFETCH"}, }, + &cli.BoolFlag{ + Name: "oidc-allow-private-endpoints", + Usage: "allow OIDC provider URLs that resolve to loopback/private/link-local addresses and that carry an explicit port; needed for an identity provider that only exists on an internal network, and also re-permits cloud metadata endpoints as fetch targets", + EnvVars: []string{"VGW_IAM_OIDC_ALLOW_PRIVATE_ENDPOINTS"}, + }, + &cli.BoolFlag{ + Name: "oidc-allow-insecure-transport", + 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: "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 447f8a15..93254f4a 100644 --- a/cmd/versitygw/iam.go +++ b/cmd/versitygw/iam.go @@ -71,6 +71,8 @@ func runIAM(ctx *cli.Context) error { VaultClientCert: ctx.String("vault-client-cert"), VaultClientCertKey: ctx.String("vault-client-cert-key"), DisableOIDCThumbprintAutoFetch: ctx.Bool("disable-oidc-thumbprint-autofetch"), + OIDCAllowPrivateEndpoints: ctx.Bool("oidc-allow-private-endpoints"), + OIDCAllowInsecureTransport: ctx.Bool("oidc-allow-insecure-transport"), CORSAllowOrigin: corsAllowOrigin, Region: region, WebuiPorts: webuiPorts, diff --git a/embedgw/iam.go b/embedgw/iam.go index 5d6e20ac..9bbf1431 100644 --- a/embedgw/iam.go +++ b/embedgw/iam.go @@ -193,6 +193,21 @@ type IAMConfig struct { // outbound TLS connection to the caller-supplied URL — for restricted // or air-gapped deployments. DisableOIDCThumbprintAutoFetch bool + + // OIDCAllowPrivateEndpoints permits OIDC provider URLs that resolve to + // loopback/private/link-local addresses and that carry an explicit port, + // both refused by default. Required to use an IdP that exists only on an + // internal network, such as a SPIFFE/SPIRE OIDC discovery provider on a + // cluster-internal Service. Transport stays https and fully verified. + OIDCAllowPrivateEndpoints bool + + // OIDCAllowInsecureTransport permits plaintext http OIDC provider URLs + // and drops TLS certificate verification (ThumbprintList pinning + // included) for https ones, leaving the network path as the only thing + // authenticating the IdP. For an IdP reachable only over a path that is + // itself trusted, such as a discovery provider bound to loopback as a + // sidecar in this process's own pod. + OIDCAllowInsecureTransport bool } // privateAPIServer is the standalone IAM service's private endpoint set @@ -420,6 +435,12 @@ func RunIAMAPI(ctx context.Context, cfg *IAMConfig) error { if cfg.DisableOIDCThumbprintAutoFetch { opts = append(opts, iamapi.WithOIDCThumbprintAutoFetchDisabled()) } + if cfg.OIDCAllowPrivateEndpoints { + opts = append(opts, iamapi.WithOIDCAllowPrivateEndpoints()) + } + if cfg.OIDCAllowInsecureTransport { + opts = append(opts, iamapi.WithOIDCAllowInsecureTransport()) + } 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 48877029..5ae70692 100644 --- a/extra/example-iam.conf +++ b/extra/example-iam.conf @@ -141,4 +141,16 @@ ROOT_SECRET_ACCESS_KEY= ################# # Reject OIDC provider creation without an explicit certificate thumbprint. -#VGW_IAM_DISABLE_OIDC_THUMBPRINT_AUTOFETCH=false \ No newline at end of file +#VGW_IAM_DISABLE_OIDC_THUMBPRINT_AUTOFETCH=false + +# Allow OIDC provider URLs that resolve to loopback/private/link-local +# addresses and that carry an explicit port; needed for an identity provider +# that only exists on an internal network, and also re-permits cloud +# metadata endpoints as fetch targets. +#VGW_IAM_OIDC_ALLOW_PRIVATE_ENDPOINTS=false + +# 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. +#VGW_IAM_OIDC_ALLOW_INSECURE_TRANSPORT=false \ No newline at end of file diff --git a/iamapi/controller.go b/iamapi/controller.go index 4b9ed6b1..b6bcdc66 100644 --- a/iamapi/controller.go +++ b/iamapi/controller.go @@ -32,18 +32,20 @@ import ( type IAMApiController struct { store storage.Storer - // oidcThumbprintAutoFetchDisabled disables CreateOpenIDConnectProvider's - // TLS auto-fetch fallback when ThumbprintList is omitted (operational - // safety valve for restricted/air-gapped deployments); set via - // iamapi.WithOIDCThumbprintAutoFetchDisabled(). Defaults to false - // (auto-fetch enabled), matching real AWS behavior. - oidcThumbprintAutoFetchDisabled bool + // oidc holds the OIDC provider settings; see OIDCConfig. Its zero value + // is the default AWS-matching posture: auto-fetch enabled, and only + // verified https endpoints at publicly routable addresses. + oidc OIDCConfig + // oidcPolicy is oidc's endpoint relaxations in the form iamutil's URL + // validation and fetch helpers take, projected once at construction. + oidcPolicy iamutil.OIDCEndpointPolicy } -func NewController(store storage.Storer, oidcThumbprintAutoFetchDisabled bool) IAMApiController { +func NewController(store storage.Storer, oidc OIDCConfig) IAMApiController { return IAMApiController{ - store: store, - oidcThumbprintAutoFetchDisabled: oidcThumbprintAutoFetchDisabled, + store: store, + oidc: oidc, + oidcPolicy: oidc.endpointPolicy(), } } @@ -1035,7 +1037,7 @@ func (c IAMApiController) CreateOpenIDConnectProvider(ctx fiber.Ctx) (*Response, debuglogger.Logf("missing required CreateOpenIDConnectProvider parameter: Url") return nil, iamerr.MissingValue("url") } - url, err := iamutil.ValidateOIDCProviderURL(rawURL) + url, err := iamutil.ValidateOIDCProviderURL(rawURL, c.oidcPolicy) if err != nil { return nil, err } @@ -1052,16 +1054,24 @@ func (c IAMApiController) CreateOpenIDConnectProvider(ctx fiber.Ctx) (*Response, thumbprints := iamutil.ParseStringList(ctx, "ThumbprintList") if len(thumbprints) == 0 { - if c.oidcThumbprintAutoFetchDisabled { + switch { + case iamutil.IsInsecureOIDCProviderURL(url): + // A plaintext http provider 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) + case c.oidc.ThumbprintAutoFetchDisabled: debuglogger.Logf("CreateOpenIDConnectProvider: ThumbprintList omitted and auto-fetch is disabled") return nil, iamerr.MissingValue("thumbprintList") + default: + fetched, err := iamutil.FetchThumbprint(ctx.Context(), url, c.oidcPolicy) + if err != nil { + debuglogger.Logf("failed to auto-fetch OIDC thumbprint for url %q: %v", url, err) + return nil, err + } + thumbprints = []string{fetched} } - fetched, err := iamutil.FetchThumbprint(ctx.Context(), url) - if err != nil { - debuglogger.Logf("failed to auto-fetch OIDC thumbprint for url %q: %v", url, err) - return nil, err - } - thumbprints = []string{fetched} } else { if err := iamutil.ValidateThumbprintList(thumbprints, false); err != nil { return nil, err @@ -1450,7 +1460,7 @@ func (c IAMApiController) AssumeRoleWithWebIdentity(ctx fiber.Ctx) (*Response, e return nil, iamerr.InvalidIdentityTokenClaims() } - verifiedClaims, err := iamutil.VerifyWebIdentitySignature(ctx.Context(), webIdentityToken, provider.Url, provider.ThumbprintList) + verifiedClaims, err := iamutil.VerifyWebIdentitySignature(ctx.Context(), webIdentityToken, provider.Url, provider.ThumbprintList, c.oidcPolicy) if err != nil { return nil, err } diff --git a/iamapi/controller_test.go b/iamapi/controller_test.go index 86ceed67..a2527a72 100644 --- a/iamapi/controller_test.go +++ b/iamapi/controller_test.go @@ -16,12 +16,15 @@ package iamapi import ( "bytes" "context" + "crypto/rand" + "crypto/rsa" "crypto/sha256" "encoding/base64" "encoding/hex" "encoding/json" "encoding/xml" "fmt" + "math/big" "net/http" "net/http/httptest" "net/url" @@ -34,6 +37,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" awsv4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" "github.com/gofiber/fiber/v3" + "github.com/golang-jwt/jwt/v5" "github.com/versity/versitygw/iamapi/iamerr" "github.com/versity/versitygw/iamapi/internal/iammiddleware" "github.com/versity/versitygw/iamapi/internal/iamutil" @@ -3632,6 +3636,234 @@ func TestIAMApiControllerCreateOIDCProviderAutoFetchSSRFGuard(t *testing.T) { "Could not connect to https://127.0.0.1") } +// newIAMControllerTestServerWith is newIAMControllerTestServer for the tests +// that need a non-default server option. +func newIAMControllerTestServerWith(t *testing.T, opts ...Option) *IAMApiServer { + t.Helper() + + store, err := storage.New(storage.Config{Dir: t.TempDir()}) + if err != nil { + t.Fatalf("storage.New: %v", err) + } + server, err := New(store, testRoot, append([]Option{WithQuiet()}, opts...)...) + if err != nil { + t.Fatalf("New: %v", err) + } + return server +} + +// TestIAMApiControllerCreateOIDCProviderEndpointRelaxations covers both +// endpoint-relaxation options end to end through the HTTP action handler: +// the two Url shapes an IdP on an isolated network needs (an explicit port, +// and plaintext http) are rejected by default and accepted once the +// corresponding option is set — including the record each one leaves behind, +// since the stored Url is what a later token's iss claim is matched against. +func TestIAMApiControllerCreateOIDCProviderEndpointRelaxations(t *testing.T) { + const thumbprint = "6938fd4d98bab03faadb97b34396831e3780aea1" + + create := func(t *testing.T, server *IAMApiServer, providerURL string, withThumbprint bool) *http.Response { + t.Helper() + params := url.Values{ + "Action": {"CreateOpenIDConnectProvider"}, + "Url": {providerURL}, + } + if withThumbprint { + params.Set("ThumbprintList.member.1", thumbprint) + } + return doIAMAction(t, server, params) + } + + getProvider := func(t *testing.T, server *IAMApiServer, arn string) iamtypes.GetOpenIDConnectProviderResult { + t.Helper() + resp := doIAMAction(t, server, url.Values{ + "Action": {"GetOpenIDConnectProvider"}, + "OpenIDConnectProviderArn": {arn}, + }) + if resp.StatusCode != http.StatusOK { + t.Fatalf("GetOpenIDConnectProvider status = %d, body=%s", resp.StatusCode, readBody(t, resp)) + } + var out iamtypes.GetOpenIDConnectProviderResponse + unmarshalXML(t, readBody(t, resp), &out) + return out.Result + } + + requireCreated := func(t *testing.T, resp *http.Response, wantArn string) { + t.Helper() + if resp.StatusCode != http.StatusOK { + t.Fatalf("CreateOpenIDConnectProvider status = %d, body=%s", resp.StatusCode, readBody(t, resp)) + } + var out iamtypes.CreateOpenIDConnectProviderResponse + unmarshalXML(t, readBody(t, resp), &out) + if out.Result.OpenIDConnectProviderArn != wantArn { + t.Fatalf("OpenIDConnectProviderArn = %q, want %q", out.Result.OpenIDConnectProviderArn, wantArn) + } + } + + t.Run("explicit port rejected by default", func(t *testing.T) { + server := newIAMControllerTestServer(t) + requireIAMError(t, create(t, server, "https://spire-oidc.spire.svc:8443", true), + http.StatusBadRequest, "Sender", "InvalidInput", "Invalid Open ID Connect Provider URL.") + }) + + t.Run("http rejected by default", func(t *testing.T) { + server := newIAMControllerTestServer(t) + requireIAMError(t, create(t, server, "http://127.0.0.1:8080", true), + http.StatusBadRequest, "Sender", "InvalidInput", + "Invalid Open ID Connect Provider URL. The URL must begin with https://.") + }) + + t.Run("http still rejected with only private endpoints allowed", func(t *testing.T) { + server := newIAMControllerTestServerWith(t, WithOIDCAllowPrivateEndpoints()) + requireIAMError(t, create(t, server, "http://127.0.0.1:8080", true), + http.StatusBadRequest, "Sender", "InvalidInput", + "Invalid Open ID Connect Provider URL. The URL must begin with https://.") + }) + + t.Run("explicit port accepted with private endpoints allowed", func(t *testing.T) { + server := newIAMControllerTestServerWith(t, WithOIDCAllowPrivateEndpoints()) + requireCreated(t, create(t, server, "https://spire-oidc.spire.svc:8443", true), + "arn:aws:iam::000000000000:oidc-provider/spire-oidc.spire.svc:8443") + + // The port survives into the stored Url, so it is part of what a + // token's iss claim must match. + got := getProvider(t, server, "arn:aws:iam::000000000000:oidc-provider/spire-oidc.spire.svc:8443") + if got.Url != "spire-oidc.spire.svc:8443" { + t.Errorf("stored Url = %q, want %q", got.Url, "spire-oidc.spire.svc:8443") + } + if len(got.ThumbprintList) != 1 || got.ThumbprintList[0] != thumbprint { + t.Errorf("ThumbprintList = %v, want [%s]", got.ThumbprintList, thumbprint) + } + }) + + t.Run("http provider accepted with insecure transport allowed", func(t *testing.T) { + server := newIAMControllerTestServerWith(t, WithOIDCAllowPrivateEndpoints(), WithOIDCAllowInsecureTransport()) + const arn = "arn:aws:iam::000000000000:oidc-provider/http://127.0.0.1:8080" + + // No ThumbprintList, and no auto-fetch attempt either: a plaintext + // provider presents no certificate, so an empty list is stored + // rather than the request failing. + requireCreated(t, create(t, server, "http://127.0.0.1:8080", false), arn) + + got := getProvider(t, server, arn) + if got.Url != "http://127.0.0.1:8080" { + t.Errorf("stored Url = %q, want the scheme to be retained", got.Url) + } + if len(got.ThumbprintList) != 0 { + t.Errorf("ThumbprintList = %v, want empty for a plaintext provider", got.ThumbprintList) + } + }) + + t.Run("http and https providers for the same host coexist", func(t *testing.T) { + // The retained scheme is what keeps these two distinct resources: + // stored stripped, both would collide on one ARN and one storage key. + server := newIAMControllerTestServerWith(t, WithOIDCAllowPrivateEndpoints(), WithOIDCAllowInsecureTransport()) + requireCreated(t, create(t, server, "http://idp.example", false), + "arn:aws:iam::000000000000:oidc-provider/http://idp.example") + requireCreated(t, create(t, server, "https://idp.example", true), + "arn:aws:iam::000000000000:oidc-provider/idp.example") + }) +} + +// TestIAMApiControllerAssumeRoleWithWebIdentityLoopbackIdP is the only test +// that drives AssumeRoleWithWebIdentity all the way to a real verified +// signature: with the two endpoint relaxations set, a plaintext OIDC +// provider on loopback — the spire-oidc-discovery-provider-as-a-sidecar +// shape — is reachable, so the discovery document and JWKS are really +// fetched over the network and a real RS256 signature is really checked +// against the published key. Every other AssumeRoleWithWebIdentity test +// stops at the fetch, which the default posture refuses outright. +func TestIAMApiControllerAssumeRoleWithWebIdentityLoopbackIdP(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa.GenerateKey: %v", err) + } + + var issuer string + idp := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/openid-configuration": + json.NewEncoder(w).Encode(map[string]any{ + "issuer": issuer, + "jwks_uri": issuer + "/keys", + }) + 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() + issuer = idp.URL // "http://127.0.0.1:" + + server := newIAMControllerTestServerWith(t, WithOIDCAllowPrivateEndpoints(), WithOIDCAllowInsecureTransport()) + providerArn := createTestOIDCProviderForTrust(t, server, issuer, "versitygw") + createTestRoleForTrust(t, server, "spire-role", + `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Federated":"`+providerArn+`"},`+ + `"Action":"sts:AssumeRoleWithWebIdentity","Condition":{"StringEquals":{"`+issuer+`:aud":"versitygw"}}}]}`) + + signedToken := func(t *testing.T, signingKey *rsa.PrivateKey, subject string) string { + t.Helper() + token := jwt.NewWithClaims(jwt.SigningMethodRS256, jwt.MapClaims{ + "iss": issuer, + "aud": "versitygw", + "sub": subject, + "iat": time.Now().Unix(), + "exp": time.Now().Add(time.Hour).Unix(), + }) + token.Header["kid"] = "k1" + signed, err := token.SignedString(signingKey) + if err != nil { + t.Fatalf("sign token: %v", err) + } + return signed + } + + assume := func(t *testing.T, token, sessionName string) *http.Response { + t.Helper() + return doSTSAction(t, server, url.Values{ + "Action": {"AssumeRoleWithWebIdentity"}, + "RoleArn": {"arn:aws:iam::000000000000:role/spire-role"}, + "RoleSessionName": {sessionName}, + "WebIdentityToken": {token}, + }) + } + + t.Run("correctly signed token is accepted", func(t *testing.T) { + resp := assume(t, signedToken(t, key, "spiffe://example.org/ns/default/sa/versitygw"), "spire-session") + 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.SubjectFromWebIdentityToken != "spiffe://example.org/ns/default/sa/versitygw" { + t.Errorf("SubjectFromWebIdentityToken = %q", out.Result.SubjectFromWebIdentityToken) + } + if out.Result.Provider != issuer { + t.Errorf("Provider = %q, want %q", out.Result.Provider, issuer) + } + if out.Result.Credentials.SessionToken == "" || out.Result.Credentials.AccessKeyId == "" { + t.Errorf("no session credentials returned: %+v", out.Result.Credentials) + } + }) + + t.Run("token signed by a different key is rejected", func(t *testing.T) { + // The relaxations reach the endpoint; they must not weaken what + // happens once the real JWKS is in hand. + forgedKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa.GenerateKey: %v", err) + } + resp := assume(t, signedToken(t, forgedKey, "attacker"), "attacker-session") + requireSTSError(t, resp, http.StatusBadRequest, "Sender", "InvalidIdentityToken", + "The web identity token provided could not be validated. See the 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/iammiddleware/policy.go b/iamapi/internal/iammiddleware/policy.go index cf1739b9..9fef1568 100644 --- a/iamapi/internal/iammiddleware/policy.go +++ b/iamapi/internal/iammiddleware/policy.go @@ -52,7 +52,7 @@ const iamActionPrefix = "iam:" // requestConditionContext supplies the request's aws:SourceIp/aws:username/ // aws:PrincipalArn/aws:CurrentTime/aws:EpochTime values for a statement's // Condition block. -func VerifyIAMPolicy(store iamutil.IdentityStore) fiber.Handler { +func VerifyIAMPolicy(store iamutil.IdentityStore, oidcPolicy iamutil.OIDCEndpointPolicy) fiber.Handler { return func(ctx fiber.Ctx) error { identity, _ := httpctx.ContextKeyCallerIdentity.Get(ctx).(types.Identity) if identity.IsRoot { @@ -62,7 +62,7 @@ func VerifyIAMPolicy(store iamutil.IdentityStore) fiber.Handler { action, _ := iamutil.RequestParam(ctx, "Action") fullAction := iamActionPrefix + action - resourceArn, resourceTags := resourceForAction(ctx, store, action) + resourceArn, resourceTags := resourceForAction(ctx, store, action, oidcPolicy) reqCtx := policy.RequestContext{ Action: fullAction, Resource: resourceArn, @@ -166,7 +166,7 @@ func AuthorizeSplit(identity types.Identity, reqCtx policy.RequestContext) (iden // request still reaches the controller afterward, which reports the // specific NoSuchEntity/MissingValue error if authorization happens to pass // on a wildcard grant, or AccessDenied first if it doesn't. -func resourceForAction(ctx fiber.Ctx, store iamutil.IdentityStore, action string) (string, []types.Tag) { +func resourceForAction(ctx fiber.Ctx, store iamutil.IdentityStore, action string, oidcPolicy iamutil.OIDCEndpointPolicy) (string, []types.Tag) { switch action { case "CreateUser": return newUserResource(ctx), nil @@ -183,7 +183,7 @@ func resourceForAction(ctx fiber.Ctx, store iamutil.IdentityStore, action string "TagRole", "UntagRole", "ListRoleTags": return existingRoleResource(ctx, store) case "CreateOpenIDConnectProvider": - return newOIDCProviderResource(ctx), nil + return newOIDCProviderResource(ctx, oidcPolicy), nil case "GetOpenIDConnectProvider", "DeleteOpenIDConnectProvider", "AddClientIDToOpenIDConnectProvider", "RemoveClientIDFromOpenIDConnectProvider", "UpdateOpenIDConnectProviderThumbprint", "TagOpenIDConnectProvider", "UntagOpenIDConnectProvider", "ListOpenIDConnectProviderTags": @@ -326,12 +326,17 @@ func existingRoleResource(ctx fiber.Ctx, store iamutil.IdentityStore) (string, [ return role.Arn, role.Tags } -func newOIDCProviderResource(ctx fiber.Ctx) string { +// newOIDCProviderResource builds the ARN of the provider a +// CreateOpenIDConnectProvider request would create, canonicalizing the Url +// exactly as the controller will. oidcPolicy must therefore be the same one +// the controller holds: a Url the controller would accept but this rejects +// resolves to "", which only a wildcard Resource statement matches. +func newOIDCProviderResource(ctx fiber.Ctx, oidcPolicy iamutil.OIDCEndpointPolicy) string { rawURL, ok := iamutil.RequestParam(ctx, "Url") if !ok || rawURL == "" { return "*" } - url, err := iamutil.ValidateOIDCProviderURL(rawURL) + url, err := iamutil.ValidateOIDCProviderURL(rawURL, oidcPolicy) if err != nil { return "" } diff --git a/iamapi/internal/iamutil/oidc.go b/iamapi/internal/iamutil/oidc.go index 36f9e365..b0643bda 100644 --- a/iamapi/internal/iamutil/oidc.go +++ b/iamapi/internal/iamutil/oidc.go @@ -39,6 +39,70 @@ const ( var oidcHostLabelPattern = regexp.MustCompile(`^[A-Za-z0-9]([A-Za-z0-9-]{0,61}[A-Za-z0-9])?$`) +// insecureOIDCScheme is the plaintext scheme an OIDC provider Url may carry +// only when OIDCEndpointPolicy.AllowInsecureTransport is set. +const insecureOIDCScheme = "http://" + +// OIDCEndpointPolicy relaxes the endpoint checks applied to an OIDC +// provider's Url and to every outbound fetch made against it (thumbprint +// auto-fetch at CreateOpenIDConnectProvider time, and the discovery +// document plus JWKS at AssumeRoleWithWebIdentity time). +// +// The zero value is the default, AWS-matching posture for an +// internet-facing IdP: https only, on the implicit :443, at a publicly +// routable address, with full hostname and chain verification against the +// system trust store (or a registered ThumbprintList). That posture makes +// the IAM API unusable with an IdP that is deliberately unreachable from +// the public internet — a SPIFFE/SPIRE OIDC discovery provider on a +// cluster-internal Service, or one bound to loopback as a sidecar in the +// gateway's own pod — because every address such an IdP can have is +// rejected outright, and no combination of the other settings can express +// "this private address is the IdP". +type OIDCEndpointPolicy struct { + // AllowPrivateEndpoints permits a provider Url that resolves to a + // loopback, private, link-local, unspecified, or multicast address, and + // permits an explicit port in that Url (an IdP on an internal network + // rarely gets to own :443 on its host). Transport is otherwise + // unchanged: still https, still fully verified. + // + // This necessarily also re-permits cloud metadata endpoints + // (e.g. 169.254.169.254) as fetch targets, so enable it only + // where registering an OIDC provider is already a trusted, + // administrator-only operation. + AllowPrivateEndpoints bool + + // AllowInsecureTransport permits a plaintext http:// provider Url — + // along with the http discovery/JWKS endpoints and redirects that + // implies — and disables TLS certificate verification, ThumbprintList + // pinning included, for https ones. It makes the network path itself + // the only thing authenticating the IdP, so it belongs only where that + // path is trustworthy on its own, such as a sidecar bound to loopback + // inside the gateway's own pod. + AllowInsecureTransport bool +} + +// IsInsecureOIDCProviderURL reports whether providerURL, a stored provider +// Url, names a plaintext http endpoint. +// +// An https provider is stored scheme-stripped, the canonical form AWS uses; +// an http one (creatable only under AllowInsecureTransport) deliberately +// keeps its scheme in storage, in its ARN, and in the iss claim it is +// matched against, so "http://host" and "https://host" can never be taken +// for one another — the same reason WebIdentityIssuer strips only "https://". +func IsInsecureOIDCProviderURL(providerURL string) bool { + return strings.HasPrefix(providerURL, insecureOIDCScheme) +} + +// OIDCEndpointURL restores the full endpoint URL of a stored provider Url: +// the "https://" ValidateOIDCProviderURL stripped, or the "http://" it +// deliberately kept. +func OIDCEndpointURL(providerURL string) string { + if IsInsecureOIDCProviderURL(providerURL) { + return providerURL + } + return "https://" + providerURL +} + // 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 @@ -57,15 +121,17 @@ func ParseStringList(ctx fiber.Ctx, paramName string) []string { } // BuildOIDCProviderArn constructs the ARN for an IAM OIDC identity -// provider. url must already have its "https://" scheme stripped. +// provider. url must already be in ValidateOIDCProviderURL's canonical +// stored form: an https provider with its scheme stripped, an http one +// (AllowInsecureTransport only) with its scheme intact. func BuildOIDCProviderArn(accountID, url string) string { return fmt.Sprintf("arn:aws:iam::%s:oidc-provider/%s", accountID, url) } // ParseOIDCProviderArn validates arn's overall length and structural shape // (arn:aws:iam:::/) and, on success, -// returns the resource segment — the provider's Url with "https://" already -// stripped, exactly as stored. The account-id segment must match +// returns the resource segment — the provider's Url exactly as stored (see +// BuildOIDCProviderArn for that form). The account-id segment must match // DefaultAccountID; any other value is rejected with AccessDenied, matching // real AWS's behavior for a well-formed ARN referencing a foreign account. // @@ -134,9 +200,9 @@ func GetOIDCProviderArn(ctx fiber.Ctx, operation string) (string, error) { } // ValidateOIDCProviderURL validates the Url parameter of -// CreateOpenIDConnectProvider and returns it with its "https://" scheme -// stripped (the canonical form used for ARN construction, storage keys, and -// GetOpenIDConnectProvider's own Url response field). +// CreateOpenIDConnectProvider and returns its canonical stored form — the +// form used for ARN construction, storage keys, iss-claim matching, and +// GetOpenIDConnectProvider's own Url response field. // // This implements a pragmatic subset of AWS's real validation: scheme must // be exactly "https", no userinfo/port/query/fragment, host must be a @@ -144,7 +210,16 @@ func GetOIDCProviderArn(ctx fiber.Ctx, operation string) (string, error) { // length <= MaxOIDCProviderURLLen. It does not attempt to reproduce every // hostname-shape check AWS performs; it returns clear InvalidInput/ // ValidationError messages instead of chasing every malformed edge case. -func ValidateOIDCProviderURL(rawURL string) (string, error) { +// +// policy relaxes two of those rules for non-public IdPs: +// AllowPrivateEndpoints additionally accepts an explicit port, and +// AllowInsecureTransport additionally accepts an "http://" scheme. +// +// An https Url is returned scheme-stripped, as AWS canonicalizes it; an +// http one keeps its scheme, so that it stays distinguishable from the same +// host over https everywhere the stored form is used (see +// IsInsecureOIDCProviderURL). +func ValidateOIDCProviderURL(rawURL string, policy OIDCEndpointPolicy) (string, error) { if rawURL == "" { return "", iamerr.MissingValue("url") } @@ -152,27 +227,38 @@ func ValidateOIDCProviderURL(rawURL string) (string, error) { return "", iamerr.ValueTooLong("url", MaxOIDCProviderURLLen) } // A URL with no scheme delimiter at all (e.g. "example.com") is - // rejected as ValidationError; one with a scheme other than https - // (e.g. "http://example.com") is rejected as InvalidInput — distinct - // error codes for distinct malformed inputs. + // rejected as ValidationError; one with a scheme the policy doesn't + // permit (e.g. "http://example.com" by default) is rejected as + // InvalidInput — distinct error codes for distinct malformed inputs. if !strings.Contains(rawURL, "://") { return "", iamerr.ValidationError("Invalid Open ID Connect Provider URL") } - if !strings.HasPrefix(rawURL, "https://") { + insecure := policy.AllowInsecureTransport && strings.HasPrefix(rawURL, insecureOIDCScheme) + if !insecure && !strings.HasPrefix(rawURL, "https://") { return "", iamerr.InvalidInput("Invalid Open ID Connect Provider URL. The URL must begin with https://.") } + wantScheme := "https" + if insecure { + wantScheme = "http" + } parsed, err := url.Parse(rawURL) - if err != nil || parsed.Scheme != "https" || parsed.Host == "" { + if err != nil || parsed.Scheme != wantScheme || parsed.Host == "" { return "", iamerr.ValidationError("Invalid Open ID Connect Provider URL") } - if parsed.User != nil || parsed.RawQuery != "" || parsed.Fragment != "" || parsed.Port() != "" { + if parsed.User != nil || parsed.RawQuery != "" || parsed.Fragment != "" { + return "", iamerr.InvalidInput("Invalid Open ID Connect Provider URL.") + } + if parsed.Port() != "" && !policy.AllowPrivateEndpoints { return "", iamerr.InvalidInput("Invalid Open ID Connect Provider URL.") } if !isValidOIDCHostname(parsed.Hostname()) { return "", iamerr.InvalidInput("Invalid Open ID Connect Provider URL.") } + if insecure { + return rawURL, nil + } return strings.TrimPrefix(rawURL, "https://"), nil } diff --git a/iamapi/internal/iamutil/oidc_test.go b/iamapi/internal/iamutil/oidc_test.go new file mode 100644 index 00000000..bb77f7e0 --- /dev/null +++ b/iamapi/internal/iamutil/oidc_test.go @@ -0,0 +1,158 @@ +// 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 iamutil + +import ( + "strings" + "testing" +) + +var ( + strictOIDCPolicy = OIDCEndpointPolicy{} + privateOIDCPolicy = OIDCEndpointPolicy{AllowPrivateEndpoints: true} + insecureOIDCPolicy = OIDCEndpointPolicy{AllowPrivateEndpoints: true, AllowInsecureTransport: true} +) + +func TestValidateOIDCProviderURL(t *testing.T) { + tests := []struct { + name string + rawURL string + policy OIDCEndpointPolicy + want string // "" means the URL must be rejected + }{ + // Default posture: AWS's own rules. + {"https host", "https://example.com", strictOIDCPolicy, "example.com"}, + {"https host with path", "https://example.com/oidc", strictOIDCPolicy, "example.com/oidc"}, + {"no scheme", "example.com", strictOIDCPolicy, ""}, + {"http rejected by default", "http://example.com", strictOIDCPolicy, ""}, + {"port rejected by default", "https://example.com:8443", strictOIDCPolicy, ""}, + {"userinfo", "https://user@example.com", strictOIDCPolicy, ""}, + {"query", "https://example.com?a=b", strictOIDCPolicy, ""}, + {"fragment", "https://example.com#frag", strictOIDCPolicy, ""}, + {"too long", "https://" + strings.Repeat("a", MaxOIDCProviderURLLen) + ".com", strictOIDCPolicy, ""}, + {"empty", "", strictOIDCPolicy, ""}, + + // AllowPrivateEndpoints: an explicit port becomes legal. A private + // address was always legal *syntax* - it is the fetch that refuses + // it - so a bare private host is accepted under both policies. + {"port allowed", "https://spire-oidc.spire.svc:8443", privateOIDCPolicy, "spire-oidc.spire.svc:8443"}, + {"loopback with port", "https://127.0.0.1:8443", privateOIDCPolicy, "127.0.0.1:8443"}, + {"ipv6 literal with port", "https://[::1]:8443", privateOIDCPolicy, "[::1]:8443"}, + {"cluster service no port", "https://spire-oidc.spire.svc", privateOIDCPolicy, "spire-oidc.spire.svc"}, + {"http still rejected", "http://127.0.0.1:8080", privateOIDCPolicy, ""}, + + // AllowInsecureTransport: http is accepted and, unlike https, keeps + // its scheme in the stored form. + {"http kept verbatim", "http://127.0.0.1:8080", insecureOIDCPolicy, "http://127.0.0.1:8080"}, + {"http with path", "http://127.0.0.1:8080/oidc", insecureOIDCPolicy, "http://127.0.0.1:8080/oidc"}, + {"https still stripped", "https://example.com", insecureOIDCPolicy, "example.com"}, + {"other scheme still rejected", "ftp://example.com", insecureOIDCPolicy, ""}, + {"http userinfo still rejected", "http://user@127.0.0.1:8080", insecureOIDCPolicy, ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ValidateOIDCProviderURL(tt.rawURL, tt.policy) + if tt.want == "" { + if err == nil { + t.Fatalf("ValidateOIDCProviderURL(%q, %+v) = %q, want an error", tt.rawURL, tt.policy, got) + } + return + } + if err != nil { + t.Fatalf("ValidateOIDCProviderURL(%q, %+v): %v", tt.rawURL, tt.policy, err) + } + if got != tt.want { + t.Errorf("ValidateOIDCProviderURL(%q, %+v) = %q, want %q", tt.rawURL, tt.policy, got, tt.want) + } + }) + } +} + +// TestOIDCProviderURLSchemeStaysDistinguishable pins down why an http +// provider keeps its scheme in the stored form: the stored Url is what an +// incoming token's iss claim is matched against, so if "http://host" were +// stored stripped it would be indistinguishable from a separately +// registered "https://host", and a token from either issuer would satisfy +// the other's trust policy. +func TestOIDCProviderURLSchemeStaysDistinguishable(t *testing.T) { + secure, err := ValidateOIDCProviderURL("https://idp.example", insecureOIDCPolicy) + if err != nil { + t.Fatalf("ValidateOIDCProviderURL(https): %v", err) + } + insecure, err := ValidateOIDCProviderURL("http://idp.example", insecureOIDCPolicy) + if err != nil { + t.Fatalf("ValidateOIDCProviderURL(http): %v", err) + } + if secure == insecure { + t.Fatalf("http and https providers for the same host both stored as %q", secure) + } + + // WebIdentityIssuer is the other half: it must map each token's iss back + // onto exactly the provider that issued it. + for iss, want := range map[string]string{ + "https://idp.example": secure, + "http://idp.example": insecure, + } { + got, ok := WebIdentityIssuer(map[string]any{"iss": iss}) + if !ok || got != want { + t.Errorf("WebIdentityIssuer(%q) = (%q, %v), want (%q, true)", iss, got, ok, want) + } + } +} + +func TestOIDCEndpointURL(t *testing.T) { + tests := []struct{ providerURL, want string }{ + {"example.com", "https://example.com"}, + {"example.com/oidc", "https://example.com/oidc"}, + {"spire-oidc.spire.svc:8443", "https://spire-oidc.spire.svc:8443"}, + {"http://127.0.0.1:8080", "http://127.0.0.1:8080"}, + {"http://127.0.0.1:8080/oidc", "http://127.0.0.1:8080/oidc"}, + } + for _, tt := range tests { + t.Run(tt.providerURL, func(t *testing.T) { + if got := OIDCEndpointURL(tt.providerURL); got != tt.want { + t.Errorf("OIDCEndpointURL(%q) = %q, want %q", tt.providerURL, got, tt.want) + } + if got := IsInsecureOIDCProviderURL(tt.providerURL); got != strings.HasPrefix(tt.want, "http://") { + t.Errorf("IsInsecureOIDCProviderURL(%q) = %v", tt.providerURL, got) + } + }) + } +} + +// TestBuildOIDCProviderArnRoundTripsRelaxedURLs confirms the ARN encoding +// survives the two new Url shapes: an explicit port and a retained +// "http://" both contain characters ParseOIDCProviderArn splits on, so a +// naive split would truncate the provider Url or misread the account id. +func TestBuildOIDCProviderArnRoundTripsRelaxedURLs(t *testing.T) { + for _, url := range []string{ + "example.com", + "spire-oidc.spire.svc:8443", + "http://127.0.0.1:8080", + "http://127.0.0.1:8080/oidc", + } { + t.Run(url, func(t *testing.T) { + arn := BuildOIDCProviderArn(DefaultAccountID, url) + got, err := ParseOIDCProviderArn(arn) + if err != nil { + t.Fatalf("ParseOIDCProviderArn(%q): %v", arn, err) + } + if got != url { + t.Errorf("ParseOIDCProviderArn(%q) = %q, want %q", arn, got, url) + } + }) + } +} diff --git a/iamapi/internal/iamutil/oidc_thumbprint.go b/iamapi/internal/iamutil/oidc_thumbprint.go index ab8dacc2..0157e2f7 100644 --- a/iamapi/internal/iamutil/oidc_thumbprint.go +++ b/iamapi/internal/iamutil/oidc_thumbprint.go @@ -33,13 +33,14 @@ const oidcThumbprintFetchTimeout = 8 * time.Second // FetchThumbprint implements CreateOpenIDConnectProvider's auto-fetch // behavior: it opens a TLS handshake (crypto/tls, not a full HTTP GET) to -// host:443, where host is derived from providerURL (a scheme-stripped OIDC -// provider Url), verifying 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. +// the host authority of providerURL (a stored OIDC provider Url) on its +// explicit port or, as is normally the only possibility, 443 — verifying +// 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. // -// SSRF hardening (mandatory): the hostname is resolved once via +// SSRF hardening: the hostname is resolved once via // net.DefaultResolver.LookupIP; if any resolved address is // loopback/private/link-local/unspecified/multicast (this range covers // 169.254.169.254 and other cloud metadata endpoints), the fetch is @@ -47,8 +48,10 @@ const oidcThumbprintFetchTimeout = 8 * time.Second // the pre-validated IPs directly (never re-resolving the hostname at dial // time, closing the DNS-rebinding TOCTOU gap) while presenting the original // hostname via tls.Config.ServerName for SNI/certificate purposes. +// policy.AllowPrivateEndpoints waives only the address check — the single +// resolution and pinned-IP dial stay in place either way. // -// Verification is deliberately NOT skipped here: unlike a one-shot +// Verification is deliberately not skipped by default: unlike a one-shot // connection whose result is used and discarded, the certificate observed // during this handshake is persisted as a long-lived trust anchor, compared // against every future JWKS fetch for this provider. An unauthenticated @@ -62,10 +65,19 @@ const oidcThumbprintFetchTimeout = 8 * time.Second // obtained the fingerprint through some independently verified channel — // the same operational shape WithOIDCThumbprintAutoFetchDisabled already // provides unconditionally, scoped here to just the providers that fail -// public verification. -func FetchThumbprint(ctx context.Context, providerURL string) (string, error) { - host := hostFromOIDCUrl(providerURL) - displayURL := "https://" + providerURL +// public verification — or, for an IdP whose certificate cannot chain to a +// public root by construction, policy.AllowInsecureTransport, which drops +// 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 + // 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) + return "", iamerr.OpenIdIdpCommunicationError(displayURL) + } + host, port := splitOIDCHostPort(hostFromOIDCUrl(providerURL)) ctx, cancel := context.WithTimeout(ctx, oidcThumbprintFetchTimeout) defer cancel() @@ -75,14 +87,16 @@ func FetchThumbprint(ctx context.Context, providerURL string) (string, error) { debuglogger.Logf("oidc thumbprint fetch: dns lookup failed for %q: %v", host, err) return "", iamerr.OpenIdIdpCommunicationError(displayURL) } - for _, ip := range ips { - if isDisallowedFetchTarget(ip) { - debuglogger.Logf("oidc thumbprint fetch: refusing to dial disallowed address %q for host %q", ip, host) - return "", iamerr.OpenIdIdpCommunicationError(displayURL) + if !policy.AllowPrivateEndpoints { + for _, ip := range ips { + if isDisallowedFetchTarget(ip) { + debuglogger.Logf("oidc thumbprint fetch: refusing to dial disallowed address %q for host %q", ip, host) + return "", iamerr.OpenIdIdpCommunicationError(displayURL) + } } } - thumbprint, err := dialAndVerifyThumbprint(ctx, net.JoinHostPort(ips[0].String(), "443"), host, nil) + thumbprint, err := dialAndVerifyThumbprint(ctx, net.JoinHostPort(ips[0].String(), port), host, nil, policy.AllowInsecureTransport) if err != nil { debuglogger.Logf("oidc thumbprint fetch: tls dial/verify failed for %q (%s): %v — supply ThumbprintList explicitly for providers that fail public CA verification", host, ips[0], err) return "", iamerr.OpenIdIdpCommunicationError(displayURL) @@ -93,15 +107,19 @@ func FetchThumbprint(ctx context.Context, providerURL string) (string, error) { // dialAndVerifyThumbprint dials addr over TLS, presenting host via SNI and // verifying the peer's certificate against roots (nil selects the host -// system's trust store, FetchThumbprint's real usage), then returns -// ThumbprintFromChain's result for the now-verified presented chain. Split -// out from FetchThumbprint so the verification behavior itself is -// unit-testable with an explicit root pool — the same rationale as -// ThumbprintFromChain's own split, and for the same reason: FetchThumbprint's -// SSRF guard must always reject loopback targets, so it can never itself be -// exercised against a same-process test server. -func dialAndVerifyThumbprint(ctx context.Context, addr, host string, roots *x509.CertPool) (string, error) { - dialer := &tls.Dialer{Config: &tls.Config{ServerName: host, RootCAs: roots}} +// system's trust store, FetchThumbprint's real usage) unless insecure drops +// verification altogether, then returns ThumbprintFromChain's result for the +// presented chain. Split out from FetchThumbprint so the verification +// behavior itself is unit-testable with an explicit root pool — the same +// rationale as ThumbprintFromChain's own split, and for the same reason: +// FetchThumbprint's SSRF guard rejects loopback targets by default, so it +// can never itself be exercised against a same-process test server. +func dialAndVerifyThumbprint(ctx context.Context, addr, host string, roots *x509.CertPool, insecure bool) (string, error) { + dialer := &tls.Dialer{Config: &tls.Config{ + ServerName: host, + RootCAs: roots, + InsecureSkipVerify: insecure, + }} conn, err := dialer.DialContext(ctx, "tcp", addr) if err != nil { return "", err @@ -121,7 +139,7 @@ func dialAndVerifyThumbprint(ctx context.Context, addr, host string, roots *x509 // in chain, hex-encoded and lowercased. Split out from FetchThumbprint as a // pure function specifically so it is unit-testable (e.g. against a chain // obtained from httptest.NewTLSServer) without going through -// FetchThumbprint's SSRF guard, which must always reject loopback targets +// FetchThumbprint's SSRF guard, which rejects loopback targets by default // and therefore can never itself be exercised against a same-process test // server. func ThumbprintFromChain(chain []*x509.Certificate) (string, error) { @@ -133,17 +151,33 @@ func ThumbprintFromChain(chain []*x509.Certificate) (string, error) { return hex.EncodeToString(sum[:]), nil } +// isDisallowedFetchTarget reports whether ip is off-limits as an outbound +// OIDC fetch target under the default posture. Callers skip it entirely +// when OIDCEndpointPolicy.AllowPrivateEndpoints is set. func isDisallowedFetchTarget(ip net.IP) bool { return ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsUnspecified() || ip.IsMulticast() } -// hostFromOIDCUrl extracts the host (no scheme, no path — OIDC provider -// URLs are validated to disallow explicit ports) from a scheme-stripped +// hostFromOIDCUrl extracts the host authority (no scheme, no path, but +// including an explicit port when the Url carries one) from a stored // provider Url. func hostFromOIDCUrl(providerURL string) string { + providerURL = strings.TrimPrefix(providerURL, insecureOIDCScheme) if before, _, ok := strings.Cut(providerURL, "/"); ok { return before } return providerURL } + +// splitOIDCHostPort splits a provider Url's host authority into hostname +// and port, defaulting to 443 — the only port reachable unless +// OIDCEndpointPolicy.AllowPrivateEndpoints permitted an explicit one — and +// unwrapping the brackets around a port-less IPv6 literal so the result is +// always a dialable hostname. +func splitOIDCHostPort(hostport string) (host, port string) { + if h, p, err := net.SplitHostPort(hostport); err == nil { + return h, p + } + return strings.TrimSuffix(strings.TrimPrefix(hostport, "["), "]"), "443" +} diff --git a/iamapi/internal/iamutil/oidc_thumbprint_test.go b/iamapi/internal/iamutil/oidc_thumbprint_test.go index 66fc72fd..677164ec 100644 --- a/iamapi/internal/iamutil/oidc_thumbprint_test.go +++ b/iamapi/internal/iamutil/oidc_thumbprint_test.go @@ -85,11 +85,32 @@ func TestDialAndVerifyThumbprintRejectsUntrustedCert(t *testing.T) { // roots=nil selects the host system's real trust store, the same as // FetchThumbprint's actual usage - httptest's self-signed certificate // must not verify against it. - if _, err := dialAndVerifyThumbprint(context.Background(), srv.Listener.Addr().String(), "example.com", nil); err == nil { + if _, err := dialAndVerifyThumbprint(context.Background(), srv.Listener.Addr().String(), "example.com", nil, false); err == nil { t.Fatal("dialAndVerifyThumbprint: expected verification error for untrusted self-signed certificate, got nil") } } +// TestDialAndVerifyThumbprintInsecureAcceptsUntrustedCert covers the +// AllowInsecureTransport path: the very chain +// TestDialAndVerifyThumbprintRejectsUntrustedCert requires to be rejected +// must be accepted and hashed once the operator has declared the network +// path itself to be the trust boundary — otherwise an IdP whose certificate +// cannot chain to a public root by construction could never use auto-fetch. +func TestDialAndVerifyThumbprintInsecureAcceptsUntrustedCert(t *testing.T) { + srv := httptest.NewTLSServer(nil) + defer srv.Close() + + got, err := dialAndVerifyThumbprint(context.Background(), srv.Listener.Addr().String(), "example.com", nil, true) + if err != nil { + t.Fatalf("dialAndVerifyThumbprint(insecure=true): %v", err) + } + + sum := sha1.Sum(srv.Certificate().Raw) + if want := hex.EncodeToString(sum[:]); got != want { + t.Fatalf("dialAndVerifyThumbprint thumbprint = %q, want %q", got, want) + } +} + // TestDialAndVerifyThumbprintAcceptsVerifiedCert is the positive // counterpart: once the peer's certificate does verify (here, against an // explicit pool containing the test server's own certificate, standing in @@ -104,7 +125,7 @@ func TestDialAndVerifyThumbprintAcceptsVerifiedCert(t *testing.T) { roots := x509.NewCertPool() roots.AddCert(srv.Certificate()) - got, err := dialAndVerifyThumbprint(context.Background(), srv.Listener.Addr().String(), "example.com", roots) + got, err := dialAndVerifyThumbprint(context.Background(), srv.Listener.Addr().String(), "example.com", roots, false) if err != nil { t.Fatalf("dialAndVerifyThumbprint: %v", err) } @@ -128,7 +149,7 @@ func TestFetchThumbprintSSRFGuard(t *testing.T) { } for _, host := range tests { t.Run(host, func(t *testing.T) { - _, err := FetchThumbprint(context.Background(), host) + _, err := FetchThumbprint(context.Background(), host, OIDCEndpointPolicy{}) if err == nil { t.Fatalf("FetchThumbprint(%q): expected SSRF guard error, got nil", host) } @@ -136,13 +157,86 @@ func TestFetchThumbprintSSRFGuard(t *testing.T) { } } +// TestFetchThumbprintAllowPrivateEndpoints confirms AllowPrivateEndpoints +// waives the address check rather than merely reordering it: with the guard +// off, a loopback provider gets as far as a real TLS handshake against a +// same-process server and yields that server's own thumbprint — something +// TestFetchThumbprintSSRFGuard shows is impossible by default. +func TestFetchThumbprintAllowPrivateEndpoints(t *testing.T) { + srv := httptest.NewTLSServer(nil) + defer srv.Close() + + policy := OIDCEndpointPolicy{AllowPrivateEndpoints: true, AllowInsecureTransport: true} + // The listener's host:port becomes the provider Url's authority, which + // only parses as one because AllowPrivateEndpoints also permits a port. + got, err := FetchThumbprint(context.Background(), srv.Listener.Addr().String(), policy) + if err != nil { + t.Fatalf("FetchThumbprint(loopback, AllowPrivateEndpoints): %v", err) + } + + sum := sha1.Sum(srv.Certificate().Raw) + if want := hex.EncodeToString(sum[:]); got != want { + t.Fatalf("FetchThumbprint thumbprint = %q, want %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. +func TestFetchThumbprintRejectsPlaintextProvider(t *testing.T) { + policy := OIDCEndpointPolicy{AllowPrivateEndpoints: true, AllowInsecureTransport: true} + if _, err := FetchThumbprint(context.Background(), "http://127.0.0.1:8080", policy); err == nil { + t.Fatal("FetchThumbprint(http provider): expected an error, got nil") + } +} + func TestFetchThumbprintDNSFailure(t *testing.T) { - _, err := FetchThumbprint(context.Background(), "this-host-should-not-resolve.invalid") + _, err := FetchThumbprint(context.Background(), "this-host-should-not-resolve.invalid", OIDCEndpointPolicy{}) if err == nil { t.Fatal("expected error for unresolvable host") } } +func TestSplitOIDCHostPort(t *testing.T) { + tests := []struct { + hostport string + wantHost string + wantPort string + }{ + {"example.com", "example.com", "443"}, + {"spire-oidc.spire.svc:8443", "spire-oidc.spire.svc", "8443"}, + {"127.0.0.1", "127.0.0.1", "443"}, + {"127.0.0.1:8443", "127.0.0.1", "8443"}, + {"[::1]:8443", "::1", "8443"}, + {"[::1]", "::1", "443"}, + } + for _, tt := range tests { + t.Run(tt.hostport, func(t *testing.T) { + host, port := splitOIDCHostPort(tt.hostport) + if host != tt.wantHost || port != tt.wantPort { + t.Errorf("splitOIDCHostPort(%q) = (%q, %q), want (%q, %q)", tt.hostport, host, port, tt.wantHost, tt.wantPort) + } + }) + } +} + +func TestHostFromOIDCUrl(t *testing.T) { + tests := []struct{ providerURL, want string }{ + {"example.com", "example.com"}, + {"example.com/path", "example.com"}, + {"spire-oidc.spire.svc:8443", "spire-oidc.spire.svc:8443"}, + {"http://127.0.0.1:8080", "127.0.0.1:8080"}, + {"http://127.0.0.1:8080/oidc", "127.0.0.1:8080"}, + } + for _, tt := range tests { + t.Run(tt.providerURL, func(t *testing.T) { + if got := hostFromOIDCUrl(tt.providerURL); got != tt.want { + t.Errorf("hostFromOIDCUrl(%q) = %q, want %q", tt.providerURL, got, tt.want) + } + }) + } +} + func TestIsDisallowedFetchTarget(t *testing.T) { tests := []struct { ip string diff --git a/iamapi/internal/iamutil/webidentity.go b/iamapi/internal/iamutil/webidentity.go index 61782ac1..e72f1cb8 100644 --- a/iamapi/internal/iamutil/webidentity.go +++ b/iamapi/internal/iamutil/webidentity.go @@ -222,11 +222,13 @@ func ParseWebIdentityClaims(tokenString string) (jwt.MapClaims, error) { // // Only an "https://" prefix is stripped — OIDC issuer identifiers are // compared exactly, scheme included, and CreateOpenIDConnectProvider already -// requires every registered provider's Url to be https. An iss using any -// other scheme (or none at all) therefore can never legitimately equal a -// registered provider; returning it unstripped in that case (rather than -// also trimming a bare "http://") guarantees it stays distinguishable from a -// same-host https issuer instead of being silently treated as equivalent. +// stores every https provider's Url scheme-stripped. An iss using any other +// scheme is returned unstripped, which is exactly what makes it comparable: +// a plaintext provider (registrable only under +// OIDCEndpointPolicy.AllowInsecureTransport) is stored with its "http://" +// intact, so trimming it here would collapse "http://host" and +// "https://host" into the same value and let one issuer stand in for the +// other. func WebIdentityIssuer(claims jwt.MapClaims) (string, bool) { iss, ok := claims["iss"].(string) if !ok || iss == "" { @@ -416,12 +418,13 @@ func VerifyWebIdentityRequiredClaims(claims jwt.MapClaims, now time.Time) error // thumbprints is the OIDC provider's registered ThumbprintList, used as a // pinned-certificate fallback when the JWKS endpoint's TLS certificate // doesn't chain to a trusted root (self-signed/private-CA providers). +// policy carries the deployment's endpoint relaxations, if any. // // If the cached key set doesn't contain the token's kid, the cache is // bypassed for one forced refresh before giving up — the provider may have // rotated its signing key since the cache entry was fetched. -func VerifyWebIdentitySignature(ctx context.Context, tokenString, issuerURL string, thumbprints []string) (jwt.MapClaims, error) { - keys, err := cachedJWKS(ctx, issuerURL, thumbprints) +func VerifyWebIdentitySignature(ctx context.Context, tokenString, issuerURL string, thumbprints []string, policy OIDCEndpointPolicy) (jwt.MapClaims, error) { + keys, err := cachedJWKS(ctx, issuerURL, thumbprints, policy) if err != nil { debuglogger.Logf("failed to fetch JWKS for web identity provider %q: %v", issuerURL, err) return nil, iamerr.InvalidIdentityTokenIDPCommunicationError() @@ -429,7 +432,7 @@ func VerifyWebIdentitySignature(ctx context.Context, tokenString, issuerURL stri claims, err := verifySignatureWithKeys(tokenString, keys) if err != nil && errors.Is(err, errUnknownKID) { - keys, refreshErr := forceRefreshJWKSCache(ctx, issuerURL, thumbprints) + keys, refreshErr := forceRefreshJWKSCache(ctx, issuerURL, thumbprints, policy) if refreshErr != nil { debuglogger.Logf("failed to refresh JWKS for web identity provider %q: %v", issuerURL, refreshErr) return nil, iamerr.InvalidIdentityTokenIDPCommunicationError() @@ -451,7 +454,7 @@ var errUnknownKID = errors.New("no matching JWKS key for kid") // verifySignatureWithKeys is VerifyWebIdentitySignature's network-free core, // split out so it can be exercised directly against an in-memory key set // (the SSRF guard in fetchJWKS's dialer means it can never itself be -// exercised against a same-process test server — the same split +// exercised against a same-process test server by default — the same split // FetchThumbprint/ThumbprintFromChain use). The returned error is the raw // parse/verification failure (not yet converted to an iamerr), so callers // can distinguish errUnknownKID from every other failure. @@ -561,7 +564,7 @@ type oidcDiscoveryDoc struct { // reachable from the provider's own URL — otherwise a provider could return, // or be redirected/misdirected to, an entirely different issuer's metadata. func validateDiscoveryIssuer(doc oidcDiscoveryDoc, issuerURL string) error { - want := "https://" + issuerURL + want := OIDCEndpointURL(issuerURL) if doc.Issuer != want { return fmt.Errorf("discovery document for %q has mismatched issuer %q", issuerURL, doc.Issuer) } @@ -616,7 +619,7 @@ func jwksCacheKey(issuerURL string, thumbprints []string) string { // 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) (*jwkSet, error) { +func cachedJWKS(ctx context.Context, issuerURL string, thumbprints []string, policy OIDCEndpointPolicy) (*jwkSet, error) { key := jwksCacheKey(issuerURL, thumbprints) jwksCacheMu.Lock() entry, ok := jwksCache[key] @@ -624,7 +627,7 @@ func cachedJWKS(ctx context.Context, issuerURL string, thumbprints []string) (*j if ok && time.Now().Before(entry.expiresAt) { return entry.keys, nil } - return fetchAndCacheJWKS(ctx, issuerURL, thumbprints) + return fetchAndCacheJWKS(ctx, issuerURL, thumbprints, policy) } // forceRefreshJWKSCache is VerifyWebIdentitySignature's fallback when a @@ -644,7 +647,7 @@ func cachedJWKS(ctx context.Context, issuerURL string, thumbprints []string) (*j // backoff, since a failed attempt never set the timestamp that would have // 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) (*jwkSet, error) { +func forceRefreshJWKSCache(ctx context.Context, issuerURL string, thumbprints []string, policy OIDCEndpointPolicy) (*jwkSet, error) { key := jwksCacheKey(issuerURL, thumbprints) jwksCacheMu.Lock() entry, ok := jwksCache[key] @@ -664,7 +667,7 @@ func forceRefreshJWKSCache(ctx context.Context, issuerURL string, thumbprints [] jwksCache[key] = entry jwksCacheMu.Unlock() - return fetchAndCacheJWKS(ctx, issuerURL, thumbprints) + return fetchAndCacheJWKS(ctx, issuerURL, thumbprints, policy) } // fetchAndCacheJWKS fetches issuerURL's key set and, on success, replaces @@ -672,10 +675,10 @@ func forceRefreshJWKSCache(ctx context.Context, issuerURL string, thumbprints [] // thumbprints 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) (*jwkSet, error) { +func fetchAndCacheJWKS(ctx context.Context, issuerURL string, thumbprints []string, policy OIDCEndpointPolicy) (*jwkSet, error) { key := jwksCacheKey(issuerURL, thumbprints) v, err, _ := jwksFetchGroup.Do(key, func() (any, error) { - keys, err := fetchJWKS(ctx, issuerURL, thumbprints) + keys, err := fetchJWKS(ctx, issuerURL, thumbprints, policy) if err != nil { return nil, err } @@ -694,14 +697,13 @@ func fetchAndCacheJWKS(ctx context.Context, issuerURL string, thumbprints []stri } // fetchJWKS retrieves issuerURL's OIDC discovery document, then the JWKS it -// points to. issuerURL is the provider's stored Url (scheme stripped). -// 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) (*jwkSet, error) { - client := ssrfSafeHTTPClient(thumbprints) - base := "https://" + issuerURL +// 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) { + 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 { @@ -710,7 +712,7 @@ func fetchJWKS(ctx context.Context, issuerURL string, thumbprints []string) (*jw if err := validateDiscoveryIssuer(doc, issuerURL); err != nil { return nil, err } - if !strings.HasPrefix(doc.JWKSUri, "https://") { + if !isFetchableOIDCEndpoint(doc.JWKSUri, policy) { return nil, fmt.Errorf("discovery document for %q has non-https jwks_uri %q", issuerURL, doc.JWKSUri) } @@ -773,6 +775,24 @@ func fetchJSON(ctx context.Context, client *http.Client, url string, out any) er return json.Unmarshal(body, out) } +// isFetchableOIDCEndpoint reports whether a URL a discovery document points +// at may be fetched: https always, http only where policy has already +// accepted a plaintext IdP. Matched on the literal prefix, so a jwks_uri +// naming anything else — including a scheme net/http would otherwise +// happily dial — never reaches the client. +func isFetchableOIDCEndpoint(rawURL string, policy OIDCEndpointPolicy) bool { + if strings.HasPrefix(rawURL, "https://") { + return true + } + return policy.AllowInsecureTransport && strings.HasPrefix(rawURL, insecureOIDCScheme) +} + +// isFetchableOIDCScheme is isFetchableOIDCEndpoint for an already-parsed +// URL, used on the redirect path where net/http hands over a *url.URL. +func isFetchableOIDCScheme(scheme string, policy OIDCEndpointPolicy) bool { + return scheme == "https" || (policy.AllowInsecureTransport && scheme == "http") +} + // ssrfSafeHTTPClient returns an http.Client whose transport resolves each // dial target's DNS once and rejects loopback/private/link-local/multicast // addresses before connecting, mirroring FetchThumbprint's SSRF guard. It @@ -789,7 +809,13 @@ func fetchJSON(ctx context.Context, client *http.Client, url string, out any) er // standard CA-based verification would otherwise reject it, and falls back // to ordinary hostname+CA verification against the system root pool // whenever thumbprints is empty or doesn't match. -func ssrfSafeHTTPClient(thumbprints []string) *http.Client { +// +// policy relaxes exactly two of those behaviors, and nothing else: +// AllowPrivateEndpoints drops the resolved-address check (the DNS-once, +// dial-the-resolved-IP shape stays, so a rebind still can't redirect the +// connection), and AllowInsecureTransport additionally admits http targets +// and redirects and makes verifyOIDCConnection accept any chain. +func ssrfSafeHTTPClient(thumbprints []string, policy OIDCEndpointPolicy) *http.Client { dialer := &net.Dialer{} return &http.Client{ Timeout: oidcFetchTimeout, @@ -797,8 +823,8 @@ func ssrfSafeHTTPClient(thumbprints []string) *http.Client { if len(via) >= maxOIDCFetchRedirects { return fmt.Errorf("stopped after %d redirects", maxOIDCFetchRedirects) } - if req.URL.Scheme != "https" { - return fmt.Errorf("refusing to follow non-https redirect to %q", req.URL) + if !isFetchableOIDCScheme(req.URL.Scheme, policy) { + return fmt.Errorf("refusing to follow redirect to %q: disallowed scheme", req.URL) } return nil }, @@ -812,9 +838,11 @@ func ssrfSafeHTTPClient(thumbprints []string) *http.Client { if err != nil || len(ips) == 0 { return nil, fmt.Errorf("dns lookup failed for %q", host) } - for _, ip := range ips { - if isDisallowedFetchTarget(ip) { - return nil, fmt.Errorf("refusing to dial disallowed address %q for host %q", ip, host) + if !policy.AllowPrivateEndpoints { + for _, ip := range ips { + if isDisallowedFetchTarget(ip) { + return nil, fmt.Errorf("refusing to dial disallowed address %q for host %q", ip, host) + } } } return dialer.DialContext(ctx, network, net.JoinHostPort(ips[0].String(), port)) @@ -822,7 +850,7 @@ func ssrfSafeHTTPClient(thumbprints []string) *http.Client { TLSClientConfig: &tls.Config{ InsecureSkipVerify: true, // verified ourselves via VerifyConnection below VerifyConnection: func(cs tls.ConnectionState) error { - return verifyOIDCConnection(cs, thumbprints) + return verifyOIDCConnection(cs, thumbprints, policy) }, }, }, @@ -841,10 +869,18 @@ func ssrfSafeHTTPClient(thumbprints []string) *http.Client { // cryptographically issue the leaf and the leaf must match cs.ServerName. // Falls back to standard hostname+CA verification against the system root // pool whenever thumbprints is empty or none matches. -func verifyOIDCConnection(cs tls.ConnectionState, thumbprints []string) error { +// +// policy.AllowInsecureTransport accepts any chain outright, pinning +// included: the operator has declared the network path itself to be the +// trust boundary, and a partial check that silently passed on some +// certificates and not others would only obscure that. +func verifyOIDCConnection(cs tls.ConnectionState, thumbprints []string, policy OIDCEndpointPolicy) error { if len(cs.PeerCertificates) == 0 { return errors.New("iamutil: no certificate presented") } + if policy.AllowInsecureTransport { + return nil + } if len(thumbprints) > 0 { top := cs.PeerCertificates[len(cs.PeerCertificates)-1] diff --git a/iamapi/internal/iamutil/webidentity_test.go b/iamapi/internal/iamutil/webidentity_test.go index 49b53812..de4d8547 100644 --- a/iamapi/internal/iamutil/webidentity_test.go +++ b/iamapi/internal/iamutil/webidentity_test.go @@ -269,14 +269,14 @@ func TestVerifyOIDCConnection(t *testing.T) { // net/http/internal/testcert), and it is self-signed, so it forms a // valid one-certificate chain rooted at itself for that name. cs := tls.ConnectionState{PeerCertificates: chain, ServerName: "example.com"} - if err := verifyOIDCConnection(cs, []string{thumbprint}); err != nil { + if err := verifyOIDCConnection(cs, []string{thumbprint}, OIDCEndpointPolicy{}); err != nil { t.Fatalf("expected pinned thumbprint to be accepted for a matching hostname: %v", err) } }) t.Run("matching pinned thumbprint does not bypass hostname verification", func(t *testing.T) { cs := tls.ConnectionState{PeerCertificates: chain, ServerName: "totally-different-host.example"} - if err := verifyOIDCConnection(cs, []string{thumbprint}); err == nil { + if err := verifyOIDCConnection(cs, []string{thumbprint}, OIDCEndpointPolicy{}); err == nil { t.Fatal("expected pinned thumbprint to still be rejected for a non-matching hostname") } }) @@ -290,30 +290,72 @@ func TestVerifyOIDCConnection(t *testing.T) { forged := append([]*x509.Certificate{unrelatedLeaf}, chain...) cs := tls.ConnectionState{PeerCertificates: forged, ServerName: "example.com"} - if err := verifyOIDCConnection(cs, []string{thumbprint}); err == nil { + if err := verifyOIDCConnection(cs, []string{thumbprint}, OIDCEndpointPolicy{}); err == nil { t.Fatal("expected forged chain (unrelated leaf + appended pinned cert) to be rejected") } }) t.Run("non-matching thumbprint falls back to standard verification and fails", func(t *testing.T) { cs := tls.ConnectionState{PeerCertificates: chain, ServerName: "example.com"} - if err := verifyOIDCConnection(cs, []string{"0000000000000000000000000000000000000000"}); err == nil { + if err := verifyOIDCConnection(cs, []string{"0000000000000000000000000000000000000000"}, OIDCEndpointPolicy{}); err == nil { t.Fatal("expected standard verification to fail for a self-signed cert not in the system pool") } }) t.Run("no thumbprints falls back to standard verification and fails", func(t *testing.T) { cs := tls.ConnectionState{PeerCertificates: chain, ServerName: "example.com"} - if err := verifyOIDCConnection(cs, nil); err == nil { + if err := verifyOIDCConnection(cs, nil, OIDCEndpointPolicy{}); err == nil { t.Fatal("expected standard verification to fail for a self-signed cert not in the system pool") } }) t.Run("no certificates presented", func(t *testing.T) { - if err := verifyOIDCConnection(tls.ConnectionState{}, nil); err == nil { + if err := verifyOIDCConnection(tls.ConnectionState{}, nil, OIDCEndpointPolicy{}); err == nil { t.Fatal("expected error when no certificate is presented") } }) + + t.Run("insecure transport accepts a chain every other case rejects", func(t *testing.T) { + insecure := OIDCEndpointPolicy{AllowInsecureTransport: true} + // Untrusted chain, wrong hostname, and no pinned thumbprint - each + // on its own is a rejection above. + cs := tls.ConnectionState{PeerCertificates: chain, ServerName: "totally-different-host.example"} + if err := verifyOIDCConnection(cs, nil, insecure); err != nil { + t.Fatalf("expected AllowInsecureTransport to accept any chain: %v", err) + } + }) + + t.Run("insecure transport still requires a certificate", func(t *testing.T) { + insecure := OIDCEndpointPolicy{AllowInsecureTransport: true} + if err := verifyOIDCConnection(tls.ConnectionState{}, nil, insecure); err == nil { + t.Fatal("expected error when no certificate is presented at all") + } + }) +} + +func TestIsFetchableOIDCEndpoint(t *testing.T) { + insecure := OIDCEndpointPolicy{AllowInsecureTransport: true} + tests := []struct { + rawURL string + policy OIDCEndpointPolicy + want bool + }{ + {"https://example.com/keys", OIDCEndpointPolicy{}, true}, + {"http://example.com/keys", OIDCEndpointPolicy{}, false}, + {"http://127.0.0.1:8080/keys", insecure, true}, + {"https://127.0.0.1:8080/keys", insecure, true}, + {"file:///etc/passwd", insecure, false}, + {"//example.com/keys", insecure, false}, + // AllowPrivateEndpoints alone is about addresses, not schemes. + {"http://10.0.0.1/keys", OIDCEndpointPolicy{AllowPrivateEndpoints: true}, false}, + } + for _, tt := range tests { + t.Run(tt.rawURL, func(t *testing.T) { + if got := isFetchableOIDCEndpoint(tt.rawURL, tt.policy); got != tt.want { + t.Errorf("isFetchableOIDCEndpoint(%q, %+v) = %v, want %v", tt.rawURL, tt.policy, got, tt.want) + } + }) + } } func TestVerifySignatureWithKeys(t *testing.T) { @@ -560,7 +602,7 @@ func TestForceRefreshJWKSCacheGatesFailedAttempts(t *testing.T) { ctx := context.Background() - if _, err := forceRefreshJWKSCache(ctx, issuer, nil); err == nil { + if _, err := forceRefreshJWKSCache(ctx, issuer, nil, OIDCEndpointPolicy{}); err == nil { t.Fatal("forceRefreshJWKSCache() = nil error, want an error for a disallowed loopback target") } @@ -575,7 +617,7 @@ func TestForceRefreshJWKSCacheGatesFailedAttempts(t *testing.T) { // A second forced refresh within jwksMinForcedRefreshInterval must be // gated - failing immediately with no cached keys to fall back on - // rather than attempting another fetch. - if _, err := forceRefreshJWKSCache(ctx, issuer, nil); err == nil { + if _, err := forceRefreshJWKSCache(ctx, issuer, nil, OIDCEndpointPolicy{}); err == nil { t.Fatal("forceRefreshJWKSCache() = nil error on gated retry, want an error (no cached keys available)") } jwksCacheMu.Lock() diff --git a/iamapi/router.go b/iamapi/router.go index d68e1e8d..802967b9 100644 --- a/iamapi/router.go +++ b/iamapi/router.go @@ -50,13 +50,13 @@ type IAMApiRouter struct { Ctrl IAMApiController actions map[string]ActionHandler rootCreds *RootCredentials - // oidcThumbprintAutoFetchDisabled is threaded into the controller; - // see IAMApiController.oidcThumbprintAutoFetchDisabled. - oidcThumbprintAutoFetchDisabled bool + // oidc is threaded into the controller and the policy middleware, both + // of which validate caller-supplied OIDC provider URLs + oidc OIDCConfig } func (r *IAMApiRouter) Init() { - r.Ctrl = NewController(r.store, r.oidcThumbprintAutoFetchDisabled) + r.Ctrl = NewController(r.store, r.oidc) r.actions = map[string]ActionHandler{ // User CRUD @@ -114,7 +114,7 @@ func (r *IAMApiRouter) Init() { iamRoute := ProcessHandlers(r.routeAction, iammiddleware.VerifyIAMAuth(sigv4auth.ServiceIAM, r.rootCreds, r.store), - iammiddleware.VerifyIAMPolicy(r.store), + iammiddleware.VerifyIAMPolicy(r.store, r.oidc.endpointPolicy()), ) stsAuthRoute := ProcessHandlers(r.routeAction, iammiddleware.VerifyIAMAuth(sigv4auth.ServiceSTS, r.rootCreds, r.store), diff --git a/iamapi/server.go b/iamapi/server.go index 939e2134..023e8a81 100644 --- a/iamapi/server.go +++ b/iamapi/server.go @@ -27,6 +27,7 @@ import ( "github.com/gofiber/fiber/v3/middleware/recover" "github.com/versity/versitygw/debuglogger" "github.com/versity/versitygw/iamapi/internal/iammiddleware" + "github.com/versity/versitygw/iamapi/internal/iamutil" "github.com/versity/versitygw/iamapi/storage" "github.com/versity/versitygw/internal/netutil" ) @@ -59,13 +60,41 @@ type IAMApiServer struct { maxRequests int socketPerm os.FileMode onListen func() - // oidcThumbprintAutoFetchDisabled disables CreateOpenIDConnectProvider's - // TLS auto-fetch fallback; see WithOIDCThumbprintAutoFetchDisabled. - oidcThumbprintAutoFetchDisabled bool + // oidc holds the OIDC provider settings threaded into the router, + // controller, and policy middleware; see OIDCConfig. + oidc OIDCConfig // corsAllowOrigin is the single origin browsers may call this API from corsAllowOrigin string } +// OIDCConfig groups the settings that govern how this API treats OIDC +// identity providers: whether CreateOpenIDConnectProvider may reach out for +// a thumbprint at all, and how strictly a provider's endpoint is validated +// and fetched from. The zero value is the default AWS-matching posture. +type OIDCConfig struct { + // ThumbprintAutoFetchDisabled disables CreateOpenIDConnectProvider's + // TLS auto-fetch fallback when ThumbprintList is omitted; see + // WithOIDCThumbprintAutoFetchDisabled. + ThumbprintAutoFetchDisabled bool + // AllowPrivateEndpoints permits OIDC provider URLs that resolve to + // loopback/private/link-local addresses, and that carry an explicit + // port; see WithOIDCAllowPrivateEndpoints. + AllowPrivateEndpoints bool + // AllowInsecureTransport permits plaintext http OIDC provider URLs and + // drops TLS verification for https ones; see + // WithOIDCAllowInsecureTransport. + AllowInsecureTransport bool +} + +// endpointPolicy projects the two 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, + } +} + func New(store storage.Storer, root RootCredentials, opts ...Option) (*IAMApiServer, error) { if store == nil { return nil, fmt.Errorf("iamapi: storer is required") @@ -96,7 +125,7 @@ func New(store storage.Storer, root RootCredentials, opts ...Option) (*IAMApiSer server.app = app server.Router.app = app server.Router.rootCreds = server.rootCreds - server.Router.oidcThumbprintAutoFetchDisabled = server.oidcThumbprintAutoFetchDisabled + server.Router.oidc = server.oidc app.Use("*", recover.New(recover.Config{ EnableStackTrace: true, @@ -184,7 +213,27 @@ func WithOnListen(fn func()) Option { // the gateway making an outbound TLS connection to the caller-supplied URL // — an operational safety valve for restricted/air-gapped deployments. func WithOIDCThumbprintAutoFetchDisabled() Option { - return func(s *IAMApiServer) { s.oidcThumbprintAutoFetchDisabled = true } + return func(s *IAMApiServer) { s.oidc.ThumbprintAutoFetchDisabled = true } +} + +// WithOIDCAllowPrivateEndpoints permits an OIDC provider Url that resolves +// to a loopback/private/link-local address, and one carrying an explicit +// port. Both are refused by default, which makes an IdP that only exists on +// an internal network — a SPIFFE/SPIRE OIDC discovery provider on a cluster +// Service, say — impossible to register or verify tokens against. Transport +// is unaffected: still https, still fully verified. +func WithOIDCAllowPrivateEndpoints() Option { + return func(s *IAMApiServer) { s.oidc.AllowPrivateEndpoints = true } +} + +// WithOIDCAllowInsecureTransport permits plaintext http OIDC provider URLs +// and drops TLS certificate verification (ThumbprintList pinning included) +// for https ones, leaving the network path as the only thing authenticating +// the IdP. Intended for an IdP reachable only over a path that is itself +// trusted — a discovery provider bound to loopback as a sidecar in this +// process's own pod. +func WithOIDCAllowInsecureTransport() Option { + return func(s *IAMApiServer) { s.oidc.AllowInsecureTransport = true } } func (s *IAMApiServer) ServeMultiPort(ports []string) error { diff --git a/iamapi/storage/internal.go b/iamapi/storage/internal.go index b786e551..d51c53d9 100644 --- a/iamapi/storage/internal.go +++ b/iamapi/storage/internal.go @@ -1264,7 +1264,7 @@ func (s *InternalStore) CreateOIDCProvider(_ context.Context, provider types.OID } if _, ok := conf.OIDCProviders[provider.Url]; ok { - return nil, iamerr.EntityAlreadyExistsOIDCProvider("https://" + provider.Url) + return nil, iamerr.EntityAlreadyExistsOIDCProvider(iamutil.OIDCEndpointURL(provider.Url)) } if len(conf.OIDCProviders) >= MaxOIDCProvidersPerAccount { return nil, iamerr.OIDCProvidersPerAccountLimitExceeded(MaxOIDCProvidersPerAccount) diff --git a/iamapi/storage/vault.go b/iamapi/storage/vault.go index 365c0d17..a94c2c40 100644 --- a/iamapi/storage/vault.go +++ b/iamapi/storage/vault.go @@ -1528,7 +1528,7 @@ func oidcProviderPathSegment(url string) string { func (s *VaultStore) CreateOIDCProvider(_ context.Context, provider types.OIDCProvider) (*types.OIDCProvider, error) { segment := oidcProviderPathSegment(provider.Url) path := s.oidcProvidersPath() + "/" + segment - displayURL := "https://" + provider.Url + displayURL := iamutil.OIDCEndpointURL(provider.Url) resp, err := s.client.Secrets.KvV2List(context.Background(), s.oidcProvidersPath(), s.kvReqOpts...) if err != nil && !vault.IsErrorStatus(err, http.StatusNotFound) { diff --git a/tests/integration/iam_access_control.go b/tests/integration/iam_access_control.go index 1c634ca1..8ef9e8c2 100644 --- a/tests/integration/iam_access_control.go +++ b/tests/integration/iam_access_control.go @@ -24,11 +24,12 @@ package integration // that mints a session in this codebase, and a real successful call requires // the server to fetch a real JWKS from the token's issuer and verify a real // cryptographic signature. The SSRF guard in iamutil's OIDC fetch path -// (isDisallowedFetchTarget) unconditionally rejects loopback, private -// (RFC1918), and link-local addresses as fetch targets — so no JWKS server -// this test process stands up on the same machine can ever be reachable, -// and a real successful AssumeRoleWithWebIdentity is unreachable from this -// suite by design. Every test below that needs to observe a trust-policy +// (isDisallowedFetchTarget) rejects loopback, private (RFC1918), and +// link-local addresses as fetch targets unless the gateway under test was +// started with --oidc-allow-private-endpoints, which this suite's gateway +// never is — so no JWKS server this test process stands up on the same +// machine can ever be reachable, and a real successful +// AssumeRoleWithWebIdentity is unreachable from this suite by design. Every test below that needs to observe a trust-policy // "Allowed" decision instead uses the same technique the rest of this // package's AssumeRoleWithWebIdentity tests already use (see // IAMAssumeRoleWithWebIdentity_oaud_condition_matches in diff --git a/tests/integration/iam_assume_role_with_web_identity.go b/tests/integration/iam_assume_role_with_web_identity.go index 84622aea..5a0f2838 100644 --- a/tests/integration/iam_assume_role_with_web_identity.go +++ b/tests/integration/iam_assume_role_with_web_identity.go @@ -374,8 +374,9 @@ func IAMAssumeRoleWithWebIdentity_empty_client_id_list(s *S3Conf) error { // IAMAssumeRoleWithWebIdentity_idp_communication_error confirms the // network-dependent signature-verification step is wired all the way // through the real HTTP action handler: a provider Url that's a loopback IP -// literal is rejected by VerifyWebIdentitySignature's mandatory SSRF guard -// before any real network attempt, deterministically and without requiring +// literal is rejected by VerifyWebIdentitySignature's SSRF guard (on by +// default, and never waived for this suite's gateway) before any real +// network attempt, deterministically and without requiring // outbound network access from the test environment — the same technique // IAMCreateOpenIDConnectProvider_thumbprint_autofetch_communication_error // uses for CreateOpenIDConnectProvider's own auto-fetch path. diff --git a/tests/integration/iam_create_oidc_provider.go b/tests/integration/iam_create_oidc_provider.go index cedcd89e..bf378097 100644 --- a/tests/integration/iam_create_oidc_provider.go +++ b/tests/integration/iam_create_oidc_provider.go @@ -194,8 +194,9 @@ func IAMCreateOpenIDConnectProvider_already_exists(s *S3Conf) error { // IAMCreateOpenIDConnectProvider_thumbprint_autofetch_communication_error // confirms the network-dependent auto-fetch fallback (triggered by // omitting ThumbprintList) is wired all the way through the real HTTP -// action handler: a loopback URL is rejected by the fetch's mandatory -// SSRF guard before any real network attempt, deterministically and +// action handler: a loopback URL is rejected by the fetch's SSRF guard (on +// by default, and never waived for this suite's gateway) before any real +// network attempt, deterministically and // without requiring outbound network access from the test environment. func IAMCreateOpenIDConnectProvider_thumbprint_autofetch_communication_error(s *S3Conf) error { testName := "IAMCreateOpenIDConnectProvider_thumbprint_autofetch_communication_error" diff --git a/webui/web/iam-oidc.html b/webui/web/iam-oidc.html index 157330b2..a63cdba3 100644 --- a/webui/web/iam-oidc.html +++ b/webui/web/iam-oidc.html @@ -195,7 +195,7 @@ under the License.
-

Must start with https://. No port, user info, query string or fragment. The URL cannot be changed after creation.

+

Must start with https://. No user info, query string or fragment. A port, or an http:// URL, is accepted only if the service was started with --oidc-allow-private-endpoints / --oidc-allow-insecure-transport. The URL cannot be changed after creation.

@@ -454,9 +454,14 @@ under the License. openModal('create-provider-modal'); } + // Whether http:// and an explicit port are accepted depends on the IAM + // service's own --oidc-allow-private-endpoints/--oidc-allow-insecure-transport + // settings, which this page has no way to read. Those two rules are left + // to the server, whose rejection surfaces as a toast like any other API + // error; everything checked here holds regardless of configuration. function validateProviderUrl(url) { if (!url) return 'Provider URL is required.'; - if (!url.startsWith('https://')) return 'Provider URL must start with https://.'; + if (!url.startsWith('https://') && !url.startsWith('http://')) return 'Provider URL must start with https://.'; if (url.length > IAM_LIMITS.oidcUrlChars) return `Provider URL must be ${IAM_LIMITS.oidcUrlChars} characters or fewer.`; let parsed; try { @@ -464,7 +469,6 @@ under the License. } catch (e) { return 'Provider URL is not a valid URL.'; } - if (parsed.port) return 'Provider URL must not include a port.'; if (parsed.username || parsed.password) return 'Provider URL must not include user info.'; if (parsed.search) return 'Provider URL must not include a query string.'; if (parsed.hash) return 'Provider URL must not include a fragment.';