From 3826d2598ccdda80a4559b32d450239ae3677cf3 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jun 2016 10:02:36 +0200 Subject: [PATCH] Move seccomp annotation validation into api/validation --- pkg/api/helpers.go | 8 ++ pkg/api/validation/validation.go | 29 +++++ pkg/api/validation/validation_test.go | 140 ++++++++++++++++++++++++ pkg/kubelet/dockertools/manager.go | 12 +- pkg/kubelet/dockertools/manager_test.go | 38 ++----- test/e2e/security_context.go | 16 +-- 6 files changed, 196 insertions(+), 47 deletions(-) diff --git a/pkg/api/helpers.go b/pkg/api/helpers.go index d173c1fb887..1349ef62b0b 100644 --- a/pkg/api/helpers.go +++ b/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 diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index a24ef85ca36..3b61fcc812e 100644 --- a/pkg/api/validation/validation.go +++ b/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{} diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 5ca2d8fa092..474bbe88e33 100644 --- a/pkg/api/validation/validation_test.go +++ b/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 { diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 1dd213db595..6e3bc3440b5 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/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/") - cleanName := strings.TrimPrefix(path.Clean("/"+name), "/") - if name != cleanName { - return nil, fmt.Errorf("invalid seccomp profile name: %s", name) - } - fname := filepath.Join(dm.seccompProfileRoot, filepath.FromSlash(cleanName)) + name := strings.TrimPrefix(profile, "localhost/") // by pod annotation validation, name is a valid subpath + fname := filepath.Join(dm.seccompProfileRoot, filepath.FromSlash(name)) file, err := ioutil.ReadFile(fname) if err != nil { return nil, fmt.Errorf("cannot load seccomp profile %q: %v", name, err) diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index 09bba49f408..6924728780f 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/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", - "container.seccomp.security.alpha.kubernetes.io/bar2": "docker/default", + api.SeccompPodAnnotationKey: "unconfined", + 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 { diff --git a/test/e2e/security_context.go b/test/e2e/security_context.go index af9d3bac33f..f9a64f758c0 100644 --- a/test/e2e/security_context.go +++ b/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["seccomp.security.alpha.kubernetes.io/pod"] = "docker/default" + pod.Annotations[api.SeccompContainerAnnotationKeyPrefix+"test-container"] = "unconfined" + 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 }) })