Improve the apparmor read-write file permissions to allow any file in a directory if already two files are allowed

internal/pkg/daemon/bpfrecorder/bpfrecorder_apparmor.go

@@ -23,6 +23,7 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"path/filepath"
 	"regexp"
 	"slices"
 	"strings"
@@ -264,26 +265,7 @@ func (b *AppArmorRecorder) processExecFsEvents(mid mntnsID) BpfAppArmorFileProce
 	}
 
 	for fileName, access := range b.recordedFiles[mid] {
-		// Workaround for HUGETLB support with apparmor:
-		// AppArmor treats mmap(..., MAP_ANONYMOUS | MAP_HUGETLB) calls as
-		// file access to "", which is then attached to "/" (attach_disconnected).
-		// So for HUGETLB to work with AppArmor, we need a `/ rw` rule in our profile.
-		// (note that there is no wildcard here - subdirectories/files are not affected).
-		// https://gitlab.com/apparmor/apparmor/-/issues/345
-		//
-		// At the same time, eBPF's bpf_d_path is also slightly confused and reports
-		// access to a path named "/anon_hugepage (deleted)" on mmap. Instead of building complex
-		// workarounds and hooking mmap, we just treat that as a canary for HUGETLB usage.
-		if fileName == "/anon_hugepage (deleted)" {
-			processedEvents.ReadWritePaths = append(processedEvents.ReadWritePaths, "/")
-			continue
-		}
-
-		// This is returned by the kernel when a dentry is removed.
-		// https://github.com/torvalds/linux/blob/2e1b3cc9d7f790145a80cb705b168f05dab65df2/fs/d_path.c#L255-L288
-		//
-		// It should be ignored since is an invalid path in the apparmor profile.
-		if fileName == "/ (deleted)" {
+		if ok := processDeletedFiles(fileName, &processedEvents); ok {
 			continue
 		}
 
@@ -313,6 +295,14 @@ func (b *AppArmorRecorder) processExecFsEvents(mid mntnsID) BpfAppArmorFileProce
 		}
 	}
 
+	// Allow any files in a directory if already at least two files are allowed to have read-write
+	// permissions. There are binaries like nginx which typically create files with random name on
+	// every start in the /etc/nginx/config.d/ directory. These random named files cannot be captured
+	// in advance and allowed in the apparmor profile. This logic SHOULD NOT be applied to read-only
+	// files because in that case the file paths are static and should be captured up-front by the
+	// recorder.
+	processedEvents.ReadWritePaths = allowAnyFiles(processedEvents.ReadWritePaths)
+
 	slices.Sort(processedEvents.AllowedExecutables)
 	slices.Sort(processedEvents.AllowedLibraries)
 	slices.Sort(processedEvents.ReadOnlyPaths)
@@ -322,6 +312,54 @@ func (b *AppArmorRecorder) processExecFsEvents(mid mntnsID) BpfAppArmorFileProce
 	return processedEvents
 }
 
+// processDeletedFiles process file paths which are marked as deleted by the Linux kernel.
+func processDeletedFiles(fileName string, processedEvents *BpfAppArmorFileProcessed) bool {
+	// Workaround for HUGETLB support with apparmor:
+	// AppArmor treats mmap(..., MAP_ANONYMOUS | MAP_HUGETLB) calls as
+	// file access to "", which is then attached to "/" (attach_disconnected).
+	// So for HUGETLB to work with AppArmor, we need a `/ rw` rule in our profile.
+	// (note that there is no wildcard here - subdirectories/files are not affected).
+	// https://gitlab.com/apparmor/apparmor/-/issues/345
+	//
+	// At the same time, eBPF's bpf_d_path is also slightly confused and reports
+	// access to a path named "/anon_hugepage (deleted)" on mmap. Instead of building complex
+	// workarounds and hooking mmap, we just treat that as a canary for HUGETLB usage.
+	if fileName == "/anon_hugepage (deleted)" {
+		processedEvents.ReadWritePaths = append(processedEvents.ReadWritePaths, "/")
+		return true
+	}
+
+	// This is returned by the kernel when a dentry is removed.
+	// https://github.com/torvalds/linux/blob/2e1b3cc9d7f790145a80cb705b168f05dab65df2/fs/d_path.c#L255-L288
+	//
+	// It should be ignored since is an invalid path in the apparmor profile.
+	if fileName == "/ (deleted)" {
+		return true
+	}
+
+	return false
+}
+
+// allowAnyFiles allows any file in a directory if more than two files are allowed.
+func allowAnyFiles(filePaths []string) []string {
+	dupDirs := map[string]int{}
+	for _, fp := range filePaths {
+		dir := filepath.Dir(fp)
+		dupDirs[dir] += 1
+	}
+	result := []string{}
+	for _, fp := range filePaths {
+		dir := filepath.Dir(fp)
+		if dupDirs[dir] > 1 {
+			result = append(result, filepath.Join(dir, "*"))
+			dupDirs[dir] = 0
+		} else if dupDirs[dir] == 1 {
+			result = append(result, fp)
+		}
+	}
+	return result
+}
+
 func (b *AppArmorRecorder) processCapabilities(mid mntnsID) []string {
 	if _, ok := b.recordedCapabilities[mid]; !ok {
 		return []string{}

internal/pkg/daemon/bpfrecorder/bpfrecorder_apparmor_test.go

@@ -63,3 +63,45 @@ func TestReplaceVarianceInFilePath(t *testing.T) {
 		})
 	}
 }
+
+func TestAllowAnyFiles(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name  string
+		paths []string
+		want  []string
+	}{
+		{
+			name:  "allow any files if at least two files are already allowed",
+			paths: []string{"/etc/nginx/conf.d/default.conf", "/dev/null", "/etc/nginx/conf.d/sedIWASqqq"},
+			want:  []string{"/etc/nginx/conf.d/*", "/dev/null"},
+		},
+		{
+			name: "allow any files if more than two files are already allowed",
+			paths: []string{
+				"/etc/nginx/conf.d/default.conf", "/dev/null",
+				"/etc/nginx/conf.d/sedIWASqqq", "/etc/nginx/conf.d/abcd",
+			},
+			want: []string{"/etc/nginx/conf.d/*", "/dev/null"},
+		},
+		{
+			name:  "do not allow any files ",
+			paths: []string{"/etc/nginx/conf.d/default.conf", "/dev/null"},
+			want:  []string{"/etc/nginx/conf.d/default.conf", "/dev/null"},
+		},
+		{
+			name:  "do not allow anything if nothing is allowed",
+			paths: []string{},
+			want:  []string{},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			got := allowAnyFiles(test.paths)
+			require.Equal(t, test.want, got)
+		})
+	}
+}