Set privileged to false unless on OpenShift without CNI (#2755)

* Set privileged to false unless on OpenShift without CNI
hashicorp · Aug 11, 2023 · 479d79d · 479d79d
1 parent 65cecc2
commit 479d79d
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 14 deletions.
diff --git a/.changelog/2755.txt b/.changelog/2755.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+control-plane: When using transparent proxy or CNI, reduced required permissions by setting privileged to false. Privileged must be true when using OpenShift without CNI.
+```
diff --git a/control-plane/connect-inject/webhook/container_init.go b/control-plane/connect-inject/webhook/container_init.go
@@ -220,6 +220,12 @@ func (w *MeshWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod,
 			})
 	}
 
+	// OpenShift without CNI is the only environment where privileged must be true.
+	privileged := false
+	if w.EnableOpenShift && !w.EnableCNI {
+		privileged = true
+	}
+
 	if tproxyEnabled {
 		if !w.EnableCNI {
 			// Set redirect traffic config for the container so that we can apply iptables rules.
@@ -240,7 +246,7 @@ func (w *MeshWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod,
 				RunAsGroup: pointer.Int64(rootUserAndGroupID),
 				// RunAsNonRoot overrides any setting in the Pod so that we can still run as root here as required.
 				RunAsNonRoot: pointer.Bool(false),
-				Privileged:   pointer.Bool(true),
+				Privileged:   pointer.Bool(privileged),
 				Capabilities: &corev1.Capabilities{
 					Add: []corev1.Capability{netAdminCapability},
 				},
@@ -250,7 +256,7 @@ func (w *MeshWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod,
 				RunAsUser:    pointer.Int64(initContainersUserAndGroupID),
 				RunAsGroup:   pointer.Int64(initContainersUserAndGroupID),
 				RunAsNonRoot: pointer.Bool(true),
-				Privileged:   pointer.Bool(false),
+				Privileged:   pointer.Bool(privileged),
 				Capabilities: &corev1.Capabilities{
 					Drop: []corev1.Capability{"ALL"},
 				},

diff --git a/control-plane/connect-inject/webhook/container_init_test.go b/control-plane/connect-inject/webhook/container_init_test.go
@@ -173,77 +173,104 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 		annotations      map[string]string
 		expTproxyEnabled bool
 		namespaceLabel   map[string]string
+		openShiftEnabled bool
 	}{
-		"enabled globally, ns not set, annotation not provided, cni disabled": {
+		"enabled globally, ns not set, annotation not provided, cni disabled, openshift disabled": {
 			true,
 			false,
 			nil,
 			true,
 			nil,
+			false,
 		},
-		"enabled globally, ns not set, annotation is false, cni disabled": {
+		"enabled globally, ns not set, annotation is false, cni disabled, openshift disabled": {
 			true,
 			false,
 			map[string]string{constants.KeyTransparentProxy: "false"},
 			false,
 			nil,
+			false,
 		},
-		"enabled globally, ns not set, annotation is true, cni disabled": {
+		"enabled globally, ns not set, annotation is true, cni disabled, openshift disabled": {
 			true,
 			false,
 			map[string]string{constants.KeyTransparentProxy: "true"},
 			true,
 			nil,
+			false,
 		},
-		"disabled globally, ns not set, annotation not provided, cni disabled": {
+		"disabled globally, ns not set, annotation not provided, cni disabled, openshift disabled": {
 			false,
 			false,
 			nil,
 			false,
 			nil,
+			false,
 		},
-		"disabled globally, ns not set, annotation is false, cni disabled": {
+		"disabled globally, ns not set, annotation is false, cni disabled, openshift disabled": {
 			false,
 			false,
 			map[string]string{constants.KeyTransparentProxy: "false"},
 			false,
 			nil,
+			false,
 		},
-		"disabled globally, ns not set, annotation is true, cni disabled": {
+		"disabled globally, ns not set, annotation is true, cni disabled, openshift disabled": {
 			false,
 			false,
 			map[string]string{constants.KeyTransparentProxy: "true"},
 			true,
 			nil,
+			false,
 		},
-		"disabled globally, ns enabled, annotation not set, cni disabled": {
+		"disabled globally, ns enabled, annotation not set, cni disabled, openshift disabled": {
 			false,
 			false,
 			nil,
 			true,
 			map[string]string{constants.KeyTransparentProxy: "true"},
+			false,
 		},
-		"enabled globally, ns disabled, annotation not set, cni disabled": {
+		"enabled globally, ns disabled, annotation not set, cni disabled, openshift disabled": {
 			true,
 			false,
 			nil,
 			false,
 			map[string]string{constants.KeyTransparentProxy: "false"},
+			false,
 		},
-		"disabled globally, ns enabled, annotation not set, cni enabled": {
+		"disabled globally, ns enabled, annotation not set, cni enabled, openshift disabled": {
 			false,
 			true,
 			nil,
 			false,
 			map[string]string{constants.KeyTransparentProxy: "true"},
+			false,
 		},
 
-		"enabled globally, ns not set, annotation not set, cni enabled": {
+		"enabled globally, ns not set, annotation not set, cni enabled, openshift disabled": {
+			true,
+			true,
+			nil,
+			false,
+			nil,
+			false,
+		},
+		"enabled globally, ns not set, annotation not set, cni enabled, openshift enabled": {
 			true,
 			true,
 			nil,
 			false,
 			nil,
+			true,
+		},
+		"enabled globally, ns not set, annotation not set, cni disabled, openshift enabled": {
+			true,
+			false,
+			nil,
+			true,
+			nil,
+			true,
 		},
 	}
 	for name, c := range cases {
@@ -252,17 +279,23 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 				EnableTransparentProxy: c.globalEnabled,
 				EnableCNI:              c.cniEnabled,
 				ConsulConfig:           &consul.Config{HTTPPort: 8500},
+				EnableOpenShift:        c.openShiftEnabled,
 			}
 			pod := minimal()
 			pod.Annotations = c.annotations
 
+			privileged := false
+			if c.openShiftEnabled && !c.cniEnabled {
+				privileged = true
+			}
+
 			var expectedSecurityContext *corev1.SecurityContext
 			if c.cniEnabled {
 				expectedSecurityContext = &corev1.SecurityContext{
 					RunAsUser:    pointer.Int64(initContainersUserAndGroupID),
 					RunAsGroup:   pointer.Int64(initContainersUserAndGroupID),
 					RunAsNonRoot: pointer.Bool(true),
-					Privileged:   pointer.Bool(false),
+					Privileged:   pointer.Bool(privileged),
 					Capabilities: &corev1.Capabilities{
 						Drop: []corev1.Capability{"ALL"},
 					},
@@ -272,7 +305,7 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 					RunAsUser:    pointer.Int64(0),
 					RunAsGroup:   pointer.Int64(0),
 					RunAsNonRoot: pointer.Bool(false),
-					Privileged:   pointer.Bool(true),
+					Privileged:   pointer.Bool(privileged),
 					Capabilities: &corev1.Capabilities{
 						Add: []corev1.Capability{netAdminCapability},
 					},