From b4727eafbed15604766b9f9cfab66b5d5424c294 Mon Sep 17 00:00:00 2001 From: Mike Brown Date: Fri, 6 Nov 2020 17:49:35 -0600 Subject: [PATCH] adding code to support seccomp apparmor securityprofile Signed-off-by: Mike Brown --- pkg/cri/server/container_create_linux.go | 74 ++++++++++- pkg/cri/server/container_create_linux_test.go | 125 +++++++++++++++++- pkg/cri/server/sandbox_run_linux.go | 3 +- 3 files changed, 193 insertions(+), 9 deletions(-) diff --git a/pkg/cri/server/container_create_linux.go b/pkg/cri/server/container_create_linux.go index a551b2efb..d5f6d038e 100644 --- a/pkg/cri/server/container_create_linux.go +++ b/pkg/cri/server/container_create_linux.go @@ -307,7 +307,8 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr)) apparmorSpecOpts, err := generateApparmorSpecOpts( - securityContext.GetApparmorProfile(), + securityContext.GetApparmor(), + securityContext.GetApparmorProfile(), // nolint:staticcheck deprecated but we don't want to remove yet securityContext.GetPrivileged(), c.apparmorEnabled()) if err != nil { @@ -318,7 +319,8 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon } seccompSpecOpts, err := c.generateSeccompSpecOpts( - securityContext.GetSeccompProfilePath(), + securityContext.GetSeccomp(), + securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet securityContext.GetPrivileged(), c.seccompEnabled()) if err != nil { @@ -331,12 +333,12 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon } // generateSeccompSpecOpts generates containerd SpecOpts for seccomp. -func (c *criService) generateSeccompSpecOpts(seccompProf string, privileged, seccompEnabled bool) (oci.SpecOpts, error) { +func (c *criService) generateSeccompSpecOpts(sp *runtime.SecurityProfile, seccompProf string, privileged, seccompEnabled bool) (oci.SpecOpts, error) { if privileged { // Do not set seccomp profile when container is privileged return nil, nil } - if seccompProf == "" { + if seccompProf == "" && sp == nil { seccompProf = c.config.UnsetSeccompProfile } // Set seccomp profile @@ -348,8 +350,33 @@ func (c *criService) generateSeccompSpecOpts(seccompProf string, privileged, sec if seccompProf != "" && seccompProf != unconfinedProfile { return nil, errors.New("seccomp is not supported") } + if sp != nil { + if sp.ProfileType != runtime.SecurityProfile_Unconfined { + return nil, errors.New("seccomp is not supported") + } + } return nil, nil } + + if sp != nil { + if sp.ProfileType != runtime.SecurityProfile_Localhost && sp.LocalhostRef != "" { + return nil, errors.New("seccomp config invalid LocalhostRef must only be set if ProfileType is Localhost") + } + switch sp.ProfileType { + case runtime.SecurityProfile_Unconfined: + // Do not set seccomp profile. + return nil, nil + case runtime.SecurityProfile_RuntimeDefault: + return seccomp.WithDefaultProfile(), nil + case runtime.SecurityProfile_Localhost: + // trimming the localhost/ prefix just in case even through it should not + // be necessary with the new SecurityProfile struct + return seccomp.WithProfile(strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix)), nil + default: + return nil, errors.New("seccomp unknown ProfileType") + } + } + switch seccompProf { case "", unconfinedProfile: // Do not set seccomp profile. @@ -367,15 +394,52 @@ func (c *criService) generateSeccompSpecOpts(seccompProf string, privileged, sec } // generateApparmorSpecOpts generates containerd SpecOpts for apparmor. -func generateApparmorSpecOpts(apparmorProf string, privileged, apparmorEnabled bool) (oci.SpecOpts, error) { +func generateApparmorSpecOpts(sp *runtime.SecurityProfile, apparmorProf string, privileged, apparmorEnabled bool) (oci.SpecOpts, error) { if !apparmorEnabled { // Should fail loudly if user try to specify apparmor profile // but we don't support it. if apparmorProf != "" && apparmorProf != unconfinedProfile { return nil, errors.New("apparmor is not supported") } + if sp != nil { + if sp.ProfileType != runtime.SecurityProfile_Unconfined { + return nil, errors.New("apparmor is not supported") + } + } return nil, nil } + + if sp != nil { + if sp.ProfileType != runtime.SecurityProfile_Localhost && sp.LocalhostRef != "" { + return nil, errors.New("apparmor config invalid LocalhostRef must only be set if ProfileType is Localhost") + } + + switch sp.ProfileType { + case runtime.SecurityProfile_Unconfined: + // Do not set apparmor profile. + return nil, nil + case runtime.SecurityProfile_RuntimeDefault: + if privileged { + // Do not set apparmor profile when container is privileged + return nil, nil + } + return apparmor.WithDefaultProfile(appArmorDefaultProfileName), nil + case runtime.SecurityProfile_Localhost: + // trimming the localhost/ prefix just in case even through it should not + // be necessary with the new SecurityProfile struct + appArmorProfile := strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix) + if profileExists, err := appArmorProfileExists(appArmorProfile); !profileExists { + if err != nil { + return nil, errors.Wrap(err, "failed to generate apparmor spec opts") + } + return nil, errors.Errorf("apparmor profile not found %s", appArmorProfile) + } + return apparmor.WithProfile(appArmorProfile), nil + default: + return nil, errors.New("apparmor unknown ProfileType") + } + } + switch apparmorProf { // Based on kubernetes#51746, default apparmor profile should be applied // for when apparmor is not specified. diff --git a/pkg/cri/server/container_create_linux_test.go b/pkg/cri/server/container_create_linux_test.go index be6811e28..c5cc4fcfd 100644 --- a/pkg/cri/server/container_create_linux_test.go +++ b/pkg/cri/server/container_create_linux_test.go @@ -787,7 +787,7 @@ func TestNoDefaultRunMount(t *testing.T) { } } -func TestGenerateSeccompSpecOpts(t *testing.T) { +func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { for desc, test := range map[string]struct { profile string privileged bool @@ -795,6 +795,7 @@ func TestGenerateSeccompSpecOpts(t *testing.T) { specOpts oci.SpecOpts expectErr bool defaultProfile string + sp *runtime.SecurityProfile }{ "should return error if seccomp is specified when seccomp is not supported": { profile: runtimeDefault, @@ -843,11 +844,66 @@ func TestGenerateSeccompSpecOpts(t *testing.T) { defaultProfile: runtimeDefault, specOpts: seccomp.WithDefaultProfile(), }, + //----------------------------------------------- + // now buckets for the SecurityProfile variants + //----------------------------------------------- + "sp should return error if seccomp is specified when seccomp is not supported": { + disable: true, + expectErr: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_RuntimeDefault, + }, + }, + "sp should not return error if seccomp is unconfined when seccomp is not supported": { + disable: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Unconfined, + }, + }, + "sp should not set seccomp when privileged is true": { + privileged: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_RuntimeDefault, + }, + }, + "sp should not set seccomp when seccomp is unconfined": { + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Unconfined, + }, + }, + "sp should not set seccomp when seccomp is not specified": {}, + "sp should set default seccomp when seccomp is runtime/default": { + specOpts: seccomp.WithDefaultProfile(), + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_RuntimeDefault, + }, + }, + "sp should set specified profile when local profile is specified": { + specOpts: seccomp.WithProfile("test-profile"), + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Localhost, + LocalhostRef: profileNamePrefix + "test-profile", + }, + }, + "sp should set specified profile when local profile is specified even without prefix": { + specOpts: seccomp.WithProfile("test-profile"), + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Localhost, + LocalhostRef: "test-profile", + }, + }, + "sp should return error if specified profile is invalid": { + expectErr: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_RuntimeDefault, + LocalhostRef: "test-profile", + }, + }, } { t.Run(fmt.Sprintf("TestCase %q", desc), func(t *testing.T) { cri := &criService{} cri.config.UnsetSeccompProfile = test.defaultProfile - specOpts, err := cri.generateSeccompSpecOpts(test.profile, test.privileged, !test.disable) + specOpts, err := cri.generateSeccompSpecOpts(test.sp, test.profile, test.privileged, !test.disable) assert.Equal(t, reflect.ValueOf(test.specOpts).Pointer(), reflect.ValueOf(specOpts).Pointer()) @@ -867,6 +923,7 @@ func TestGenerateApparmorSpecOpts(t *testing.T) { disable bool specOpts oci.SpecOpts expectErr bool + sp *runtime.SecurityProfile }{ "should return error if apparmor is specified when apparmor is not supported": { profile: runtimeDefault, @@ -918,9 +975,71 @@ func TestGenerateApparmorSpecOpts(t *testing.T) { profile: "test-profile", expectErr: true, }, + //-------------------------------------- + // buckets for SecurityProfile struct + //-------------------------------------- + "sp should return error if apparmor is specified when apparmor is not supported": { + disable: true, + expectErr: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_RuntimeDefault, + }, + }, + "sp should not return error if apparmor is unconfined when apparmor is not supported": { + disable: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Unconfined, + }, + }, + "sp should not apparmor when apparmor is unconfined": { + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Unconfined, + }, + }, + "sp should not apparmor when apparmor is unconfined and privileged is true": { + privileged: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Unconfined, + }, + }, + "sp should set default apparmor when apparmor is runtime/default": { + specOpts: apparmor.WithDefaultProfile(appArmorDefaultProfileName), + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_RuntimeDefault, + }, + }, + "sp should not apparmor when apparmor is default and privileged is true": { + privileged: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_RuntimeDefault, + }, + }, + "sp should return error when undefined local profile is specified": { + expectErr: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Localhost, + LocalhostRef: profileNamePrefix + "test-profile", + }, + }, + "sp should return error when undefined local profile is specified even without prefix": { + profile: profileNamePrefix + "test-profile", + expectErr: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Localhost, + LocalhostRef: "test-profile", + }, + }, + "sp should return error when undefined local profile is specified and privileged is true": { + privileged: true, + expectErr: true, + sp: &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Localhost, + LocalhostRef: profileNamePrefix + "test-profile", + }, + }, } { t.Logf("TestCase %q", desc) - specOpts, err := generateApparmorSpecOpts(test.profile, test.privileged, !test.disable) + specOpts, err := generateApparmorSpecOpts(test.sp, test.profile, test.privileged, !test.disable) assert.Equal(t, reflect.ValueOf(test.specOpts).Pointer(), reflect.ValueOf(specOpts).Pointer()) diff --git a/pkg/cri/server/sandbox_run_linux.go b/pkg/cri/server/sandbox_run_linux.go index f043504e6..575f85c45 100644 --- a/pkg/cri/server/sandbox_run_linux.go +++ b/pkg/cri/server/sandbox_run_linux.go @@ -165,7 +165,8 @@ func (c *criService) sandboxContainerSpecOpts(config *runtime.PodSandboxConfig, specOpts []oci.SpecOpts ) seccompSpecOpts, err := c.generateSeccompSpecOpts( - securityContext.GetSeccompProfilePath(), + securityContext.GetSeccomp(), + securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet securityContext.GetPrivileged(), c.seccompEnabled()) if err != nil {