Merge pull request #11669 from pweil-/sc-nonroot

add non-root directive to SC and kubelet checking

api/swagger-spec/v1.json

@@ -12782,6 +12782,10 @@
       "type": "integer",
       "format": "int64",
       "description": "the user id that runs the first process in the container; see http://releases.k8s.io/HEAD/docs/design/security_context.md#security-context"
+     },
+     "runAsNonRoot": {
+      "type": "boolean",
+      "description": "indicates the container must be run as a non-root user either by specifying the runAsUser or in the image specification"
      }
     }
    },

pkg/api/deep_copy_generated.go

@@ -1803,6 +1803,7 @@ func deepCopy_api_SecurityContext(in SecurityContext, out *SecurityContext, c *c
 	} else {
 		out.RunAsUser = nil
 	}
+	out.RunAsNonRoot = in.RunAsNonRoot
 	return nil
 }
 

pkg/api/types.go

@@ -2166,6 +2166,11 @@ type SecurityContext struct {
 
 	// RunAsUser is the UID to run the entrypoint of the container process.
 	RunAsUser *int64 `json:"runAsUser,omitempty"`
+
+	// RunAsNonRoot indicates that the container should be run as a non-root user.  If the RunAsUser
+	// field is not explicitly set then the kubelet may check the image for a specified user or
+	// perform defaulting to specify a user.
+	RunAsNonRoot bool
 }
 
 // SELinuxOptions are the labels to be applied to the container.

pkg/api/v1/conversion_generated.go

@@ -2002,6 +2002,7 @@ func convert_api_SecurityContext_To_v1_SecurityContext(in *api.SecurityContext,
 	} else {
 		out.RunAsUser = nil
 	}
+	out.RunAsNonRoot = in.RunAsNonRoot
 	return nil
 }
 
@@ -4344,6 +4345,7 @@ func convert_v1_SecurityContext_To_api_SecurityContext(in *SecurityContext, out
 	} else {
 		out.RunAsUser = nil
 	}
+	out.RunAsNonRoot = in.RunAsNonRoot
 	return nil
 }
 

pkg/api/v1/deep_copy_generated.go

@@ -1812,6 +1812,7 @@ func deepCopy_v1_SecurityContext(in SecurityContext, out *SecurityContext, c *co
 	} else {
 		out.RunAsUser = nil
 	}
+	out.RunAsNonRoot = in.RunAsNonRoot
 	return nil
 }
 

pkg/api/v1/types.go

@@ -2030,6 +2030,11 @@ type SecurityContext struct {
 
 	// RunAsUser is the UID to run the entrypoint of the container process.
 	RunAsUser *int64 `json:"runAsUser,omitempty" description:"the user id that runs the first process in the container; see http://releases.k8s.io/HEAD/docs/design/security_context.md#security-context"`
+
+	// RunAsNonRoot indicates that the container should be run as a non-root user.  If the RunAsUser
+	// field is not explicitly set then the kubelet may check the image for a specified user or
+	// perform defaulting to specify a user.
+	RunAsNonRoot bool `json:"runAsNonRoot,omitempty" description:"indicates the container must be run as a non-root user either by specifying the runAsUser or in the image specification"`
 }
 
 // SELinuxOptions are the labels to be applied to the container

pkg/kubelet/dockertools/manager.go

@@ -1619,6 +1619,15 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, pod
 			continue
 		}
 
+		if container.SecurityContext != nil && container.SecurityContext.RunAsNonRoot {
+			err := dm.verifyNonRoot(container)
+			dm.updateReasonCache(pod, container, err)
+			if err != nil {
+				glog.Errorf("Error running pod %q container %q: %v", kubecontainer.GetPodFullName(pod), container.Name, err)
+				continue
+			}
+		}
+
 		// TODO(dawnchen): Check RestartPolicy.DelaySeconds before restart a container
 		namespaceMode := fmt.Sprintf("container:%v", podInfraContainerID)
 		_, err = dm.runContainerInPod(pod, container, namespaceMode, namespaceMode)
@@ -1635,3 +1644,62 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, pod
 
 	return nil
 }
