From b25e70f3ad50409c8219d66ca784e943b3c1657a Mon Sep 17 00:00:00 2001 From: zylxjtu Date: Thu, 3 Oct 2024 21:59:44 +0000 Subject: [PATCH] Create random hostname for GMSA This will only apply to gmsa pods which have the corresponding security context Disabling/enabling of this can be controlled through ENV --- .github/workflows/build.yaml | 4 +- .../integration_tests/integration_test.go | 64 +++++++++++ .../templates/simple-with-gmsa-hostname.yml | 30 ++++++ .../templates/simple-without-gmsa.yml | 26 +++++ admission-webhook/main.go | 20 +++- admission-webhook/utils_test.go | 15 ++- admission-webhook/webhook.go | 40 ++++++- admission-webhook/webhook_test.go | 100 ++++++++++++++++-- charts/gmsa/templates/deployment.yaml | 2 + charts/gmsa/values.yaml | 1 + 10 files changed, 289 insertions(+), 13 deletions(-) create mode 100644 admission-webhook/integration_tests/templates/simple-with-gmsa-hostname.yml create mode 100644 admission-webhook/integration_tests/templates/simple-without-gmsa.yml diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 64bae13d..cf4a5c47 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -113,7 +113,7 @@ jobs: env: T: integration DEPLOY_METHOD: chart - integration-rotation-enabled: + integration-optional-features: runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v4 @@ -126,5 +126,5 @@ jobs: env: T: integration DEPLOY_METHOD: chart - HELM_INSTALL_FLAGS_FLAGS: --set certificates.certReload.enabled=true + HELM_INSTALL_FLAGS_FLAGS: --set certificates.certReload.enabled=true, --set randomHostname=true diff --git a/admission-webhook/integration_tests/integration_test.go b/admission-webhook/integration_tests/integration_test.go index 28c476a5..79611c60 100644 --- a/admission-webhook/integration_tests/integration_test.go +++ b/admission-webhook/integration_tests/integration_test.go @@ -463,6 +463,70 @@ func TestPossibleToUpdatePodWithNewCert(t *testing.T) { assert.Equal(t, expectedCredSpec0, extractContainerCredSpecContents(t, pod3, testName3)) } +func TestPossibleHostnameRandomization(t *testing.T) { + deployMethod := os.Getenv("DEPLOY_METHOD") + if deployMethod != "chart" { + t.Skip("Non chart deployment method not supported for this test") + } + + webHookNs := os.Getenv("NAMESPACE") + webHookDeploymentName := os.Getenv("DEPLOYMENT_NAME") + webhook, err := kubeClient(t).AppsV1().Deployments(webHookNs).Get(context.Background(), webHookDeploymentName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + randomHostnameEnabled := false + for _, envVar := range webhook.Spec.Template.Spec.Containers[0].Env { + if strings.EqualFold(envVar.Name, "RANDOM_HOSTNAME") && strings.EqualFold(envVar.Value, "true") { + randomHostnameEnabled = true + } + } + + if randomHostnameEnabled { + testName1 := "happy-path-with-hostname-randomization" + credSpecTemplates1 := []string{"credspec-0"} + templates1 := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-with-gmsa"} + + testConfig1, tearDownFunc1 := integrationTestSetup(t, testName1, credSpecTemplates1, templates1) + defer tearDownFunc1() + + pod := waitForPodToComeUp(t, testConfig1.Namespace, "app="+testName1) + assert.NotEqual(t, testName1, pod.Spec.Hostname) + assert.Equal(t, 15, len(pod.Spec.Hostname)) + + testName2 := "hostnameset-no-hostname-randomization" + credSpecTemplates2 := []string{"credspec-0"} + templates2 := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-with-gmsa-hostname"} + + testConfig2, tearDownFunc2 := integrationTestSetup(t, testName2, credSpecTemplates2, templates2) + defer tearDownFunc2() + + pod = waitForPodToComeUp(t, testConfig2.Namespace, "app="+testName2) + assert.Equal(t, testName2, pod.Spec.Hostname) + + testName3 := "nogmsa-hostname-randomization" + credSpecTemplates3 := []string{"credspec-0"} + templates3 := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-without-gmsa"} + + testConfig3, tearDownFunc3 := integrationTestSetup(t, testName3, credSpecTemplates3, templates3) + defer tearDownFunc3() + pod = waitForPodToComeUp(t, testConfig3.Namespace, "app="+testName3) + + assert.Equal(t, "", pod.Spec.Hostname) + } else { + testName4 := "notenabled-hostname-randomization" + credSpecTemplates4 := []string{"credspec-0"} + templates4 := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-with-gmsa"} + + testConfig4, tearDownFunc4 := integrationTestSetup(t, testName4, credSpecTemplates4, templates4) + defer tearDownFunc4() + pod := waitForPodToComeUp(t, testConfig4.Namespace, "app="+testName4) + + assert.Equal(t, "", pod.Spec.Hostname) + } +} + /* Helpers */ type testConfig struct { diff --git a/admission-webhook/integration_tests/templates/simple-with-gmsa-hostname.yml b/admission-webhook/integration_tests/templates/simple-with-gmsa-hostname.yml new file mode 100644 index 00000000..e5e415b3 --- /dev/null +++ b/admission-webhook/integration_tests/templates/simple-with-gmsa-hostname.yml @@ -0,0 +1,30 @@ +## a simple deployment with a pod-level GMSA cred spec + +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: {{ .TestName }} + name: {{ .TestName }} + namespace: {{ .Namespace }} +spec: + replicas: 1 + selector: + matchLabels: + app: {{ .TestName }} + template: + metadata: + labels: + app: {{ .TestName }} + spec: + hostname: {{ .TestName }} + serviceAccountName: {{ .ServiceAccountName }} + securityContext: + windowsOptions: + gmsaCredentialSpecName: {{ index .CredSpecNames 0 }} + containers: + - image: registry.k8s.io/pause + name: nginx +{{- range $line := .ExtraSpecLines }} + {{ $line }} +{{- end }} diff --git a/admission-webhook/integration_tests/templates/simple-without-gmsa.yml b/admission-webhook/integration_tests/templates/simple-without-gmsa.yml new file mode 100644 index 00000000..adcf1b0e --- /dev/null +++ b/admission-webhook/integration_tests/templates/simple-without-gmsa.yml @@ -0,0 +1,26 @@ +## a simple deployment with a pod-level GMSA cred spec + +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: {{ .TestName }} + name: {{ .TestName }} + namespace: {{ .Namespace }} +spec: + replicas: 1 + selector: + matchLabels: + app: {{ .TestName }} + template: + metadata: + labels: + app: {{ .TestName }} + spec: + serviceAccountName: {{ .ServiceAccountName }} + containers: + - image: registry.k8s.io/pause + name: nginx +{{- range $line := .ExtraSpecLines }} + {{ $line }} +{{- end }} diff --git a/admission-webhook/main.go b/admission-webhook/main.go index 882138d4..7d718826 100644 --- a/admission-webhook/main.go +++ b/admission-webhook/main.go @@ -22,7 +22,12 @@ func main() { panic(err) } - webhook := newWebhookWithOptions(kubeClient, WithCertReload(*enableCertReload)) + options := []WebhookOption{WithCertReload(*enableCertReload)} + + randomHostname := env_bool("RANDOM_HOSTNAME") + options = append(options, WithRandomHostname(randomHostname)) + + webhook := newWebhookWithOptions(kubeClient, options...) tlsConfig := &tlsConfig{ crtPath: env("TLS_CRT"), @@ -98,6 +103,19 @@ func env_float(key string, defaultFloat float32) float32 { return defaultFloat } +func env_bool(key string) bool { + if v, found := os.LookupEnv(key); found { + // Convert string to bool + if boolValue, err := strconv.ParseBool(v); err == nil { + return boolValue + } + logrus.Warningf("unable to parse environment variable %s with value %s to bool, use default value false", key, v) + } + + // return bool default value: false + return false +} + func env_int(key string, defaultInt int) int { if v, found := os.LookupEnv(key); found { if i, err := strconv.Atoi(v); err == nil { diff --git a/admission-webhook/utils_test.go b/admission-webhook/utils_test.go index 80c6cd74..4ba14d73 100644 --- a/admission-webhook/utils_test.go +++ b/admission-webhook/utils_test.go @@ -22,7 +22,6 @@ type dummyKubeClient struct { retrieveCredSpecContentsFunc func(ctx context.Context, credSpecName string) (contents string, httpCode int, err error) } - func (dkc *dummyKubeClient) isAuthorizedToUseCredSpec(ctx context.Context, serviceAccountName, namespace, credSpecName string) (authorized bool, reason string) { if dkc.isAuthorizedToUseCredSpecFunc != nil { return dkc.isAuthorizedToUseCredSpecFunc(ctx, serviceAccountName, namespace, credSpecName) @@ -59,6 +58,14 @@ func setWindowsOptions(winOptions *corev1.WindowsSecurityContextOptions, credSpe // case a `*corev1.WindowsSecurityContextOptions` is built using that string as the name of the cred spec to use. // Same goes for the values of `containerNamesAndWindowsOptions`. func buildPod(serviceAccountName string, podWindowsOptions *corev1.WindowsSecurityContextOptions, containerNamesAndWindowsOptions map[string]*corev1.WindowsSecurityContextOptions) *corev1.Pod { + return buildPodWithHostName(serviceAccountName, nil, podWindowsOptions, containerNamesAndWindowsOptions) +} + +// buildPod builds a pod for unit tests. +// `podWindowsOptions` should be either a full `*corev1.WindowsSecurityContextOptions` or a string, in which +// case a `*corev1.WindowsSecurityContextOptions` is built using that string as the name of the cred spec to use. +// Same goes for the values of `containerNamesAndWindowsOptions`. +func buildPodWithHostName(serviceAccountName string, hostname *string, podWindowsOptions *corev1.WindowsSecurityContextOptions, containerNamesAndWindowsOptions map[string]*corev1.WindowsSecurityContextOptions) *corev1.Pod { containers := make([]corev1.Container, len(containerNamesAndWindowsOptions)) i := 0 for name, winOptions := range containerNamesAndWindowsOptions { @@ -70,10 +77,16 @@ func buildPod(serviceAccountName string, podWindowsOptions *corev1.WindowsSecuri } shuffleContainers(containers) + podSpec := corev1.PodSpec{ ServiceAccountName: serviceAccountName, Containers: containers, } + + if hostname != nil { + podSpec.Hostname = *hostname + } + if podWindowsOptions != nil { podSpec.SecurityContext = &corev1.PodSecurityContext{WindowsOptions: podWindowsOptions} } diff --git a/admission-webhook/webhook.go b/admission-webhook/webhook.go index 7c2244bd..1b727358 100644 --- a/admission-webhook/webhook.go +++ b/admission-webhook/webhook.go @@ -13,6 +13,8 @@ import ( "strings" "time" + "github.com/google/uuid" + "github.com/sirupsen/logrus" admissionV1 "k8s.io/api/admission/v1" corev1 "k8s.io/api/core/v1" @@ -48,7 +50,8 @@ type podAdmissionError struct { } type WebhookConfig struct { - EnableCertReload bool + EnableCertReload bool + EnableRandomHostName bool } type WebhookOption func(*WebhookConfig) @@ -59,12 +62,18 @@ func WithCertReload(enabled bool) WebhookOption { } } +func WithRandomHostname(enabled bool) WebhookOption { + return func(cfg *WebhookConfig) { + cfg.EnableRandomHostName = enabled + } +} + func newWebhook(client kubeClientInterface) *webhook { return newWebhookWithOptions(client) } func newWebhookWithOptions(client kubeClientInterface, options ...WebhookOption) *webhook { - config := &WebhookConfig{EnableCertReload: false} + config := &WebhookConfig{EnableCertReload: false, EnableRandomHostName: false} for _, option := range options { option(config) @@ -358,9 +367,11 @@ func compareCredSpecContents(fromResource, fromCRD string) (bool, error) { // mutateCreateRequest inlines the requested GMSA's into the pod's and containers' `WindowsSecurityOptions` structs. func (webhook *webhook) mutateCreateRequest(ctx context.Context, pod *corev1.Pod) (*admissionV1.AdmissionResponse, *podAdmissionError) { var patches []map[string]string + hasGMSA := false if err := iterateOverWindowsSecurityOptions(pod, func(windowsOptions *corev1.WindowsSecurityContextOptions, resourceKind gmsaResourceKind, resourceName string, containerIndex int) *podAdmissionError { if credSpecName := windowsOptions.GMSACredentialSpecName; credSpecName != nil { + hasGMSA = true // if the user has pre-set the GMSA's contents, we won't override it - it'll be down // to the validation endpoint to make sure the contents actually are what they should if credSpecContents := windowsOptions.GMSACredentialSpec; credSpecContents == nil { @@ -390,8 +401,23 @@ func (webhook *webhook) mutateCreateRequest(ctx context.Context, pod *corev1.Pod return nil, err } - admissionResponse := &admissionV1.AdmissionResponse{Allowed: true} + if hasGMSA && webhook.config.EnableRandomHostName { + // Pods are GMSA related, Env enabled, patch the hostname only if it is empty + hostName := pod.Spec.Hostname + if hostName == "" { + hostName = generateUUID() + patches = append(patches, map[string]string{ + "op": "add", + "path": "/spec/hostname", + "value": hostName, + }) + } else { + // Will honor the hostname set in the spec, print out a message + logrus.Warnf("hostname is set in spec and will be hornored instead of being randomized") + } + } + admissionResponse := &admissionV1.AdmissionResponse{Allowed: true} if len(patches) != 0 { patchesBytes, err := json.Marshal(patches) if err != nil { @@ -537,3 +563,11 @@ func (ln tcpKeepAliveListener) Accept() (net.Conn, error) { tc.SetKeepAlivePeriod(3 * time.Minute) return tc, nil } + +func generateUUID() string { + // Generate a new UUID + id := uuid.New() + // Convert to string and get the first 15 characters in lower case + shortUUID := strings.ToLower(id.String()[:15]) + return shortUUID +} diff --git a/admission-webhook/webhook_test.go b/admission-webhook/webhook_test.go index 5c6c354a..d1f947ab 100644 --- a/admission-webhook/webhook_test.go +++ b/admission-webhook/webhook_test.go @@ -186,7 +186,7 @@ func TestMutateCreateRequest(t *testing.T) { }, } { t.Run(testCaseName, func(t *testing.T) { - webhook := newWebhook(nil) + webhook := newWebhookWithOptions(nil, WithRandomHostname(false)) pod := buildPod(dummyServiceAccoutName, winOptionsFactory(), map[string]*corev1.WindowsSecurityContextOptions{dummyContainerName: winOptionsFactory()}) response, err := webhook.mutateCreateRequest(context.Background(), pod) @@ -194,9 +194,86 @@ func TestMutateCreateRequest(t *testing.T) { require.NotNil(t, response) assert.True(t, response.Allowed) + + assert.Nil(t, response.Patch) + }) } + for testCaseName, winOptionsFactory := range map[string]func() *corev1.WindowsSecurityContextOptions{ + "with random hostname env set and empty GMSA settings, it passes and does nothing": func() *corev1.WindowsSecurityContextOptions { + return &corev1.WindowsSecurityContextOptions{} + }, + "with random hostname env set and no GMSA settings, it passes and does nothing": func() *corev1.WindowsSecurityContextOptions { + return nil + }, + } { + t.Run(testCaseName, func(t *testing.T) { + webhook := newWebhookWithOptions(nil, WithRandomHostname(true)) + pod := buildPod(dummyServiceAccoutName, winOptionsFactory(), map[string]*corev1.WindowsSecurityContextOptions{dummyContainerName: winOptionsFactory()}) + + response, err := webhook.mutateCreateRequest(context.Background(), pod) + assert.Nil(t, err) + + require.NotNil(t, response) + assert.True(t, response.Allowed) + + assert.Nil(t, response.Patch) + + }) + } + + testCaseName, winOptionsFactory1 := "with random hostname env set and dummy GMSA settings, it passes and set random hostname", func() *corev1.WindowsSecurityContextOptions { + dummyCredSpecNameVar := dummyCredSpecName + dummyCredSpecContentsVar := dummyCredSpecContents + return &corev1.WindowsSecurityContextOptions{GMSACredentialSpecName: &dummyCredSpecNameVar, GMSACredentialSpec: &dummyCredSpecContentsVar} + } + t.Run(testCaseName, func(t *testing.T) { + webhook := newWebhookWithOptions(nil, WithRandomHostname(true)) + pod := buildPod(dummyServiceAccoutName, winOptionsFactory1(), map[string]*corev1.WindowsSecurityContextOptions{dummyContainerName: winOptionsFactory1()}) + + response, err := webhook.mutateCreateRequest(context.Background(), pod) + assert.Nil(t, err) + + require.NotNil(t, response) + assert.True(t, response.Allowed) + + var patches []map[string]string + // one more because we're adding the hostname + if err := json.Unmarshal(response.Patch, &patches); assert.Nil(t, err) && assert.Equal(t, 1, len(patches)) { + foundHostname := false + for _, patch := range patches { + if value, hasValue := patch["value"]; assert.True(t, hasValue) { + if patch["path"] == "/spec/hostname" { + foundHostname = true + assert.Equal(t, "add", patch["op"]) + assert.Equal(t, 15, len(value)) + } + } + } + assert.True(t, foundHostname) + } + }) + + testCaseName, winOptionsFactory1 = "with random hostname env set and dummy GMSA settings and hostname set in spec, it passes and do nothing", func() *corev1.WindowsSecurityContextOptions { + dummyCredSpecNameVar := dummyCredSpecName + dummyCredSpecContentsVar := dummyCredSpecContents + return &corev1.WindowsSecurityContextOptions{GMSACredentialSpecName: &dummyCredSpecNameVar, GMSACredentialSpec: &dummyCredSpecContentsVar} + } + t.Run(testCaseName, func(t *testing.T) { + webhook := newWebhookWithOptions(nil, WithRandomHostname(true)) + dummyPodNameVar := dummyPodName + pod := buildPodWithHostName(dummyServiceAccoutName, &dummyPodNameVar, winOptionsFactory1(), map[string]*corev1.WindowsSecurityContextOptions{dummyContainerName: winOptionsFactory1()}) + + response, err := webhook.mutateCreateRequest(context.Background(), pod) + assert.Nil(t, err) + + require.NotNil(t, response) + assert.True(t, response.Allowed) + + assert.Nil(t, response.Patch) + }) + kubeClientFactory := func() *dummyKubeClient { return &dummyKubeClient{ retrieveCredSpecContentsFunc: func(ctx context.Context, credSpecName string) (contents string, httpCode int, err error) { @@ -215,8 +292,8 @@ func TestMutateCreateRequest(t *testing.T) { } runWebhookValidateOrMutateTests(t, winOptionsFactory, map[string]webhookValidateOrMutateTest{ - "with a GMSA cred spec name, it passes and inlines the cred-spec's contents": func(t *testing.T, pod *corev1.Pod, optionsSelector winOptionsSelector, resourceKind gmsaResourceKind, resourceName string) { - webhook := newWebhook(kubeClientFactory()) + "with random hostname env and a GMSA cred spec name, it passes and inlines the cred-spec's contents and generate random hostname": func(t *testing.T, pod *corev1.Pod, optionsSelector winOptionsSelector, resourceKind gmsaResourceKind, resourceName string) { + webhook := newWebhookWithOptions(kubeClientFactory(), WithRandomHostname(true)) setWindowsOptions(optionsSelector(pod), dummyCredSpecName, "") @@ -269,17 +346,25 @@ func TestMutateCreateRequest(t *testing.T) { } var patches []map[string]string - if err := json.Unmarshal(response.Patch, &patches); assert.Nil(t, err) && assert.Equal(t, len(pod.Spec.Containers), len(patches)) { + // len(pod.Spec.Containers)+1 because we're adding the hostname + if err := json.Unmarshal(response.Patch, &patches); assert.Nil(t, err) && assert.Equal(t, len(pod.Spec.Containers)+1, len(patches)) { + foundHostname := false for _, patch := range patches { if value, hasValue := patch["value"]; assert.True(t, hasValue) { - if expectedPatch, present := expectedPatches[value]; assert.True(t, present, "value %s not found in expected patches", value) { + if patch["path"] == "/spec/hostname" { + foundHostname = true + assert.Equal(t, "add", patch["op"]) + assert.Equal(t, 15, len(value)) + } else if expectedPatch, present := expectedPatches[value]; assert.True(t, present, "value %s not found in expected patches", value) { assert.Equal(t, expectedPatch, patch) } } } + assert.True(t, foundHostname) } }, + // random hostname env not set in the following cases, and validated no hostname is set (implicitly) "it the cred spec's contents are already set, along with its name, it passes and doesn't overwrite the provided contents": func(t *testing.T, pod *corev1.Pod, optionsSelector winOptionsSelector, _ gmsaResourceKind, _ string) { webhook := newWebhook(kubeClientFactory()) @@ -421,8 +506,11 @@ func TestDefaultWebhookConfig(t *testing.T) { func TestSetWebhookConfig(t *testing.T) { expectedCertReload := true - webhook := newWebhookWithOptions(nil, WithCertReload(expectedCertReload)) + expectedRandomHostname := true + randomHostname := true + webhook := newWebhookWithOptions(nil, WithCertReload(expectedCertReload), WithRandomHostname(randomHostname)) assert.Equal(t, expectedCertReload, webhook.config.EnableCertReload) + assert.Equal(t, expectedRandomHostname, webhook.config.EnableRandomHostName) } func TestEqualStringPointers(t *testing.T) { diff --git a/charts/gmsa/templates/deployment.yaml b/charts/gmsa/templates/deployment.yaml index 3eebd509..f2f5d390 100644 --- a/charts/gmsa/templates/deployment.yaml +++ b/charts/gmsa/templates/deployment.yaml @@ -57,6 +57,8 @@ spec: value: "{{ .Values.burst }}" - name: QPS value: "{{ .Values.qps }}" + - name: RANDOM_HOSTNAME + value: "{{ .Values.randomHostname }}" {{- if .Values.securityContext }} securityContext: {{ toYaml .Values.securityContext | nindent 12 }} {{- end }} diff --git a/charts/gmsa/values.yaml b/charts/gmsa/values.yaml index e5bc1be6..5e9e8f50 100644 --- a/charts/gmsa/values.yaml +++ b/charts/gmsa/values.yaml @@ -51,3 +51,4 @@ securityContext: {} tolerations: [] qps: 30.0 burst: 50 +randomHostname: false