From 38f19f991e85e4ed6f0c3f3b0a125a22967cf506 Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Fri, 8 May 2020 13:24:38 -0700 Subject: [PATCH 1/2] Add config flag to default empty seccomp profile This changes adds `default_seccomp_profile` config switch to apply default seccomp profile when not provided by k8s.a Signed-off-by: Maksym Pavlenko --- pkg/config/config.go | 2 ++ pkg/server/container_create_unix.go | 7 ++-- pkg/server/container_create_unix_test.go | 43 +++++++++++++++--------- pkg/server/sandbox_run_unix.go | 2 +- 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 1307dd1b7..57e9c021a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -225,6 +225,8 @@ type PluginConfig struct { // DisableProcMount disables Kubernetes ProcMount support. This MUST be set to `true` // when using containerd with Kubernetes <=1.11. DisableProcMount bool `toml:"disable_proc_mount" json:"disableProcMount"` + // DefaultSeccompProfile is a seccomp profile to use if not provided by k8s. + DefaultSeccompProfile string `toml:"default_seccomp_profile" json:"defaultSeccompProfile"` } // X509KeyPairStreaming contains the x509 configuration for streaming diff --git a/pkg/server/container_create_unix.go b/pkg/server/container_create_unix.go index ef6cce39b..e44902ea1 100644 --- a/pkg/server/container_create_unix.go +++ b/pkg/server/container_create_unix.go @@ -286,7 +286,7 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon specOpts = append(specOpts, apparmorSpecOpts) } - seccompSpecOpts, err := generateSeccompSpecOpts( + seccompSpecOpts, err := c.generateSeccompSpecOpts( securityContext.GetSeccompProfilePath(), securityContext.GetPrivileged(), c.seccompEnabled()) @@ -300,11 +300,14 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon } // generateSeccompSpecOpts generates containerd SpecOpts for seccomp. -func generateSeccompSpecOpts(seccompProf string, privileged, seccompEnabled bool) (oci.SpecOpts, error) { +func (c *criService) generateSeccompSpecOpts(seccompProf string, privileged, seccompEnabled bool) (oci.SpecOpts, error) { if privileged { // Do not set seccomp profile when container is privileged return nil, nil } + if seccompProf == "" { + seccompProf = c.config.DefaultSeccompProfile + } // Set seccomp profile if seccompProf == runtimeDefault || seccompProf == dockerDefault { // use correct default profile (Eg. if not configured otherwise, the default is docker/default) diff --git a/pkg/server/container_create_unix_test.go b/pkg/server/container_create_unix_test.go index 4b54ae49c..a4f4856ef 100644 --- a/pkg/server/container_create_unix_test.go +++ b/pkg/server/container_create_unix_test.go @@ -20,6 +20,7 @@ package server import ( "context" + "fmt" "os" "path/filepath" "reflect" @@ -779,11 +780,12 @@ func TestNoDefaultRunMount(t *testing.T) { func TestGenerateSeccompSpecOpts(t *testing.T) { for desc, test := range map[string]struct { - profile string - privileged bool - disable bool - specOpts oci.SpecOpts - expectErr bool + profile string + privileged bool + disable bool + specOpts oci.SpecOpts + expectErr bool + defaultProfile string }{ "should return error if seccomp is specified when seccomp is not supported": { profile: runtimeDefault, @@ -824,17 +826,28 @@ func TestGenerateSeccompSpecOpts(t *testing.T) { profile: "test-profile", expectErr: true, }, + "should use default profile when seccomp is empty": { + defaultProfile: profileNamePrefix + "test-profile", + specOpts: seccomp.WithProfile("test-profile"), + }, + "should fallback to docker/default when seccomp is empty and default is runtime/default": { + defaultProfile: runtimeDefault, + specOpts: seccomp.WithDefaultProfile(), + }, } { - t.Logf("TestCase %q", desc) - specOpts, err := generateSeccompSpecOpts(test.profile, test.privileged, !test.disable) - assert.Equal(t, - reflect.ValueOf(test.specOpts).Pointer(), - reflect.ValueOf(specOpts).Pointer()) - if test.expectErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } + t.Run(fmt.Sprintf("TestCase %q", desc), func(t *testing.T) { + cri := &criService{} + cri.config.DefaultSeccompProfile = test.defaultProfile + specOpts, err := cri.generateSeccompSpecOpts(test.profile, test.privileged, !test.disable) + assert.Equal(t, + reflect.ValueOf(test.specOpts).Pointer(), + reflect.ValueOf(specOpts).Pointer()) + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) } } diff --git a/pkg/server/sandbox_run_unix.go b/pkg/server/sandbox_run_unix.go index 2f4fd1a54..cf460722c 100644 --- a/pkg/server/sandbox_run_unix.go +++ b/pkg/server/sandbox_run_unix.go @@ -161,7 +161,7 @@ func (c *criService) sandboxContainerSpecOpts(config *runtime.PodSandboxConfig, securityContext = config.GetLinux().GetSecurityContext() specOpts []oci.SpecOpts ) - seccompSpecOpts, err := generateSeccompSpecOpts( + seccompSpecOpts, err := c.generateSeccompSpecOpts( securityContext.GetSeccompProfilePath(), securityContext.GetPrivileged(), c.seccompEnabled()) From 674fe72aa85c7329aed7a292a44853bb1066fc3c Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Sun, 10 May 2020 10:46:58 -0700 Subject: [PATCH 2/2] Update docs for unset seccomp profile Signed-off-by: Maksym Pavlenko --- docs/config.md | 4 ++++ pkg/config/config.go | 5 +++-- pkg/server/container_create_unix.go | 2 +- pkg/server/container_create_unix_test.go | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/config.md b/docs/config.md index c6529dd9a..402967001 100644 --- a/docs/config.md +++ b/docs/config.md @@ -78,6 +78,10 @@ version = 2 # when using containerd with Kubernetes <=1.11. disable_proc_mount = false + # unsetSeccompProfile is the profile containerd/cri will use if the provided seccomp profile is + # unset (`""`) for a container (default is `unconfined`) + unset_seccomp_profile = "" + # 'plugins."io.containerd.grpc.v1.cri".containerd' contains config related to containerd [plugins."io.containerd.grpc.v1.cri".containerd] diff --git a/pkg/config/config.go b/pkg/config/config.go index 57e9c021a..d2e10f192 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -225,8 +225,9 @@ type PluginConfig struct { // DisableProcMount disables Kubernetes ProcMount support. This MUST be set to `true` // when using containerd with Kubernetes <=1.11. DisableProcMount bool `toml:"disable_proc_mount" json:"disableProcMount"` - // DefaultSeccompProfile is a seccomp profile to use if not provided by k8s. - DefaultSeccompProfile string `toml:"default_seccomp_profile" json:"defaultSeccompProfile"` + // UnsetSeccompProfile is the profile containerd/cri will use If the provided seccomp profile is + // unset (`""`) for a container (default is `unconfined`) + UnsetSeccompProfile string `toml:"unset_seccomp_profile" json:"unsetSeccompProfile"` } // X509KeyPairStreaming contains the x509 configuration for streaming diff --git a/pkg/server/container_create_unix.go b/pkg/server/container_create_unix.go index e44902ea1..0324bc206 100644 --- a/pkg/server/container_create_unix.go +++ b/pkg/server/container_create_unix.go @@ -306,7 +306,7 @@ func (c *criService) generateSeccompSpecOpts(seccompProf string, privileged, sec return nil, nil } if seccompProf == "" { - seccompProf = c.config.DefaultSeccompProfile + seccompProf = c.config.UnsetSeccompProfile } // Set seccomp profile if seccompProf == runtimeDefault || seccompProf == dockerDefault { diff --git a/pkg/server/container_create_unix_test.go b/pkg/server/container_create_unix_test.go index a4f4856ef..b27bb38e2 100644 --- a/pkg/server/container_create_unix_test.go +++ b/pkg/server/container_create_unix_test.go @@ -837,7 +837,7 @@ func TestGenerateSeccompSpecOpts(t *testing.T) { } { t.Run(fmt.Sprintf("TestCase %q", desc), func(t *testing.T) { cri := &criService{} - cri.config.DefaultSeccompProfile = test.defaultProfile + cri.config.UnsetSeccompProfile = test.defaultProfile specOpts, err := cri.generateSeccompSpecOpts(test.profile, test.privileged, !test.disable) assert.Equal(t, reflect.ValueOf(test.specOpts).Pointer(),