From d8514c406ab4e3344c4352afd7fbc89d74b68f1a Mon Sep 17 00:00:00 2001 From: Dan Gerdesmeier Date: Fri, 10 May 2019 16:21:55 +0000 Subject: [PATCH 1/3] Unprivileged user conformance test This adds a test to validate that a container specified user is respected when executed within Knative. Fixes #3223 --- .ko.yaml | 7 +++ test/conformance/conformancetest_helper.go | 32 ++++++++++---- test/conformance/user_test.go | 42 +++++++++++++++--- test/conformance/util.go | 15 ++++--- test/test_images/runtime-unprivileged/main.go | 44 +++++++++++++++++++ .../runtime-unprivileged/service.yaml | 10 +++++ test/upload-test-images.sh | 5 ++- 7 files changed, 134 insertions(+), 21 deletions(-) create mode 100644 .ko.yaml create mode 100644 test/test_images/runtime-unprivileged/main.go create mode 100644 test/test_images/runtime-unprivileged/service.yaml diff --git a/.ko.yaml b/.ko.yaml new file mode 100644 index 000000000000..fadf4adf006c --- /dev/null +++ b/.ko.yaml @@ -0,0 +1,7 @@ +# 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 diff --git a/test/conformance/conformancetest_helper.go b/test/conformance/conformancetest_helper.go index 2455e98697ba..87b7fd5ad21b 100644 --- a/test/conformance/conformancetest_helper.go +++ b/test/conformance/conformancetest_helper.go @@ -22,6 +22,7 @@ package conformance import ( "encoding/json" + "fmt" "testing" pkgTest "github.com/knative/pkg/test" @@ -31,18 +32,33 @@ import ( . "github.com/knative/serving/pkg/reconciler/testing" ) -// fetchRuntimeInfo creates a Service that uses the 'runtime' test image, and extracts the returned output into the +// The 'runtime-unprivileged' test image uses uid 1000. +const unprivilegedUserId = 1000 + +// 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, options *test.Options, opts ...ServiceOption) (*test.ResourceNames, *types.RuntimeInfo, error) { + return runtimeInfo(t, clients, &test.ResourceNames{Image: runtimeUnprivileged}, options, 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. func fetchRuntimeInfo(t *testing.T, clients *test.Clients, options *test.Options, opts ...ServiceOption) (*test.ResourceNames, *types.RuntimeInfo, error) { - names := test.ResourceNames{ - Service: test.ObjectNameForTest(t), - Image: runtime, + return runtimeInfo(t, clients, &test.ResourceNames{}, options, opts...) +} + +func runtimeInfo(t *testing.T, clients *test.Clients, names *test.ResourceNames, options *test.Options, opts ...ServiceOption) (*test.ResourceNames, *types.RuntimeInfo, error) { + names.Service = test.ObjectNameForTest(t) + if names.Image == "" { + names.Image = runtime + } else if names.Image != runtimeUnprivileged { + return nil, nil, fmt.Errorf("Invalid image provided: %s", names.Image) } - defer test.TearDown(clients, names) - test.CleanupOnInterrupt(func() { test.TearDown(clients, names) }) + defer test.TearDown(clients, *names) + test.CleanupOnInterrupt(func() { test.TearDown(clients, *names) }) - objects, err := test.CreateRunLatestServiceReady(t, clients, &names, options, opts...) + objects, err := test.CreateRunLatestServiceReady(t, clients, names, options, opts...) if err != nil { return nil, nil, err } @@ -63,5 +79,5 @@ func fetchRuntimeInfo(t *testing.T, clients *test.Clients, options *test.Options if err != nil { return nil, nil, err } - return &names, &ri, nil + return names, &ri, nil } diff --git a/test/conformance/user_test.go b/test/conformance/user_test.go index 488140062de8..72ebe52c3cd0 100644 --- a/test/conformance/user_test.go +++ b/test/conformance/user_test.go @@ -26,15 +26,15 @@ import ( corev1 "k8s.io/api/core/v1" ) -const userID = 2020 +const securityContextUserID = 2020 // TestMustRunAsUser verifies that a supplied runAsUser through securityContext takes -// effect as delared by "MUST" in the runtime-contract. +// effect as declared by "MUST" in the runtime-contract. func TestMustRunAsUser(t *testing.T) { t.Parallel() clients := setup(t) - runAsUser := int64(userID) + runAsUser := int64(securityContextUserID) securityContext := &corev1.SecurityContext{ RunAsUser: &runAsUser, } @@ -52,13 +52,45 @@ func TestMustRunAsUser(t *testing.T) { t.Fatal("Missing user information from runtime info.") } - if got, want := ri.Host.User.UID, userID; got != want { + if got, want := ri.Host.User.UID, securityContextUserID; got != want { t.Errorf("uid = %d, want: %d", got, want) } // We expect the effective userID to match the userID as we // did not use setuid. - if got, want := ri.Host.User.EUID, userID; got != want { + if got, want := ri.Host.User.EUID, securityContextUserID; got != want { t.Errorf("euid = %d, want: %d", got, want) } } + +// TestShouldRunAsUserContainerDefault verifies that a container that sets runAsUser +// in the Dockerfile is respected when executed in Knative as declared by "SHOULD" +// in the runtime-contract. +func TestShouldRunAsUserContainerDefault(t *testing.T) { + t.Parallel() + clients := setup(t) + _, ri, err := fetchRuntimeInfoUnprivileged(t, clients, &test.Options{}) + + if err != nil { + t.Fatalf("Error fetching runtime info: %v", err) + } + + if ri.Host == nil { + t.Fatal("Missing host information from runtime info.") + } + + if ri.Host.User == nil { + t.Fatal("Missing user information from runtime info.") + } + + if got, want := ri.Host.User.UID, unprivilegedUserId; got != want { + t.Errorf("uid = %d, want: %d", got, want) + } + + // We expect the effective userID to match the userID as we + // did not use setuid. + if got, want := ri.Host.User.EUID, unprivilegedUserId; got != want { + t.Errorf("euid = %d, want: %d", got, want) + } + +} diff --git a/test/conformance/util.go b/test/conformance/util.go index 164e612d4855..c493efc2da89 100644 --- a/test/conformance/util.go +++ b/test/conformance/util.go @@ -40,17 +40,18 @@ import ( // Constants for test images located in test/test_images. const ( - pizzaPlanet1 = "pizzaplanetv1" - pizzaPlanet2 = "pizzaplanetv2" + failing = "failing" helloworld = "helloworld" httpproxy = "httpproxy" - singleThreadedImage = "singlethreaded" - timeout = "timeout" + invalidhelloworld = "invalidhelloworld" + pizzaPlanet1 = "pizzaplanetv1" + pizzaPlanet2 = "pizzaplanetv2" printport = "printport" - runtime = "runtime" protocols = "protocols" - failing = "failing" - invalidhelloworld = "invalidhelloworld" + runtime = "runtime" + runtimeUnprivileged = "runtime-unprivileged" + singleThreadedImage = "singlethreaded" + timeout = "timeout" concurrentRequests = 50 // We expect to see 100% of requests succeed for traffic sent directly to revisions. diff --git a/test/test_images/runtime-unprivileged/main.go b/test/test_images/runtime-unprivileged/main.go new file mode 100644 index 000000000000..3716860d37a3 --- /dev/null +++ b/test/test_images/runtime-unprivileged/main.go @@ -0,0 +1,44 @@ +/* +Copyright 2019 The Knative Authors + Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "fmt" + "log" + "net/http" + "os" + + "github.com/knative/serving/test/test_images/runtime/handlers" +) + +func main() { + + // We expect PORT to be defined in a Knative environment + // and don't want to mask this failure in a test image. + port, isSet := os.LookupEnv("PORT") + if !isSet { + log.Fatal("Environment variable PORT is not set.") + } + + mux := http.NewServeMux() + handlers.InitHandlers(mux) + + server := &http.Server{ + Addr: fmt.Sprintf(":%s", port), + Handler: mux, + } + + log.Printf("Server starting on port %s", port) + log.Fatal(server.ListenAndServe()) +} diff --git a/test/test_images/runtime-unprivileged/service.yaml b/test/test_images/runtime-unprivileged/service.yaml new file mode 100644 index 000000000000..125ee65862f2 --- /dev/null +++ b/test/test_images/runtime-unprivileged/service.yaml @@ -0,0 +1,10 @@ +apiVersion: serving.knative.dev/v1alpha1 +kind: Service +metadata: + name: runtime-unprivileged-test-image + namespace: default +spec: + template: + spec: + containers: + - image: github.com/knative/serving/test/test_images/runtime-unprivileged diff --git a/test/upload-test-images.sh b/test/upload-test-images.sh index a3cd296625a3..d02c76f9d084 100755 --- a/test/upload-test-images.sh +++ b/test/upload-test-images.sh @@ -18,7 +18,10 @@ set -o errexit function upload_test_images() { echo ">> Publishing test images" - local image_dir="$(dirname $0)/test_images" + # Script needs to be executed from the root directory + # to pickup .ko.yaml + cd "$( dirname "$0")/.." + local image_dir="test/test_images" local docker_tag=$1 local tag_option="" if [ -n "${docker_tag}" ]; then From 1fb279ac0590c02cc0d30fb2ce829fbaf31c9eb5 Mon Sep 17 00:00:00 2001 From: Dan Gerdesmeier Date: Fri, 10 May 2019 17:25:47 +0000 Subject: [PATCH 2/3] Capitalization fix --- test/conformance/conformancetest_helper.go | 2 +- test/conformance/user_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/conformance/conformancetest_helper.go b/test/conformance/conformancetest_helper.go index 87b7fd5ad21b..e09e1e789ee5 100644 --- a/test/conformance/conformancetest_helper.go +++ b/test/conformance/conformancetest_helper.go @@ -33,7 +33,7 @@ import ( ) // The 'runtime-unprivileged' test image uses uid 1000. -const unprivilegedUserId = 1000 +const unprivilegedUserID = 1000 // fetchRuntimeInfoUnprivileged creates a Service that uses the 'runtime-unprivileged' test image, and extracts the returned output into the // RuntimeInfo object. diff --git a/test/conformance/user_test.go b/test/conformance/user_test.go index 72ebe52c3cd0..1166a54d9df2 100644 --- a/test/conformance/user_test.go +++ b/test/conformance/user_test.go @@ -83,13 +83,13 @@ func TestShouldRunAsUserContainerDefault(t *testing.T) { t.Fatal("Missing user information from runtime info.") } - if got, want := ri.Host.User.UID, unprivilegedUserId; got != want { + if got, want := ri.Host.User.UID, unprivilegedUserID; got != want { t.Errorf("uid = %d, want: %d", got, want) } // We expect the effective userID to match the userID as we // did not use setuid. - if got, want := ri.Host.User.EUID, unprivilegedUserId; got != want { + if got, want := ri.Host.User.EUID, unprivilegedUserID; got != want { t.Errorf("euid = %d, want: %d", got, want) } From f1071d3875bdc20423d08b046c1ec7a7e5514fc7 Mon Sep 17 00:00:00 2001 From: Dan Gerdesmeier Date: Fri, 10 May 2019 17:48:15 +0000 Subject: [PATCH 3/3] Code Review comments --- test/conformance/conformancetest_helper.go | 2 +- test/test_images/runtime-unprivileged/main.go | 3 +-- test/test_images/runtime/main.go | 3 +-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/test/conformance/conformancetest_helper.go b/test/conformance/conformancetest_helper.go index e09e1e789ee5..566a2ae2697b 100644 --- a/test/conformance/conformancetest_helper.go +++ b/test/conformance/conformancetest_helper.go @@ -52,7 +52,7 @@ func runtimeInfo(t *testing.T, clients *test.Clients, names *test.ResourceNames, if names.Image == "" { names.Image = runtime } else if names.Image != runtimeUnprivileged { - return nil, nil, fmt.Errorf("Invalid image provided: %s", names.Image) + return nil, nil, fmt.Errorf("invalid image provided: %s", names.Image) } defer test.TearDown(clients, *names) diff --git a/test/test_images/runtime-unprivileged/main.go b/test/test_images/runtime-unprivileged/main.go index 3716860d37a3..7d2418f6f426 100644 --- a/test/test_images/runtime-unprivileged/main.go +++ b/test/test_images/runtime-unprivileged/main.go @@ -14,7 +14,6 @@ limitations under the License. package main import ( - "fmt" "log" "net/http" "os" @@ -35,7 +34,7 @@ func main() { handlers.InitHandlers(mux) server := &http.Server{ - Addr: fmt.Sprintf(":%s", port), + Addr: ":" + port, Handler: mux, } diff --git a/test/test_images/runtime/main.go b/test/test_images/runtime/main.go index 3716860d37a3..7d2418f6f426 100644 --- a/test/test_images/runtime/main.go +++ b/test/test_images/runtime/main.go @@ -14,7 +14,6 @@ limitations under the License. package main import ( - "fmt" "log" "net/http" "os" @@ -35,7 +34,7 @@ func main() { handlers.InitHandlers(mux) server := &http.Server{ - Addr: fmt.Sprintf(":%s", port), + Addr: ":" + port, Handler: mux, }