Skip to content

Commit 32e911d

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 d420adf commit 32e911d

File tree

6 files changed

+1107
-2
lines changed

6 files changed

+1107
-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
"encoding/json"
1113
"fmt"
1214
"maps"
@@ -24,6 +26,7 @@ import (
2426
"k8s.io/client-go/tools/record"
2527
ctrl "sigs.k8s.io/controller-runtime"
2628
"sigs.k8s.io/controller-runtime/pkg/client"
29+
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
2730
"sigs.k8s.io/controller-runtime/pkg/handler"
2831
"sigs.k8s.io/controller-runtime/pkg/log"
2932
"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -101,7 +104,7 @@ type VirtualMCPServerReconciler struct {
101104
// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=roles,verbs=create;delete;get;list;patch;update;watch
102105
// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=rolebindings,verbs=create;delete;get;list;patch;update;watch
103106
// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
104-
// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch
107+
// +kubebuilder:rbac:groups="",resources=secrets,verbs=create;get;list;watch
105108
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;delete;get;list;patch;update;watch
106109
// +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;delete;get;list;patch;update;watch
107110
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=embeddingservers,verbs=get;list;watch
@@ -593,6 +596,12 @@ func (r *VirtualMCPServerReconciler) ensureAllResources(
593596
return ctrl.Result{}, err
594597
}
595598

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

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

cmd/thv-operator/controllers/virtualmcpserver_deployment.go

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

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

@@ -321,6 +326,25 @@ func (*VirtualMCPServerReconciler) buildOIDCEnvVars(vmcp *mcpv1alpha1.VirtualMCP
321326
return env
322327
}
323328

329+
// buildHMACSecretEnvVar builds environment variable for HMAC secret mounting.
330+
// This secret is used for session token binding in Session Management V2.
331+
// The operator automatically generates and manages this secret if it doesn't exist.
332+
func (*VirtualMCPServerReconciler) buildHMACSecretEnvVar(vmcp *mcpv1alpha1.VirtualMCPServer) corev1.EnvVar {
333+
secretName := fmt.Sprintf("%s-hmac-secret", vmcp.Name)
334+
335+
return corev1.EnvVar{
336+
Name: "VMCP_SESSION_HMAC_SECRET",
337+
ValueFrom: &corev1.EnvVarSource{
338+
SecretKeyRef: &corev1.SecretKeySelector{
339+
LocalObjectReference: corev1.LocalObjectReference{
340+
Name: secretName,
341+
},
342+
Key: "hmac-secret",
343+
},
344+
},
345+
}
346+
}
347+
324348
// buildOutgoingAuthEnvVars builds environment variables for outgoing auth secrets.
325349
func (r *VirtualMCPServerReconciler) buildOutgoingAuthEnvVars(
326350
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)