Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run as non-privilege and non-root user #2037

Merged
merged 3 commits into from
Jun 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions pkg/render/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ import (
"github.com/tigera/operator/pkg/ptr"
rmeta "github.com/tigera/operator/pkg/render/common/meta"
"github.com/tigera/operator/pkg/render/common/podaffinity"
"github.com/tigera/operator/pkg/render/common/podsecuritycontext"
"github.com/tigera/operator/pkg/render/common/podsecuritypolicy"
"github.com/tigera/operator/pkg/render/common/secret"
"github.com/tigera/operator/pkg/render/common/securitycontext"
"github.com/tigera/operator/pkg/tls/certificatemanagement"
)

Expand Down Expand Up @@ -943,7 +943,8 @@ func (c *apiServerComponent) queryServerContainer() corev1.Container {
InitialDelaySeconds: 90,
PeriodSeconds: 10,
},
SecurityContext: podsecuritycontext.NewBaseContext(),
// UID 1001 is used in the queryserver Dockerfile.
SecurityContext: securitycontext.NewBaseContext(1001, 0),
VolumeMounts: volumeMounts,
}
return container
Expand Down
8 changes: 7 additions & 1 deletion pkg/render/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ var _ = Describe("API server rendering tests (Calico Enterprise)", func() {
Expect(d.Spec.Template.Spec.Tolerations).To(ConsistOf(rmeta.TolerateMaster))

Expect(d.Spec.Template.Spec.ImagePullSecrets).To(BeEmpty())
Expect(len(d.Spec.Template.Spec.Containers)).To(Equal(2))
Expect(d.Spec.Template.Spec.Containers).To(HaveLen(2))
Expect(d.Spec.Template.Spec.Containers[0].Name).To(Equal("tigera-apiserver"))
Expect(d.Spec.Template.Spec.Containers[0].Image).To(Equal(
fmt.Sprintf("testregistry.com/%s:%s", components.ComponentAPIServer.Image, components.ComponentAPIServer.Version),
Expand Down Expand Up @@ -267,6 +267,12 @@ var _ = Describe("API server rendering tests (Calico Enterprise)", func() {
Expect(d.Spec.Template.Spec.Containers[1].LivenessProbe.InitialDelaySeconds).To(BeEquivalentTo(90))
Expect(d.Spec.Template.Spec.Containers[1].LivenessProbe.PeriodSeconds).To(BeEquivalentTo(10))

Expect(*d.Spec.Template.Spec.Containers[1].SecurityContext.AllowPrivilegeEscalation).To(BeFalse())
Expect(*d.Spec.Template.Spec.Containers[1].SecurityContext.Privileged).To(BeFalse())
Expect(*d.Spec.Template.Spec.Containers[1].SecurityContext.RunAsGroup).To(BeEquivalentTo(0))
Expect(*d.Spec.Template.Spec.Containers[1].SecurityContext.RunAsNonRoot).To(BeTrue())
Expect(*d.Spec.Template.Spec.Containers[1].SecurityContext.RunAsUser).To(BeEquivalentTo(1001))

Expect(len(d.Spec.Template.Spec.Volumes)).To(Equal(3))

Expect(d.Spec.Template.Spec.Volumes[0].Name).To(Equal("tigera-apiserver-certs"))
Expand Down
7 changes: 4 additions & 3 deletions pkg/render/aws-securitygroup-setup.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2019 Tigera, Inc. All rights reserved.
// Copyright (c) 2019,2022 Tigera, Inc. All rights reserved.

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -25,7 +25,7 @@ import (
"github.com/tigera/operator/pkg/common"
"github.com/tigera/operator/pkg/components"
rmeta "github.com/tigera/operator/pkg/render/common/meta"
"github.com/tigera/operator/pkg/render/common/podsecuritycontext"
"github.com/tigera/operator/pkg/render/common/securitycontext"
)

func AWSSecurityGroupSetup(cfg *AWSSGSetupConfiguration) (Component, error) {
Expand Down Expand Up @@ -110,7 +110,8 @@ func (c *awsSGSetupComponent) setupJob() *batchv1.Job {
Value: "/etc/kubernetes/kubeconfig",
},
},
SecurityContext: podsecuritycontext.NewBaseContext(),
// UID 1001 is used in the operator Dockerfile.
SecurityContext: securitycontext.NewBaseContext(1001, 0),
}},
},
},
Expand Down
85 changes: 85 additions & 0 deletions pkg/render/aws-securitygroup-setup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright (c) 2022 Tigera, Inc. All rights reserved.

// 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 render

import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"

operatorv1 "github.com/tigera/operator/api/v1"
rtest "github.com/tigera/operator/pkg/render/common/test"
)

var _ = Describe("AWS SecurityGroup Setup rendering tests", func() {
var cfg *AWSSGSetupConfiguration

BeforeEach(func() {
cfg = &AWSSGSetupConfiguration{
PullSecrets: []corev1.LocalObjectReference{},
Installation: &operatorv1.InstallationSpec{},
}
})

It("should render AWS SecurityGroup Setup resources", func() {
component, err := AWSSecurityGroupSetup(cfg)
Expect(err).NotTo(HaveOccurred())
Expect(component.ResolveImages(nil)).NotTo(HaveOccurred())

toCreate, toDelete := component.Objects()

expectedResources := []struct {
name string
ns string
group string
version string
kind string
}{
{"tigera-aws-security-group-setup", "tigera-operator", "", "v1", "ServiceAccount"},
{"tigera-aws-security-group-setup", "kube-system", "rbac.authorization.k8s.io", "v1", "Role"},
{"tigera-aws-security-group-setup", "kube-system", "rbac.authorization.k8s.io", "v1", "RoleBinding"},
{"aws-security-group-setup-1", "tigera-operator", "batch", "v1", "Job"},
}

Expect(len(toCreate)).To(Equal(len(expectedResources)))

for i, expectedRes := range expectedResources {
obj := toCreate[i]
rtest.ExpectResource(obj, expectedRes.name, expectedRes.ns, expectedRes.group, expectedRes.version, expectedRes.kind)
}

Expect(toDelete).To(BeNil())
})

It("should render Setup Job specs correctly", func() {
component, err := AWSSecurityGroupSetup(cfg)
Expect(err).NotTo(HaveOccurred())
Expect(component.ResolveImages(nil)).NotTo(HaveOccurred())
toCreate, _ := component.Objects()

job, ok := rtest.GetResource(toCreate, "aws-security-group-setup-1", "tigera-operator", "batch", "v1", "Job").(*batchv1.Job)
Expect(ok).To(BeTrue())

Expect(job.Spec.Template.Spec.Containers).To(HaveLen(1))

Expect(*job.Spec.Template.Spec.Containers[0].SecurityContext.AllowPrivilegeEscalation).To(BeFalse())
Expect(*job.Spec.Template.Spec.Containers[0].SecurityContext.Privileged).To(BeFalse())
Expect(*job.Spec.Template.Spec.Containers[0].SecurityContext.RunAsGroup).To(BeEquivalentTo(0))
Expect(*job.Spec.Template.Spec.Containers[0].SecurityContext.RunAsNonRoot).To(BeTrue())
Expect(*job.Spec.Template.Spec.Containers[0].SecurityContext.RunAsUser).To(BeEquivalentTo(1001))
})
})
15 changes: 0 additions & 15 deletions pkg/render/common/podsecuritycontext/pod_security_context.go

This file was deleted.

42 changes: 42 additions & 0 deletions pkg/render/common/securitycontext/security_context.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) 2021-2022 Tigera, Inc. All rights reserved.

