scc-admission: make the admission respect the securityContext.seccomp…

…Profile fields

Upstream moved seccomp configuration to GA in 1.19 and introduced a new field
in the securityContext. This needs to be taken into consideration during
SCC admission.

pkg/securitycontextconstraints/sccmatching/provider.go

@@ -3,20 +3,21 @@ package sccmatching
 import (
 	"fmt"
 
-	securityv1 "github.com/openshift/api/security/v1"
-	"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/capabilities"
-	"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/group"
-	"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/seccomp"
-	"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/selinux"
-	"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/user"
-	sccutil "github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/util"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/security/podsecuritypolicy/sysctl"
 	"k8s.io/kubernetes/pkg/securitycontext"
 	"k8s.io/kubernetes/pkg/util/maps"
+
+	securityv1 "github.com/openshift/api/security/v1"
+	"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/capabilities"
+	"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/group"
+	"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/seccomp"
+	"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/selinux"
+	"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/user"
+	sccutil "github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/util"
 )
 
 // used to pass in the field being validated for reusable group strategies so they
@@ -126,22 +127,17 @@ func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurit
 		sc.SetSELinuxOptions(seLinux)
 	}
 
-	// we only generate a seccomp annotation for the entire pod.  Validation
-	// will catch any container annotations that are invalid and containers
-	// will inherit the pod annotation.
-	_, hasPodProfile := pod.Annotations[api.SeccompPodAnnotationKey]
-	if !hasPodProfile {
-		profile, err := s.seccompStrategy.Generate(pod)
-		if err != nil {
-			return nil, nil, err
-		}
-
-		if profile != "" {
-			if annotationsCopy == nil {
-				annotationsCopy = map[string]string{}
-			}
-			annotationsCopy[api.SeccompPodAnnotationKey] = profile
+	// This is only generated on the pod level.  Containers inherit the pod's profile.  If the
+	// container has a specific profile set then it will be caught in the validation step.
+	seccompProfile, err := s.seccompStrategy.Generate(pod.Annotations, pod)
+	if err != nil {
+		return nil, nil, err
+	}
+	if seccompProfile != "" {
+		if annotationsCopy == nil {
+			annotationsCopy = map[string]string{}
 		}
+		annotationsCopy[api.SeccompPodAnnotationKey] = seccompProfile
 	}
 
 	return sc.PodSecurityContext(), annotationsCopy, nil
@@ -461,7 +457,7 @@ func createCapabilitiesStrategy(defaultAddCaps, requiredDropCaps, allowedCaps []
 
 // createSeccompStrategy creates a new seccomp strategy
 func createSeccompStrategy(allowedProfiles []string) (seccomp.SeccompStrategy, error) {
-	return seccomp.NewWithSeccompProfile(allowedProfiles)
+	return seccomp.NewSeccompStrategy(allowedProfiles), nil
 }
 
 // createSysctlsStrategy creates a new sysctls strategy

pkg/securitycontextconstraints/sccmatching/provider_test.go

@@ -8,13 +8,14 @@ import (
 
 	"github.com/stretchr/testify/require"
 
-	securityv1 "github.com/openshift/api/security/v1"
-	sccutil "github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/util"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	api "k8s.io/kubernetes/pkg/apis/core"
+
+	securityv1 "github.com/openshift/api/security/v1"
+	sccutil "github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/util"
 )
 
 func TestCreatePodSecurityContextNonmutating(t *testing.T) {
@@ -227,6 +228,9 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
 		},
 	}
 
+	failSeccompProfilePod := defaultPod()
+	failSeccompProfilePod.Annotations = map[string]string{api.SeccompPodAnnotationKey: "foo"}
+
 	errorCases := map[string]struct {
 		pod           *api.Pod
 		scc           *securityv1.SecurityContextConstraints
@@ -285,7 +289,7 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
 		"failInvalidSeccompPod": {
 			pod:           failInvalidSeccompProfile,
 			scc:           failInvalidSeccompProfileSCC,
-			expectedError: "bar is not a valid seccomp profile",
+			expectedError: "Forbidden: bar is not an allowed seccomp profile. Valid values are [foo]",
 		},
 		"failHostDirSCC": {
 			pod:           failHostDirPod,
@@ -317,6 +321,11 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
 			scc:           failAllUnsafeSysctlsSCC,
 			expectedError: "unsafe sysctl \"kernel.sem\" is not allowed",
 		},
+		"failInvalidSeccomp": {
+			pod:           failSeccompProfilePod,
+			scc:           defaultSCC(),
+			expectedError: "Forbidden: seccomp may not be set",
+		},
 	}
 	for k, v := range errorCases {
 		provider, err := NewSimpleProvider(v.scc)
@@ -388,6 +397,16 @@ func TestValidateContainerSecurityContextFailures(t *testing.T) {
 	failInvalidSeccompProfileSCC := defaultSCC()
 	failInvalidSeccompProfileSCC.SeccompProfiles = []string{"foo"}
 
+	failSeccompPod := defaultPod()
+	failSeccompPod.Annotations = map[string]string{
+		api.SeccompContainerAnnotationKeyPrefix + failSeccompPod.Spec.Containers[0].Name: "foo",
+	}
+
+	failSeccompPodInheritPodAnnotation := defaultPod()
+	failSeccompPodInheritPodAnnotation.Annotations = map[string]string{
+		api.SeccompPodAnnotationKey: "foo",
+	}
+
 	errorCases := map[string]struct {
 		pod           *api.Pod
 		scc           *securityv1.SecurityContextConstraints
@@ -436,7 +455,17 @@ func TestValidateContainerSecurityContextFailures(t *testing.T) {
 		"failInvalidSeccompPod": {
 			pod:           failInvalidSeccompProfile,
 			scc:           failInvalidSeccompProfileSCC,
-			expectedError: "bar is not a valid seccomp profile",
+			expectedError: "Forbidden: bar is not an allowed seccomp profile. Valid values are [foo]",
+		},
+		"failSeccompContainerAnnotation": {
+			pod:           failSeccompPod,
+			scc:           defaultSCC(),
+			expectedError: "Forbidden: seccomp may not be set",
+		},
+		"failSeccompContainerPodAnnotation": {
+			pod:           failSeccompPodInheritPodAnnotation,
+			scc:           defaultSCC(),
+			expectedError: "Forbidden: seccomp may not be set",
 		},
 	}
 
@@ -557,6 +586,14 @@ func TestValidatePodSecurityContextSuccess(t *testing.T) {
 		},
 	}
 
+	seccompSCC := defaultSCC()
+	seccompSCC.SeccompProfiles = []string{"foo"}
+
+	seccompPod := defaultPod()
+	seccompPod.Annotations = map[string]string{
+		api.SeccompPodAnnotationKey: "foo",
+	}
+
 	successCases := map[string]struct {
 		pod *api.Pod
 		scc *securityv1.SecurityContextConstraints
@@ -625,6 +662,10 @@ func TestValidatePodSecurityContextSuccess(t *testing.T) {
 			pod: unsafeSysctlKernelPod,
 			scc: sysctlAllowAllSCC,
 		},
+		"pass seccomp validating SCC": {
+			pod: seccompPod,
+			scc: seccompSCC,
+		},
 	}
 
 	for k, v := range successCases {
@@ -726,6 +767,17 @@ func TestValidateContainerSecurityContextSuccess(t *testing.T) {
 	seccompFooPod := defaultPod()
 	seccompFooPod.Annotations[api.SeccompContainerAnnotationKeyPrefix+seccompFooPod.Spec.Containers[0].Name] = "foo"
 
+	seccompPod := defaultPod()
+	seccompPod.Annotations = map[string]string{
+		api.SeccompPodAnnotationKey: "foo",
+		api.SeccompContainerAnnotationKeyPrefix + seccompPod.Spec.Containers[0].Name: "foo",
+	}
+
+	seccompPodInherit := defaultPod()
+	seccompPodInherit.Annotations = map[string]string{
+		api.SeccompPodAnnotationKey: "foo",
+	}
+
 	successCases := map[string]struct {
 		pod *api.Pod
 		scc *securityv1.SecurityContextConstraints