github/containerd
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

Merge pull request #4700 from mikebrow/cri-security-profile-update

Browse Source 
CRI security profile update for CRI graduation
This commit is contained in:
Mike Brown
2021-01-12 12:21:56 -06:00
committed by GitHub
parent 3b6a386803 6467c3374d
commit 550b4949cb
8 changed files with 1306 additions and 538 deletions
Download Patch File Download Diff File Expand all files Collapse all files

141
pkg/cri/server/container_create_linux.go
View File

@@ -306,8 +306,15 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
}
specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr))
asp := securityContext.GetApparmor()
if asp == nil {
asp, err = generateApparmorSecurityProfile(securityContext.GetApparmorProfile()) // nolint:staticcheck deprecated but we don't want to remove yet
if err != nil {
return nil, errors.Wrap(err, "failed to generate apparmor spec opts")
}
}
apparmorSpecOpts, err := generateApparmorSpecOpts(
securityContext.GetApparmorProfile(),
asp,
securityContext.GetPrivileged(),
c.apparmorEnabled())
if err != nil {
@@ -317,8 +324,17 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
specOpts = append(specOpts, apparmorSpecOpts)
}
ssp := securityContext.GetSeccomp()
if ssp == nil {
ssp, err = generateSeccompSecurityProfile(
securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet
c.config.UnsetSeccompProfile)
if err != nil {
return nil, errors.Wrap(err, "failed to generate seccomp spec opts")
}
}
seccompSpecOpts, err := c.generateSeccompSpecOpts(
securityContext.GetSeccompProfilePath(),
ssp,
securityContext.GetPrivileged(),
c.seccompEnabled())
if err != nil {
@@ -330,70 +346,119 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
return specOpts, nil
}
func generateSeccompSecurityProfile(profilePath string, unsetProfilePath string) (*runtime.SecurityProfile, error) {
if profilePath != "" {
return generateSecurityProfile(profilePath)
}
if unsetProfilePath != "" {
return generateSecurityProfile(unsetProfilePath)
}
return nil, nil
}
func generateApparmorSecurityProfile(profilePath string) (*runtime.SecurityProfile, error) {
if profilePath != "" {
return generateSecurityProfile(profilePath)
}
return nil, nil
}
func generateSecurityProfile(profilePath string) (*runtime.SecurityProfile, error) {
switch profilePath {
case runtimeDefault, dockerDefault, "":
return &runtime.SecurityProfile{
ProfileType: runtime.SecurityProfile_RuntimeDefault,
}, nil
case unconfinedProfile:
return &runtime.SecurityProfile{
ProfileType: runtime.SecurityProfile_Unconfined,
}, nil
default:
// Require and Trim default profile name prefix
if !strings.HasPrefix(profilePath, profileNamePrefix) {
return nil, errors.Errorf("invalid profile %q", profilePath)
}
return &runtime.SecurityProfile{
ProfileType: runtime.SecurityProfile_Localhost,
LocalhostRef: strings.TrimPrefix(profilePath, profileNamePrefix),
}, nil
}
}
// 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, 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.UnsetSeccompProfile
}
// Set seccomp profile
if seccompProf == runtimeDefault || seccompProf == dockerDefault {
// use correct default profile (Eg. if not configured otherwise, the default is docker/default)
seccompProf = seccompDefaultProfile
}
if !seccompEnabled {
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
}
switch seccompProf {
case "", unconfinedProfile:
if sp == nil {
return nil, 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 dockerDefault:
// Note: WithDefaultProfile specOpts must be added after capabilities
case runtime.SecurityProfile_RuntimeDefault:
return seccomp.WithDefaultProfile(), nil
case runtime.SecurityProfile_Localhost:
// trimming the localhost/ prefix just in case even though it should not
// be necessary with the new SecurityProfile struct
return seccomp.WithProfile(strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix)), nil
default:
// Require and Trim default profile name prefix
if !strings.HasPrefix(seccompProf, profileNamePrefix) {
return nil, errors.Errorf("invalid seccomp profile %q", seccompProf)
}
return seccomp.WithProfile(strings.TrimPrefix(seccompProf, profileNamePrefix)), nil
return nil, errors.New("seccomp unknown ProfileType")
}
}
// generateApparmorSpecOpts generates containerd SpecOpts for apparmor.
func generateApparmorSpecOpts(apparmorProf string, privileged, apparmorEnabled bool) (oci.SpecOpts, error) {
func generateApparmorSpecOpts(sp *runtime.SecurityProfile, 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
}
switch apparmorProf {
// Based on kubernetes#51746, default apparmor profile should be applied
// for when apparmor is not specified.
case runtimeDefault, "":
if sp == nil {
// Based on kubernetes#51746, default apparmor profile should be applied
// for when apparmor is not specified.
sp, _ = generateSecurityProfile("")
}
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
}
// TODO (mikebrow): delete created apparmor default profile
return apparmor.WithDefaultProfile(appArmorDefaultProfileName), nil
case unconfinedProfile:
return nil, nil
default:
// Require and Trim default profile name prefix
if !strings.HasPrefix(apparmorProf, profileNamePrefix) {
return nil, errors.Errorf("invalid apparmor profile %q", apparmorProf)
}
appArmorProfile := strings.TrimPrefix(apparmorProf, profileNamePrefix)
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")
@@ -401,6 +466,8 @@ func generateApparmorSpecOpts(apparmorProf string, privileged, apparmorEnabled b
return nil, errors.Errorf("apparmor profile not found %s", appArmorProfile)
}
return apparmor.WithProfile(appArmorProfile), nil
default:
return nil, errors.New("apparmor unknown ProfileType")
}
}