+
+// verifyNonRoot returns an error if the container or image will run as the root user.
+func (dm *DockerManager) verifyNonRoot(container *api.Container) error {
+	if securitycontext.HasRunAsUser(container) {
+		if securitycontext.HasRootRunAsUser(container) {
+			return fmt.Errorf("container's runAsUser breaks non-root policy")
+		}
+		return nil
+	}
+
+	imgRoot, err := dm.isImageRoot(container.Image)
+	if err != nil {
+		return err
+	}
+	if imgRoot {
+		return fmt.Errorf("container has no runAsUser and image will run as root")
+	}
+
+	return nil
+}
+
+// isImageRoot returns true if the user directive is not set on the image, the user is set to 0
+// or the user is set to root.  If there is an error inspecting the image this method will return
+// false and return the error.
+func (dm *DockerManager) isImageRoot(image string) (bool, error) {
+	img, err := dm.client.InspectImage(image)
+	if err != nil {
+		return false, err
+	}
+	if img == nil || img.Config == nil {
+		return false, fmt.Errorf("unable to inspect image %s, nil Config", image)
+	}
+
+	user := getUidFromUser(img.Config.User)
+	// if no user is defined container will run as root
+	if user == "" {
+		return true, nil
+	}
+	// do not allow non-numeric user directives
+	uid, err := strconv.Atoi(user)
+	if err != nil {
+		return false, fmt.Errorf("unable to validate image is non-root, non-numeric user (%s) is not allowed", user)
+	}
+	// user is numeric, check for 0
+	return uid == 0, nil
+}
+
+// getUidFromUser splits the uid out of a uid:gid string.
+func getUidFromUser(id string) string {
+	if id == "" {
+		return id
+	}
+	// split instances where the id may contain uid:gid
+	if strings.Contains(id, ":") {
+		return strings.Split(id, ":")[0]
+	}
+	// no gid, just return the id
+	return id
+}

pkg/kubelet/dockertools/manager_test.go

@@ -2085,3 +2085,129 @@ func TestGetPodStatusSortedContainers(t *testing.T) {
 		}
 	}
 }
+
+func TestVerifyNonRoot(t *testing.T) {
+	dm, fakeDocker := newTestDockerManager()
+
+	// setup test cases.
+	var rootUid int64 = 0
+	var nonRootUid int64 = 1
+
+	tests := map[string]struct {
+		container     *api.Container
+		inspectImage  *docker.Image
+		expectedError string
+	}{
+		// success cases
+		"non-root runAsUser": {
+			container: &api.Container{
+				SecurityContext: &api.SecurityContext{
+					RunAsUser: &nonRootUid,
+				},
+			},
+		},
+		"numeric non-root image user": {
+			container: &api.Container{},
+			inspectImage: &docker.Image{
+				Config: &docker.Config{
+					User: "1",
+				},
+			},
+		},
+		"numeric non-root image user with gid": {
+			container: &api.Container{},
+			inspectImage: &docker.Image{
+				Config: &docker.Config{
+					User: "1:2",
+				},
+			},
+		},
+
+		// failure cases
+		"root runAsUser": {
+			container: &api.Container{
+				SecurityContext: &api.SecurityContext{
+					RunAsUser: &rootUid,
+				},
+			},
+			expectedError: "container's runAsUser breaks non-root policy",
+		},
+		"non-numeric image user": {
+			container: &api.Container{},
+			inspectImage: &docker.Image{
+				Config: &docker.Config{
+					User: "foo",
+				},
+			},
+			expectedError: "unable to validate image is non-root, non-numeric user",
+		},
+		"numeric root image user": {
+			container: &api.Container{},
+			inspectImage: &docker.Image{
+				Config: &docker.Config{
+					User: "0",
+				},
+			},
+			expectedError: "container has no runAsUser and image will run as root",
+		},
+		"numeric root image user with gid": {
+			container: &api.Container{},
+			inspectImage: &docker.Image{
+				Config: &docker.Config{
+					User: "0:1",
+				},
+			},
+			expectedError: "container has no runAsUser and image will run as root",
+		},
+		"nil image in inspect": {
+			container:     &api.Container{},
+			expectedError: "unable to inspect image",
+		},
+		"nil config in image inspect": {
+			container:     &api.Container{},
+			inspectImage:  &docker.Image{},
+			expectedError: "unable to inspect image",
+		},
+	}
+
+	for k, v := range tests {
+		fakeDocker.Image = v.inspectImage
+		err := dm.verifyNonRoot(v.container)
+		if v.expectedError == "" && err != nil {
+			t.Errorf("%s had unexpected error %v", k, err)
+		}
+		if v.expectedError != "" && !strings.Contains(err.Error(), v.expectedError) {
+			t.Errorf("%s expected error %s but received %s", k, v.expectedError, err.Error())
+		}
+	}
+}
+
+func TestGetUidFromUser(t *testing.T) {
+	tests := map[string]struct {
+		input  string
+		expect string
+	}{
+		"no gid": {
+			input:  "0",
+			expect: "0",
+		},
+		"uid/gid": {
+			input:  "0:1",
+			expect: "0",
+		},
+		"empty input": {
+			input:  "",
+			expect: "",
+		},
+		"multiple spearators": {
+			input:  "1:2:3",
+			expect: "1",
+		},
+	}
+	for k, v := range tests {
+		actual := getUidFromUser(v.input)
+		if actual != v.expect {
+			t.Errorf("%s failed.  Expected %s but got %s", k, v.expect, actual)
+		}
+	}
+}

