Skip to content

Commit e783bf5

Browse files
yroblataskbot
yrobla
and
taskbot
authored
Automate HMAC secret management (#4008)
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 Co-authored-by: taskbot <taskbot@users.noreply.github.com>
1 parent 5fa6252 commit e783bf5

6 files changed

Lines changed: 1128 additions & 2 deletions

File tree

‎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,

‎cmd/thv-operator/controllers/virtualmcpserver_hmac_secret_test.go‎

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)