// 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 securitycontext

import (
corev1 "k8s.io/api/core/v1"

"github.com/tigera/operator/pkg/ptr"
)

var (
// It is recommended to choose UID and GID that don't collide with existing system users and groups.
// Non-system UID and GID range is normally from 1000 to 60000 (Debian derived systems define this
// in /etc/login.defs). On a normal Linux host, it is unlikely to have more than 10k non-system users.
// 10001 is chosen based on this assumption.
RunAsUserID int64 = 10001
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preferably, I want this UID and GID consistent across all components. It is not possible to make the Operator only change because some components chown folders to a hard-coded UID in Dockerfile. When running as a different user, I got permission errors. 10001 is chosen for the reason mentioned in the comments. The UIDs we are currently using (999 or 1001 etc.) may not be desired because they will collide with system or non-system users. Do we want to make them consistent? @caseydavenport or @tmjd WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistency would be nice, but like you point out it would require some extra work. I don't think we have any special attachment to 999 or 1001, but there are probably other places where we (or users) make assumptions based on those, so I might be inclined to leave them as-is for now. I don't think we've had any reports of those values causing issues.

Maybe just make these configurable for now and each component can pass in the user / group that it expects to be run as, and we can then do a follow-up series of PRs to standardize?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made uid and gid configurable in NewBaseContext. The UID and GID are taken from Dockerfiles (unchanged) for each component.

RunAsGroupID int64 = 10001
)

// NewBaseContext returns the non root non privileged security context that most of the containers running should
// be using.
func NewBaseContext(uid, gid int64) *corev1.SecurityContext {
return &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.BoolToPtr(false),
Privileged: ptr.BoolToPtr(false),
RunAsGroup: &gid,
RunAsNonRoot: ptr.BoolToPtr(true),
RunAsUser: &uid,
}
}
13 changes: 7 additions & 6 deletions pkg/render/dex.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
"github.com/tigera/operator/pkg/components"
rmeta "github.com/tigera/operator/pkg/render/common/meta"
"github.com/tigera/operator/pkg/render/common/podaffinity"
"github.com/tigera/operator/pkg/render/common/podsecuritycontext"
"github.com/tigera/operator/pkg/render/common/secret"
"github.com/tigera/operator/pkg/render/common/securitycontext"
"github.com/tigera/operator/pkg/tls/certificatemanagement"
"gopkg.in/yaml.v2"

