VDS: add support for drift detection for static-creds

api/v1alpha1/vaultdynamicsecret_types.go

@@ -74,6 +74,15 @@ type VaultDynamicSecretStatus struct {
 	// LastRuntimePodUID used for tracking the transition from one Pod to the next.
 	// It is used to mitigate the effects of a Vault lease renewal storm.
 	LastRuntimePodUID types.UID `json:"lastRuntimePodUID,omitempty"`
+	// SecretMAC used when deciding whether new Vault secret data should be synced.
+	//
+	// The controller will compare the "new" Vault secret data to this value using HMAC,
+	// if they are different, then the data will be synced to the Destination.
+	//
+	// The SecretMac is also used to detect drift in the Destination Secret's Data.
+	// If drift is detected the data will be synced to the Destination.
+	// SecretMAC will only be stored when VaultDynamicSecretSpec.AllowStaticCreds is true.
+	SecretMAC string `json:"secretMAC,omitempty"`
 }
 
 type VaultSecretLease struct {

chart/crds/secrets.hashicorp.com_vaultdynamicsecrets.yaml

@@ -198,6 +198,16 @@ spec:
                 - renewable
                 - requestID
                 type: object
+              secretMAC:
+                description: "SecretMAC used when deciding whether new Vault secret
+                  data should be synced. \n The controller will compare the \"new\"
+                  Vault secret data to this value using HMAC, if they are different,
+                  then the data will be synced to the Destination. \n The SecretMac
+                  is also used to detect drift in the Destination Secret's Data. If
+                  drift is detected the data will be synced to the Destination. SecretMAC
+                  will only be stored when VaultDynamicSecretSpec.AllowStaticCreds
+                  is true."
+                type: string
               staticCredsMetaData:
                 description: StaticCredsMetaData contains the static creds response
                   meta-data

config/crd/bases/secrets.hashicorp.com_vaultdynamicsecrets.yaml

@@ -198,6 +198,16 @@ spec:
                 - renewable
                 - requestID
                 type: object
+              secretMAC:
+                description: "SecretMAC used when deciding whether new Vault secret
+                  data should be synced. \n The controller will compare the \"new\"
+                  Vault secret data to this value using HMAC, if they are different,
+                  then the data will be synced to the Destination. \n The SecretMac
+                  is also used to detect drift in the Destination Secret's Data. If
+                  drift is detected the data will be synced to the Destination. SecretMAC
+                  will only be stored when VaultDynamicSecretSpec.AllowStaticCreds
+                  is true."
+                type: string
               staticCredsMetaData:
                 description: StaticCredsMetaData contains the static creds response
                   meta-data

controllers/vaultdynamicsecret_controller.go

@@ -5,6 +5,7 @@ package controllers
 
 import (
 	"context"
+	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -41,6 +42,7 @@ type VaultDynamicSecretReconciler struct {
 	Scheme        *runtime.Scheme
 	Recorder      record.EventRecorder
 	ClientFactory vault.ClientFactory
+	HMACValidator vault.HMACValidator
 	// runtimePodUID should always be set when updating resource's Status.
 	// This is done via the downwardAPI. We get the current Pod's UID from either the
 	// OPERATOR_POD_UID environment variable, or the /var/run/podinfo/uid file; in that order.
@@ -182,11 +184,13 @@ func (r *VaultDynamicSecretReconciler) Reconcile(ctx context.Context, req ctrl.R
 		return ctrl.Result{}, err
 	}
 
-	secretLease, err := r.syncSecret(ctx, vClient, o)
+	secretLease, updated, err := r.syncSecret(ctx, vClient, o)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
 
+	doRolloutRestart = updated
+
 	o.Status.SecretLease = *secretLease
 	o.Status.LastRenewalTime = time.Now().Unix()
 	o.Status.LastGeneration = o.GetGeneration()
@@ -261,9 +265,7 @@ func (r *VaultDynamicSecretReconciler) isStaticCreds(meta *secretsv1alpha1.Vault
 	return meta.LastVaultRotation > 0 && meta.RotationPeriod > 1
 }
 
-func (r *VaultDynamicSecretReconciler) syncSecret(
-	ctx context.Context, c vault.ClientBase, o *secretsv1alpha1.VaultDynamicSecret,
-) (*secretsv1alpha1.VaultSecretLease, error) {
+func (r *VaultDynamicSecretReconciler) syncSecret(ctx context.Context, c vault.ClientBase, o *secretsv1alpha1.VaultDynamicSecret) (*secretsv1alpha1.VaultSecretLease, bool, error) {
 	path := vault.JoinPath(o.Spec.Mount, o.Spec.Path)
 	var err error
 	var resp *api.Secret
@@ -295,24 +297,20 @@ func (r *VaultDynamicSecretReconciler) syncSecret(
 	case http.MethodGet:
 		resp, err = c.Read(ctx, path)
 	default:
-		return nil, fmt.Errorf("unsupported HTTP method %q for sync", method)
+		return nil, false, fmt.Errorf("unsupported HTTP method %q for sync", method)
 	}
 
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 
 	if resp == nil {
-		return nil, fmt.Errorf("nil response from vault for path %s", path)
+		return nil, false, fmt.Errorf("nil response from vault for path %s", path)
 	}
 
 	data, err := vault.MarshalSecretData(resp)
 	if err != nil {
-		return nil, err
-	}
-
-	if err := helpers.SyncSecret(ctx, r.Client, o, data); err != nil {
-		return nil, err
+		return nil, false, err
 	}
 
 	secretLease := r.getVaultSecretLease(resp)
@@ -341,9 +339,25 @@ func (r *VaultDynamicSecretReconciler) syncSecret(
 				}
 			}
 		}
+
+		if r.isStaticCreds(&o.Status.StaticCredsMetaData) {
+			macsEqual, messageMAC, err := helpers.HandleSecretHMAC(ctx, r.Client, r.HMACValidator, o, data)
+			if err != nil {
+				return nil, false, err
+			}
+
+			o.Status.SecretMAC = base64.StdEncoding.EncodeToString(messageMAC)
+			if macsEqual {
+				return secretLease, false, nil
+			}
+		}
+	}
+
+	if err := helpers.SyncSecret(ctx, r.Client, o, data); err != nil {
+		return nil, false, err
 	}
 
-	return secretLease, nil
+	return secretLease, true, nil
 }
 
 func (r *VaultDynamicSecretReconciler) updateStatus(ctx context.Context, o *secretsv1alpha1.VaultDynamicSecret) error {

controllers/vaultdynamicsecret_controller_test.go

@@ -499,7 +499,7 @@ func TestVaultDynamicSecretReconciler_syncSecret(t *testing.T) {
 			r := &VaultDynamicSecretReconciler{
 				Client: tt.fields.Client,
 			}
-			got, err := r.syncSecret(tt.args.ctx, tt.args.vClient, tt.args.o)
+			got, _, err := r.syncSecret(tt.args.ctx, tt.args.vClient, tt.args.o)
 			if !tt.wantErr(t, err, fmt.Sprintf("syncSecret(%v, %v, %v)", tt.args.ctx, tt.args.vClient, tt.args.o)) {
 				return
 			}

controllers/vaultstaticsecret_controller.go

@@ -29,11 +29,10 @@ import (
 // VaultStaticSecretReconciler reconciles a VaultStaticSecret object
 type VaultStaticSecretReconciler struct {
 	client.Client
-	Scheme          *runtime.Scheme
-	Recorder        record.EventRecorder
-	ClientFactory   vault.ClientFactory
-	HMACFunc        vault.HMACFromSecretFunc
-	ValidateMACFunc vault.ValidateMACFromSecretFunc
+	Scheme        *runtime.Scheme
+	Recorder      record.EventRecorder
+	ClientFactory vault.ClientFactory
+	HMACValidator vault.HMACValidator
 }
 
 //+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=vaultstaticsecrets,verbs=get;list;watch;create;update;patch;delete
@@ -142,7 +141,7 @@ func (r *VaultStaticSecretReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		// doRolloutRestart only if this is not the first time this secret has been synced
 		doRolloutRestart = o.Status.SecretMAC != ""
 
-		macsEqual, messageMAC, err := r.handleSecretHMAC(ctx, o, data)
+		macsEqual, messageMAC, err := helpers.HandleSecretHMAC(ctx, r.Client, r.HMACValidator, o, data)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
@@ -183,66 +182,6 @@ func (r *VaultStaticSecretReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	}, nil
 }
 
-// handleSecretHMAC compares the HMAC of data to its previously computed value stored in o.Status.SecretHMAC,
-// returning true if they are equal. The computed new-MAC will be returned so that o.Status.SecretHMAC can be updated.
-func (r *VaultStaticSecretReconciler) handleSecretHMAC(ctx context.Context, o *secretsv1alpha1.VaultStaticSecret, data map[string][]byte) (bool, []byte, error) {
-	logger := log.FromContext(ctx)
-
-	// HMAC the Vault secret data so that it can be compared to the what's in the destination Secret.
-	message, err := json.Marshal(data)
-	if err != nil {
-		return false, nil, err
-	}
-
-	newMAC, err := r.HMACFunc(ctx, r.Client, message)
-	if err != nil {
-		return false, nil, err
-	}
-
-	// we have never computed the Vault secret data HMAC,
-	// so there is no need to perform Secret data drift detection.
-	if o.Status.SecretMAC == "" {
-		return false, newMAC, nil
-	}
-
-	lastMAC, err := base64.StdEncoding.DecodeString(o.Status.SecretMAC)
-	if err != nil {
-		return false, nil, err
-	}
-
-	macsEqual := vault.EqualMACS(lastMAC, newMAC)
-	if macsEqual {
-		// check to see if the Secret.Data has drifted since the last sync,
-		// if it has then it will be overwritten with the Vault secret data
-		// this would indicate an out-of-band change made to the Secret's data
-		// in this case the controller should do the sync.
-		if cur, ok, _ := helpers.GetSecret(ctx, r.Client, o); ok {
-			curMessage, err := json.Marshal(cur.Data)
-			if err != nil {
-				return false, nil, err
-			}
-
-			logger.V(consts.LogLevelDebug).Info("Doing Secret data drift detection", "lastMAC", lastMAC)
-			// we only care of the MAC has changed, it's new value is not important here.
-			valid, foundMAC, err := r.ValidateMACFunc(ctx, r.Client, curMessage, lastMAC)
-			if err != nil {
-				return false, nil, err
-			}
-			if !valid {
-				logger.V(consts.LogLevelDebug).Info("Secret data drift detected",
-					"lastMAC", lastMAC, "foundMAC", foundMAC, "curMessage", curMessage, "message", message)
-			}
-
-			macsEqual = valid
-		} else {
-			// assume MACs are not equal if the secret does not exist or an error (ignored) has occurred
-			macsEqual = false
-		}
-	}
-
-	return macsEqual, newMAC, nil
-}
-
 // SetupWithManager sets up the controller with the Manager.
 func makeK8sSecret(vaultSecret *api.KVSecret) (map[string][]byte, error) {
 	if vaultSecret.Raw == nil {

internal/helpers/hmac.go

@@ -0,0 +1,96 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package helpers
+
+import (
+	"context"
+	"encoding/base64"
+	"encoding/json"
+	"fmt"
+
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+
+	"github.com/hashicorp/vault-secrets-operator/api/v1alpha1"
+	"github.com/hashicorp/vault-secrets-operator/internal/consts"
+	"github.com/hashicorp/vault-secrets-operator/internal/vault"
+)
+
+// HandleSecretHMAC compares the HMAC of data to its previously computed value
+// stored in o.Status.SecretHMAC, returning true if they are equal. The computed
+// new-MAC will be returned so that o.Status.SecretHMAC can be updated.
+//
+// Supported types for obj are: VaultDynamicSecret, VaultStaticSecret
+func HandleSecretHMAC(ctx context.Context, client ctrlclient.Client,
+	validator vault.HMACValidator, obj ctrlclient.Object, data map[string][]byte,
+) (bool, []byte, error) {
+	var cur string
+	switch t := obj.(type) {
+	case *v1alpha1.VaultDynamicSecret:
+		cur = t.Status.SecretMAC
+	case *v1alpha1.VaultStaticSecret:
+		cur = t.Status.SecretMAC
+	default:
+		return false, nil, fmt.Errorf("unsupported object type %T", t)
+	}
+	logger := log.FromContext(ctx)
+
+	// HMAC the Vault secret data so that it can be compared to the what's in the
+	// destination Secret.
+	message, err := json.Marshal(data)
+	if err != nil {
+		return false, nil, err
+	}
+
+	newMAC, err := validator.HMAC(ctx, client, message)
+	if err != nil {
+		return false, nil, err
+	}
+
+	// we have never computed the Vault secret data HMAC,
+	// so there is no need to perform Secret data drift detection.
+	if cur == "" {
+		return false, newMAC, nil
+	}
+
+	lastMAC, err := base64.StdEncoding.DecodeString(cur)
+	if err != nil {
+		return false, nil, err
+	}
+
+	macsEqual := vault.EqualMACS(lastMAC, newMAC)
+	if macsEqual {
+		// check to see if the Secret.Data has drifted since the last sync, if it has
+		// then it will be overwritten with the Vault secret data this would indicate an
+		// out-of-band change made to the Secret's data in this case the controller
+		// should do the sync.
+		if cur, ok, _ := GetSecret(ctx, client, obj); ok {
+			curMessage, err := json.Marshal(cur.Data)
+			if err != nil {
+				return false, nil, err
+			}
+
+			logger.V(consts.LogLevelDebug).Info(
+				"Doing Secret data drift detection", "lastMAC", lastMAC)
+			// we only care of the MAC has changed, it's new value is not important here.
+			valid, foundMAC, err := validator.Validate(ctx, client, curMessage, lastMAC)
+			if err != nil {
+				return false, nil, err
+			}
+			if !valid {
+				logger.V(consts.LogLevelDebug).Info("Secret data drift detected",
+					"lastMAC", lastMAC, "foundMAC", foundMAC,
+					"curMessage", curMessage, "message", message)
+			}
+
+			macsEqual = valid
+		} else {
+			// assume MACs are not equal if the secret does not exist or an error (ignored)
+			// has occurred
+			macsEqual = false
+		}
+	}
+
+	return macsEqual, newMAC, nil
+}