diff --git a/pkg/aa/apparmor_test.go b/pkg/aa/apparmor_test.go index ffdf107d..10cf366b 100644 --- a/pkg/aa/apparmor_test.go +++ b/pkg/aa/apparmor_test.go @@ -144,8 +144,8 @@ func TestAppArmorProfileFile_Sort(t *testing.T) { Rules: []Rule{ include1, all1, rlimit3, userns1, capability1, capability2, network2, network1, mount2, mount1, remount2, umount2, - pivotroot1, changeprofile2, mqueue2, iouring2, signal2, - signal1, ptrace1, unix2, dbus2, dbus1, file1, file2, + pivotroot1, changeprofile2, mqueue2, iouring2, signal1, + signal2, ptrace1, unix2, dbus2, dbus1, file1, file2, link2, includeLocal1, }, }}, diff --git a/pkg/aa/base.go b/pkg/aa/base.go index fb817497..37d3873a 100644 --- a/pkg/aa/base.go +++ b/pkg/aa/base.go @@ -15,6 +15,7 @@ type RuleBase struct { FileInherit bool Prefix string Padding string + Suffix string Optional bool } diff --git a/pkg/aa/rules.go b/pkg/aa/rules.go index cdd36fc7..f292ee55 100644 --- a/pkg/aa/rules.go +++ b/pkg/aa/rules.go @@ -6,6 +6,9 @@ package aa import ( "slices" + "strings" + + "github.com/samber/lo" ) type requirement map[string][]string @@ -145,9 +148,17 @@ func (r Rules) GetIncludes() []*Include { func (r Rules) Merge() Rules { for i := 0; i < len(r); i++ { for j := i + 1; j < len(r); j++ { - typeOfI := r[i].Kind() - typeOfJ := r[j].Kind() - if typeOfI != typeOfJ { + if r[i] == nil && r[j] == nil { + r = r.Delete(j) + j-- + continue + } + if r[i] == nil || r[j] == nil { + continue + } + kindOfI := r[i].Kind() + kindOfJ := r[j].Kind() + if kindOfI != kindOfJ { continue } @@ -159,7 +170,7 @@ func (r Rules) Merge() Rules { } // File rule - if typeOfI == FILE && typeOfJ == FILE { + if kindOfI == FILE && kindOfJ == FILE { // Merge access fileI := r[i].(*File) fileJ := r[j].(*File) @@ -200,27 +211,75 @@ func (r Rules) Sort() Rules { // Follow: https://apparmor.pujol.io/development/guidelines/#the-file-block func (r Rules) Format() Rules { const prefixOwner = " " + suffixMaxlen := 36 + transitions := append(requirements[FILE]["transition"], "m") - hasOwnerRule := false - for i := len(r) - 1; i > 0; i-- { - j := i - 1 - typeOfI := r[i].Kind() - typeOfJ := r[j].Kind() + paddingIndex := []int{} + paddingMaxLenght := 0 + for i, rule := range r { + if rule == nil { + continue + } - // File rule - if typeOfI == FILE && typeOfJ == FILE { - letterI := getLetterIn(fileAlphabet, r[i].(*File).Path) - letterJ := getLetterIn(fileAlphabet, r[j].(*File).Path) + if rule.Kind() == FILE { + rule := r[i].(*File) - // Add prefix before rule path to align with other rule - if r[i].(*File).Owner { - hasOwnerRule = true - } else if hasOwnerRule { - r[i].(*File).Prefix = prefixOwner + // Add padding to align with other transition rule + isTransition := lo.Intersect(transitions, rule.Access) + if len(isTransition) > 0 { + ruleLen := len(rule.Path) + 1 + paddingMaxLenght = max(ruleLen, paddingMaxLenght) + paddingIndex = append(paddingIndex, i) } - if letterI != letterJ { - // Add a new empty line between Files rule of different type + // Add suffix to align comment on udev/data rule + if rule.Comment != "" && strings.HasPrefix(rule.Path, "@{run}/udev/data/") { + suffixlen := suffixMaxlen - len(rule.Path) + if suffixlen < 0 { + suffixlen = 0 + } + rule.Suffix = strings.Repeat(" ", suffixlen) + } + } + } + if len(paddingIndex) > 1 { + r.setPadding(paddingIndex, paddingMaxLenght) + } + + hasOwnerRule := false + for i := len(r) - 1; i >= 0; i-- { + if r[i] == nil { + hasOwnerRule = false + continue + } + + // File rule + if r[i].Kind() == FILE { + rule := r[i].(*File) + + // Add prefix before rule path to align with other rule + if rule.Owner { + hasOwnerRule = true + } else if hasOwnerRule { + rule.Prefix = prefixOwner + } + + // Do not add new line on executable rule + isTransition := lo.Intersect(transitions, rule.Access) + if len(isTransition) > 0 { + continue + } + + // Add a new line between Files rule of different group type + j := i - 1 + if j < 0 || r[j] == nil || r[j].Kind() != FILE { + continue + } + letterI := getLetterIn(fileAlphabet, rule.Path) + letterJ := getLetterIn(fileAlphabet, r[j].(*File).Path) + groupI, ok1 := fileAlphabetGroups[letterI] + groupJ, ok2 := fileAlphabetGroups[letterJ] + if letterI != letterJ && !(ok1 && ok2 && groupI == groupJ) { hasOwnerRule = false r = r.Insert(i, nil) } @@ -228,3 +287,10 @@ func (r Rules) Format() Rules { } return r } + +// setPadding adds padding to the rule path to align with other rules. +func (r *Rules) setPadding(paddingIndex []int, paddingMaxLenght int) { + for _, i := range paddingIndex { + (*r)[i].(*File).Padding = strings.Repeat(" ", paddingMaxLenght-len((*r)[i].(*File).Path)) + } +} diff --git a/pkg/aa/template.go b/pkg/aa/template.go index 28a8d3e4..0fdea74a 100644 --- a/pkg/aa/template.go +++ b/pkg/aa/template.go @@ -94,9 +94,11 @@ var ( fileAlphabet = []string{ "@{exec_path}", // 1. entry point "@{sh_path}", // 2.1 shells - "@{bin}", // 2.1 binaries - "@{lib}", // 2.2 libraries - "/opt", // 2.3 opt binaries & libraries + "@{coreutils_path}", // 2.2 coreutils + "@{open_path}", // 2.3 binaries paths + "@{bin}", // 2.3 binaries + "@{lib}", // 2.4 libraries + "/opt", // 2.5 opt binaries & libraries "/usr/share", // 3. shared data "/etc", // 4. system configuration "/var", // 5.1 system read/write data @@ -107,8 +109,9 @@ var ( "@{user_config_dirs}", // 7.2 user config "@{user_share_dirs}", // 7.3 user shared "/tmp", // 8.1 Temporary data - "@{run}", // 8.2 Runtime data - "/dev/shm", // 8.3 Shared memory + "@{tmp}", // 8.1. User temporary data + "/dev/shm", // 8.2 Shared memory + "@{run}", // 8.3 Runtime data "@{sys}", // 9. Sys files "@{PROC}", // 10. Proc files "/dev", // 11. Dev files @@ -117,6 +120,20 @@ var ( } fileWeights = generateWeights(fileAlphabet) + // Some file rule should be sorted in the same group + fileAlphabetGroups = map[string]string{ + "@{exec_path}": "exec", + "@{sh_path}": "exec", + "@{coreutils_path}": "exec", + "@{open_path}": "exec", + "@{bin}": "exec", + "@{lib}": "exec", + "/opt": "exec", + "/tmp": "tmp", + "@{tmp}": "tmp", + "/dev/shm": "tmp", + } + // The order AARE should be sorted stringAlphabet = []byte( "!\"#$%&'(){}[]*+,-./:;<=>?@\\^_`|~0123456789abcdefghijklmnopqrstuvwxyz",