From 604a8dc062aabd3f8e872470a6026566af24f3a3 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 --- .../integration_tests/integration_test.go | 39 +++++++ .../templates/simple-with-gmsa-hostname.yml | 30 ++++++ .../templates/simple-without-gmsa.yml | 26 +++++ admission-webhook/main.go | 20 +++- admission-webhook/utils_test.go | 26 ++++- admission-webhook/webhook.go | 56 ++++++++-- admission-webhook/webhook_test.go | 100 ++++++++++++++++-- charts/gmsa/templates/deployment.yaml | 2 + charts/gmsa/values.yaml | 1 + 9 files changed, 280 insertions(+), 20 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/admission-webhook/integration_tests/integration_test.go b/admission-webhook/integration_tests/integration_test.go index 28c476a5..25271a7a 100644 --- a/admission-webhook/integration_tests/integration_test.go +++ b/admission-webhook/integration_tests/integration_test.go @@ -463,6 +463,45 @@ func TestPossibleToUpdatePodWithNewCert(t *testing.T) { assert.Equal(t, expectedCredSpec0, extractContainerCredSpecContents(t, pod3, testName3)) } +func TestHappyPathWithHostnameRandomization(t *testing.T) { + testName := "happy-path-with-hostname-randomization" + credSpecTemplates := []string{"credspec-0"} + templates := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-with-gmsa"} + + testConfig, tearDownFunc := integrationTestSetup(t, testName, credSpecTemplates, templates) + defer tearDownFunc() + + pod := waitForPodToComeUp(t, testConfig.Namespace, "app="+testName) + + assert.Equal(t, 15, len(pod.Spec.Hostname)) +} + +func TestHostnameSetNoHostnameRandomization(t *testing.T) { + testName := "hostnameset-no-hostname-randomization" + credSpecTemplates := []string{"credspec-0"} + templates := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-with-gmsa-hostname"} + + testConfig, tearDownFunc := integrationTestSetup(t, testName, credSpecTemplates, templates) + defer tearDownFunc() + + pod := waitForPodToComeUp(t, testConfig.Namespace, "app="+testName) + + assert.Equal(t, testName, pod.Spec.Hostname) +} + +func TestNoGMSANoHostnameRandomization(t *testing.T) { + testName := "nogmsa-hostname-randomization" + credSpecTemplates := []string{"credspec-0"} + templates := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-without-gmsa"} + + testConfig, tearDownFunc := integrationTestSetup(t, testName, credSpecTemplates, templates) + defer tearDownFunc() + + pod := waitForPodToComeUp(t, testConfig.Namespace, "app="+testName) + + 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..e23b12bf 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,21 @@ func buildPod(serviceAccountName string, podWindowsOptions *corev1.WindowsSecuri } shuffleContainers(containers) - podSpec := corev1.PodSpec{ - ServiceAccountName: serviceAccountName, - Containers: containers, + + var podSpec corev1.PodSpec + if hostname != nil { + podSpec = corev1.PodSpec{ + ServiceAccountName: serviceAccountName, + Containers: containers, + Hostname: *hostname, + } + } else { + podSpec = corev1.PodSpec{ + ServiceAccountName: serviceAccountName, + Containers: containers, + } } + if podWindowsOptions != nil { podSpec.SecurityContext = &corev1.PodSecurityContext{WindowsOptions: podWindowsOptions} } diff --git a/admission-webhook/webhook.go b/admission-webhook/webhook.go index 7c2244bd..7357ca5f 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 { @@ -392,15 +403,34 @@ func (webhook *webhook) mutateCreateRequest(ctx context.Context, pod *corev1.Pod admissionResponse := &admissionV1.AdmissionResponse{Allowed: true} - if len(patches) != 0 { - patchesBytes, err := json.Marshal(patches) - if err != nil { - return nil, &podAdmissionError{error: fmt.Errorf("unable to marshall patch JSON %v: %v", patches, err), pod: pod, code: http.StatusInternalServerError} + if hasGMSA { + // pods are GMSA related, we will need further check if we need to randomize the hostname + hostName := pod.Spec.Hostname + // Patch the hostname only if it is empty + if webhook.config.EnableRandomHostName { + 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.Infof("hostname is set in spec and will be hornored instead of being randomized") + } } - admissionResponse.Patch = patchesBytes - patchType := admissionV1.PatchTypeJSONPatch - admissionResponse.PatchType = &patchType + if len(patches) != 0 { + patchesBytes, err := json.Marshal(patches) + if err != nil { + return nil, &podAdmissionError{error: fmt.Errorf("unable to marshall patch JSON %v: %v", patches, err), pod: pod, code: http.StatusInternalServerError} + } + + admissionResponse.Patch = patchesBytes + patchType := admissionV1.PatchTypeJSONPatch + admissionResponse.PatchType = &patchType + } } return admissionResponse, nil @@ -537,3 +567,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..96eba44d 100644 --- a/charts/gmsa/values.yaml +++ b/charts/gmsa/values.yaml @@ -51,3 +51,4 @@ securityContext: {} tolerations: [] qps: 30.0 burst: 50 +randomHostname: true