Merge pull request #49799 from thaJeztah/apparmor_cleanups

profiles/apparmor: add some optimisations and tests

profiles/apparmor/apparmor.go

@@ -1,6 +1,6 @@
 //go:build linux
 
-package apparmor // import "github.com/docker/docker/profiles/apparmor"
+package apparmor
 
 import (
 	"bufio"
@@ -57,69 +57,63 @@ func macroExists(m string) bool {
 // InstallDefault generates a default profile in a temp directory determined by
 // os.TempDir(), then loads the profile into the kernel using 'apparmor_parser'.
 func InstallDefault(name string) error {
-	p := profileData{
-		Name: name,
-	}
-
 	// Figure out the daemon profile.
-	currentProfile, err := os.ReadFile("/proc/self/attr/current")
-	if err != nil {
-		// If we couldn't get the daemon profile, assume we are running
-		// unconfined which is generally the default.
-		currentProfile = nil
+	daemonProfile := "unconfined"
+	if currentProfile, err := os.ReadFile("/proc/self/attr/current"); err == nil {
+		// Normally profiles are suffixed by " (enforcing)" or similar. AppArmor
+		// profiles cannot contain spaces so this doesn't restrict daemon profile
+		// names.
+		if profile, _, _ := strings.Cut(string(currentProfile), " "); profile != "" {
+			daemonProfile = profile
+		}
 	}
-	daemonProfile := string(currentProfile)
-	// Normally profiles are suffixed by " (enforcing)" or similar. AppArmor
-	// profiles cannot contain spaces so this doesn't restrict daemon profile
-	// names.
-	if parts := strings.SplitN(daemonProfile, " ", 2); len(parts) >= 1 {
-		daemonProfile = parts[0]
-	}
-	if daemonProfile == "" {
-		daemonProfile = "unconfined"
-	}
-	p.DaemonProfile = daemonProfile
 
 	// Install to a temporary directory.
-	f, err := os.CreateTemp("", name)
+	tmpFile, err := os.CreateTemp("", name)
 	if err != nil {
 		return err
 	}
-	profilePath := f.Name()
 
-	defer f.Close()
-	defer os.Remove(profilePath)
+	defer func() {
+		_ = tmpFile.Close()
+		_ = os.Remove(tmpFile.Name())
+	}()
 
-	if err := p.generateDefault(f); err != nil {
+	p := profileData{
+		Name:          name,
+		DaemonProfile: daemonProfile,
+	}
+	if err := p.generateDefault(tmpFile); err != nil {
 		return err
 	}
 
-	return loadProfile(profilePath)
+	return loadProfile(tmpFile.Name())
 }
 
 // IsLoaded checks if a profile with the given name has been loaded into the
 // kernel.
 func IsLoaded(name string) (bool, error) {
-	file, err := os.Open("/sys/kernel/security/apparmor/profiles")
+	return isLoaded(name, "/sys/kernel/security/apparmor/profiles")
+}
+
+func isLoaded(name string, fileName string) (bool, error) {
+	file, err := os.Open(fileName)
 	if err != nil {
 		return false, err
 	}
 	defer file.Close()
 
-	r := bufio.NewReader(file)
-	for {
-		p, err := r.ReadString('\n')
-		if err == io.EOF {
-			break
-		}
-		if err != nil {
-			return false, err
-		}
-		if strings.HasPrefix(p, name+" ") {
+	scanner := bufio.NewScanner(file)
+	for scanner.Scan() {
+		if prefix, _, ok := strings.Cut(scanner.Text(), " "); ok && prefix == name {
 			return true, nil
 		}
 	}
 
+	if err := scanner.Err(); err != nil {
+		return false, err
+	}
+
 	return false, nil
 }
 
@@ -130,9 +124,8 @@ func loadProfile(profilePath string) error {
 	c := exec.Command("apparmor_parser", "-Kr", profilePath)
 	c.Dir = ""
 
-	output, err := c.CombinedOutput()
-	if err != nil {
-		return fmt.Errorf("running `%s %s` failed with output: %s\nerror: %v", c.Path, strings.Join(c.Args, " "), output, err)
+	if output, err := c.CombinedOutput(); err != nil {
+		return fmt.Errorf("running '%s' failed with output: %s\nerror: %v", c, output, err)
 	}
 
 	return nil

profiles/apparmor/apparmor_linux_test.go

@@ -0,0 +1,197 @@
+package apparmor
+
+import (
+	"errors"
+	"os"
+	"path"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// testAppArmorProfiles fixture "/sys/kernel/security/apparmor/profiles"
+// from an Ubuntu 24.10 host.
+const testAppArmorProfiles = `wpcom (unconfined)
+wike (unconfined)
+vpnns (unconfined)
+vivaldi-bin (unconfined)
+virtiofsd (unconfined)
+vdens (unconfined)
+uwsgi-core (unconfined)
+rsyslogd (enforce)
+/usr/lib/snapd/snap-confine (enforce)
+/usr/lib/snapd/snap-confine//mount-namespace-capture-helper (enforce)
+tcpdump (enforce)
+man_groff (enforce)
+man_filter (enforce)
+/usr/bin/man (enforce)
+userbindmount (unconfined)
+unprivileged_userns (enforce)
+unix-chkpwd (enforce)
+ubuntu_pro_esm_cache_systemd_detect_virt (enforce)
+ubuntu_pro_esm_cache_systemctl (enforce)
+ubuntu_pro_esm_cache (enforce)
+ubuntu_pro_esm_cache//ubuntu_distro_info (enforce)
+ubuntu_pro_esm_cache//ps (enforce)
+ubuntu_pro_esm_cache//dpkg (enforce)
+ubuntu_pro_esm_cache//cloud_id (enforce)
+ubuntu_pro_esm_cache//apt_methods_gpgv (enforce)
+ubuntu_pro_esm_cache//apt_methods (enforce)
+ubuntu_pro_apt_news (enforce)
+tuxedo-control-center (unconfined)
+tup (unconfined)
+trinity (unconfined)
+transmission-qt (complain)
+transmission-gtk (complain)
+transmission-daemon (complain)
+transmission-cli (complain)
+toybox (unconfined)
+thunderbird (unconfined)
+systemd-coredump (unconfined)
+surfshark (unconfined)
+stress-ng (unconfined)
+steam (unconfined)
+slirp4netns (unconfined)
+slack (unconfined)
+signal-desktop (unconfined)
+scide (unconfined)
+sbuild-upgrade (unconfined)
+sbuild-update (unconfined)
+sbuild-unhold (unconfined)
+sbuild-shell (unconfined)
+sbuild-hold (unconfined)
+sbuild-distupgrade (unconfined)
+sbuild-destroychroot (unconfined)
+sbuild-createchroot (unconfined)
+sbuild-clean (unconfined)
+sbuild-checkpackages (unconfined)
+sbuild-apt (unconfined)
+sbuild-adduser (unconfined)
+sbuild-abort (unconfined)
+sbuild (unconfined)
+runc (unconfined)
+rssguard (unconfined)
+rpm (unconfined)
+rootlesskit (unconfined)
+qutebrowser (unconfined)
+qmapshack (unconfined)
+qcam (unconfined)
+privacybrowser (unconfined)
+polypane (unconfined)
+podman (unconfined)
+plasmashell (enforce)
+plasmashell//QtWebEngineProcess (enforce)
+pageedit (unconfined)
+opera (unconfined)
+opam (unconfined)
+obsidian (unconfined)
+nvidia_modprobe (enforce)
+nvidia_modprobe//kmod (enforce)
+notepadqq (unconfined)
+nautilus (unconfined)
+msedge (unconfined)
+mmdebstrap (unconfined)
+lxc-usernsexec (unconfined)
+lxc-unshare (unconfined)
+lxc-stop (unconfined)
+lxc-execute (unconfined)
+lxc-destroy (unconfined)
+lxc-create (unconfined)
+lxc-attach (unconfined)
+lsb_release (enforce)
+loupe (unconfined)
+linux-sandbox (unconfined)
+libcamerify (unconfined)
+lc-compliance (unconfined)
+keybase (unconfined)
+kchmviewer (unconfined)
+ipa_verify (unconfined)
+goldendict (unconfined)
+github-desktop (unconfined)
+geary (unconfined)
+foliate (unconfined)
+flatpak (unconfined)
+firefox (unconfined)
+evolution (unconfined)
+epiphany (unconfined)
+element-desktop (unconfined)
+devhelp (unconfined)
+crun (unconfined)
+vscode (unconfined)
+chromium (unconfined)
+chrome (unconfined)
+ch-run (unconfined)
+ch-checkns (unconfined)
+cam (unconfined)
+busybox (unconfined)
+buildah (unconfined)
+brave (unconfined)
+balena-etcher (unconfined)
+Xorg (complain)
+QtWebEngineProcess (unconfined)
+MongoDB Compass (unconfined)
+Discord (unconfined)
+1password (unconfined)
+`
+
+func TestIsLoaded(t *testing.T) {
+	tmpDir := t.TempDir()
+	profiles := path.Join(tmpDir, "apparmor_profiles")
+	if err := os.WriteFile(profiles, []byte(testAppArmorProfiles), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	t.Run("loaded", func(t *testing.T) {
+		found, err := isLoaded("busybox", profiles)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !found {
+			t.Fatal("expected profile to be loaded")
+		}
+	})
+	t.Run("not loaded", func(t *testing.T) {
+		found, err := isLoaded("no-such-profile", profiles)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if found {
+			t.Fatal("expected profile to not be loaded")
+		}
+	})
+	t.Run("error", func(t *testing.T) {
+		_, err := isLoaded("anything", path.Join(tmpDir, "no_such_file"))
+		if err == nil || !errors.Is(err, os.ErrNotExist) {
+			t.Fatalf("expected error to be os.ErrNotExist, got %v", err)
+		}
+	})
+}
+
+func createTestProfiles(b *testing.B, lines int, targetProfile string) string {
+	b.Helper()
+
+	var sb strings.Builder
+	for i := 0; i < lines-1; i++ {
+		sb.WriteString("someprofile (enforcing)\n")
+	}
+	sb.WriteString(targetProfile + " (enforcing)\n")
+
+	fileName := filepath.Join(b.TempDir(), "apparmor_profiles")
+	if err := os.WriteFile(fileName, []byte(sb.String()), 0o644); err != nil {
+		b.Fatal(err)
+	}
+	return fileName
+}
+
+func BenchmarkIsLoaded(b *testing.B) {
+	const target = "myprofile"
+	profiles := createTestProfiles(b, 10000, target)
+
+	b.ReportAllocs()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		found, err := isLoaded(target, profiles)
+		if err != nil || !found {
+			b.Fatalf("expected profile to be found, got found=%v, err=%v", found, err)
+		}
+	}
+}

profiles/apparmor/template.go

@@ -1,6 +1,6 @@
 //go:build linux
 
-package apparmor // import "github.com/docker/docker/profiles/apparmor"
+package apparmor
 
 // NOTE: This profile is replicated in containerd and libpod. If you make a
 //       change to this profile, please make follow-up PRs to those projects so