CVE-2024-45496

@@ -39,7 +39,8 @@ func (bs *DockerBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 	}
 
 	strategy := build.Spec.Strategy.DockerStrategy
-	securityContext := securityContextForBuild(strategy.Env)
+	buildSecurityContext := securityContextForBuild(strategy.Env)
+	initSecurityContext := builderMinSecurityContext()
 	hostPathFile := v1.HostPathFile
 
 	containerEnv := []v1.EnvVar{
@@ -73,7 +74,7 @@ func (bs *DockerBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 					Image:                    bs.Image,
 					Args:                     []string{"openshift-docker-build"},
 					Env:                      copyEnvVarSlice(containerEnv),
-					SecurityContext:          securityContext,
+					SecurityContext:          buildSecurityContext,
 					TerminationMessagePolicy: v1.TerminationMessageFallbackToLogsOnError,
 					VolumeMounts: []v1.VolumeMount{
 						{
@@ -130,7 +131,7 @@ func (bs *DockerBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 			Image:                    bs.Image,
 			Args:                     []string{"openshift-git-clone"},
 			Env:                      copyEnvVarSlice(containerEnv),
-			SecurityContext:          securityContext,
+			SecurityContext:          initSecurityContext,
 			TerminationMessagePolicy: v1.TerminationMessageFallbackToLogsOnError,
 			VolumeMounts: []v1.VolumeMount{
 				{
@@ -149,12 +150,14 @@ func (bs *DockerBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 		pod.Spec.InitContainers = append(pod.Spec.InitContainers, gitCloneContainer)
 	}
 	if len(build.Spec.Source.Images) > 0 {
+		// We use buildah to extract the image content as source. Using the build security context
+		// since buildah needs to create its own isolation environment.
 		extractImageContentContainer := v1.Container{
 			Name:                     ExtractImageContentContainer,
 			Image:                    bs.Image,
 			Args:                     []string{"openshift-extract-image-content"},
 			Env:                      copyEnvVarSlice(containerEnv),
-			SecurityContext:          securityContext,
+			SecurityContext:          buildSecurityContext,
 			TerminationMessagePolicy: v1.TerminationMessageFallbackToLogsOnError,
 			VolumeMounts: []v1.VolumeMount{
 				{
@@ -183,7 +186,7 @@ func (bs *DockerBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 			Image:                    bs.Image,
 			Args:                     []string{"openshift-manage-dockerfile"},
 			Env:                      copyEnvVarSlice(containerEnv),
-			SecurityContext:          securityContext,
+			SecurityContext:          initSecurityContext,
 			TerminationMessagePolicy: v1.TerminationMessageFallbackToLogsOnError,
 			VolumeMounts: []v1.VolumeMount{
 				{
@@ -209,7 +212,7 @@ func (bs *DockerBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 	setupContainersConfigs(build, pod)
 	setupBuildCAs(build, pod, additionalCAs, internalRegistryHost)
 	setupContainersStorage(pod, &pod.Spec.Containers[0])
-	if securityContext == nil || securityContext.Privileged == nil || !*securityContext.Privileged {
+	if buildSecurityContext == nil || buildSecurityContext.Privileged == nil || !*buildSecurityContext.Privileged {
 		setupBuilderAutonsUser(build, strategy.Env, pod)
 		setupBuilderDeviceFUSE(pod)
 	}

@@ -151,6 +151,7 @@ func TestDockerCreateBuildPod(t *testing.T) {
 	}
 
 	checkAliasing(t, actual)
+	checkPodSecurityContexts(t, actual)
 }
 
 func TestDockerBuildLongName(t *testing.T) {

@@ -68,7 +68,8 @@ func (bs *SourceBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 	}
 
 	hostPathFile := corev1.HostPathFile
-	securityContext := securityContextForBuild(strategy.Env)
+	buildSecurityContext := securityContextForBuild(strategy.Env)
+	initSecurityContext := builderMinSecurityContext()
 	pod := &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      buildutil.GetBuildPodName(build),
@@ -83,7 +84,7 @@ func (bs *SourceBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 					Image:                    bs.Image,
 					Args:                     []string{"openshift-sti-build"},
 					Env:                      copyEnvVarSlice(containerEnv),
-					SecurityContext:          securityContext,
+					SecurityContext:          buildSecurityContext,
 					TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
 					VolumeMounts: []corev1.VolumeMount{
 						{
@@ -137,7 +138,7 @@ func (bs *SourceBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 			Image:                    bs.Image,
 			Args:                     []string{"openshift-git-clone"},
 			Env:                      copyEnvVarSlice(containerEnv),
-			SecurityContext:          securityContext,
+			SecurityContext:          initSecurityContext,
 			TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
 			VolumeMounts: []corev1.VolumeMount{
 				{
@@ -156,12 +157,14 @@ func (bs *SourceBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 		pod.Spec.InitContainers = append(pod.Spec.InitContainers, gitCloneContainer)
 	}
 	if len(build.Spec.Source.Images) > 0 {
+		// We use buildah to extract the image content as source. Using the build security context
+		// since buildah needs to create its own isolation environment.
 		extractImageContentContainer := corev1.Container{
 			Name:                     ExtractImageContentContainer,
 			Image:                    bs.Image,
 			Args:                     []string{"openshift-extract-image-content"},
 			Env:                      copyEnvVarSlice(containerEnv),
-			SecurityContext:          securityContext,
+			SecurityContext:          buildSecurityContext,
 			TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
 			VolumeMounts: []corev1.VolumeMount{
 				{
@@ -190,7 +193,7 @@ func (bs *SourceBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 			Image:                    bs.Image,
 			Args:                     []string{"openshift-manage-dockerfile"},
 			Env:                      copyEnvVarSlice(containerEnv),
-			SecurityContext:          securityContext,
+			SecurityContext:          initSecurityContext,
 			TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
 			VolumeMounts: []corev1.VolumeMount{
 				{
@@ -216,7 +219,7 @@ func (bs *SourceBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 	setupContainersConfigs(build, pod)
 	setupBuildCAs(build, pod, additionalCAs, internalRegistryHost)
 	setupContainersStorage(pod, &pod.Spec.Containers[0])
-	if securityContext == nil || securityContext.Privileged == nil || !*securityContext.Privileged {
+	if buildSecurityContext == nil || buildSecurityContext.Privileged == nil || !*buildSecurityContext.Privileged {
 		setupBuilderAutonsUser(build, strategy.Env, pod)
 		setupBuilderDeviceFUSE(pod)
 	}

@@ -210,6 +210,7 @@ func testSTICreateBuildPod(t *testing.T, rootAllowed bool) {
 	}
 
 	checkAliasing(t, actual)
+	checkPodSecurityContexts(t, actual)
 }
 
 func TestS2IBuildLongName(t *testing.T) {

@@ -149,9 +149,6 @@ func mountVolume(pod *corev1.Pod, container *corev1.Container, objName, mountPat
 		}
 	}
 	mode := int32(0o600)
-	if container.SecurityContext == nil || container.SecurityContext.Privileged == nil || !*container.SecurityContext.Privileged {
-		mode = int32(0o644) // make sure unprivileged builders can read them
-	}
 	if !volumeExists {
 		volume := makeVolume(volumeName, objName, mode, volumeSourceType, volumeSource)
 		pod.Spec.Volumes = append(pod.Spec.Volumes, volume)
@@ -559,6 +556,45 @@ func securityContextForBuild(vars []corev1.EnvVar) *corev1.SecurityContext {
 	return securityContext
 }
 
+// builderMinSecurityContext returns a SecurityContext that has the minimum privileges needed to
+// run the openshift-builder-container's non-buildah actions. This includes cloning source code,
+// accepting source code from a command line, and manipulating a provided Dockerfile.
+//
+// These containers need to run as root in order to generate a full ca-trust chain from the mounted
+// cluster trust bundle. This trust chain is used to clone source code from private git
+// hosted with self/enterprise-internal SSL certificates, amongst other actions [1]. Linux
+// capabilities are otherwise minimized to reduce attack surfaces [2].
+//
+// See also:
+// [1] https://bugzilla.redhat.com/show_bug.cgi?id=1826183
+// [2] https://kubernetes.io/docs/concepts/security/pod-security-standards/#baseline
+func builderMinSecurityContext() *corev1.SecurityContext {
+	isNonRoot := false
+	isPrivileged := false
+	uidGid := int64(0)
+	// Try run as root, but only have permission to CHOWN files.
+	return &corev1.SecurityContext{
+		Privileged:   &isPrivileged,
+		RunAsNonRoot: &isNonRoot,
+		RunAsUser:    &uidGid,
+		RunAsGroup:   &uidGid,
+		SeccompProfile: &corev1.SeccompProfile{
+			Type: corev1.SeccompProfileTypeRuntimeDefault,
+		},
+		// DAC_OVERRIDE required to override the permission checks on ssh keys and .gitconfig files.
+		// TODO: Set appropriate file permission bits on ssh keys and .gitconfig files.
+		Capabilities: &corev1.Capabilities{
+			Drop: []corev1.Capability{
+				"ALL",
+			},
+			Add: []corev1.Capability{
+				"CHOWN",
+				"DAC_OVERRIDE",
+			},
+		},
+	}
+}
+
 // Add annotations that should tell CRI-O to provide /dev/fuse in the pod's
 // containers' device control group and in the /dev that the runtime will set
 // up for them.

@@ -110,6 +110,15 @@ func TestSetupDockerSecrets(t *testing.T) {
 			t.Errorf("Duplicate volume name %s", v.Name)
 		}
 		seenName[v.Name] = true
+
+		if v.VolumeSource.Secret == nil {
+			t.Errorf("expected volume %s to have source type Secret", v.Name)
+		} else {
+			defaultMode := v.VolumeSource.Secret.DefaultMode
+			if *defaultMode != int32(0600) {
+				t.Errorf("expected volume source to default file permissions to read-write-user (0600), got %o", *defaultMode)
+			}
+		}
 	}
 
 	if !seenName["my-pushSecret-with-full-stops-and-longer-than-six-c6eb4d75-push"] {
@@ -886,18 +895,18 @@ func testCreateBuildPodAutonsUser(t *testing.T, build *buildv1.Build, strategy b
 				t.Errorf("Unexpected error: %v", err)
 				return
 			}
-			for ctrIndex, ctr := range append(actual.Spec.Containers, actual.Spec.InitContainers...) {
+			for _, ctr := range append(actual.Spec.Containers, actual.Spec.InitContainers...) {
 				sc := ctr.SecurityContext
 				if sc == nil {
-					t.Errorf("Container %d in pod spec has no SecurityContext", ctrIndex)
+					t.Errorf("Container %s in pod spec has no SecurityContext", ctr.Name)
 					continue
 				}
 				if sc.Privileged == nil {
-					t.Errorf("Container %d in pod spec has no privileged field", ctrIndex)
+					t.Errorf("Container %s in pod spec has no privileged field", ctr.Name)
 					continue
 				}
-				if *sc.Privileged != testCase.privileged {
-					t.Errorf("Expected privileged: %q to produce privileged=%v, got %v", testCase.env, testCase.privileged, *sc.Privileged)
+				if isPrivilegedContainerAllowed(ctr.Name) && *sc.Privileged != testCase.privileged {
+					t.Errorf("Expected privileged: %q to produce privileged=%v for container %s, got %v", testCase.env, testCase.privileged, ctr.Name, *sc.Privileged)
 				}
 			}
 			for annotation, value := range testCase.annotations {
@@ -918,3 +927,115 @@ func testCreateBuildPodAutonsUser(t *testing.T, build *buildv1.Build, strategy b
 		})
 	}
 }
+
+func TestMinimalSecurityContext(t *testing.T) {
+	securityCtx := builderMinSecurityContext()
+	checkSecurityContextMeetsBaseline(t, "test", securityCtx)
+}
+
+var allowedCapabilities = []string{
+	"AUDIT_WRITE",
+	"CHOWN",
+	"DAC_OVERRIDE",
+	"FOWNER",
+	"FSETID",
+	"KILL",
+	"MKNOD",
+	"NET_BIND_SERVICE",
+	"SETFCAP",
+	"SETGID",
+	"SETPCAP",
+	"SETUID",
+	"SYS_CHROOT",
+}
+
+func isCapabilityAllowedBaseline(capability corev1.Capability) bool {
+	for _, allowed := range allowedCapabilities {
+		if capability == corev1.Capability(allowed) {
+			return true
+		}
+	}
+	return false
+}
+
+// checkSecurityContextMeetsBaseline verifes if the security context meets the "baseline" Pod Security Standard.
+func checkSecurityContextMeetsBaseline(t *testing.T, containerName string, securityCtx *corev1.SecurityContext) {
+	// Baseline pod security standard allows containers to run as root, but with extra protections
+	// to prevent common/known attack surfaces.
+
+	// Privileged containers are not allowed
+	if securityCtx.Privileged != nil && *securityCtx.Privileged {
+		t.Errorf("container %s should not be privileged", containerName)
+	}
+
+	// A subset of Linux capabilities are allowed for "baseline" pod security standard
+	if securityCtx.Capabilities != nil {
+		for _, cap := range securityCtx.Capabilities.Add {
+			if !isCapabilityAllowedBaseline(cap) {
+				t.Errorf("container %s adds privileged capability %q", containerName, cap)
+			}
+		}
+	}
+
+	// SELinux cannot be disabled, or use user/role overrides
+	if securityCtx.SELinuxOptions != nil {
+		seLinuxType := securityCtx.SELinuxOptions.Type
+		if seLinuxType != "container_t" && seLinuxType != "container_init_t" &&
+			seLinuxType != "container_kvm_t" && seLinuxType != "container_engine_t" {
+			t.Errorf("container %s uses privileged SELinux type %q", containerName, seLinuxType)
+		}
+		if len(securityCtx.SELinuxOptions.User) > 0 {
+			t.Errorf("container %s overrides SELinux user", containerName)
+		}
+		if len(securityCtx.SELinuxOptions.Role) > 0 {
+			t.Errorf("container %s overrides SELinux role", containerName)
+		}
+	}
+
+	// /proc mount should use defaults (masked)
+	if securityCtx.ProcMount != nil && *securityCtx.ProcMount != corev1.DefaultProcMount {
+		t.Errorf("container %s does not use default /proc mount type", containerName)
+	}
+
+	// Seccomp profile cannot be set to "Unconfined"
+	if securityCtx.SeccompProfile != nil && securityCtx.SeccompProfile.Type == corev1.SeccompProfileTypeUnconfined {
+		t.Errorf("container %s uses Unconfined Seccomp profile", containerName)
+	}
+
+}
+
+// checkPodSecurityContexts verifies if the build pod containers have appropriate security
+// contexts. Only a subset of build pod containers are allowed to use a non-restricted security
+// context.
+func checkPodSecurityContexts(t *testing.T, pod *corev1.Pod) {
+	for _, c := range pod.Spec.Containers {
+		if isPrivilegedContainerAllowed(c.Name) {
+			continue
+		}
+		checkSecurityContextMeetsBaseline(t, c.Name, c.SecurityContext)
+	}
+	for _, c := range pod.Spec.InitContainers {
+		if isPrivilegedContainerAllowed(c.Name) {
+			continue
+		}
+		checkSecurityContextMeetsBaseline(t, c.Name, c.SecurityContext)
+	}
+}
+
+// isPrivilegedContainerAllowed returns true if the container is allowed to run as privileged,
+// based on its name. Only the following containers in the build pod are allowed to run as
+// privileged:
+//
+// - DockerBuild
+// - StiBuild
+// - ExtractImageContentContainer
+//
+// TODO: Remove need for privileged containers by having cri-o mount /dev/fuse safely.
+func isPrivilegedContainerAllowed(containerName string) bool {
+	switch containerName {
+	case DockerBuild, StiBuild, ExtractImageContentContainer:
+		return true
+	default:
+		return false
+	}
+}