Run all containers as non-root user (knative#3237)

* Run all serving containers as nonroot user

Change-Id: I8e380a7b5c2f533feb244325459f5143bf6a979f

* Fix conformance tests

Signed-off-by: Brad Hoekstra <[email protected]>
Change-Id: Id0ac1e41f36c04ddd119edb7d625827b8ea02695

* Use pkg.ptr

Change-Id: I77370d1f198276f59e6033d8515d2c438c8f47c7

.ko.yaml

@@ -1,7 +1,2 @@
-# Use the Jenkins base image as it is a public image that
-# reliably sets up and uses a non-root user (uid 1000).
-# https://github.com/jenkinsci/docker/blob/master/Dockerfile-slim#L15
-# Ideally we should use an unprivileged distroless user if
-# https://github.com/GoogleContainerTools/distroless/issues/235 is implemented.
-baseImageOverrides:
-        github.com/knative/serving/test/test_images/runtime-unprivileged: jenkins/jenkins:lts-slim
+# Use :nonroot base image for all containers
+defaultBaseImage: gcr.io/distroless/static:nonroot

config/activator.yaml

@@ -100,6 +100,8 @@ spec:
           mountPath: /etc/config-logging
         - name: config-observability
           mountPath: /etc/config-observability
+        securityContext:
+          allowPrivilegeEscalation: false
       volumes:
         - name: config-logging
           configMap:

config/autoscaler.yaml

@@ -76,6 +76,8 @@ spec:
           value: config-observability
         - name: METRICS_DOMAIN
           value: knative.dev/serving
+        securityContext:
+          allowPrivilegeEscalation: false
       volumes:
         - name: config-autoscaler
           configMap:

config/controller.yaml

@@ -64,6 +64,8 @@ spec:
           value: config-observability
         - name: METRICS_DOMAIN
           value: knative.dev/serving
+        securityContext:
+          allowPrivilegeEscalation: false
       volumes:
         - name: config-logging
           configMap:

config/networking-certmanager.yaml

@@ -62,6 +62,8 @@ spec:
           value: config-observability
         - name: METRICS_DOMAIN
           value: knative.dev/serving
+        securityContext:
+          allowPrivilegeEscalation: false
       volumes:
         - name: config-logging
           configMap:

config/networking-istio.yaml

@@ -62,6 +62,8 @@ spec:
           value: config-observability
         - name: METRICS_DOMAIN
           value: knative.dev/serving
+        securityContext:
+          allowPrivilegeEscalation: false
       volumes:
         - name: config-logging
           configMap:

config/webhook.yaml

@@ -60,6 +60,8 @@ spec:
               fieldPath: metadata.namespace
         - name: CONFIG_LOGGING_NAME
           value: config-logging
+        securityContext:
+          allowPrivilegeEscalation: false
       volumes:
         - name: config-logging
           configMap:

docs/resources-overview.md

@@ -41,6 +41,9 @@ the `knative-serving` namespace. For example, if you create a Knative Serivce in
 namespace 'foo' the corresponding Istio resources will also be in namespace
 'foo'.
 
+All of these components are run as a non-root user (uid: 1337) and disallow
+privilege escalation.
+
 ## Kubernetes Resource Configs
 
 The various Kubernetes resource configurations are organized as follows:

pkg/reconciler/revision/resources/deploy_test.go

@@ -69,10 +69,11 @@ var (
 	}
 
 	defaultQueueContainer = &corev1.Container{
-		Name:           QueueContainerName,
-		Resources:      createQueueResources(make(map[string]string), &corev1.Container{}),
-		Ports:          append(queueNonServingPorts, queueHTTPPort),
-		ReadinessProbe: queueReadinessProbe,
+		Name:            QueueContainerName,
+		Resources:       createQueueResources(make(map[string]string), &corev1.Container{}),
+		Ports:           append(queueNonServingPorts, queueHTTPPort),
+		ReadinessProbe:  queueReadinessProbe,
+		SecurityContext: queueSecurityContext,
 		Env: []corev1.EnvVar{{
 			Name:  "SERVING_NAMESPACE",
 			Value: "foo", // matches namespace

pkg/reconciler/revision/resources/queue.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/knative/pkg/logging"
 	pkgmetrics "github.com/knative/pkg/metrics"
+	"github.com/knative/pkg/ptr"
 	"github.com/knative/pkg/system"
 	"github.com/knative/serving/pkg/apis/networking"
 	"github.com/knative/serving/pkg/apis/serving"
@@ -78,6 +79,10 @@ var (
 		// thus don't want to be limited by K8s granularity here.
 		TimeoutSeconds: 10,
 	}
+
+	queueSecurityContext = &corev1.SecurityContext{
+		AllowPrivilegeEscalation: ptr.Bool(false),
+	}
 )
 
 func createQueueResources(annotations map[string]string, userContainer *corev1.Container) corev1.ResourceRequirements {
@@ -192,12 +197,13 @@ func makeQueueContainer(rev *v1alpha1.Revision, loggingConfig *logging.Config, o
 	}
 
 	return &corev1.Container{
-		Name:           QueueContainerName,
-		Image:          deploymentConfig.QueueSidecarImage,
-		Resources:      createQueueResources(rev.GetAnnotations(), rev.Spec.GetContainer()),
-		Ports:          ports,
-		ReadinessProbe: queueReadinessProbe,
-		VolumeMounts:   volumeMounts,
+		Name:            QueueContainerName,
+		Image:           deploymentConfig.QueueSidecarImage,
+		Resources:       createQueueResources(rev.GetAnnotations(), rev.Spec.GetContainer()),
+		Ports:           ports,
+		ReadinessProbe:  queueReadinessProbe,
+		VolumeMounts:    volumeMounts,
+		SecurityContext: queueSecurityContext,
 		Env: []corev1.EnvVar{{
 			Name:  "SERVING_NAMESPACE",
 			Value: rev.Namespace,

pkg/reconciler/revision/resources/queue_test.go

@@ -74,10 +74,11 @@ func TestMakeQueueContainer(t *testing.T) {
 		cc: &deployment.Config{},
 		want: &corev1.Container{
 			// These are effectively constant
-			Name:           QueueContainerName,
-			Resources:      createQueueResources(make(map[string]string), &corev1.Container{}),
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Name:            QueueContainerName,
+			Resources:       createQueueResources(make(map[string]string), &corev1.Container{}),
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Env: env(nil),
 		},
@@ -113,10 +114,11 @@ func TestMakeQueueContainer(t *testing.T) {
 		},
 		want: &corev1.Container{
 			// These are effectively constant
-			Name:           QueueContainerName,
-			Resources:      createQueueResources(make(map[string]string), &corev1.Container{}),
-			Ports:          append(queueNonServingPorts, queueHTTP2Port),
-			ReadinessProbe: queueReadinessProbe,
+			Name:            QueueContainerName,
+			Resources:       createQueueResources(make(map[string]string), &corev1.Container{}),
+			Ports:           append(queueNonServingPorts, queueHTTP2Port),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Image: "alpine",
 			Env: env(map[string]string{
@@ -150,10 +152,11 @@ func TestMakeQueueContainer(t *testing.T) {
 		},
 		want: &corev1.Container{
 			// These are effectively constant
-			Name:           QueueContainerName,
-			Resources:      createQueueResources(make(map[string]string), &corev1.Container{}),
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Name:            QueueContainerName,
+			Resources:       createQueueResources(make(map[string]string), &corev1.Container{}),
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Image: "alpine",
 			Env: env(map[string]string{
@@ -187,10 +190,11 @@ func TestMakeQueueContainer(t *testing.T) {
 		cc: &deployment.Config{},
 		want: &corev1.Container{
 			// These are effectively constant
-			Name:           QueueContainerName,
-			Resources:      createQueueResources(make(map[string]string), &corev1.Container{}),
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Name:            QueueContainerName,
+			Resources:       createQueueResources(make(map[string]string), &corev1.Container{}),
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Env: env(map[string]string{
 				"CONTAINER_CONCURRENCY": "0",
@@ -225,10 +229,11 @@ func TestMakeQueueContainer(t *testing.T) {
 		cc: &deployment.Config{},
 		want: &corev1.Container{
 			// These are effectively constant
-			Name:           QueueContainerName,
-			Resources:      createQueueResources(make(map[string]string), &corev1.Container{}),
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Name:            QueueContainerName,
+			Resources:       createQueueResources(make(map[string]string), &corev1.Container{}),
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Env: env(map[string]string{
 				"CONTAINER_CONCURRENCY":  "0",
@@ -259,10 +264,11 @@ func TestMakeQueueContainer(t *testing.T) {
 		cc: &deployment.Config{},
 		want: &corev1.Container{
 			// These are effectively constant
-			Name:           QueueContainerName,
-			Resources:      createQueueResources(make(map[string]string), &corev1.Container{}),
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Name:            QueueContainerName,
+			Resources:       createQueueResources(make(map[string]string), &corev1.Container{}),
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Env: env(map[string]string{
 				"CONTAINER_CONCURRENCY": "10",
@@ -289,10 +295,11 @@ func TestMakeQueueContainer(t *testing.T) {
 		cc: &deployment.Config{},
 		want: &corev1.Container{
 			// These are effectively constant
-			Name:           QueueContainerName,
-			Resources:      createQueueResources(make(map[string]string), &corev1.Container{}),
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Name:            QueueContainerName,
+			Resources:       createQueueResources(make(map[string]string), &corev1.Container{}),
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Env: env(map[string]string{
 				"CONTAINER_CONCURRENCY":        "0",
@@ -322,10 +329,11 @@ func TestMakeQueueContainer(t *testing.T) {
 		cc: &deployment.Config{},
 		want: &corev1.Container{
 			// These are effectively constant
-			Name:           QueueContainerName,
-			Resources:      createQueueResources(make(map[string]string), &corev1.Container{}),
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Name:            QueueContainerName,
+			Resources:       createQueueResources(make(map[string]string), &corev1.Container{}),
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Env: env(map[string]string{
 				"CONTAINER_CONCURRENCY":           "0",
@@ -412,8 +420,9 @@ func TestMakeQueueContainerWithPercentageAnnotation(t *testing.T) {
 					corev1.ResourceName("memory"): *resource.NewQuantity(429496736, resource.BinarySI),
 				},
 			},
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Image: "alpine",
 			Env: env(map[string]string{
@@ -466,8 +475,9 @@ func TestMakeQueueContainerWithPercentageAnnotation(t *testing.T) {
 					corev1.ResourceName("memory"): resource.MustParse("50Mi"),
 				},
 			},
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Image: "alpine",
 			Env: env(map[string]string{
@@ -519,8 +529,9 @@ func TestMakeQueueContainerWithPercentageAnnotation(t *testing.T) {
 					corev1.ResourceName("cpu"): resource.MustParse("25m"),
 				},
 			},
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Image: "alpine",
 			Env: env(map[string]string{
@@ -572,8 +583,9 @@ func TestMakeQueueContainerWithPercentageAnnotation(t *testing.T) {
 					corev1.ResourceName("memory"): resource.MustParse("200Mi"),
 				},
 			},
-			Ports:          append(queueNonServingPorts, queueHTTPPort),
-			ReadinessProbe: queueReadinessProbe,
+			Ports:           append(queueNonServingPorts, queueHTTPPort),
+			ReadinessProbe:  queueReadinessProbe,
+			SecurityContext: queueSecurityContext,
 			// These changed based on the Revision and configs passed in.
 			Image: "alpine",
 			Env: env(map[string]string{

pkg/testing/v1alpha1/service.go

@@ -439,3 +439,10 @@ func WithSecurityContext(sc *corev1.SecurityContext) ServiceOption {
 		s.Spec.Template.Spec.Containers[0].SecurityContext = sc
 	}
 }
+
+// WithWorkingDir configures the Service to use the provided working directory.
+func WithWorkingDir(wd string) ServiceOption {
+	return func(s *v1alpha1.Service) {
+		s.Spec.Template.Spec.Containers[0].WorkingDir = wd
+	}
+}

pkg/testing/v1beta1/service.go

@@ -187,3 +187,10 @@ func WithSecurityContext(sc *corev1.SecurityContext) ServiceOption {
 		s.Spec.Template.Spec.Containers[0].SecurityContext = sc
 	}
 }
+
+// WithWorkingDir configures the Service to use the provided working directory.
+func WithWorkingDir(wd string) ServiceOption {
+	return func(s *v1beta1.Service) {
+		s.Spec.Template.Spec.Containers[0].WorkingDir = wd
+	}
+}

test/conformance.go

@@ -39,7 +39,6 @@ const (
 	PrintPort           = "printport"
 	Protocols           = "protocols"
 	Runtime             = "runtime"
-	RuntimeUnprivileged = "runtime-unprivileged"
 	SingleThreadedImage = "singlethreaded"
 	Timeout             = "timeout"
 	WorkingDir          = "workingdir"

test/conformance/runtime/user_test.go

@@ -29,7 +29,7 @@ import (
 
 const (
 	securityContextUserID = 2020
-	unprivilegedUserID    = 1000
+	unprivilegedUserID    = 65532
 )
 
 // TestMustRunAsUser verifies that a supplied runAsUser through securityContext takes
@@ -43,7 +43,9 @@ func TestMustRunAsUser(t *testing.T) {
 		RunAsUser: &runAsUser,
 	}
 
-	_, ri, err := fetchRuntimeInfo(t, clients, WithSecurityContext(securityContext))
+	// We need to modify the working dir because the specified user cannot access the
+	// default user's working dir.
+	_, ri, err := fetchRuntimeInfo(t, clients, WithSecurityContext(securityContext), WithWorkingDir("/"))
 	if err != nil {
 		t.Fatalf("Error fetching runtime info: %v", err)
 	}
@@ -73,7 +75,7 @@ func TestMustRunAsUser(t *testing.T) {
 func TestShouldRunAsUserContainerDefault(t *testing.T) {
 	t.Parallel()
 	clients := test.Setup(t)
-	_, ri, err := fetchRuntimeInfoUnprivileged(t, clients)
+	_, ri, err := fetchRuntimeInfo(t, clients)
 
 	if err != nil {
 		t.Fatalf("Error fetching runtime info: %v", err)

test/conformance/runtime/util.go

@@ -29,18 +29,8 @@ import (
 	. "github.com/knative/serving/pkg/testing/v1alpha1"
 )
 
-// fetchRuntimeInfoUnprivileged creates a Service that uses the 'runtime-unprivileged' test image, and extracts the returned output into the
-// RuntimeInfo object.
-func fetchRuntimeInfoUnprivileged(
-	t *testing.T,
-	clients *test.Clients,
-	opts ...interface{}) (*test.ResourceNames, *types.RuntimeInfo, error) {
-
-	return runtimeInfo(t, clients, &test.ResourceNames{Image: test.RuntimeUnprivileged}, opts...)
-}
-
 // fetchRuntimeInfo creates a Service that uses the 'runtime' test image, and extracts the returned output into the
-// RuntimeInfo object. The 'runtime' image uses uid 0.
+// RuntimeInfo object. The 'runtime' image uses uid 65532.
 func fetchRuntimeInfo(
 	t *testing.T,
 	clients *test.Clients,
@@ -59,7 +49,7 @@ func runtimeInfo(
 	names.Service = test.ObjectNameForTest(t)
 	if names.Image == "" {
 		names.Image = test.Runtime
-	} else if names.Image != test.RuntimeUnprivileged {
+	} else if names.Image != test.Runtime {
 		return nil, nil, fmt.Errorf("invalid image provided: %s", names.Image)
 	}