From de104142057e6b66ec0a0c26460c4e5f79fc09a2 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 3 Oct 2024 15:34:46 -0500 Subject: [PATCH 1/7] Use privileged dataplane entrypoint for ingress-gateway which may bind to privileged ports --- charts/consul/templates/ingress-gateways-deployment.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml index c7a38bb040..1486e81107 100644 --- a/charts/consul/templates/ingress-gateways-deployment.yaml +++ b/charts/consul/templates/ingress-gateways-deployment.yaml @@ -292,8 +292,9 @@ spec: - name: DP_SERVICE_NODE_NAME value: $(NODE_NAME)-virtual command: - - consul-dataplane + - privileged-consul-dataplane args: + - -privileged - -envoy-ready-bind-port=21000 {{- if $root.Values.externalServers.enabled }} - -addresses={{ $root.Values.externalServers.hosts | first }} From cb5c7ffc56cf15f667b475dec8a251c6b89b6295 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 9 Oct 2024 18:03:36 -0400 Subject: [PATCH 2/7] Use new envoy-executable-path flag for ingress-gateway container --- charts/consul/templates/ingress-gateways-deployment.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml index 1486e81107..b23a252fcc 100644 --- a/charts/consul/templates/ingress-gateways-deployment.yaml +++ b/charts/consul/templates/ingress-gateways-deployment.yaml @@ -294,7 +294,7 @@ spec: command: - privileged-consul-dataplane args: - - -privileged + - -envoy-executable-path=/usr/local/bin/privileged-envoy - -envoy-ready-bind-port=21000 {{- if $root.Values.externalServers.enabled }} - -addresses={{ $root.Values.externalServers.hosts | first }} From dbdcbbdcbbea2b1488479d3befc8b412173baa87 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 9 Oct 2024 18:33:15 -0400 Subject: [PATCH 3/7] Use custom securityContext for ingress-gateway container This will enable a future commit to remove NET_BIND_SERVICE from the widely shared consul.restrictedSecurityContext --- .../ingress-gateways-deployment.yaml | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml index b23a252fcc..21bcd8699a 100644 --- a/charts/consul/templates/ingress-gateways-deployment.yaml +++ b/charts/consul/templates/ingress-gateways-deployment.yaml @@ -247,7 +247,29 @@ spec: - name: ingress-gateway image: {{ $root.Values.global.imageConsulDataplane | quote }} {{ template "consul.imagePullPolicy" $root }} - {{- include "consul.restrictedSecurityContext" $ | nindent 8 }} + {{- if not $root.Values.global.enablePodSecurityPolicies }} + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: + - ALL + add: + - NET_BIND_SERVICE + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + {{- if not $root.Values.global.openshift.enabled }} + {{/* + We must set runAsUser or else the root user will be used in some cases and + containers will fail to start due to runAsNonRoot above (e.g. + tls-init-cleanup). On OpenShift, runAsUser is automatically. We pick user 100 + because it is a non-root user id that exists in the consul, consul-dataplane, + and consul-k8s-control-plane images. + */}} + runAsUser: 100 + {{- end }} + {{- end }} {{- if (default $defaults.resources .resources) }} resources: {{ toYaml (default $defaults.resources .resources) | nindent 10 }} {{- end }} From 86fe96527e1d4e6576ef613e22cc07e044ae7776 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 9 Oct 2024 18:34:16 -0400 Subject: [PATCH 4/7] Remove NET_BIND_SERVICE everywhere other than the ingress-gateway container --- charts/consul/templates/_helpers.tpl | 2 -- charts/consul/templates/mesh-gateway-deployment.yaml | 6 ++---- .../templates/mesh-gateway-podsecuritypolicy.yaml | 2 -- .../telemetry-collector-podsecuritypolicy.yaml | 2 -- .../terminating-gateways-podsecuritypolicy.yaml | 2 -- charts/consul/test/unit/server-statefulset.bats | 2 -- control-plane/api-gateway/gatekeeper/dataplane.go | 3 --- .../api-gateway/gatekeeper/gatekeeper_test.go | 4 +--- .../webhook/consul_dataplane_sidecar.go | 10 +++------- .../webhook/consul_dataplane_sidecar_test.go | 12 ------------ 10 files changed, 6 insertions(+), 39 deletions(-) diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl index ded6485418..8e123395b1 100644 --- a/charts/consul/templates/_helpers.tpl +++ b/charts/consul/templates/_helpers.tpl @@ -23,8 +23,6 @@ securityContext: capabilities: drop: - ALL - add: - - NET_BIND_SERVICE runAsNonRoot: true seccompProfile: type: RuntimeDefault diff --git a/charts/consul/templates/mesh-gateway-deployment.yaml b/charts/consul/templates/mesh-gateway-deployment.yaml index c6c33966ee..abfff75871 100644 --- a/charts/consul/templates/mesh-gateway-deployment.yaml +++ b/charts/consul/templates/mesh-gateway-deployment.yaml @@ -187,14 +187,12 @@ spec: - name: mesh-gateway image: {{ .Values.global.imageConsulDataplane | quote }} {{ template "consul.imagePullPolicy" . }} + {{ if not .Values.meshGateway.hostNetwork}} securityContext: capabilities: - {{ if not .Values.meshGateway.hostNetwork}} drop: - ALL - {{- end }} - add: - - NET_BIND_SERVICE + {{- end }} {{- if .Values.meshGateway.resources }} resources: {{- if eq (typeOf .Values.meshGateway.resources) "string" }} diff --git a/charts/consul/templates/mesh-gateway-podsecuritypolicy.yaml b/charts/consul/templates/mesh-gateway-podsecuritypolicy.yaml index 04576fe926..b5bbb2fa03 100644 --- a/charts/consul/templates/mesh-gateway-podsecuritypolicy.yaml +++ b/charts/consul/templates/mesh-gateway-podsecuritypolicy.yaml @@ -18,8 +18,6 @@ spec: # but we can provide it for defense in depth. requiredDropCapabilities: - ALL - defaultAddCapabilities: - - NET_BIND_SERVICE # Allow core volume types. volumes: - 'configMap' diff --git a/charts/consul/templates/telemetry-collector-podsecuritypolicy.yaml b/charts/consul/templates/telemetry-collector-podsecuritypolicy.yaml index f4c05a2f33..286a92d0bd 100644 --- a/charts/consul/templates/telemetry-collector-podsecuritypolicy.yaml +++ b/charts/consul/templates/telemetry-collector-podsecuritypolicy.yaml @@ -18,8 +18,6 @@ spec: # but we can provide it for defense in depth. requiredDropCapabilities: - ALL - defaultAddCapabilities: - - NET_BIND_SERVICE # Allow core volume types. volumes: - 'configMap' diff --git a/charts/consul/templates/terminating-gateways-podsecuritypolicy.yaml b/charts/consul/templates/terminating-gateways-podsecuritypolicy.yaml index 7307fb8be9..97ad2af961 100644 --- a/charts/consul/templates/terminating-gateways-podsecuritypolicy.yaml +++ b/charts/consul/templates/terminating-gateways-podsecuritypolicy.yaml @@ -21,8 +21,6 @@ spec: # but we can provide it for defense in depth. requiredDropCapabilities: - ALL - defaultAddCapabilities: - - NET_BIND_SERVICE # Allow core volume types. volumes: - 'configMap' diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats index 96b5501e79..ecff198dfc 100755 --- a/charts/consul/test/unit/server-statefulset.bats +++ b/charts/consul/test/unit/server-statefulset.bats @@ -1383,7 +1383,6 @@ load _helpers "allowPrivilegeEscalation": false, "capabilities": { "drop": ["ALL"], - "add": ["NET_BIND_SERVICE"] }, "readOnlyRootFilesystem": true, "runAsNonRoot": true, @@ -1416,7 +1415,6 @@ load _helpers "allowPrivilegeEscalation": false, "capabilities": { "drop": ["ALL"], - "add": ["NET_BIND_SERVICE"] }, "readOnlyRootFilesystem": true, "runAsNonRoot": true, diff --git a/control-plane/api-gateway/gatekeeper/dataplane.go b/control-plane/api-gateway/gatekeeper/dataplane.go index fb488509c1..033fbb5604 100644 --- a/control-plane/api-gateway/gatekeeper/dataplane.go +++ b/control-plane/api-gateway/gatekeeper/dataplane.go @@ -19,7 +19,6 @@ import ( const ( allCapabilities = "ALL" - netBindCapability = "NET_BIND_SERVICE" consulDataplaneDNSBindHost = "127.0.0.1" consulDataplaneDNSBindPort = 8600 defaultEnvoyProxyConcurrency = 1 @@ -114,9 +113,7 @@ func consulDataplaneContainer(metrics common.MetricsConfig, config common.HelmCo // otherwise, allow the user to be assigned by OpenShift. container.SecurityContext = &corev1.SecurityContext{ ReadOnlyRootFilesystem: ptr.To(true), - // Drop any Linux capabilities you'd get as root other than NET_BIND_SERVICE. Capabilities: &corev1.Capabilities{ - Add: []corev1.Capability{netBindCapability}, Drop: []corev1.Capability{allCapabilities}, }, } diff --git a/control-plane/api-gateway/gatekeeper/gatekeeper_test.go b/control-plane/api-gateway/gatekeeper/gatekeeper_test.go index 0c60aa610b..18324fe647 100644 --- a/control-plane/api-gateway/gatekeeper/gatekeeper_test.go +++ b/control-plane/api-gateway/gatekeeper/gatekeeper_test.go @@ -1506,8 +1506,7 @@ func validateResourcesExist(t *testing.T, client client.Client, helmConfig commo } assert.True(t, hasInitContainer) - // Ensure there is a consul-dataplane container dropping ALL capabilities, adding - // back the NET_BIND_SERVICE capability, and establishing a read-only root filesystem + // Ensure there is a consul-dataplane container dropping ALL capabilities and establishing a read-only root filesystem hasDataplaneContainer := false for _, container := range actual.Spec.Template.Spec.Containers { if container.Image == dataplaneImage { @@ -1516,7 +1515,6 @@ func validateResourcesExist(t *testing.T, client client.Client, helmConfig commo require.NotNil(t, container.SecurityContext.Capabilities) require.NotNil(t, container.SecurityContext.ReadOnlyRootFilesystem) assert.True(t, *container.SecurityContext.ReadOnlyRootFilesystem) - assert.Equal(t, []corev1.Capability{netBindCapability}, container.SecurityContext.Capabilities.Add) assert.Equal(t, []corev1.Capability{allCapabilities}, container.SecurityContext.Capabilities.Drop) } } diff --git a/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go b/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go index a18c15b999..cd2d3f0672 100644 --- a/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go +++ b/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go @@ -10,12 +10,13 @@ import ( "strings" "github.com/google/shlex" - "github.com/hashicorp/consul-k8s/control-plane/connect-inject/common" - "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" + + "github.com/hashicorp/consul-k8s/control-plane/connect-inject/common" + "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" ) const ( @@ -264,11 +265,6 @@ func (w *MeshWebhook) consulDataplaneSidecar(namespace corev1.Namespace, pod cor RunAsGroup: ptr.To(group), RunAsNonRoot: ptr.To(true), AllowPrivilegeEscalation: ptr.To(false), - // consul-dataplane requires the NET_BIND_SERVICE capability regardless of binding port #. - // See https://developer.hashicorp.com/consul/docs/connect/dataplane#technical-constraints - Capabilities: &corev1.Capabilities{ - Add: []corev1.Capability{"NET_BIND_SERVICE"}, - }, ReadOnlyRootFilesystem: ptr.To(true), } return container, nil diff --git a/control-plane/connect-inject/webhook/consul_dataplane_sidecar_test.go b/control-plane/connect-inject/webhook/consul_dataplane_sidecar_test.go index 9edd91f9e2..3242a40e7a 100644 --- a/control-plane/connect-inject/webhook/consul_dataplane_sidecar_test.go +++ b/control-plane/connect-inject/webhook/consul_dataplane_sidecar_test.go @@ -807,9 +807,6 @@ func TestHandlerConsulDataplaneSidecar_withSecurityContext(t *testing.T) { RunAsNonRoot: ptr.To(true), ReadOnlyRootFilesystem: ptr.To(true), AllowPrivilegeEscalation: ptr.To(false), - Capabilities: &corev1.Capabilities{ - Add: []corev1.Capability{"NET_BIND_SERVICE"}, - }, }, }, "tproxy enabled; openshift disabled": { @@ -821,9 +818,6 @@ func TestHandlerConsulDataplaneSidecar_withSecurityContext(t *testing.T) { RunAsNonRoot: ptr.To(true), ReadOnlyRootFilesystem: ptr.To(true), AllowPrivilegeEscalation: ptr.To(false), - Capabilities: &corev1.Capabilities{ - Add: []corev1.Capability{"NET_BIND_SERVICE"}, - }, }, }, "tproxy disabled; openshift enabled": { @@ -835,9 +829,6 @@ func TestHandlerConsulDataplaneSidecar_withSecurityContext(t *testing.T) { RunAsNonRoot: ptr.To(true), ReadOnlyRootFilesystem: ptr.To(true), AllowPrivilegeEscalation: ptr.To(false), - Capabilities: &corev1.Capabilities{ - Add: []corev1.Capability{"NET_BIND_SERVICE"}, - }, }, }, "tproxy enabled; openshift enabled": { @@ -849,9 +840,6 @@ func TestHandlerConsulDataplaneSidecar_withSecurityContext(t *testing.T) { RunAsNonRoot: ptr.To(true), ReadOnlyRootFilesystem: ptr.To(true), AllowPrivilegeEscalation: ptr.To(false), - Capabilities: &corev1.Capabilities{ - Add: []corev1.Capability{"NET_BIND_SERVICE"}, - }, }, }, } From ece21310de6c0c22b2642ec24be7b36a2780810b Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 17 Oct 2024 14:43:00 -0400 Subject: [PATCH 5/7] Allow ingress gateway users who do not bind to privileged ports to opt out of NET_BIND_SERVICE --- charts/consul/templates/ingress-gateways-deployment.yaml | 6 ++++++ charts/consul/values.yaml | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml index 21bcd8699a..96874e933e 100644 --- a/charts/consul/templates/ingress-gateways-deployment.yaml +++ b/charts/consul/templates/ingress-gateways-deployment.yaml @@ -254,8 +254,10 @@ spec: capabilities: drop: - ALL + {{- if $root.Values.ingressGateways.supportPrivilegedPorts }} add: - NET_BIND_SERVICE + {{- end }} runAsNonRoot: true seccompProfile: type: RuntimeDefault @@ -313,10 +315,14 @@ spec: value: component=ingress-gateway - name: DP_SERVICE_NODE_NAME value: $(NODE_NAME)-virtual + {{- if $root.Values.ingressGateways.supportPrivilegedPorts }} command: - privileged-consul-dataplane args: - -envoy-executable-path=/usr/local/bin/privileged-envoy + {{- else }} + args: + {{- end }} - -envoy-ready-bind-port=21000 {{- if $root.Values.externalServers.enabled }} - -addresses={{ $root.Values.externalServers.hosts | first }} diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 4a6aa20060..5bd2df89c4 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -3217,6 +3217,11 @@ ingressGateways: # @type: string logLevel: "" + # When set, ingress-gateway containers use a privileged entrypoint in consul-dataplane which requires the + # NET_BIND_SERVICE capability at runtime. Disabling this setting will use the non-privileged entrypoint and + # drop the requirement for NET_BIND_SERVICE. Only disable this if you are *not* using privileged ports (< 1024). + supportPrivilegedPorts: true + # Defaults sets default values for all gateway fields. With the exception # of annotations, defining any of these values in the `gateways` list # will override the default values provided here. Annotations will From 642b4d034ebfc989fc23e39f4424da1d6e320f9c Mon Sep 17 00:00:00 2001 From: jm96441n Date: Thu, 17 Oct 2024 15:44:34 -0400 Subject: [PATCH 6/7] use named constant for security context with net bind service --- charts/consul/templates/_helpers.tpl | 27 +++++++++++++++++++ .../ingress-gateways-deployment.yaml | 26 +----------------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl index 8e123395b1..c9131381eb 100644 --- a/charts/consul/templates/_helpers.tpl +++ b/charts/consul/templates/_helpers.tpl @@ -39,6 +39,33 @@ and consul-k8s-control-plane images. {{- end -}} {{- end -}} + +{{- define "consul.restrictedSecurityContextWithNetBindService" -}} +{{- if not .Values.global.enablePodSecurityPolicies -}} +securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + add: + - NET_BIND_SERVICE + drop: + - ALL + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault +{{- if not .Values.global.openshift.enabled -}} +{{/* +We must set runAsUser or else the root user will be used in some cases and +containers will fail to start due to runAsNonRoot above (e.g. +tls-init-cleanup). On OpenShift, runAsUser is automatically. We pick user 100 +because it is a non-root user id that exists in the consul, consul-dataplane, +and consul-k8s-control-plane images. +*/}} + runAsUser: 100 +{{- end -}} +{{- end -}} +{{- end -}} + {{- define "consul.vaultSecretTemplate" -}} | {{ "{{" }}- with secret "{{ .secretName }}" -{{ "}}" }} diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml index 96874e933e..46e94730de 100644 --- a/charts/consul/templates/ingress-gateways-deployment.yaml +++ b/charts/consul/templates/ingress-gateways-deployment.yaml @@ -247,31 +247,7 @@ spec: - name: ingress-gateway image: {{ $root.Values.global.imageConsulDataplane | quote }} {{ template "consul.imagePullPolicy" $root }} - {{- if not $root.Values.global.enablePodSecurityPolicies }} - securityContext: - allowPrivilegeEscalation: false - readOnlyRootFilesystem: true - capabilities: - drop: - - ALL - {{- if $root.Values.ingressGateways.supportPrivilegedPorts }} - add: - - NET_BIND_SERVICE - {{- end }} - runAsNonRoot: true - seccompProfile: - type: RuntimeDefault - {{- if not $root.Values.global.openshift.enabled }} - {{/* - We must set runAsUser or else the root user will be used in some cases and - containers will fail to start due to runAsNonRoot above (e.g. - tls-init-cleanup). On OpenShift, runAsUser is automatically. We pick user 100 - because it is a non-root user id that exists in the consul, consul-dataplane, - and consul-k8s-control-plane images. - */}} - runAsUser: 100 - {{- end }} - {{- end }} + {{- include "consul.restrictedSecurityContextWithNetBindService" $ | nindent 8 }} {{- if (default $defaults.resources .resources) }} resources: {{ toYaml (default $defaults.resources .resources) | nindent 10 }} {{- end }} From e8453dc9298ce5f4118047150f1ff944e0589eb2 Mon Sep 17 00:00:00 2001 From: jm96441n Date: Mon, 21 Oct 2024 13:14:29 -0400 Subject: [PATCH 7/7] gofmt --- .../connect-inject/webhook/consul_dataplane_sidecar.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go b/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go index cd2d3f0672..27f659d712 100644 --- a/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go +++ b/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go @@ -265,7 +265,7 @@ func (w *MeshWebhook) consulDataplaneSidecar(namespace corev1.Namespace, pod cor RunAsGroup: ptr.To(group), RunAsNonRoot: ptr.To(true), AllowPrivilegeEscalation: ptr.To(false), - ReadOnlyRootFilesystem: ptr.To(true), + ReadOnlyRootFilesystem: ptr.To(true), } return container, nil }