From 79593c051d384e1b5bda616df31d06297b4b0e7e Mon Sep 17 00:00:00 2001 From: Jimmi Dyson Date: Thu, 9 May 2024 20:34:06 +0100 Subject: [PATCH 1/3] feat: Enable unprivileged ports sysctl in containerd config This enabled pods to run as non-root and bind to privileged ports as long as they have the necessary capability, `CAP_NET_BIND_SERVICE` added. This fixes an issue on AWS when bringing up coredns which binds to port 53 but runs as an unprivileged user. Overall this is a net security improvement for clusters, meaning users can stop giving too many privileged to pods - see https://github.com/kubernetes/kubernetes/issues/102612 for discussion. --- .../files/unprivileged-ports-config.toml | 3 + .../containerdunprivilegedports/inject.go | 75 +++++++++++++++++ .../inject_test.go | 84 +++++++++++++++++++ .../unprivileged_ports.go | 32 +++++++ pkg/handlers/generic/mutation/handlers.go | 2 + test/e2e/config/caren.yaml | 6 +- 6 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 pkg/handlers/generic/mutation/containerdunprivilegedports/files/unprivileged-ports-config.toml create mode 100644 pkg/handlers/generic/mutation/containerdunprivilegedports/inject.go create mode 100644 pkg/handlers/generic/mutation/containerdunprivilegedports/inject_test.go create mode 100644 pkg/handlers/generic/mutation/containerdunprivilegedports/unprivileged_ports.go diff --git a/pkg/handlers/generic/mutation/containerdunprivilegedports/files/unprivileged-ports-config.toml b/pkg/handlers/generic/mutation/containerdunprivilegedports/files/unprivileged-ports-config.toml new file mode 100644 index 000000000..47a558971 --- /dev/null +++ b/pkg/handlers/generic/mutation/containerdunprivilegedports/files/unprivileged-ports-config.toml @@ -0,0 +1,3 @@ +[plugins."io.containerd.grpc.v1.cri"] + enable_unprivileged_ports = true + enable_unprivileged_icmp = true diff --git a/pkg/handlers/generic/mutation/containerdunprivilegedports/inject.go b/pkg/handlers/generic/mutation/containerdunprivilegedports/inject.go new file mode 100644 index 000000000..73caa27f2 --- /dev/null +++ b/pkg/handlers/generic/mutation/containerdunprivilegedports/inject.go @@ -0,0 +1,75 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +package containerdunprivilegedports + +import ( + "context" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" + ctrl "sigs.k8s.io/controller-runtime" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches/selectors" +) + +type containerdUnprivilegedPortsPatchHandler struct{} + +func NewPatch() *containerdUnprivilegedPortsPatchHandler { + return &containerdUnprivilegedPortsPatchHandler{} +} + +func (h *containerdUnprivilegedPortsPatchHandler) Mutate( + ctx context.Context, + obj *unstructured.Unstructured, + vars map[string]apiextensionsv1.JSON, + holderRef runtimehooksv1.HolderReference, + _ ctrlclient.ObjectKey, + _ mutation.ClusterGetter, +) error { + log := ctrl.LoggerFrom(ctx).WithValues( + "holderRef", holderRef, + ) + + unprivilegedPortsConfigDropIn := generateUnprivilegedPortsConfigDropIn() + + if err := patches.MutateIfApplicable( + obj, vars, &holderRef, selectors.ControlPlane(), log, + func(obj *controlplanev1.KubeadmControlPlaneTemplate) error { + log.WithValues( + "patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(), + "patchedObjectName", ctrlclient.ObjectKeyFromObject(obj), + ).Info("adding containerd unprivileged ports config to control plane kubeadm config spec") + obj.Spec.Template.Spec.KubeadmConfigSpec.Files = append( + obj.Spec.Template.Spec.KubeadmConfigSpec.Files, + unprivilegedPortsConfigDropIn, + ) + + return nil + }); err != nil { + return err + } + + if err := patches.MutateIfApplicable( + obj, vars, &holderRef, selectors.WorkersKubeadmConfigTemplateSelector(), log, + func(obj *bootstrapv1.KubeadmConfigTemplate) error { + log.WithValues( + "patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(), + "patchedObjectName", ctrlclient.ObjectKeyFromObject(obj), + ).Info("adding containerd unprivileged ports config to worker node kubeadm config template") + obj.Spec.Template.Spec.Files = append( + obj.Spec.Template.Spec.Files, + unprivilegedPortsConfigDropIn) + + return nil + }); err != nil { + return err + } + + return nil +} diff --git a/pkg/handlers/generic/mutation/containerdunprivilegedports/inject_test.go b/pkg/handlers/generic/mutation/containerdunprivilegedports/inject_test.go new file mode 100644 index 000000000..d4dc275c6 --- /dev/null +++ b/pkg/handlers/generic/mutation/containerdunprivilegedports/inject_test.go @@ -0,0 +1,84 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package containerdunprivilegedports + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" + + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest/request" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/test/helpers" +) + +func TestContainerdUnprivilegedPortsPatch(t *testing.T) { + gomega.RegisterFailHandler(Fail) + RunSpecs(t, "Containerd unprivileged ports mutator suite") +} + +var _ = Describe("Generate containerd unprivileged ports patches", func() { + // only add aws region patch + patchGenerator := func() mutation.GeneratePatches { + return mutation.NewMetaGeneratePatchesHandler("", helpers.TestEnv.Client, NewPatch()).(mutation.GeneratePatches) + } + + testDefs := []capitest.PatchTestDef{ + { + Name: "containerd unprivileged ports config added to control plane kubeadm config spec", + RequestItem: request.NewKubeadmControlPlaneTemplateRequestItem(""), + ExpectedPatchMatchers: []capitest.JSONPatchMatcher{ + { + Operation: "add", + Path: "/spec/template/spec/kubeadmConfigSpec/files", + ValueMatcher: gomega.ContainElements( + gomega.HaveKeyWithValue( + "path", unprivilegedPortsConfigDropInFileOnRemote, + ), + ), + }, + }, + }, + { + Name: "containerd unprivileged ports config added to worker node kubeadm config template", + Vars: []runtimehooksv1.Variable{ + capitest.VariableWithValue( + "builtin", + map[string]any{ + "machineDeployment": map[string]any{ + "class": "*", + }, + }, + ), + }, + RequestItem: request.NewKubeadmConfigTemplateRequestItem(""), + ExpectedPatchMatchers: []capitest.JSONPatchMatcher{ + { + Operation: "add", + Path: "/spec/template/spec/files", + ValueMatcher: gomega.ContainElements( + gomega.HaveKeyWithValue( + "path", unprivilegedPortsConfigDropInFileOnRemote, + ), + ), + }, + }, + }, + } + + // create test node for each case + for testIdx := range testDefs { + tt := testDefs[testIdx] + It(tt.Name, func() { + capitest.AssertGeneratePatches( + GinkgoT(), + patchGenerator, + &tt, + ) + }) + } +}) diff --git a/pkg/handlers/generic/mutation/containerdunprivilegedports/unprivileged_ports.go b/pkg/handlers/generic/mutation/containerdunprivilegedports/unprivileged_ports.go new file mode 100644 index 000000000..13c4a2b2c --- /dev/null +++ b/pkg/handlers/generic/mutation/containerdunprivilegedports/unprivileged_ports.go @@ -0,0 +1,32 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +package containerdunprivilegedports + +import ( + _ "embed" + "path" + + cabpkv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" +) + +const ( + // TODO Factor out this constant to a common package. + containerdPatchesDirOnRemote = "/etc/containerd/cre.d" +) + +var ( + //go:embed files/unprivileged-ports-config.toml + unprivilegedPortsConfigDropIn []byte + unprivilegedPortsConfigDropInFileOnRemote = path.Join( + containerdPatchesDirOnRemote, + "unprivileged-ports-config.toml", + ) +) + +func generateUnprivilegedPortsConfigDropIn() cabpkv1.File { + return cabpkv1.File{ + Path: unprivilegedPortsConfigDropInFileOnRemote, + Content: string(unprivilegedPortsConfigDropIn), + Permissions: "0600", + } +} diff --git a/pkg/handlers/generic/mutation/handlers.go b/pkg/handlers/generic/mutation/handlers.go index e5eb287b0..e5d8f1965 100644 --- a/pkg/handlers/generic/mutation/handlers.go +++ b/pkg/handlers/generic/mutation/handlers.go @@ -11,6 +11,7 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/auditpolicy" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/containerdapplypatchesandrestart" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/containerdmetrics" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/containerdunprivilegedports" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/etcd" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/extraapiservercertsans" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/httpproxy" @@ -33,6 +34,7 @@ func MetaMutators(mgr manager.Manager) []mutation.MetaMutator { calico.NewPatch(), users.NewPatch(), containerdmetrics.NewPatch(), + containerdunprivilegedports.NewPatch(), // Some patches may have changed containerd configuration. // We write the configuration changes to disk, and must run a command diff --git a/test/e2e/config/caren.yaml b/test/e2e/config/caren.yaml index 6e560ecd2..e9d0ee37b 100644 --- a/test/e2e/config/caren.yaml +++ b/test/e2e/config/caren.yaml @@ -184,15 +184,15 @@ variables: AMI_LOOKUP_ORG: "999867407951" # To run Nutanix provider tests, set following variables here or as an env var # # IP/FQDN of Prism Central. - # NUTANIX_ENDPOINT: "" + NUTANIX_ENDPOINT: "" # # Port of Prism Central. Default: 9440 # NUTANIX_PORT: 9440 # # Disable Prism Central certificate checking. Default: false # NUTANIX_INSECURE: false # # Prism Central user - # NUTANIX_USER: "" + NUTANIX_USER: "" # # Prism Central password - # NUTANIX_PASSWORD: "" + NUTANIX_PASSWORD: "" # # Host IP to be assigned to the CAPX Kubernetes cluster. # CONTROL_PLANE_ENDPOINT_IP: "" # # Port of the CAPX Kubernetes cluster. Default: 6443 From 3211a0609d83608942cc39fc0c00600a63f7a64e Mon Sep 17 00:00:00 2001 From: Jimmi Dyson Date: Tue, 14 May 2024 15:22:29 +0100 Subject: [PATCH 2/3] fixup! revert: Remove unintentional e2e test config change --- test/e2e/config/caren.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/config/caren.yaml b/test/e2e/config/caren.yaml index e9d0ee37b..6e560ecd2 100644 --- a/test/e2e/config/caren.yaml +++ b/test/e2e/config/caren.yaml @@ -184,15 +184,15 @@ variables: AMI_LOOKUP_ORG: "999867407951" # To run Nutanix provider tests, set following variables here or as an env var # # IP/FQDN of Prism Central. - NUTANIX_ENDPOINT: "" + # NUTANIX_ENDPOINT: "" # # Port of Prism Central. Default: 9440 # NUTANIX_PORT: 9440 # # Disable Prism Central certificate checking. Default: false # NUTANIX_INSECURE: false # # Prism Central user - NUTANIX_USER: "" + # NUTANIX_USER: "" # # Prism Central password - NUTANIX_PASSWORD: "" + # NUTANIX_PASSWORD: "" # # Host IP to be assigned to the CAPX Kubernetes cluster. # CONTROL_PLANE_ENDPOINT_IP: "" # # Port of the CAPX Kubernetes cluster. Default: 6443 From cd58ce2d401ba02d70b8754f787c6c29011ce2c0 Mon Sep 17 00:00:00 2001 From: Jimmi Dyson Date: Tue, 14 May 2024 16:30:59 +0100 Subject: [PATCH 3/3] fixup! refactor: Use common function for deriving containerd config patch path --- .../containerdunprivilegedports/unprivileged_ports.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/pkg/handlers/generic/mutation/containerdunprivilegedports/unprivileged_ports.go b/pkg/handlers/generic/mutation/containerdunprivilegedports/unprivileged_ports.go index 13c4a2b2c..70fa6206d 100644 --- a/pkg/handlers/generic/mutation/containerdunprivilegedports/unprivileged_ports.go +++ b/pkg/handlers/generic/mutation/containerdunprivilegedports/unprivileged_ports.go @@ -4,21 +4,16 @@ package containerdunprivilegedports import ( _ "embed" - "path" cabpkv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" -) -const ( - // TODO Factor out this constant to a common package. - containerdPatchesDirOnRemote = "/etc/containerd/cre.d" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/common" ) var ( //go:embed files/unprivileged-ports-config.toml unprivilegedPortsConfigDropIn []byte - unprivilegedPortsConfigDropInFileOnRemote = path.Join( - containerdPatchesDirOnRemote, + unprivilegedPortsConfigDropInFileOnRemote = common.ContainerdPatchPathOnRemote( "unprivileged-ports-config.toml", ) )