Move seccomp annotation validation into api/validation

pkg/api/helpers.go

@@ -416,6 +416,14 @@ const (
 	// TaintsAnnotationKey represents the key of taints data (json serialized)
 	// in the Annotations of a Node.
 	TaintsAnnotationKey string = "scheduler.alpha.kubernetes.io/taints"
+
+	// SeccompPodAnnotationKey represents the key of a seccomp profile applied
+	// to all containers of a pod.
+	SeccompPodAnnotationKey string = "seccomp.security.alpha.kubernetes.io/pod"
+
+	// SeccompContainerAnnotationKeyPrefix represents the key of a seccomp profile applied
+	// to one container of a pod.
+	SeccompContainerAnnotationKeyPrefix string = "container.seccomp.security.alpha.kubernetes.io/"
 )
 
 // GetAffinityFromPod gets the json serialized affinity data from Pod.Annotations

pkg/api/validation/validation.go

@@ -122,6 +122,8 @@ func ValidatePodSpecificAnnotations(annotations map[string]string, fldPath *fiel
 		}
 	}
 
+	allErrs = append(allErrs, ValidateSeccompPodAnnotations(annotations, fldPath)...)
+
 	return allErrs
 }
 
@@ -1846,6 +1848,33 @@ func ValidateTolerationsInPodAnnotations(annotations map[string]string, fldPath
 	return allErrs
 }
 
+func validateSeccompProfile(p string, fldPath *field.Path) field.ErrorList {
+	if p == "docker/default" {
+		return nil
+	}
+	if p == "unconfined" {
+		return nil
+	}
+	if strings.HasPrefix(p, "localhost/") {
+		return validateSubPath(strings.TrimPrefix(p, "localhost/"), fldPath)
+	}
+	return field.ErrorList{field.Invalid(fldPath, p, "must be a valid seccomp profile")}
+}
+
+func ValidateSeccompPodAnnotations(annotations map[string]string, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	if p, exists := annotations[api.SeccompPodAnnotationKey]; exists {
+		allErrs = append(allErrs, validateSeccompProfile(p, fldPath.Child(api.SeccompPodAnnotationKey))...)
+	}
+	for k, p := range annotations {
+		if strings.HasPrefix(k, api.SeccompContainerAnnotationKeyPrefix) {
+			allErrs = append(allErrs, validateSeccompProfile(p, fldPath.Child(k))...)
+		}
+	}
+
+	return allErrs
+}
+
 // ValidatePodSecurityContext test that the specified PodSecurityContext has valid data.
 func ValidatePodSecurityContext(securityContext *api.PodSecurityContext, spec *api.PodSpec, specPath, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}

pkg/api/validation/validation_test.go

@@ -2299,6 +2299,62 @@ func TestValidatePod(t *testing.T) {
 				DNSPolicy:     api.DNSClusterFirst,
 			},
 		},