Expand Down Expand Up @@ -216,11 +216,12 @@ func (c *dexComponent) deployment() client.Object {
InitContainers: initContainers,
Containers: []corev1.Container{
{
Name: DexObjectName,
Image: c.image,
Env: c.cfg.DexConfig.RequiredEnv(""),
LivenessProbe: c.probe(),
SecurityContext: podsecuritycontext.NewBaseContext(),
Name: DexObjectName,
Image: c.image,
Env: c.cfg.DexConfig.RequiredEnv(""),
LivenessProbe: c.probe(),
// UID and GID 1001:1001 are used in dex Dockerfile.
SecurityContext: securitycontext.NewBaseContext(1001, 1001),

Command: []string{"/usr/local/bin/dex", "serve", "/etc/dex/baseCfg/config.yaml"},

Expand Down
9 changes: 9 additions & 0 deletions pkg/render/dex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,15 @@ var _ = Describe("dex rendering tests", func() {
rtest.ExpectResource(resources[i], expectedRes.name, expectedRes.ns, expectedRes.group, expectedRes.version, expectedRes.kind)
}
Expect(len(resources)).To(Equal(len(expectedResources)))

d := rtest.GetResource(resources, "tigera-dex", "tigera-dex", "apps", "v1", "Deployment").(*appsv1.Deployment)

Expect(d.Spec.Template.Spec.Containers).To(HaveLen(1))
Expect(*d.Spec.Template.Spec.Containers[0].SecurityContext.AllowPrivilegeEscalation).To(BeFalse())
Expect(*d.Spec.Template.Spec.Containers[0].SecurityContext.Privileged).To(BeFalse())
Expect(*d.Spec.Template.Spec.Containers[0].SecurityContext.RunAsGroup).To(BeEquivalentTo(1001))
Expect(*d.Spec.Template.Spec.Containers[0].SecurityContext.RunAsNonRoot).To(BeTrue())
Expect(*d.Spec.Template.Spec.Containers[0].SecurityContext.RunAsUser).To(BeEquivalentTo(1001))
})

DescribeTable("should render the cluster name properly in the validator", func(clusterDomain string) {
Expand Down
5 changes: 3 additions & 2 deletions pkg/render/guardian.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import (
operatorv1 "github.com/tigera/operator/api/v1"
"github.com/tigera/operator/pkg/components"
rmeta "github.com/tigera/operator/pkg/render/common/meta"
"github.com/tigera/operator/pkg/render/common/podsecuritycontext"
"github.com/tigera/operator/pkg/render/common/secret"
"github.com/tigera/operator/pkg/render/common/securitycontext"
"github.com/tigera/operator/pkg/tls/certificatemanagement"
)

Expand Down Expand Up @@ -268,7 +268,8 @@ func (c *GuardianComponent) container() []corev1.Container {
InitialDelaySeconds: 10,
PeriodSeconds: 5,
},
SecurityContext: podsecuritycontext.NewBaseContext(),
// UID 1001 is used in the guardian Dockerfile.
SecurityContext: securitycontext.NewBaseContext(1001, 0),
},
}
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/render/guardian_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,14 @@ var _ = Describe("Rendering tests", func() {
}

deployment := rtest.GetResource(resources, render.GuardianDeploymentName, render.GuardianNamespace, "apps", "v1", "Deployment").(*appsv1.Deployment)
Expect(deployment.Spec.Template.Spec.Containers).To(HaveLen(1))
Expect(deployment.Spec.Template.Spec.Containers[0].Image).Should(Equal("my-reg/tigera/guardian:" + components.ComponentGuardian.Version))

Expect(*deployment.Spec.Template.Spec.Containers[0].SecurityContext.AllowPrivilegeEscalation).To(BeFalse())
Expect(*deployment.Spec.Template.Spec.Containers[0].SecurityContext.Privileged).To(BeFalse())
Expect(*deployment.Spec.Template.Spec.Containers[0].SecurityContext.RunAsGroup).To(BeEquivalentTo(0))
Expect(*deployment.Spec.Template.Spec.Containers[0].SecurityContext.RunAsNonRoot).To(BeTrue())
Expect(*deployment.Spec.Template.Spec.Containers[0].SecurityContext.RunAsUser).To(BeEquivalentTo(1001))
})

It("should render controlPlaneTolerations", func() {
Expand Down
12 changes: 6 additions & 6 deletions pkg/render/intrusion_detection.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ import (
operatorv1 "github.com/tigera/operator/api/v1"
"github.com/tigera/operator/pkg/components"
"github.com/tigera/operator/pkg/dns"
"github.com/tigera/operator/pkg/ptr"
relasticsearch "github.com/tigera/operator/pkg/render/common/elasticsearch"
rkibana "github.com/tigera/operator/pkg/render/common/kibana"
rmeta "github.com/tigera/operator/pkg/render/common/meta"
"github.com/tigera/operator/pkg/render/common/podsecuritypolicy"
"github.com/tigera/operator/pkg/render/common/secret"
"github.com/tigera/operator/pkg/render/common/securitycontext"
"github.com/tigera/operator/pkg/tls/certificatemanagement"
"github.com/tigera/operator/pkg/url"
)
Expand Down Expand Up @@ -543,7 +545,7 @@ func (c *intrusionDetectionComponent) intrusionDetectionControllerContainer() co
},
}

privileged := false
sc := securitycontext.NewBaseContext(securitycontext.RunAsUserID, securitycontext.RunAsGroupID)
Copy link
Contributor Author

@hjiawei hjiawei Jun 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing is set in IDS controller Dockerfile so we are safe to set UID and GID to 10001. This is also fixed in IDS master as a fallback.


// If syslog forwarding is enabled then set the necessary ENV var and volume mount to
// write logs for Fluentd.
Expand All @@ -558,7 +560,7 @@ func (c *intrusionDetectionComponent) intrusionDetectionControllerContainer() co
// On OpenShift, if we need the volume mount to hostpath volume for syslog forwarding,
// then ID controller needs privileged access to write event logs to that volume
if c.cfg.Openshift {
privileged = true
sc.Privileged = ptr.BoolToPtr(true)
}
}