181
pkg/cri/server/container_create_linux_test.go
View File

@@ -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,
@@ -831,10 +832,6 @@ func TestGenerateSeccompSpecOpts(t *testing.T) {
profile: profileNamePrefix + "test-profile",
specOpts: seccomp.WithProfile("test-profile"),
},
"should return error if specified profile is invalid": {
profile: "test-profile",
expectErr: true,
},
"should use default profile when seccomp is empty": {
defaultProfile: profileNamePrefix + "test-profile",
specOpts: seccomp.WithProfile("test-profile"),
@@ -843,18 +840,88 @@ 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)
assert.Equal(t,
reflect.ValueOf(test.specOpts).Pointer(),
reflect.ValueOf(specOpts).Pointer())
if test.expectErr {
assert.Error(t, err)
ssp := test.sp
csp, err := generateSeccompSecurityProfile(
test.profile,
test.defaultProfile)
if err != nil {
if test.expectErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
} else {
assert.NoError(t, err)
if ssp == nil {
ssp = csp
}
specOpts, err := cri.generateSeccompSpecOpts(ssp, 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)
}
}
})
}
@@ -867,6 +934,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,16 +986,91 @@ 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)
assert.Equal(t,
reflect.ValueOf(test.specOpts).Pointer(),
reflect.ValueOf(specOpts).Pointer())
if test.expectErr {
assert.Error(t, err)
asp := test.sp
csp, err := generateApparmorSecurityProfile(test.profile)
if err != nil {
if test.expectErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
} else {
assert.NoError(t, err)
if asp == nil {
asp = csp
}
specOpts, err := generateApparmorSpecOpts(asp, 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)
}
}
}
}

12
pkg/cri/server/sandbox_run_linux.go
View File

@@ -163,9 +163,19 @@ func (c *criService) sandboxContainerSpecOpts(config *runtime.PodSandboxConfig,
var (
securityContext = config.GetLinux().GetSecurityContext()
specOpts []oci.SpecOpts
err error
)
ssp := securityContext.GetSeccomp()
if ssp == nil {
ssp, err = generateSeccompSecurityProfile(
securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet
c.config.UnsetSeccompProfile)
if err != nil {
return nil, errors.Wrap(err, "failed to generate seccomp spec opts")
}
}
seccompSpecOpts, err := c.generateSeccompSpecOpts(
securityContext.GetSeccompProfilePath(),
ssp,
securityContext.GetPrivileged(),
c.seccompEnabled())
if err != nil {