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 <[email protected]>
jbhou · Mar 12, 2021 · 1f8c211 · 1f8c211
1 parent fcee7a0
commit 1f8c211
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 89 deletions.
diff --git 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
@@ -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 {