diff --git a/cmd/thv-operator/pkg/vmcpconfig/converter.go b/cmd/thv-operator/pkg/vmcpconfig/converter.go index be35dd1d5f..476870f6ea 100644 --- a/cmd/thv-operator/pkg/vmcpconfig/converter.go +++ b/cmd/thv-operator/pkg/vmcpconfig/converter.go @@ -256,6 +256,7 @@ func mapResolvedOIDCToVmcpConfigFromRef( JwksAllowPrivateIP: resolved.JWKSAllowPrivateIP, InsecureAllowHTTP: resolved.InsecureAllowHTTP, Scopes: resolved.Scopes, + CABundlePath: resolved.ThvCABundlePath, } // MCPOIDCConfig inline type may have a client secret diff --git a/cmd/thv-operator/pkg/vmcpconfig/converter_test.go b/cmd/thv-operator/pkg/vmcpconfig/converter_test.go index aad297da44..8bb08b7387 100644 --- a/cmd/thv-operator/pkg/vmcpconfig/converter_test.go +++ b/cmd/thv-operator/pkg/vmcpconfig/converter_test.go @@ -188,6 +188,52 @@ func TestConverter_OIDCResolution(t *testing.T) { assert.Equal(t, "VMCP_OIDC_CLIENT_SECRET", config.IncomingAuth.OIDC.ClientSecretEnv) }, }, + { + name: "inline resolved ThvCABundlePath maps to CABundlePath", + oidcConfigRef: &mcpv1alpha1.MCPOIDCConfigReference{Name: oidcConfigName, Audience: "test-audience"}, + oidcConfig: newTestMCPOIDCConfig(mcpv1alpha1.MCPOIDCConfigTypeInline), + mockReturn: &oidc.OIDCConfig{ + Issuer: "https://issuer.example.com", + ThvCABundlePath: "/config/certs/example-ca/ca.crt", + }, + validate: func(t *testing.T, config *vmcpconfig.Config, err error) { + t.Helper() + require.NoError(t, err) + require.NotNil(t, config.IncomingAuth.OIDC) + assert.Equal(t, "/config/certs/example-ca/ca.crt", config.IncomingAuth.OIDC.CABundlePath) + }, + }, + { + name: "k8s service account ThvCABundlePath maps to CABundlePath", + oidcConfigRef: &mcpv1alpha1.MCPOIDCConfigReference{Name: oidcConfigName, Audience: "test-audience"}, + oidcConfig: newTestMCPOIDCConfig(mcpv1alpha1.MCPOIDCConfigTypeKubernetesServiceAccount), + mockReturn: &oidc.OIDCConfig{ + Issuer: "https://kubernetes.default.svc", + ThvCABundlePath: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt", + }, + validate: func(t *testing.T, config *vmcpconfig.Config, err error) { + t.Helper() + require.NoError(t, err) + require.NotNil(t, config.IncomingAuth.OIDC) + assert.Equal(t, + "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt", + config.IncomingAuth.OIDC.CABundlePath) + }, + }, + { + name: "empty ThvCABundlePath results in empty CABundlePath", + oidcConfigRef: &mcpv1alpha1.MCPOIDCConfigReference{Name: oidcConfigName, Audience: "test-audience"}, + oidcConfig: newTestMCPOIDCConfig(mcpv1alpha1.MCPOIDCConfigTypeInline), + mockReturn: &oidc.OIDCConfig{ + Issuer: "https://issuer.example.com", + }, + validate: func(t *testing.T, config *vmcpconfig.Config, err error) { + t.Helper() + require.NoError(t, err) + require.NotNil(t, config.IncomingAuth.OIDC) + assert.Empty(t, config.IncomingAuth.OIDC.CABundlePath) + }, + }, { name: "non-inline type does not set ClientSecretEnv", oidcConfigRef: &mcpv1alpha1.MCPOIDCConfigReference{Name: oidcConfigName, Audience: "test-audience"}, diff --git a/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_virtualmcpserver_integration_test.go b/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_virtualmcpserver_integration_test.go index cda92baa98..a34bd48a55 100644 --- a/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_virtualmcpserver_integration_test.go +++ b/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_virtualmcpserver_integration_test.go @@ -6,6 +6,7 @@ package controllers import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -731,4 +732,181 @@ var _ = Describe("MCPOIDCConfig and VirtualMCPServer Cross-Resource Integration }, timeout, interval).Should(BeTrue()) }) }) + + Context("When MCPOIDCConfig inline.caBundleRef is set", Ordered, func() { + var ( + namespace string + configName string + vmcpName string + groupName string + caCMName string + caCMKey string + caConfigMap *corev1.ConfigMap + oidcConfig *mcpv1alpha1.MCPOIDCConfig + vmcpServer *mcpv1alpha1.VirtualMCPServer + mcpGroup *mcpv1alpha1.MCPGroup + ns *corev1.Namespace + ) + + BeforeAll(func() { + ns = &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "test-vmcp-oidc-cabundle-"}, + } + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + namespace = ns.Name + + configName = testOIDCConfigName + vmcpName = testVMCPServerName + groupName = testVMCPGroupName + caCMName = "vmcp-oidc-ca" + caCMKey = "ca.crt" + + // ConfigMap holding the CA bundle. Content is a placeholder — the operator + // only cares about mounting the ConfigMap at the right path. + caConfigMap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: caCMName, + Namespace: namespace, + }, + Data: map[string]string{ + caCMKey: "-----BEGIN CERTIFICATE-----\nplaceholder\n-----END CERTIFICATE-----\n", + }, + } + Expect(k8sClient.Create(ctx, caConfigMap)).Should(Succeed()) + + mcpGroup = &mcpv1alpha1.MCPGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: groupName, + Namespace: namespace, + }, + } + Expect(k8sClient.Create(ctx, mcpGroup)).Should(Succeed()) + + oidcConfig = &mcpv1alpha1.MCPOIDCConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: configName, + Namespace: namespace, + }, + Spec: mcpv1alpha1.MCPOIDCConfigSpec{ + Type: mcpv1alpha1.MCPOIDCConfigTypeInline, + Inline: &mcpv1alpha1.InlineOIDCSharedConfig{ + Issuer: "https://auth.example.internal/realms/demo", + ClientID: "test-client", + CABundleRef: &mcpv1alpha1.CABundleSource{ + ConfigMapRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: caCMName}, + Key: caCMKey, + }, + }, + JWKSAllowPrivateIP: true, + }, + }, + } + Expect(k8sClient.Create(ctx, oidcConfig)).Should(Succeed()) + + Eventually(func() bool { + updated := &mcpv1alpha1.MCPOIDCConfig{} + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: configName, + Namespace: namespace, + }, updated) + if err != nil || updated.Status.ConfigHash == "" { + return false + } + for _, cond := range updated.Status.Conditions { + if cond.Type == mcpv1alpha1.ConditionTypeOIDCConfigValid && + cond.Status == metav1.ConditionTrue { + return true + } + } + return false + }, timeout, interval).Should(BeTrue()) + + vmcpServer = &mcpv1alpha1.VirtualMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: vmcpName, + Namespace: namespace, + }, + Spec: mcpv1alpha1.VirtualMCPServerSpec{ + GroupRef: &mcpv1alpha1.MCPGroupRef{Name: groupName}, + Config: vmcpconfig.Config{Group: groupName}, + IncomingAuth: &mcpv1alpha1.IncomingAuthConfig{ + Type: "oidc", + OIDCConfigRef: &mcpv1alpha1.MCPOIDCConfigReference{ + Name: configName, + Audience: "test-vmcp-audience", + Scopes: []string{"openid"}, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, vmcpServer)).Should(Succeed()) + }) + + AfterAll(func() { + _ = k8sClient.Delete(ctx, vmcpServer) + _ = k8sClient.Delete(ctx, oidcConfig) + _ = k8sClient.Delete(ctx, mcpGroup) + _ = k8sClient.Delete(ctx, caConfigMap) + Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) + }) + + It("should render the mounted CA path into the vmcp ConfigMap's OIDC config", func() { + configMapName := vmcpName + "-vmcp-config" + expectedPath := "/config/certs/" + caCMName + "/" + caCMKey + + Eventually(func(g Gomega) { + cm := &corev1.ConfigMap{} + g.Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: configMapName, + Namespace: namespace, + }, cm)).To(Succeed()) + g.Expect(cm.Data).To(HaveKey("config.yaml")) + + var config vmcpconfig.Config + g.Expect(yaml.Unmarshal([]byte(cm.Data["config.yaml"]), &config)).To(Succeed()) + g.Expect(config.IncomingAuth).NotTo(BeNil()) + g.Expect(config.IncomingAuth.OIDC).NotTo(BeNil()) + g.Expect(config.IncomingAuth.OIDC.CABundlePath).To(Equal(expectedPath), + "rendered vmcp config must contain the mounted CA path so the OIDC middleware can trust the issuer") + }, timeout, interval).Should(Succeed()) + }) + + It("should mount the CA ConfigMap as a read-only volume at /config/certs/", func() { + expectedMountPath := "/config/certs/" + caCMName + + Eventually(func(g Gomega) { + deployment := &appsv1.Deployment{} + g.Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: vmcpName, + Namespace: namespace, + }, deployment)).To(Succeed()) + + // The CA ConfigMap must be projected into a volume that sources from caCMName. + found := false + var volumeName string + for _, v := range deployment.Spec.Template.Spec.Volumes { + if v.ConfigMap != nil && v.ConfigMap.Name == caCMName { + found = true + volumeName = v.Name + break + } + } + g.Expect(found).To(BeTrue(), "Deployment must have a Volume sourcing from ConfigMap %q", caCMName) + + // The same volume must be mounted read-only at /config/certs/. + var mount *corev1.VolumeMount + for i := range deployment.Spec.Template.Spec.Containers[0].VolumeMounts { + m := &deployment.Spec.Template.Spec.Containers[0].VolumeMounts[i] + if m.Name == volumeName { + mount = m + break + } + } + g.Expect(mount).NotTo(BeNil(), "Deployment container must mount the CA volume") + g.Expect(mount.MountPath).To(Equal(expectedMountPath)) + g.Expect(mount.ReadOnly).To(BeTrue(), "CA bundle mount must be read-only") + }, timeout, interval).Should(Succeed()) + }) + }) }) diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index d9d0552db2..56e295bef6 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -1245,6 +1245,16 @@ spec: audience: description: Audience is the required token audience. type: string + caBundlePath: + description: |- + CABundlePath is the absolute file path to a PEM-encoded CA certificate bundle + used when the OIDC middleware performs HTTPS requests to the issuer + (OIDC discovery, JWKS fetch, token introspection). When set, the CA bundle + at this path is added to the trust store used for verifying the issuer's + TLS certificate. Typically populated by the Kubernetes operator from + MCPOIDCConfig.spec.inline.caBundleRef (ConfigMap) or from the in-cluster + service-account CA when using Kubernetes service-account auth. + type: string clientId: description: ClientID is the OAuth client ID. type: string diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index 1c075ae8a5..a9fdcc880c 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -1248,6 +1248,16 @@ spec: audience: description: Audience is the required token audience. type: string + caBundlePath: + description: |- + CABundlePath is the absolute file path to a PEM-encoded CA certificate bundle + used when the OIDC middleware performs HTTPS requests to the issuer + (OIDC discovery, JWKS fetch, token introspection). When set, the CA bundle + at this path is added to the trust store used for verifying the issuer's + TLS certificate. Typically populated by the Kubernetes operator from + MCPOIDCConfig.spec.inline.caBundleRef (ConfigMap) or from the in-cluster + service-account CA when using Kubernetes service-account auth. + type: string clientId: description: ClientID is the OAuth client ID. type: string diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index d514fa4e33..39f35975b6 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -388,6 +388,7 @@ _Appears in:_ | `protectedResourceAllowPrivateIp` _boolean_ | ProtectedResourceAllowPrivateIP allows protected resource endpoint on private IP addresses
Use with caution - only enable for trusted internal IDPs or testing | | | | `jwksAllowPrivateIp` _boolean_ | JwksAllowPrivateIP allows OIDC discovery and JWKS fetches to private IP addresses.
Enable when the embedded auth server runs on a loopback address and
the OIDC middleware needs to fetch its JWKS from that address.
Use with caution - only enable for trusted internal IDPs or testing. | | | | `insecureAllowHttp` _boolean_ | InsecureAllowHTTP allows HTTP (non-HTTPS) OIDC issuers for development/testing
WARNING: This is insecure and should NEVER be used in production | | | +| `caBundlePath` _string_ | CABundlePath is the absolute file path to a PEM-encoded CA certificate bundle
used when the OIDC middleware performs HTTPS requests to the issuer
(OIDC discovery, JWKS fetch, token introspection). When set, the CA bundle
at this path is added to the trust store used for verifying the issuer's
TLS certificate. Typically populated by the Kubernetes operator from
MCPOIDCConfig.spec.inline.caBundleRef (ConfigMap) or from the in-cluster
service-account CA when using Kubernetes service-account auth. | | Optional: \{\}
| #### vmcp.config.OperationalConfig diff --git a/pkg/vmcp/auth/factory/incoming.go b/pkg/vmcp/auth/factory/incoming.go index c27d842573..feae0534a9 100644 --- a/pkg/vmcp/auth/factory/incoming.go +++ b/pkg/vmcp/auth/factory/incoming.go @@ -176,6 +176,7 @@ func newOIDCAuthMiddleware( AllowPrivateIP: oidcCfg.ProtectedResourceAllowPrivateIP || oidcCfg.JwksAllowPrivateIP, InsecureAllowHTTP: oidcCfg.InsecureAllowHTTP, Scopes: oidcCfg.Scopes, + CACertPath: oidcCfg.CABundlePath, } // Wire optional dependencies from the embedded auth server so the JWT diff --git a/pkg/vmcp/auth/factory/incoming_cabundle_test.go b/pkg/vmcp/auth/factory/incoming_cabundle_test.go new file mode 100644 index 0000000000..c4beb694e7 --- /dev/null +++ b/pkg/vmcp/auth/factory/incoming_cabundle_test.go @@ -0,0 +1,124 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package factory + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/stacklok/toolhive/pkg/vmcp/config" +) + +// TestNewOIDCAuthMiddleware_CABundlePath verifies that the CABundlePath field on +// OIDCConfig is wired into the underlying auth.TokenValidatorConfig. A bad path +// is expected to surface as an error at middleware construction time, and a +// valid PEM bundle is expected to succeed — proving the field is actually +// consumed (not silently dropped) by the downstream HTTP client builder. +func TestNewOIDCAuthMiddleware_CABundlePath(t *testing.T) { + t.Parallel() + + // Stand up a minimal OIDC discovery server so the validator has an issuer + // to talk to during construction. The CA bundle affects the TLS trust + // store but not issuer reachability here. + server, _ := newTestOIDCServer(t) + t.Cleanup(server.Close) + + validCAPath := writeTestCAPEM(t) + + tests := []struct { + name string + caBundlePath string + wantErr bool + errContains string + }{ + { + name: "empty caBundlePath succeeds (backward compatible)", + caBundlePath: "", + wantErr: false, + }, + { + name: "valid PEM caBundlePath is loaded successfully", + caBundlePath: validCAPath, + wantErr: false, + }, + { + name: "non-existent caBundlePath surfaces as an error", + caBundlePath: "/nonexistent/ca/bundle.pem", + wantErr: true, + errContains: "CA certificate", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + oidcCfg := &config.OIDCConfig{ + Issuer: server.URL, + ClientID: "test-client", + Audience: "test-audience", + InsecureAllowHTTP: true, + JwksAllowPrivateIP: true, + CABundlePath: tt.caBundlePath, + } + + authMw, _, err := newOIDCAuthMiddleware(t.Context(), oidcCfg, nil, nil) + + if tt.wantErr { + require.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + return + } + + require.NoError(t, err) + require.NotNil(t, authMw) + }) + } +} + +// writeTestCAPEM generates a minimal self-signed certificate and writes it as +// PEM into t.TempDir(). Returns the absolute path to the PEM file. The cert +// content does not need to match any issuer — the test only exercises that +// the bundle loads successfully into the HTTP client's trust store. +func writeTestCAPEM(t *testing.T) string { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "toolhive-test-ca"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageCertSign, + BasicConstraintsValid: true, + IsCA: true, + } + + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + require.NoError(t, err) + + pemBytes := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) + require.NotEmpty(t, pemBytes) + + dir := t.TempDir() + path := filepath.Join(dir, "ca.pem") + require.NoError(t, os.WriteFile(path, pemBytes, 0o600)) + return path +} diff --git a/pkg/vmcp/config/config.go b/pkg/vmcp/config/config.go index 7a2c699290..45e69e066c 100644 --- a/pkg/vmcp/config/config.go +++ b/pkg/vmcp/config/config.go @@ -249,6 +249,16 @@ type OIDCConfig struct { // InsecureAllowHTTP allows HTTP (non-HTTPS) OIDC issuers for development/testing // WARNING: This is insecure and should NEVER be used in production InsecureAllowHTTP bool `json:"insecureAllowHttp,omitempty" yaml:"insecureAllowHttp,omitempty"` + + // CABundlePath is the absolute file path to a PEM-encoded CA certificate bundle + // used when the OIDC middleware performs HTTPS requests to the issuer + // (OIDC discovery, JWKS fetch, token introspection). When set, the CA bundle + // at this path is added to the trust store used for verifying the issuer's + // TLS certificate. Typically populated by the Kubernetes operator from + // MCPOIDCConfig.spec.inline.caBundleRef (ConfigMap) or from the in-cluster + // service-account CA when using Kubernetes service-account auth. + // +optional + CABundlePath string `json:"caBundlePath,omitempty" yaml:"caBundlePath,omitempty"` } // AuthzConfig configures authorization. diff --git a/pkg/vmcp/config/validator.go b/pkg/vmcp/config/validator.go index f710a814a7..69aac14b20 100644 --- a/pkg/vmcp/config/validator.go +++ b/pkg/vmcp/config/validator.go @@ -165,6 +165,16 @@ func (v *DefaultValidator) validateIncomingAuth(auth *IncomingAuthConfig) error // - PKCE flows (public clients) // - Token validation without introspection // - Kubernetes service account token validation + + // Validate CA bundle path: reject null bytes, path traversal, and relative paths. + if auth.OIDC.CABundlePath != "" { + if strings.ContainsRune(auth.OIDC.CABundlePath, 0) || strings.Contains(auth.OIDC.CABundlePath, "..") { + return fmt.Errorf("incomingAuth.oidc.caBundlePath contains invalid path characters") + } + if !filepath.IsAbs(auth.OIDC.CABundlePath) { + return fmt.Errorf("incomingAuth.oidc.caBundlePath must be an absolute path") + } + } } // Validate authorization configuration diff --git a/pkg/vmcp/config/validator_test.go b/pkg/vmcp/config/validator_test.go index 1fabf51be2..c350ec5db5 100644 --- a/pkg/vmcp/config/validator_test.go +++ b/pkg/vmcp/config/validator_test.go @@ -188,6 +188,57 @@ func TestValidator_ValidateIncomingAuth(t *testing.T) { wantErr: true, errMsg: "issuer is required", }, + { + name: "valid OIDC auth with absolute caBundlePath", + auth: &IncomingAuthConfig{ + Type: "oidc", + OIDC: &OIDCConfig{ + Issuer: "https://example.com", + Audience: "vmcp", + CABundlePath: "/config/certs/example-ca/ca.crt", + }, + }, + wantErr: false, + }, + { + name: "OIDC rejects relative caBundlePath", + auth: &IncomingAuthConfig{ + Type: "oidc", + OIDC: &OIDCConfig{ + Issuer: "https://example.com", + Audience: "vmcp", + CABundlePath: "certs/ca.crt", + }, + }, + wantErr: true, + errMsg: "caBundlePath must be an absolute path", + }, + { + name: "OIDC rejects caBundlePath with path traversal", + auth: &IncomingAuthConfig{ + Type: "oidc", + OIDC: &OIDCConfig{ + Issuer: "https://example.com", + Audience: "vmcp", + CABundlePath: "/config/certs/../../etc/passwd", + }, + }, + wantErr: true, + errMsg: "caBundlePath contains invalid path characters", + }, + { + name: "OIDC rejects caBundlePath with null byte", + auth: &IncomingAuthConfig{ + Type: "oidc", + OIDC: &OIDCConfig{ + Issuer: "https://example.com", + Audience: "vmcp", + CABundlePath: "/config/certs/ca\x00.crt", + }, + }, + wantErr: true, + errMsg: "caBundlePath contains invalid path characters", + }, } for _, tt := range tests {