Skip to content

Commit 4d7ac7e

Browse files
committed
Automate HMAC secret management
Automatically generate and manage HMAC secrets when SessionManagementV2 is enabled in VirtualMCPServer resources, eliminating manual secret creation. Also add e2e and integration tests to validate the functionality Related-to: #3867
1 parent 0791876 commit 4d7ac7e

File tree

6 files changed

+1120
-2
lines changed

6 files changed

+1120
-2
lines changed

cmd/thv-operator/controllers/virtualmcpserver_controller.go

Lines changed: 180 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ package controllers
77

88
import (
99
"context"
10+
"crypto/rand"
11+
"encoding/base64"
1012
"fmt"
1113
"maps"
1214
"reflect"
@@ -23,6 +25,7 @@ import (
2325
"k8s.io/client-go/tools/record"
2426
ctrl "sigs.k8s.io/controller-runtime"
2527
"sigs.k8s.io/controller-runtime/pkg/client"
28+
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
2629
"sigs.k8s.io/controller-runtime/pkg/handler"
2730
"sigs.k8s.io/controller-runtime/pkg/log"
2831
"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -100,7 +103,7 @@ type VirtualMCPServerReconciler struct {
100103
// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=roles,verbs=create;delete;get;list;patch;update;watch
101104
// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=rolebindings,verbs=create;delete;get;list;patch;update;watch
102105
// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
103-
// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch
106+
// +kubebuilder:rbac:groups="",resources=secrets,verbs=create;get;list;watch
104107
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;delete;get;list;patch;update;watch
105108
// +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;delete;get;list;patch;update;watch
106109
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=embeddingservers,verbs=get;list;watch
@@ -592,6 +595,12 @@ func (r *VirtualMCPServerReconciler) ensureAllResources(
592595
return ctrl.Result{}, err
593596
}
594597

598+
// Ensure HMAC secret for session token binding (Session Management V2)
599+
if err := r.ensureHMACSecret(ctx, vmcp); err != nil {
600+
ctxLogger.Error(err, "Failed to ensure HMAC secret")
601+
return ctrl.Result{}, err
602+
}
603+
595604
// Ensure vmcp Config ConfigMap
596605
if err := r.ensureVmcpConfigConfigMap(ctx, vmcp, workloadNames, statusManager); err != nil {
597606
ctxLogger.Error(err, "Failed to ensure vmcp Config ConfigMap")
@@ -660,6 +669,161 @@ func (r *VirtualMCPServerReconciler) ensureRBACResources(
660669
return err
661670
}
662671

672+
// ensureHMACSecret ensures the HMAC secret exists for session token binding.
673+
// This secret is required when Session Management V2 is enabled.
674+
// The secret is automatically generated with a cryptographically secure random value.
675+
//
676+
// The secret follows this naming pattern: {vmcp-name}-hmac-secret
677+
// and contains a single key: hmac-secret with a 32-byte base64-encoded random value.
678+
func (r *VirtualMCPServerReconciler) ensureHMACSecret(
679+
ctx context.Context,
680+
vmcp *mcpv1alpha1.VirtualMCPServer,
681+
) error {
682+
ctxLogger := log.FromContext(ctx)
683+
684+
// Only ensure HMAC secret if Session Management V2 is enabled
685+
if vmcp.Spec.Config.Operational == nil || !vmcp.Spec.Config.Operational.SessionManagementV2 {
686+
return nil
687+
}
688+
689+
secretName := fmt.Sprintf("%s-hmac-secret", vmcp.Name)
690+
secret := &corev1.Secret{}
691+
err := r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: vmcp.Namespace}, secret)
692+
693+
if errors.IsNotFound(err) {
694+
// Generate a cryptographically secure 32-byte HMAC secret
695+
hmacSecret, err := generateHMACSecret()
696+
if err != nil {
697+
ctxLogger.Error(err, "Failed to generate HMAC secret")
698+
if r.Recorder != nil {
699+
r.Recorder.Eventf(vmcp, corev1.EventTypeWarning, "HMACSecretGenerationFailed",
700+
"Failed to generate HMAC secret: %v", err)
701+
}
702+
return fmt.Errorf("failed to generate HMAC secret: %w", err)
703+
}
704+
705+
newSecret := &corev1.Secret{
706+
ObjectMeta: metav1.ObjectMeta{
707+
Name: secretName,
708+
Namespace: vmcp.Namespace,
709+
Labels: map[string]string{
710+
"app.kubernetes.io/name": "virtualmcpserver",
711+
"app.kubernetes.io/instance": vmcp.Name,
712+
"app.kubernetes.io/component": "session-security",
713+
"app.kubernetes.io/managed-by": "toolhive-operator",
714+
},
715+
Annotations: map[string]string{
716+
"toolhive.stacklok.dev/purpose": "hmac-secret-for-session-token-binding",
717+
},
718+
},
719+
Type: corev1.SecretTypeOpaque,
720+
Data: map[string][]byte{
721+
"hmac-secret": []byte(hmacSecret),
722+
},
723+
}
724+
725+
// Set VirtualMCPServer as owner so secret is automatically deleted when VMCP is deleted
726+
if err := controllerutil.SetControllerReference(vmcp, newSecret, r.Scheme); err != nil {
727+
ctxLogger.Error(err, "Failed to set controller reference for HMAC secret")
728+
return fmt.Errorf("failed to set controller reference: %w", err)
729+
}
730+
731+
ctxLogger.Info("Creating HMAC secret for session token binding", "Secret.Name", secretName)
732+
if err := r.Create(ctx, newSecret); err != nil {
733+
ctxLogger.Error(err, "Failed to create HMAC secret")
734+
if r.Recorder != nil {
735+
r.Recorder.Eventf(vmcp, corev1.EventTypeWarning, "HMACSecretCreationFailed",
736+
"Failed to create HMAC secret: %v", err)
737+
}
738+
return fmt.Errorf("failed to create HMAC secret: %w", err)
739+
}
740+
741+
// Record success event
742+
if r.Recorder != nil {
743+
r.Recorder.Event(vmcp, corev1.EventTypeNormal, "HMACSecretCreated",
744+
"HMAC secret created for session token binding")
745+
}
746+
return nil
747+
} else if err != nil {
748+
ctxLogger.Error(err, "Failed to get HMAC secret")
749+
return fmt.Errorf("failed to get HMAC secret: %w", err)
750+
}
751+
752+
// Secret exists - validate ownership and structure before accepting it
753+
if err := r.validateHMACSecret(ctx, vmcp, secret); err != nil {
754+
ctxLogger.Error(err, "Existing HMAC secret is invalid", "Secret.Name", secretName)
755+
if r.Recorder != nil {
756+
r.Recorder.Eventf(vmcp, corev1.EventTypeWarning, "HMACSecretValidationFailed",
757+
"Existing HMAC secret validation failed: %v", err)
758+
}
759+
return fmt.Errorf("existing HMAC secret validation failed: %w", err)
760+
}
761+
762+
return nil
763+
}
764+
765+
// validateHMACSecret validates that an existing HMAC secret has the correct ownership,
766+
// structure, and content. This prevents accepting stale, malformed, or attacker-controlled
767+
// secrets that could weaken session token signing or cause pod startup failures.
768+
func (*VirtualMCPServerReconciler) validateHMACSecret(
769+
ctx context.Context,
770+
vmcp *mcpv1alpha1.VirtualMCPServer,
771+
secret *corev1.Secret,
772+
) error {
773+
ctxLogger := log.FromContext(ctx)
774+
775+
// Verify the secret is owned by this VirtualMCPServer
776+
// This prevents accepting secrets created by other actors
777+
isOwned := false
778+
for _, ownerRef := range secret.OwnerReferences {
779+
if ownerRef.UID == vmcp.UID &&
780+
ownerRef.Kind == "VirtualMCPServer" &&
781+
ownerRef.Name == vmcp.Name {
782+
isOwned = true
783+
break
784+
}
785+
}
786+
if !isOwned {
787+
return fmt.Errorf("secret is not owned by VirtualMCPServer %s/%s", vmcp.Namespace, vmcp.Name)
788+
}
789+
790+
// Verify the hmac-secret key exists
791+
hmacSecretData, exists := secret.Data["hmac-secret"]
792+
if !exists {
793+
return fmt.Errorf("secret missing required 'hmac-secret' key")
794+
}
795+
796+
// Verify it's valid base64 and decodes to exactly 32 bytes
797+
hmacSecretBase64 := string(hmacSecretData)
798+
if hmacSecretBase64 == "" {
799+
return fmt.Errorf("hmac-secret is empty")
800+
}
801+
802+
decoded, err := base64.StdEncoding.DecodeString(hmacSecretBase64)
803+
if err != nil {
804+
return fmt.Errorf("hmac-secret is not valid base64: %w", err)
805+
}
806+
807+
if len(decoded) != 32 {
808+
return fmt.Errorf("hmac-secret must be exactly 32 bytes, got %d bytes", len(decoded))
809+
}
810+
811+
// Verify it's not all zeros (would indicate a weak/predictable key)
812+
allZeros := true
813+
for _, b := range decoded {
814+
if b != 0 {
815+
allZeros = false
816+
break
817+
}
818+
}
819+
if allZeros {
820+
return fmt.Errorf("hmac-secret is all zeros (weak key)")
821+
}
822+
823+
ctxLogger.V(1).Info("HMAC secret validation passed", "Secret.Name", secret.Name)
824+
return nil
825+
}
826+
663827
// getVmcpConfigChecksum fetches the vmcp Config ConfigMap checksum annotation.
664828
// This is used to trigger deployment rollouts when the configuration changes.
665829
//
@@ -2368,3 +2532,18 @@ func setAuthConfigConditions(
23682532
// auth config errors are non-fatal. The system can continue operating with
23692533
// the auth configs that are valid.
23702534
}
2535+
2536+
// generateHMACSecret generates a cryptographically secure 32-byte HMAC secret
2537+
// encoded as base64. This secret is used for session token binding in Session Management V2.
2538+
//
2539+
// Returns a base64-encoded string suitable for use as VMCP_SESSION_HMAC_SECRET.
2540+
func generateHMACSecret() (string, error) {
2541+
// Generate 32 bytes of cryptographically secure random data
2542+
secret := make([]byte, 32)
2543+
if _, err := rand.Read(secret); err != nil {
2544+
return "", fmt.Errorf("failed to generate random bytes: %w", err)
2545+
}
2546+
2547+
// Encode as base64 for safe storage and environment variable use
2548+
return base64.StdEncoding.EncodeToString(secret), nil
2549+
}

cmd/thv-operator/controllers/virtualmcpserver_deployment.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,11 @@ func (r *VirtualMCPServerReconciler) buildEnvVarsForVmcp(
277277
// Mount outgoing auth secrets
278278
env = append(env, r.buildOutgoingAuthEnvVars(ctx, vmcp, typedWorkloads)...)
279279

280+
// Mount HMAC secret for session token binding (Session Management V2)
281+
if vmcp.Spec.Config.Operational != nil && vmcp.Spec.Config.Operational.SessionManagementV2 {
282+
env = append(env, r.buildHMACSecretEnvVar(vmcp))
283+
}
284+
280285
// Note: Other secrets (Redis passwords, service account credentials) may be added here in the future
281286
// following the same pattern of mounting from Kubernetes Secrets as environment variables.
282287

@@ -325,6 +330,25 @@ func (*VirtualMCPServerReconciler) buildOIDCEnvVars(vmcp *mcpv1alpha1.VirtualMCP
325330
return env
326331
}
327332

333+
// buildHMACSecretEnvVar builds environment variable for HMAC secret mounting.
334+
// This secret is used for session token binding in Session Management V2.
335+
// The operator automatically generates and manages this secret if it doesn't exist.
336+
func (*VirtualMCPServerReconciler) buildHMACSecretEnvVar(vmcp *mcpv1alpha1.VirtualMCPServer) corev1.EnvVar {
337+
secretName := fmt.Sprintf("%s-hmac-secret", vmcp.Name)
338+
339+
return corev1.EnvVar{
340+
Name: "VMCP_SESSION_HMAC_SECRET",
341+
ValueFrom: &corev1.EnvVarSource{
342+
SecretKeyRef: &corev1.SecretKeySelector{
343+
LocalObjectReference: corev1.LocalObjectReference{
344+
Name: secretName,
345+
},
346+
Key: "hmac-secret",
347+
},
348+
},
349+
}
350+
}
351+
328352
// buildOutgoingAuthEnvVars builds environment variables for outgoing auth secrets.
329353
func (r *VirtualMCPServerReconciler) buildOutgoingAuthEnvVars(
330354
ctx context.Context,
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package controllers
5+
6+
import (
7+
"encoding/base64"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
// TestGenerateHMACSecret tests the HMAC secret generation function.
15+
func TestGenerateHMACSecret(t *testing.T) {
16+
t.Parallel()
17+
18+
t.Run("generates valid base64 encoded secret", func(t *testing.T) {
19+
t.Parallel()
20+
21+
secret, err := generateHMACSecret()
22+
require.NoError(t, err)
23+
require.NotEmpty(t, secret)
24+
25+
// Verify it's valid base64
26+
decoded, err := base64.StdEncoding.DecodeString(secret)
27+
require.NoError(t, err)
28+
assert.Len(t, decoded, 32, "decoded secret should be exactly 32 bytes")
29+
})
30+
31+
t.Run("generates unique secrets", func(t *testing.T) {
32+
t.Parallel()
33+
34+
secret1, err := generateHMACSecret()
35+
require.NoError(t, err)
36+
37+
secret2, err := generateHMACSecret()
38+
require.NoError(t, err)
39+
40+
// Two generated secrets should be different
41+
assert.NotEqual(t, secret1, secret2, "consecutive secrets should be unique")
42+
})
43+
44+
t.Run("generates cryptographically secure random data", func(t *testing.T) {
45+
t.Parallel()
46+
47+
secret, err := generateHMACSecret()
48+
require.NoError(t, err)
49+
50+
decoded, err := base64.StdEncoding.DecodeString(secret)
51+
require.NoError(t, err)
52+
53+
// Check that it's not all zeros (would indicate failure of crypto/rand)
54+
allZeros := make([]byte, 32)
55+
assert.NotEqual(t, allZeros, decoded, "secret should not be all zeros")
56+
})
57+
58+
t.Run("generates multiple valid secrets", func(t *testing.T) {
59+
t.Parallel()
60+
61+
// Generate 100 secrets to ensure consistency
62+
secrets := make(map[string]bool)
63+
for i := 0; i < 100; i++ {
64+
secret, err := generateHMACSecret()
65+
require.NoError(t, err)
66+
67+
// Verify base64 decoding
68+
decoded, err := base64.StdEncoding.DecodeString(secret)
69+
require.NoError(t, err)
70+
assert.Len(t, decoded, 32)
71+
72+
// Track uniqueness
73+
secrets[secret] = true
74+
}
75+
76+
// All secrets should be unique
77+
assert.Len(t, secrets, 100, "all generated secrets should be unique")
78+
})
79+
}

cmd/vmcp/README.md

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,35 @@ spec:
237237
key: hmac-secret
238238
```
239239
240-
**Note**: Kubernetes deployments **require** `VMCP_SESSION_HMAC_SECRET` to be set (the server will fail to start without it). For non-Kubernetes environments (local development/testing), a default insecure secret is used as a fallback, but this is **NOT recommended for production**.
240+
**Note**: When **Session Management V2 is enabled**, Kubernetes deployments **require** `VMCP_SESSION_HMAC_SECRET` to be set (the server will fail to start without it). For non-Kubernetes environments (local development/testing), a default insecure secret is used as a fallback, but this is **NOT recommended for production**. If Session Management V2 is disabled, this environment variable is not required.
241+
242+
### Automatic Secret Management (ToolHive Operator)
243+
244+
When deploying vMCP via the **ToolHive operator** with Session Management V2 enabled, the HMAC secret is **automatically generated and managed** for you:
245+
246+
```yaml
247+
apiVersion: toolhive.stacklok.dev/v1alpha1
248+
kind: VirtualMCPServer
249+
metadata:
250+
name: my-vmcp
251+
spec:
252+
config:
253+
operational:
254+
sessionManagementV2: true # Enables automatic HMAC secret creation
255+
group: my-group
256+
```
257+
258+
The operator will:
259+
260+
- ✅ Automatically generate a cryptographically secure 32-byte HMAC secret
261+
- ✅ Store it in a Kubernetes Secret named `{vmcp-name}-hmac-secret`
262+
- ✅ Inject it into the vMCP deployment as `VMCP_SESSION_HMAC_SECRET`
263+
- ✅ Validate existing secrets (ownership, structure, and content)
264+
- ✅ Automatically delete the secret when the VirtualMCPServer is removed
265+
266+
**No manual secret generation or management required!** The operator handles all of this automatically when you enable Session Management V2.
267+
268+
> **Note**: The secret is generated once at creation time and persists for the lifetime of the VirtualMCPServer. Secret rotation is not currently supported but may be added in a future release.
241269

242270
## Tool Aggregation & Conflict Resolution
243271

0 commit comments

Comments
 (0)