+		{ // docker default seccomp profile
+			ObjectMeta: api.ObjectMeta{
+				Name:      "123",
+				Namespace: "ns",
+				Annotations: map[string]string{
+					api.SeccompPodAnnotationKey: "docker/default",
+				},
+			},
+			Spec: api.PodSpec{
+				Containers:    []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
+				RestartPolicy: api.RestartPolicyAlways,
+				DNSPolicy:     api.DNSClusterFirst,
+			},
+		},
+		{ // unconfined seccomp profile
+			ObjectMeta: api.ObjectMeta{
+				Name:      "123",
+				Namespace: "ns",
+				Annotations: map[string]string{
+					api.SeccompPodAnnotationKey: "unconfined",
+				},
+			},
+			Spec: api.PodSpec{
+				Containers:    []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
+				RestartPolicy: api.RestartPolicyAlways,
+				DNSPolicy:     api.DNSClusterFirst,
+			},
+		},
+		{ // localhost seccomp profile
+			ObjectMeta: api.ObjectMeta{
+				Name:      "123",
+				Namespace: "ns",
+				Annotations: map[string]string{
+					api.SeccompPodAnnotationKey: "localhost/foo",
+				},
+			},
+			Spec: api.PodSpec{
+				Containers:    []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
+				RestartPolicy: api.RestartPolicyAlways,
+				DNSPolicy:     api.DNSClusterFirst,
+			},
+		},
+		{ // localhost seccomp profile for a container
+			ObjectMeta: api.ObjectMeta{
+				Name:      "123",
+				Namespace: "ns",
+				Annotations: map[string]string{
+					api.SeccompContainerAnnotationKeyPrefix + "foo": "localhost/foo",
+				},
+			},
+			Spec: api.PodSpec{
+				Containers:    []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
+				RestartPolicy: api.RestartPolicyAlways,
+				DNSPolicy:     api.DNSClusterFirst,
+			},
+		},
 	}
 	for _, pod := range successCases {
 		if errs := ValidatePod(&pod); len(errs) != 0 {
@@ -2711,6 +2767,90 @@ func TestValidatePod(t *testing.T) {
 				DNSPolicy:     api.DNSClusterFirst,
 			},
 		},
+		"must be a valid pod seccomp profile": {
+			ObjectMeta: api.ObjectMeta{
+				Name:      "123",
+				Namespace: "ns",
+				Annotations: map[string]string{
+					api.SeccompPodAnnotationKey: "foo",
+				},
+			},
+			Spec: api.PodSpec{
+				Containers:    []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
+				RestartPolicy: api.RestartPolicyAlways,
+				DNSPolicy:     api.DNSClusterFirst,
+			},
+		},
+		"must be a valid container seccomp profile": {
+			ObjectMeta: api.ObjectMeta{
+				Name:      "123",
+				Namespace: "ns",
+				Annotations: map[string]string{
+					api.SeccompContainerAnnotationKeyPrefix + "foo": "foo",
+				},
+			},
+			Spec: api.PodSpec{
+				Containers:    []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
+				RestartPolicy: api.RestartPolicyAlways,
+				DNSPolicy:     api.DNSClusterFirst,
+			},
+		},
+		"must be a non-empty container name in seccomp annotation": {
+			ObjectMeta: api.ObjectMeta{
+				Name:      "123",
+				Namespace: "ns",
+				Annotations: map[string]string{
+					api.SeccompContainerAnnotationKeyPrefix: "foo",
+				},
+			},
+			Spec: api.PodSpec{
+				Containers:    []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
+				RestartPolicy: api.RestartPolicyAlways,
+				DNSPolicy:     api.DNSClusterFirst,
+			},
+		},
+		"must be a non-empty container profile in seccomp annotation": {
+			ObjectMeta: api.ObjectMeta{
+				Name:      "123",
+				Namespace: "ns",
+				Annotations: map[string]string{
+					api.SeccompContainerAnnotationKeyPrefix + "foo": "",
+				},
+			},
+			Spec: api.PodSpec{
+				Containers:    []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
+				RestartPolicy: api.RestartPolicyAlways,
+				DNSPolicy:     api.DNSClusterFirst,
+			},
+		},
+		"must be a relative path in a node-local seccomp profile annotation": {
+			ObjectMeta: api.ObjectMeta{
+				Name:      "123",
+				Namespace: "ns",
+				Annotations: map[string]string{
+					api.SeccompPodAnnotationKey: "localhost//foo",
+				},
+			},
+			Spec: api.PodSpec{
+				Containers:    []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
+				RestartPolicy: api.RestartPolicyAlways,
+				DNSPolicy:     api.DNSClusterFirst,
+			},
+		},
+		"must not start with '../'": {
+			ObjectMeta: api.ObjectMeta{
+				Name:      "123",
+				Namespace: "ns",
+				Annotations: map[string]string{
+					api.SeccompPodAnnotationKey: "localhost/../foo",
+				},
+			},
+			Spec: api.PodSpec{
+				Containers:    []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}},
+				RestartPolicy: api.RestartPolicyAlways,
+				DNSPolicy:     api.DNSClusterFirst,
+			},
+		},
 	}
 	for k, v := range errorCases {
 		if errs := ValidatePod(&v); len(errs) == 0 {

pkg/kubelet/dockertools/manager.go

@@ -991,10 +991,10 @@ func (dm *DockerManager) getSecurityOpt(pod *api.Pod, ctrName string) ([]string,
 		return nil, nil
 	}
 
-	profile, profileOK := pod.ObjectMeta.Annotations["container.seccomp.security.alpha.kubernetes.io/"+ctrName]
+	profile, profileOK := pod.ObjectMeta.Annotations[api.SeccompContainerAnnotationKeyPrefix+ctrName]
 	if !profileOK {
 		// try the pod profile
-		profile, profileOK = pod.ObjectMeta.Annotations["seccomp.security.alpha.kubernetes.io/pod"]
+		profile, profileOK = pod.ObjectMeta.Annotations[api.SeccompPodAnnotationKey]
 		if !profileOK {
 			// return early the default
 			return defaultSecurityOpt, nil
@@ -1015,12 +1015,8 @@ func (dm *DockerManager) getSecurityOpt(pod *api.Pod, ctrName string) ([]string,
 		return nil, fmt.Errorf("unknown seccomp profile option: %s", profile)
 	}
 
-	name := strings.TrimPrefix(profile, "localhost/")
+	name := strings.TrimPrefix(profile, "localhost/") // by pod annotation validation, name is a valid subpath
-	cleanName := strings.TrimPrefix(path.Clean("/"+name), "/")
+	fname := filepath.Join(dm.seccompProfileRoot, filepath.FromSlash(name))
-	if name != cleanName {
-		return nil, fmt.Errorf("invalid seccomp profile name: %s", name)
-	}
-	fname := filepath.Join(dm.seccompProfileRoot, filepath.FromSlash(cleanName))
 	file, err := ioutil.ReadFile(fname)
 	if err != nil {
 		return nil, fmt.Errorf("cannot load seccomp profile %q: %v", name, err)

pkg/kubelet/dockertools/manager_test.go

@@ -1764,7 +1764,7 @@ func TestUnconfinedSeccompProfileWithDockerV110(t *testing.T) {
 			Name:      "foo4",
 			Namespace: "new",
 			Annotations: map[string]string{
-				"seccomp.security.alpha.kubernetes.io/pod": "unconfined",
+				api.SeccompPodAnnotationKey: "unconfined",
 			},
 		},
 		Spec: api.PodSpec{
@@ -1806,7 +1806,7 @@ func TestDefaultSeccompProfileWithDockerV110(t *testing.T) {
 			Name:      "foo1",
 			Namespace: "new",
 			Annotations: map[string]string{
-				"seccomp.security.alpha.kubernetes.io/pod": "docker/default",
+				api.SeccompPodAnnotationKey: "docker/default",
 			},
 		},
 		Spec: api.PodSpec{
@@ -1848,8 +1848,8 @@ func TestSeccompContainerAnnotationTrumpsPod(t *testing.T) {
 			Name:      "foo2",
 			Namespace: "new",
 			Annotations: map[string]string{
-				"seccomp.security.alpha.kubernetes.io/pod":            "unconfined",
+				api.SeccompPodAnnotationKey:                      "unconfined",
-				"container.seccomp.security.alpha.kubernetes.io/bar2": "docker/default",
+				api.SeccompContainerAnnotationKeyPrefix + "bar2": "docker/default",
 			},
 		},
 		Spec: api.PodSpec{
@@ -1891,46 +1891,22 @@ func TestSeccompLocalhostProfileIsLoaded(t *testing.T) {
 	}{
 		{
 			annotations: map[string]string{
-				"seccomp.security.alpha.kubernetes.io/pod": "localhost/test",
+				api.SeccompPodAnnotationKey: "localhost/test",
 			},
 			expectedSecOpt: `seccomp={"foo":"bar"}`,
 		},
 		{
 			annotations: map[string]string{
-				"seccomp.security.alpha.kubernetes.io/pod": "localhost/sub/subtest",
+				api.SeccompPodAnnotationKey: "localhost/sub/subtest",
 			},
 			expectedSecOpt: `seccomp={"abc":"def"}`,
 		},
 		{
 			annotations: map[string]string{
-				"seccomp.security.alpha.kubernetes.io/pod": "localhost/not-existing",
+				api.SeccompPodAnnotationKey: "localhost/not-existing",
 			},
 			expectedError: "cannot load seccomp profile",
 		},
-		{
-			annotations: map[string]string{
-				"seccomp.security.alpha.kubernetes.io/pod": "localhost/../test",
-			},
-			expectedError: "invalid seccomp profile name",
-		},
-		{
-			annotations: map[string]string{
-				"seccomp.security.alpha.kubernetes.io/pod": "localhost//test",
-			},
-			expectedError: "invalid seccomp profile name",
-		},
-		{
-			annotations: map[string]string{
-				"seccomp.security.alpha.kubernetes.io/pod": "localhost/sub//subtest",
-			},
-			expectedError: "invalid seccomp profile name",
-		},
-		{
-			annotations: map[string]string{
-				"seccomp.security.alpha.kubernetes.io/pod": "localhost/test/",
-			},
-			expectedError: "invalid seccomp profile name",
-		},
 	}
 
 	for _, test := range tests {

test/e2e/security_context.go

@@ -110,33 +110,33 @@ var _ = framework.KubeDescribe("Security Context [Feature:SecurityContext]", fun
 	It("should support seccomp alpha unconfined annotation on the container [Feature:Seccomp]", func() {
 		// TODO: port to SecurityContext as soon as seccomp is out of alpha
 		pod := scTestPod(false, false)
-		pod.Annotations["container.seccomp.security.alpha.kubernetes.io/test-container"] = "unconfined"
+		pod.Annotations[api.SeccompContainerAnnotationKeyPrefix+"test-container"] = "unconfined"
-		pod.Annotations["seccomp.security.alpha.kubernetes.io/pod"] = "docker/default"
+		pod.Annotations[api.SeccompPodAnnotationKey] = "docker/default"
 		pod.Spec.Containers[0].Command = []string{"grep", "ecc", "/proc/self/status"}
-		f.TestContainerOutput("pod.Spec.SecurityContext.Seccomp", pod, 0, []string{"0"}) // seccomp disabled
+		f.TestContainerOutput(api.SeccompPodAnnotationKey, pod, 0, []string{"0"}) // seccomp disabled
 	})
 
 	It("should support seccomp alpha unconfined annotation on the pod [Feature:Seccomp]", func() {
 		// TODO: port to SecurityContext as soon as seccomp is out of alpha
 		pod := scTestPod(false, false)
-		pod.Annotations["seccomp.security.alpha.kubernetes.io/pod"] = "unconfined"
+		pod.Annotations[api.SeccompPodAnnotationKey] = "unconfined"
 		pod.Spec.Containers[0].Command = []string{"grep", "ecc", "/proc/self/status"}
-		f.TestContainerOutput("pod.Spec.SecurityContext.Seccomp", pod, 0, []string{"0"}) // seccomp disabled
+		f.TestContainerOutput(api.SeccompPodAnnotationKey, pod, 0, []string{"0"}) // seccomp disabled
 	})
 
 	It("should support seccomp alpha docker/default annotation [Feature:Seccomp]", func() {
 		// TODO: port to SecurityContext as soon as seccomp is out of alpha
 		pod := scTestPod(false, false)
-		pod.Annotations["container.seccomp.security.alpha.kubernetes.io/test-container"] = "docker/default"
+		pod.Annotations[api.SeccompContainerAnnotationKeyPrefix+"test-container"] = "docker/default"
 		pod.Spec.Containers[0].Command = []string{"grep", "ecc", "/proc/self/status"}
-		f.TestContainerOutput("pod.Spec.SecurityContext.Seccomp", pod, 0, []string{"2"}) // seccomp filtered
+		f.TestContainerOutput(api.SeccompPodAnnotationKey, pod, 0, []string{"2"}) // seccomp filtered
 	})
 
 	It("should support seccomp default which is unconfined [Feature:Seccomp]", func() {
 		// TODO: port to SecurityContext as soon as seccomp is out of alpha
 		pod := scTestPod(false, false)
 		pod.Spec.Containers[0].Command = []string{"grep", "ecc", "/proc/self/status"}
-		f.TestContainerOutput("pod.Spec.SecurityContext.Seccomp", pod, 0, []string{"0"}) // seccomp disabled
+		f.TestContainerOutput(api.SeccompPodAnnotationKey, pod, 0, []string{"0"}) // seccomp disabled
 	})
 })