From 1f8c21166edfd7d0ff0d5e4d347340569c1e01a0 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Tue, 12 Jan 2021 14:06:58 +0100 Subject: [PATCH] Remove AppArmor loaded profile validation In general it could be possible that init containers deploy security profiles. The existing AppArmor pre-validation would block the complete workload without this patch being applied. If we now schedule a workload which contains an unconfined init container, then we will skip the validation. The underlying container runtime will fail if the profile is not available after the execution of the init container. This synchronizes the overall behavior with seccomp. Signed-off-by: Sascha Grunert --- pkg/security/apparmor/validate.go | 57 +------------------------- pkg/security/apparmor/validate_test.go | 36 ++-------------- 2 files changed, 4 insertions(+), 89 deletions(-) diff --git a/pkg/security/apparmor/validate.go b/pkg/security/apparmor/validate.go index 210ff99fe10..8d585f7df08 100644 --- a/pkg/security/apparmor/validate.go +++ b/pkg/security/apparmor/validate.go @@ -73,14 +73,9 @@ func (v *validator) Validate(pod *v1.Pod) error { return v.validateHostErr } - loadedProfiles, err := v.getLoadedProfiles() - if err != nil { - return fmt.Errorf("could not read loaded profiles: %v", err) - } - var retErr error podutil.VisitContainers(&pod.Spec, podutil.AllContainers, func(container *v1.Container, containerType podutil.ContainerType) bool { - retErr = validateProfile(GetProfileName(pod, container.Name), loadedProfiles) + retErr = ValidateProfileFormat(GetProfileName(pod, container.Name)) if retErr != nil { return false } @@ -119,22 +114,6 @@ func validateHost(runtime string) error { return nil } -// Verify that the profile is valid and loaded. -func validateProfile(profile string, loadedProfiles map[string]bool) error { - if err := ValidateProfileFormat(profile); err != nil { - return err - } - - if strings.HasPrefix(profile, v1.AppArmorBetaProfileNamePrefix) { - profileName := strings.TrimPrefix(profile, v1.AppArmorBetaProfileNamePrefix) - if !loadedProfiles[profileName] { - return fmt.Errorf("profile %q is not loaded", profileName) - } - } - - return nil -} - // ValidateProfileFormat checks the format of the profile. func ValidateProfileFormat(profile string) error { if profile == "" || profile == v1.AppArmorBetaProfileRuntimeDefault || profile == v1.AppArmorBetaProfileNameUnconfined { @@ -146,40 +125,6 @@ func ValidateProfileFormat(profile string) error { return nil } -func (v *validator) getLoadedProfiles() (map[string]bool, error) { - profilesPath := path.Join(v.appArmorFS, "profiles") - profilesFile, err := os.Open(profilesPath) - if err != nil { - return nil, fmt.Errorf("failed to open %s: %v", profilesPath, err) - } - defer profilesFile.Close() - - profiles := map[string]bool{} - scanner := bufio.NewScanner(profilesFile) - for scanner.Scan() { - profileName := parseProfileName(scanner.Text()) - if profileName == "" { - // Unknown line format; skip it. - continue - } - profiles[profileName] = true - } - return profiles, nil -} - -// The profiles file is formatted with one profile per line, matching a form: -// namespace://profile-name (mode) -// profile-name (mode) -// Where mode is {enforce, complain, kill}. The "namespace://" is only included for namespaced -// profiles. For the purposes of Kubernetes, we consider the namespace part of the profile name. -func parseProfileName(profileLine string) string { - modeIndex := strings.IndexRune(profileLine, '(') - if modeIndex < 0 { - return "" - } - return strings.TrimSpace(profileLine[:modeIndex]) -} - func getAppArmorFS() (string, error) { mountsFile, err := os.Open("/proc/mounts") if err != nil { diff --git a/pkg/security/apparmor/validate_test.go b/pkg/security/apparmor/validate_test.go index f5aa3aa0af2..65ab4e4f4f6 100644 --- a/pkg/security/apparmor/validate_test.go +++ b/pkg/security/apparmor/validate_test.go @@ -21,7 +21,7 @@ import ( "fmt" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/stretchr/testify/assert" @@ -47,16 +47,7 @@ func TestValidateHost(t *testing.T) { assert.Error(t, validateHost("rkt")) } -func TestValidateProfile(t *testing.T) { - loadedProfiles := map[string]bool{ - "docker-default": true, - "foo-bar": true, - "baz": true, - "/usr/sbin/ntpd": true, - "/usr/lib/connman/scripts/dhclient-script": true, - "/usr/lib/NetworkManager/nm-dhcp-client.action": true, - "/usr/bin/evince-previewer//sanitized_helper": true, - } +func TestValidateProfileFormat(t *testing.T) { tests := []struct { profile string expectValid bool @@ -67,12 +58,10 @@ func TestValidateProfile(t *testing.T) { {"baz", false}, // Missing local prefix. {v1.AppArmorBetaProfileNamePrefix + "/usr/sbin/ntpd", true}, {v1.AppArmorBetaProfileNamePrefix + "foo-bar", true}, - {v1.AppArmorBetaProfileNamePrefix + "unloaded", false}, // Not loaded. - {v1.AppArmorBetaProfileNamePrefix + "", false}, } for _, test := range tests { - err := validateProfile(test.profile, loadedProfiles) + err := ValidateProfileFormat(test.profile) if test.expectValid { assert.NoError(t, err, "Profile %s should be valid", test.profile) } else { @@ -121,8 +110,6 @@ func TestValidateValidHost(t *testing.T) { {v1.AppArmorBetaProfileNamePrefix + "foo-container", true}, {v1.AppArmorBetaProfileNamePrefix + "/usr/sbin/ntpd", true}, {"docker-default", false}, - {v1.AppArmorBetaProfileNamePrefix + "foo", false}, - {v1.AppArmorBetaProfileNamePrefix + "", false}, } for _, test := range tests { @@ -155,23 +142,6 @@ func TestValidateValidHost(t *testing.T) { }, } assert.NoError(t, v.Validate(pod), "Multi-container pod should validate") - for k, val := range pod.Annotations { - pod.Annotations[k] = val + "-bad" - assert.Error(t, v.Validate(pod), fmt.Sprintf("Multi-container pod with invalid profile %s:%s", k, pod.Annotations[k])) - pod.Annotations[k] = val // Restore. - } -} - -func TestParseProfileName(t *testing.T) { - tests := []struct{ line, expected string }{ - {"foo://bar/baz (kill)", "foo://bar/baz"}, - {"foo-bar (enforce)", "foo-bar"}, - {"/usr/foo/bar/baz (complain)", "/usr/foo/bar/baz"}, - } - for _, test := range tests { - name := parseProfileName(test.line) - assert.Equal(t, test.expected, name, "Parsing %s", test.line) - } } func getPodWithProfile(profile string) *v1.Pod {