From 046eae0888b4bec8810f7b4dafc6680fbc069fac Mon Sep 17 00:00:00 2001 From: Lukas Piwowarski Date: Tue, 26 Nov 2024 05:19:30 -0500 Subject: [PATCH] Run test pods with readOnlyRootFileSystem: true Up until now the test pods were being spawned with writable root file system (readOnlyRootFilesystem: false). This is against the best security practices as setting the readOnlyRootFileSystem to false increases the size of the attack surface. This patch ensures that each pod spawned by the test-operator has the readOnlyRootFilesystem set to true by default. It is possible to run a pod with writable roo file system by setting privileged: true. Depends-On: https://github.com/openstack-k8s-operators/tcib/pull/233 --- .../test.openstack.org_ansibletests.yaml | 10 ++++---- .../test.openstack.org_horizontests.yaml | 10 ++++---- api/bases/test.openstack.org_tempests.yaml | 10 ++++---- api/bases/test.openstack.org_tobikoes.yaml | 10 ++++---- api/v1beta1/common.go | 10 ++++---- api/v1beta1/common_webhook.go | 4 ++-- .../test.openstack.org_ansibletests.yaml | 10 ++++---- .../test.openstack.org_horizontests.yaml | 10 ++++---- .../bases/test.openstack.org_tempests.yaml | 10 ++++---- .../bases/test.openstack.org_tobikoes.yaml | 10 ++++---- config/samples/test_v1beta1_tempest.yaml | 10 ++++---- pkg/ansibletest/volumes.go | 23 +++++++++++++++++++ pkg/horizontest/volumes.go | 22 ++++++++++++++++++ pkg/tempest/volumes.go | 23 +++++++++++++++++++ pkg/tobiko/volumes.go | 22 ++++++++++++++++++ pkg/util/common.go | 11 +++++++++ 16 files changed, 153 insertions(+), 52 deletions(-) diff --git a/api/bases/test.openstack.org_ansibletests.yaml b/api/bases/test.openstack.org_ansibletests.yaml index d41e4376..5a177047 100644 --- a/api/bases/test.openstack.org_ansibletests.yaml +++ b/api/bases/test.openstack.org_ansibletests.yaml @@ -146,11 +146,11 @@ spec: default: false description: |- Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true and the default capabilities on - top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW). - This parameter is deemed insecure but it is needed for certain test-operator - functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set - of tobiko tests). + pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false and the + default capabilities on top of capabilities that are usually needed by the test + pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed for + certain test-operator functionalities to work properly (e.g.: extraRPMs in Tempest + CR, or certain set of tobiko tests). type: boolean storageClass: default: local-storage diff --git a/api/bases/test.openstack.org_horizontests.yaml b/api/bases/test.openstack.org_horizontests.yaml index 6b287cdd..243a1bf6 100644 --- a/api/bases/test.openstack.org_horizontests.yaml +++ b/api/bases/test.openstack.org_horizontests.yaml @@ -159,11 +159,11 @@ spec: default: false description: |- Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true and the default capabilities on - top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW). - This parameter is deemed insecure but it is needed for certain test-operator - functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set - of tobiko tests). + pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false and the + default capabilities on top of capabilities that are usually needed by the test + pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed for + certain test-operator functionalities to work properly (e.g.: extraRPMs in Tempest + CR, or certain set of tobiko tests). type: boolean projectName: default: horizontest diff --git a/api/bases/test.openstack.org_tempests.yaml b/api/bases/test.openstack.org_tempests.yaml index 62d30607..81f54d1c 100644 --- a/api/bases/test.openstack.org_tempests.yaml +++ b/api/bases/test.openstack.org_tempests.yaml @@ -153,11 +153,11 @@ spec: default: false description: |- Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true and the default capabilities on - top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW). - This parameter is deemed insecure but it is needed for certain test-operator - functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set - of tobiko tests). + pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false and the + default capabilities on top of capabilities that are usually needed by the test + pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed for + certain test-operator functionalities to work properly (e.g.: extraRPMs in Tempest + CR, or certain set of tobiko tests). type: boolean storageClass: default: local-storage diff --git a/api/bases/test.openstack.org_tobikoes.yaml b/api/bases/test.openstack.org_tobikoes.yaml index a19c8431..6b44c6e7 100644 --- a/api/bases/test.openstack.org_tobikoes.yaml +++ b/api/bases/test.openstack.org_tobikoes.yaml @@ -143,11 +143,11 @@ spec: default: false description: |- Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true and the default capabilities on - top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW). - This parameter is deemed insecure but it is needed for certain test-operator - functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set - of tobiko tests). + pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false and the + default capabilities on top of capabilities that are usually needed by the test + pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed for + certain test-operator functionalities to work properly (e.g.: extraRPMs in Tempest + CR, or certain set of tobiko tests). type: boolean publicKey: default: "" diff --git a/api/v1beta1/common.go b/api/v1beta1/common.go index 6222b1cc..cd93ae5e 100644 --- a/api/v1beta1/common.go +++ b/api/v1beta1/common.go @@ -46,11 +46,11 @@ type CommonOptions struct { // +kubebuilder:default=false // +optional // Use with caution! This parameter specifies whether test-operator should spawn test - // pods with allowedPrivilegedEscalation: true and the default capabilities on - // top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW). - // This parameter is deemed insecure but it is needed for certain test-operator - // functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set - // of tobiko tests). + // pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false and the + // default capabilities on top of capabilities that are usually needed by the test + // pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed for + // certain test-operator functionalities to work properly (e.g.: extraRPMs in Tempest + // CR, or certain set of tobiko tests). Privileged bool `json:"privileged"` // +operator-sdk:csv:customresourcedefinitions:type=spec diff --git a/api/v1beta1/common_webhook.go b/api/v1beta1/common_webhook.go index a28d2d8a..4c3f3280 100644 --- a/api/v1beta1/common_webhook.go +++ b/api/v1beta1/common_webhook.go @@ -12,8 +12,8 @@ const ( const ( // WarnPrivilegedModeOn WarnPrivilegedModeOn = "%s.Spec.Privileged is set to true. This means that test pods " + - "are spawned with allowPrivilegedEscalation: true and default " + - "capabilities on top of those required by the test operator " + + "are spawned with allowPrivilegedEscalation: true, readOnlyRootFilesystem: false " + + "and default capabilities on top of those required by the test operator " + "(NET_ADMIN, NET_RAW)." // WarnPrivilegedModeOff diff --git a/config/crd/bases/test.openstack.org_ansibletests.yaml b/config/crd/bases/test.openstack.org_ansibletests.yaml index d41e4376..5a177047 100644 --- a/config/crd/bases/test.openstack.org_ansibletests.yaml +++ b/config/crd/bases/test.openstack.org_ansibletests.yaml @@ -146,11 +146,11 @@ spec: default: false description: |- Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true and the default capabilities on - top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW). - This parameter is deemed insecure but it is needed for certain test-operator - functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set - of tobiko tests). + pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false and the + default capabilities on top of capabilities that are usually needed by the test + pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed for + certain test-operator functionalities to work properly (e.g.: extraRPMs in Tempest + CR, or certain set of tobiko tests). type: boolean storageClass: default: local-storage diff --git a/config/crd/bases/test.openstack.org_horizontests.yaml b/config/crd/bases/test.openstack.org_horizontests.yaml index 6b287cdd..243a1bf6 100644 --- a/config/crd/bases/test.openstack.org_horizontests.yaml +++ b/config/crd/bases/test.openstack.org_horizontests.yaml @@ -159,11 +159,11 @@ spec: default: false description: |- Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true and the default capabilities on - top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW). - This parameter is deemed insecure but it is needed for certain test-operator - functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set - of tobiko tests). + pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false and the + default capabilities on top of capabilities that are usually needed by the test + pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed for + certain test-operator functionalities to work properly (e.g.: extraRPMs in Tempest + CR, or certain set of tobiko tests). type: boolean projectName: default: horizontest diff --git a/config/crd/bases/test.openstack.org_tempests.yaml b/config/crd/bases/test.openstack.org_tempests.yaml index 62d30607..81f54d1c 100644 --- a/config/crd/bases/test.openstack.org_tempests.yaml +++ b/config/crd/bases/test.openstack.org_tempests.yaml @@ -153,11 +153,11 @@ spec: default: false description: |- Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true and the default capabilities on - top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW). - This parameter is deemed insecure but it is needed for certain test-operator - functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set - of tobiko tests). + pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false and the + default capabilities on top of capabilities that are usually needed by the test + pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed for + certain test-operator functionalities to work properly (e.g.: extraRPMs in Tempest + CR, or certain set of tobiko tests). type: boolean storageClass: default: local-storage diff --git a/config/crd/bases/test.openstack.org_tobikoes.yaml b/config/crd/bases/test.openstack.org_tobikoes.yaml index a19c8431..6b44c6e7 100644 --- a/config/crd/bases/test.openstack.org_tobikoes.yaml +++ b/config/crd/bases/test.openstack.org_tobikoes.yaml @@ -143,11 +143,11 @@ spec: default: false description: |- Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true and the default capabilities on - top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW). - This parameter is deemed insecure but it is needed for certain test-operator - functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set - of tobiko tests). + pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false and the + default capabilities on top of capabilities that are usually needed by the test + pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed for + certain test-operator functionalities to work properly (e.g.: extraRPMs in Tempest + CR, or certain set of tobiko tests). type: boolean publicKey: default: "" diff --git a/config/samples/test_v1beta1_tempest.yaml b/config/samples/test_v1beta1_tempest.yaml index 4e20f0e2..dcce5ca8 100644 --- a/config/samples/test_v1beta1_tempest.yaml +++ b/config/samples/test_v1beta1_tempest.yaml @@ -31,11 +31,11 @@ spec: # Privileged # ---------- # Use with caution! This parameter specifies whether test-operator should spawn test - # pods with allowedPrivilegedEscalation: true and the default capabilities on - # top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW). - # This parameter is deemed insecure but it is needed for certain test-operator - # functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set - # tobiko tests). + # pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false and the + # default capabilities on top of capabilities that are usually needed by the test + # pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed for + # certain test-operator functionalities to work properly (e.g.: extraRPMs in Tempest CR, + # or certain set tobiko tests). # # privileged: false tempestRun: diff --git a/pkg/ansibletest/volumes.go b/pkg/ansibletest/volumes.go index ebe52558..90ccfa06 100644 --- a/pkg/ansibletest/volumes.go +++ b/pkg/ansibletest/volumes.go @@ -2,6 +2,7 @@ package ansibletest import ( testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1" + util "github.com/openstack-k8s-operators/test-operator/pkg/util" corev1 "k8s.io/api/core/v1" ) @@ -50,6 +51,18 @@ func GetVolumes( }, }, }, + { + Name: util.TestOperatorEphemeralVolumeNameWorkdir, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + { + Name: util.TestOperatorEphemeralVolumeNameTmp, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, } if mountCerts { @@ -129,6 +142,16 @@ func GetVolumes( // GetVolumeMounts - func GetVolumeMounts(mountCerts bool, instance *testv1beta1.AnsibleTest, externalWorkflowCounter int) []corev1.VolumeMount { volumeMounts := []corev1.VolumeMount{ + { + Name: util.TestOperatorEphemeralVolumeNameWorkdir, + MountPath: "/var/lib/ansible", + ReadOnly: false, + }, + { + Name: util.TestOperatorEphemeralVolumeNameTmp, + MountPath: "/tmp", + ReadOnly: false, + }, { Name: "test-operator-logs", MountPath: "/var/lib/AnsibleTests/external_files", diff --git a/pkg/horizontest/volumes.go b/pkg/horizontest/volumes.go index 44678043..e72608ce 100644 --- a/pkg/horizontest/volumes.go +++ b/pkg/horizontest/volumes.go @@ -61,6 +61,18 @@ func GetVolumes( }, }, }, + { + Name: util.TestOperatorEphemeralVolumeNameWorkdir, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + { + Name: util.TestOperatorEphemeralVolumeNameTmp, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, } if mountCerts { @@ -117,6 +129,16 @@ func GetVolumes( // GetVolumeMounts - func GetVolumeMounts(mountCerts bool, mountKeys bool, mountKubeconfig bool, instance *testv1beta1.HorizonTest) []corev1.VolumeMount { volumeMounts := []corev1.VolumeMount{ + { + Name: util.TestOperatorEphemeralVolumeNameWorkdir, + MountPath: "/var/lib/horizontest", + ReadOnly: false, + }, + { + Name: util.TestOperatorEphemeralVolumeNameTmp, + MountPath: "/tmp", + ReadOnly: false, + }, { Name: "test-operator-logs", MountPath: "/var/lib/horizontest/external_files", diff --git a/pkg/tempest/volumes.go b/pkg/tempest/volumes.go index 261874eb..f4f8d3ee 100644 --- a/pkg/tempest/volumes.go +++ b/pkg/tempest/volumes.go @@ -2,6 +2,7 @@ package tempest import ( testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1" + util "github.com/openstack-k8s-operators/test-operator/pkg/util" corev1 "k8s.io/api/core/v1" ) @@ -61,6 +62,18 @@ func GetVolumes( }, }, }, + { + Name: util.TestOperatorEphemeralVolumeNameWorkdir, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + { + Name: util.TestOperatorEphemeralVolumeNameTmp, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, } if mountCerts { @@ -119,6 +132,16 @@ func GetVolumes( // GetVolumeMounts - func GetVolumeMounts(mountCerts bool, mountSSHKey bool, instance *testv1beta1.Tempest) []corev1.VolumeMount { volumeMounts := []corev1.VolumeMount{ + { + Name: util.TestOperatorEphemeralVolumeNameWorkdir, + MountPath: "/var/lib/tempest", + ReadOnly: false, + }, + { + Name: util.TestOperatorEphemeralVolumeNameTmp, + MountPath: "/tmp", + ReadOnly: false, + }, { Name: "config-data", MountPath: "/etc/test_operator", diff --git a/pkg/tobiko/volumes.go b/pkg/tobiko/volumes.go index ff92b163..8522ca00 100644 --- a/pkg/tobiko/volumes.go +++ b/pkg/tobiko/volumes.go @@ -63,6 +63,18 @@ func GetVolumes( }, }, }, + { + Name: util.TestOperatorEphemeralVolumeNameWorkdir, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + { + Name: util.TestOperatorEphemeralVolumeNameTmp, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, } if mountCerts { @@ -150,6 +162,16 @@ func GetVolumes( // GetVolumeMounts - func GetVolumeMounts(mountCerts bool, mountKeys bool, mountKubeconfig bool, instance *testv1beta1.Tobiko) []corev1.VolumeMount { volumeMounts := []corev1.VolumeMount{ + { + Name: util.TestOperatorEphemeralVolumeNameWorkdir, + MountPath: "/var/lib/tobiko", + ReadOnly: false, + }, + { + Name: util.TestOperatorEphemeralVolumeNameTmp, + MountPath: "/tmp", + ReadOnly: false, + }, { Name: "test-operator-logs", MountPath: "/var/lib/tobiko/external_files", diff --git a/pkg/util/common.go b/pkg/util/common.go index 939734b6..81a860b7 100644 --- a/pkg/util/common.go +++ b/pkg/util/common.go @@ -9,6 +9,12 @@ const ( // modified clouds.yaml obtained from openstack-config ConfigMap. The modified // CM is needed by some test frameworks (e.g., HorizonTest and Tobiko) TestOperatorCloudsConfigMapName = "test-operator-clouds-config" + + // TestOperatorEphemeralVolumeNameWorkdir + TestOperatorEphemeralVolumeNameWorkdir = "test-operator-ephemeral-workdir" + + // TestOperatorEphemeralVolumeNameTmp + TestOperatorEphemeralVolumeNameTmp = "test-operator-ephemeral-temporary" ) func GetSecurityContext( @@ -22,6 +28,7 @@ func GetSecurityContext( securityContext := corev1.SecurityContext{ RunAsUser: &runAsUser, RunAsGroup: &runAsUser, + ReadOnlyRootFilesystem: &trueVar, AllowPrivilegeEscalation: &falseVar, Capabilities: &corev1.Capabilities{}, SeccompProfile: &corev1.SeccompProfile{ @@ -33,6 +40,10 @@ func GetSecurityContext( // We need to run pods with AllowPrivilegedEscalation: true to remove // nosuid from the pod (in order to be able to run sudo) securityContext.AllowPrivilegeEscalation = &trueVar + + // We need to run pods with ReadOnlyRootFileSystem: false when installing + // additional tests using extraRPMs parameter in Tempest CR + securityContext.ReadOnlyRootFilesystem = &falseVar securityContext.Capabilities.Add = addCapabilities }