pkg/kubelet/rkt/rkt.go

@@ -856,6 +856,8 @@ func (r *runtime) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, podStatus
 			continue
 		}
 
+		// TODO: check for non-root image directives.  See ../docker/manager.go#SyncPod
+
 		// TODO(yifan): Take care of host network change.
 		containerChanged := c.Hash != 0 && c.Hash != expectedHash
 		if containerChanged {

pkg/securitycontext/util.go

@@ -66,3 +66,24 @@ func ParseSELinuxOptions(context string) (*api.SELinuxOptions, error) {
 		Level: fields[3],
 	}, nil
 }
+
+// HasNonRootUID returns true if the runAsUser is set and is greater than 0.
+func HasRootUID(container *api.Container) bool {
+	if container.SecurityContext == nil {
+		return false
+	}
+	if container.SecurityContext.RunAsUser == nil {
+		return false
+	}
+	return *container.SecurityContext.RunAsUser == 0
+}
+
+// HasRunAsUser determines if the sc's runAsUser field is set.
+func HasRunAsUser(container *api.Container) bool {
+	return container.SecurityContext != nil && container.SecurityContext.RunAsUser != nil
+}
+
+// HasRootRunAsUser returns true if the run as user is set and it is set to 0.
+func HasRootRunAsUser(container *api.Container) bool {
+	return HasRunAsUser(container) && HasRootUID(container)
+}

pkg/securitycontext/util_test.go

@@ -83,3 +83,124 @@ func compareContexts(name string, ex, ac *api.SELinuxOptions, t *testing.T) {
 		t.Errorf("%v: expected level: %v, got: %v", name, e, a)
 	}
 }
+
+func TestHaRootUID(t *testing.T) {
+	var nonRoot int64 = 1
+	var root int64 = 0
+
+	tests := map[string]struct {
+		container *api.Container
+		expect    bool
+	}{
+		"nil sc": {
+			container: &api.Container{SecurityContext: nil},
+		},
+		"nil runAsuser": {
+			container: &api.Container{
+				SecurityContext: &api.SecurityContext{
+					RunAsUser: nil,
+				},
+			},
+		},
+		"runAsUser non-root": {
+			container: &api.Container{
+				SecurityContext: &api.SecurityContext{
+					RunAsUser: &nonRoot,
+				},
+			},
+		},
+		"runAsUser root": {
+			container: &api.Container{
+				SecurityContext: &api.SecurityContext{
+					RunAsUser: &root,
+				},
+			},
+			expect: true,
+		},
+	}
+
+	for k, v := range tests {
+		actual := HasRootUID(v.container)
+		if actual != v.expect {
+			t.Errorf("%s failed, expected %t but received %t", k, v.expect, actual)
+		}
+	}
+}
+
+func TestHasRunAsUser(t *testing.T) {
+	var runAsUser int64 = 0
+
+	tests := map[string]struct {
+		container *api.Container
+		expect    bool
+	}{
+		"nil sc": {
+			container: &api.Container{SecurityContext: nil},
+		},
+		"nil runAsUser": {
+			container: &api.Container{
+				SecurityContext: &api.SecurityContext{
+					RunAsUser: nil,
+				},
+			},
+		},
+		"valid runAsUser": {
+			container: &api.Container{
+				SecurityContext: &api.SecurityContext{
+					RunAsUser: &runAsUser,
+				},
+			},
+			expect: true,
+		},
+	}
+
+	for k, v := range tests {
+		actual := HasRunAsUser(v.container)
+		if actual != v.expect {
+			t.Errorf("%s failed, expected %t but received %t", k, v.expect, actual)
+		}
+	}
+}
+
+func TestHasRootRunAsUser(t *testing.T) {
+	var nonRoot int64 = 1
+	var root int64 = 0
+
+	tests := map[string]struct {
+		container *api.Container
+		expect    bool
+	}{
+		"nil sc": {
+			container: &api.Container{SecurityContext: nil},
+		},
+		"nil runAsuser": {
+			container: &api.Container{
+				SecurityContext: &api.SecurityContext{
+					RunAsUser: nil,
+				},
+			},
+		},
+		"runAsUser non-root": {
+			container: &api.Container{
+				SecurityContext: &api.SecurityContext{
+					RunAsUser: &nonRoot,
+				},
+			},
+		},
+		"runAsUser root": {
+			container: &api.Container{
+				SecurityContext: &api.SecurityContext{
+					RunAsUser: &root,
+				},
+			},
+			expect: true,
+		},
+	}
+
+	for k, v := range tests {
+		actual := HasRootRunAsUser(v.container)
+		if actual != v.expect {
+			t.Errorf("%s failed, expected %t but received %t", k, v.expect, actual)
+		}
+	}
+}