Expand All @@ -578,10 +580,8 @@ func (c *intrusionDetectionComponent) intrusionDetectionControllerContainer() co
},
InitialDelaySeconds: 5,
},
SecurityContext: &corev1.SecurityContext{
Privileged: &privileged,
},
VolumeMounts: volumeMounts,
SecurityContext: sc,
VolumeMounts: volumeMounts,
}
}

Expand Down
9 changes: 9 additions & 0 deletions pkg/render/intrusion_detection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,17 +143,26 @@ var _ = Describe("Intrusion Detection rendering tests", func() {
// Should mount ManagerTLSSecret for non-managed clusters
idc := rtest.GetResource(resources, "intrusion-detection-controller", render.IntrusionDetectionNamespace, "apps", "v1", "Deployment").(*appsv1.Deployment)
idji := rtest.GetResource(resources, "intrusion-detection-es-job-installer", render.IntrusionDetectionNamespace, "batch", "v1", "Job").(*batchv1.Job)
Expect(idc.Spec.Template.Spec.Containers).To(HaveLen(1))
Expect(idc.Spec.Template.Spec.Containers[0].Env).Should(ContainElements(
corev1.EnvVar{Name: "ELASTIC_INDEX_SUFFIX", Value: "clusterTestName"},
))
Expect(idji.Spec.Template.Spec.Containers).To(HaveLen(1))
Expect(idji.Spec.Template.Spec.Containers[0].Env).Should(ContainElements(
corev1.EnvVar{Name: "ELASTIC_INDEX_SUFFIX", Value: "clusterTestName"},
))
Expect(idc.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name).To(Equal(certificatemanagement.TrustedCertConfigMapName))
Expect(idc.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath).To(Equal(certificatemanagement.TrustedCertVolumeMountPath))

Expect(idc.Spec.Template.Spec.Volumes[0].Name).To(Equal(certificatemanagement.TrustedCertConfigMapName))
Expect(idc.Spec.Template.Spec.Volumes[0].ConfigMap.Name).To(Equal(certificatemanagement.TrustedCertConfigMapName))

Expect(*idc.Spec.Template.Spec.Containers[0].SecurityContext.AllowPrivilegeEscalation).To(BeFalse())
Expect(*idc.Spec.Template.Spec.Containers[0].SecurityContext.Privileged).To(BeFalse())
Expect(*idc.Spec.Template.Spec.Containers[0].SecurityContext.RunAsGroup).To(BeEquivalentTo(10001))
Expect(*idc.Spec.Template.Spec.Containers[0].SecurityContext.RunAsNonRoot).To(BeTrue())
Expect(*idc.Spec.Template.Spec.Containers[0].SecurityContext.RunAsUser).To(BeEquivalentTo(10001))

clusterRole := rtest.GetResource(resources, "intrusion-detection-controller", "", "rbac.authorization.k8s.io", "v1", "ClusterRole").(*rbacv1.ClusterRole)

Expect(clusterRole.Rules).To(ContainElement(rbacv1.PolicyRule{
Expand Down
Loading