fix(webhook): validate object-supplied vault-addr and gate skip-verify

Signed-off-by: Bence Csati <bence.csati@axoflow.com>

Author: csatib02

pkg/webhook/config.go

@@ -30,6 +30,7 @@ import (
 // VaultConfig represents vault options
 type VaultConfig struct {
 	Addr                          string
+	AddrFromObject                bool
 	AuthMethod                    string
 	Role                          string
 	Path                          string
@@ -106,6 +107,7 @@ func parseVaultConfig(obj metav1.Object, ar *model.AdmissionReview) VaultConfig
 
 	if val, ok := annotations[common.VaultAddrAnnotation]; ok {
 		vaultConfig.Addr = val
+		vaultConfig.AddrFromObject = true
 	} else {
 		vaultConfig.Addr = viper.GetString("vault_addr")
 	}
@@ -145,7 +147,7 @@ func parseVaultConfig(obj metav1.Object, ar *model.AdmissionReview) VaultConfig
 	}
 
 	if val, ok := annotations[common.VaultSkipVerifyAnnotation]; ok {
-		vaultConfig.SkipVerify, _ = strconv.ParseBool(val)
+		vaultConfig.SkipVerify = common.ResolveObjectSkipVerify(val, "vault_allow_object_skip_verify", "vault_skip_verify")
 	} else {
 		vaultConfig.SkipVerify = viper.GetBool("vault_skip_verify")
 	}
@@ -473,6 +475,9 @@ func SetConfigDefaults() {
 	viper.SetDefault("vault_ct_pull_policy", string(corev1.PullIfNotPresent))
 	viper.SetDefault("vault_addr", "https://vault:8200")
 	viper.SetDefault("vault_skip_verify", "false")
+	viper.SetDefault("vault_addr_allowlist", "")
+	viper.SetDefault("vault_allow_object_skip_verify", "false")
+	viper.SetDefault("vault_allow_private_addr", "false")
 	viper.SetDefault("vault_path", "kubernetes")
 	viper.SetDefault("vault_auth_method", "jwt")
 	viper.SetDefault("vault_role", "")

pkg/webhook/webhook.go

@@ -32,6 +32,7 @@ import (
 	"github.com/slok/kubewebhook/v2/pkg/log"
 	"github.com/slok/kubewebhook/v2/pkg/model"
 	"github.com/slok/kubewebhook/v2/pkg/webhook/mutating"
+	"github.com/spf13/viper"
 	authenticationv1 "k8s.io/api/authentication/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -193,6 +194,18 @@ func (mw *MutatingWebhook) lookForValueFrom(ctx context.Context, env corev1.EnvV
 	return nil, nil
 }
 
+func vaultAddrPolicy() common.AddrPolicy {
+	allowlist := common.SplitAndTrim(viper.GetString("vault_addr_allowlist"))
+	if addr := viper.GetString("vault_addr"); addr != "" {
+		allowlist = append(allowlist, addr) // trusted operator default is implicitly allowed
+	}
+
+	return common.AddrPolicy{
+		Allowlist:    allowlist,
+		AllowPrivate: viper.GetBool("vault_allow_private_addr"),
+	}
+}
+
 func (mw *MutatingWebhook) newVaultClient(ctx context.Context, vaultConfig VaultConfig) (*vault.Client, error) {
 	vaultAuthAttemptsCount.WithLabelValues().Inc()
 	clientConfig := vaultapi.DefaultConfig()
@@ -201,6 +214,14 @@ func (mw *MutatingWebhook) newVaultClient(ctx context.Context, vaultConfig Vault
 		return nil, clientConfig.Error
 	}
 
+	// Validate before any connection or ServiceAccount token mint.
+	if vaultConfig.AddrFromObject {
+		if err := common.ValidateObjectAddr(vaultConfig.Addr, vaultAddrPolicy()); err != nil {
+			vaultAuthAttemptsErrorsCount.WithLabelValues("config_error").Inc()
+			return nil, errors.Wrap(err, "rejected Vault address from object annotation")
+		}
+	}
+
 	clientConfig.Address = vaultConfig.Addr
 
 	tlsConfig := vaultapi.TLSConfig{Insecure: vaultConfig.SkipVerify}

pkg/webhook/webhook_test.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/testutil"
+	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
@@ -134,3 +135,77 @@ func TestNewVaultClientMetrics(t *testing.T) {
 		})
 	}
 }
+
+func TestNewVaultClientRejectsObjectAddr(t *testing.T) {
+	logger := slog.New(slog.DiscardHandler)
+	require.NoError(t, os.Setenv("KUBERNETES_NAMESPACE", "test-namespace"))
+
+	tests := []struct {
+		name      string
+		addr      string
+		allowlist string
+		wantErr   bool
+	}{
+		{
+			name:    "PoC IMDS address from object annotation is rejected",
+			addr:    "http://169.254.169.254/latest/meta-data/",
+			wantErr: true,
+		},
+		{
+			name:    "non-allowlisted external address from object annotation is rejected",
+			addr:    "https://evil.attacker.com",
+			wantErr: true,
+		},
+		{
+			name:      "userinfo-bearing address from object annotation is rejected",
+			addr:      "https://attacker:pw@vault.prod.svc:8200",
+			allowlist: "https://vault.prod.svc:8200",
+			wantErr:   true,
+		},
+		{
+			name:      "allowlisted address from object annotation passes validation",
+			addr:      "https://vault.prod.svc:8200",
+			allowlist: "https://vault.prod.svc:8200",
+			wantErr:   false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			vaultAuthAttemptsCount.Reset()
+			vaultAuthAttemptsErrorsCount.Reset()
+			viper.Set("vault_addr_allowlist", tt.allowlist)
+			t.Cleanup(viper.Reset)
+
+			mw, err := NewMutatingWebhook(logger, fake.NewClientset())
+			require.NoError(t, err)
+
+			vaultConfig := VaultConfig{
+				Addr:                tt.addr,
+				AddrFromObject:      true,
+				SkipVerify:          true,
+				Role:                "test-role",
+				Path:                "kubernetes",
+				VaultServiceAccount: "high-priv-sa",
+				ObjectNamespace:     "test-namespace",
+			}
+
+			_, err = mw.newVaultClient(t.Context(), vaultConfig)
+
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), "rejected Vault address from object annotation")
+				assert.Equal(t, float64(1), testutil.ToFloat64(vaultAuthAttemptsErrorsCount.WithLabelValues("config_error")),
+					"address rejection must record a config_error")
+				assert.Equal(t, float64(0), testutil.ToFloat64(vaultAuthAttemptsErrorsCount.WithLabelValues("kubernetes_error")),
+					"rejection must happen before the ServiceAccount token path")
+			} else {
+				if err != nil {
+					assert.NotContains(t, err.Error(), "rejected Vault address from object annotation")
+				}
+				assert.Equal(t, float64(0), testutil.ToFloat64(vaultAuthAttemptsErrorsCount.WithLabelValues("config_error")),
+					"a valid allowlisted address must not record a config_error")
+			}
+		})
+	}
+}