sandbox/apparmor: aare exclusion rule generation (canonical#13488)

* sandbox/apparmor: add GenerateAAREExclusionPatterns

This function is generic (and complex) enough to be able to handle all of the
overlapping and wildcard behavior we need in docker-support, and it could also
serve to replace numerous other places in the codebase where we need this sort
of complex behavior. It is a generalization of the existing
aareExclusionPatterns helper, though it's actually unclear if this exact
implementation will currently be able to serve the use case from that helper
directly or if more options/adjustments are needed to enable that use case as
well.

To keep the diff smaller, this patch does not actually change any of the
profiles/interfaces, just TODO's are left for where to use it.

Note that the generated rules are slightly more condensed in terms of number of
rules but significantly more verbose in terms of alternations, not sharing more
of repeated substrings between alternations inside the patterns. This was done
explicitly to keep the generating code simpler and easier to understand, but it
may prove to have performance effects, either detrimental or benevolent but
that should be measured before deciding to make the generation code even more
complex than it already is.

Signed-off-by: Ian Johnson <[email protected]>

* interfaces/docker-support: generate AARE exclusion patterns with helper func

Signed-off-by: Ian Johnson <[email protected]>

* sandbox/apparmor: unexport helper functions

These were not meant to be exported, only the fully generic one is meant to be
exported.

Signed-off-by: Ian Johnson <[email protected]>

* sandbox/apparmor: fix bug mis-sorting capitalized letters in AARE exclude patt

Thanks to Alberto for spotting this :-)

Signed-off-by: Ian Johnson <[email protected]>

* sandbox/apparmor: fix format issues introduced during rebase

* sandbox/apparmor: simplify generateAAREExclusionPatternsGenericImpl

* sandbox/apparmor: add checks for unsupported cases and improve documentation

* sandbox/apparmor: update tests to compare the apparmor binary instead of source

* interfaces/builtin/docker_support: check if userns is supported before adding it to the profile

* interfaces/builtin/docker_support: fix dependencies

* sandbox/apparmor: use placeholders

* i/b/docker_support_test: update TestGenerateAAREExclusionPatterns to use SnapAppSet

* testutil/apparmor: use go crypto/sha1 module instead of system sha1sum command

* {sandbox,testutil}/apparmor: minor format fixes

* move helper to find common prefix to strutil

* add copyright info

* use string builder

* i/b/docker_support_test.go: update accordingly to 277fbc2 (many: add components to interfaces.SnapAppSet (canonical#13837))

* strutil/commonprefix.go: remove extra empty line

* sandbox/apparmor/apparmor.go: sort prefixes to ensure profile is always the same

* sandbox/apparmor/apparmor.go: remove extra empty line

* i/b/docker_support_test: skip TestGenerateAAREExclusionPatterns is apparmor_parser is not usable

---------

Signed-off-by: Ian Johnson <[email protected]>
Co-authored-by: Ian Johnson <[email protected]>

cmd/snap-confine/snap-confine.apparmor.in

@@ -137,6 +137,9 @@
     # LD_PRELOAD to be set to a library which is in a directory configured by
     # ld.so.conf, but access to those locations is mediated by this profile
     # (which requires rules for specific locations).
+    # TODO: use GenerateAAREExclusionPatterns for this, though the first
+    # rule and the fact that the generative aspect is not an absolute filepath
+    # complicate using that function directly
     change_profile unsafe /** -> [^u/]**,
     change_profile unsafe /** -> u[^n]**,
     change_profile unsafe /** -> un[^c]**,

interfaces/apparmor/template.go

@@ -681,6 +681,7 @@ var defaultOtherBaseTemplateRules = `
   # - /lib/modules
   #
   # Everything but /lib/firmware and /lib/modules
+  # TODO: use GenerateAAREExclusionPatterns for this
   /{,usr/}lib/ r,
   /{,usr/}lib/[^fm]** mrklix,
   /{,usr/}lib/{f[^i],m[^o]}** mrklix,
@@ -709,6 +710,7 @@ var defaultOtherBaseTemplateRules = `
   #
   # Everything but /usr/lib and /usr/src, which are handled elsewhere.
   /usr/ r,
+  # TODO: use GenerateAAREExclusionPatterns for this
   /usr/[^ls]** mrklix,
   /usr/{l[^i],s[^r]}** mrklix,
   /usr/{li[^b],sr[^c]}** mrklix,

interfaces/builtin/home.go

@@ -56,6 +56,9 @@ const homeConnectedPlugAppArmor = `
 
 # Allow read/write access to all files in @{HOME}, except snap application
 # data in @{HOME}/snap and toplevel hidden directories in @{HOME}.
+# TODO: use GenerateAAREExclusionPatterns for this - though the first
+# rule here complicates using it slightly from the inclusion of the "." to 
+# prevent reading dotfiles
 ###PROMPT### owner @{HOME}/[^s.]**             rwkl###HOME_IX###,
 ###PROMPT### owner @{HOME}/s[^n]**             rwkl###HOME_IX###,
 ###PROMPT### owner @{HOME}/sn[^a]**            rwkl###HOME_IX###,
@@ -85,6 +88,9 @@ audit deny @{HOME}/bin wl,
 const homeConnectedPlugAppArmorWithAllRead = `
 # Allow non-owner read to non-hidden and non-snap files and directories
 capability dac_read_search,
+# TODO: use GenerateAAREExclusionPatterns for this - though the first
+# rule here complicates using it slightly from the inclusion of the "." to 
+# prevent reading dotfiles
 ###PROMPT### @{HOME}/               r,
 ###PROMPT### @{HOME}/[^s.]**        r,
 ###PROMPT### @{HOME}/s[^n]**        r,

interfaces/builtin/system_backup.go

@@ -34,6 +34,7 @@ const systemBackupConnectedPlugAppArmor = `
 capability dac_read_search,
 
 # read access to everything except items under /dev, /sys and /proc
+# TODO: use GenerateAAREExclusionPatterns for this
 /{,var/lib/snapd/hostfs/}[^dsp]** r,
 /{,var/lib/snapd/hostfs/}{d[^e],s[^y],p[^r]}** r,
 /{,var/lib/snapd/hostfs/}{de[^v],sy[^s],pr[^o]}** r,

interfaces/builtin/utils.go

@@ -85,6 +85,9 @@ func verifySlotPathAttribute(slotRef *interfaces.SlotRef, attrs interfaces.Attre
 //	  "f[^o]*",
 //	  "fo[^o]*",
 //	}
+//
+// For a more generic version of this see GenerateAAREExclusionPatterns
+// TODO: can this be rewritten to use/share code with that function instead?
 func aareExclusivePatterns(orig string) []string {
 	// This function currently is only intended to be used with desktop
 	// prefixes as calculated by info.DesktopPrefix (the snap name and

sandbox/apparmor/apparmor.go

@@ -22,6 +22,7 @@ package apparmor
 import (
 	"bufio"
 	"bytes"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -56,6 +57,234 @@ func ValidateNoAppArmorRegexp(s string) error {
 	return nil
 }
 
+type AAREExclusionPatternsOptions struct {
+	// Prefix is a string to include on every line in the permutations before
+	// the exclusion permutation itself.
+	Prefix string
+	// Suffix is a string to include on every line in the permutations after
+	// the exclusion permutation itself.
+	Suffix string
+
+	// TODO: add options for generating non-filepaths like what we need for the
+	// unconfined profile transition exclusion in snap-confine's profile as well
+	// as an option for adding an extra character to the very first rule like we
+	// need in the home interface
+}
+
+// InsertAAREExclusionPatterns replaces a ###EXCL{<pref>,<suf>}### snippet
+// with matching prefix and comma separated suffixes with a set of rules generated
+// by GenerateAAREExclusionPatterns.
+func InsertAAREExclusionPatterns(aaRules string, excludePatterns []string, opts *AAREExclusionPatternsOptions) (string, error) {
+	exclussionPatterns, err := GenerateAAREExclusionPatterns(excludePatterns, opts)
+	if err != nil {
+		return "", err
+	}
+
+	placeHolder := fmt.Sprintf("###EXCL{%s<>%s:%s}###", opts.Prefix, opts.Suffix, strings.Join(excludePatterns[:], ","))
+
+	if !strings.Contains(aaRules, placeHolder) {
+		return "", fmt.Errorf("placeholder pattern %q not found", placeHolder)
+	}
+	return strings.Replace(aaRules, placeHolder, strings.TrimSuffix(exclussionPatterns, "\n"), -1), nil
+}
+
+// GenerateAAREExclusionPatterns generates a series of valid AppArmor
+// regular expression negation rules such that anything except the specific
+// excludePatterns will match with the specified prefix and suffix rules. For
+// example to allow reading any file except those matching /usr/*/foo, you would
+// call this function with "/usr/*/foo" as the first argument, "" as the prefix
+// and " r," as the suffix (the suffix being the main part of the read rule) and
+// this function would return the following multi-line string with the relevant
+// rules:
+//
+// /[^u]** r,
+// /u[^s]** r,
+// /us[^r]** r,
+// /usr[^/]** r,
+// /usr/*/[^f]** r,
+// /usr/*/f[^o]** r,
+// /usr/*/fo[^o]** r,
+//
+// This function only treats '*' specially in the string and does not handle any
+// other alternations etc. that AARE may more generally support, and all
+// patterns provided must be absolute filepaths that are at least 2 runes long.
+//
+// This function also works with multiple exclude patterns such as specifying to
+// exclude "/usr/lib/snapd" and "/var/lib/snapd" with suffix " r," would yield:
+//
+// /[^uv]** r,
+// /{u[^s],v[^a]}** r,
+// /{us[^r],va[^r]}** r,
+// /{usr[^/],var[^/]}** r,
+// /{usr/[^l],var/[^l]}** r,
+// /{usr/l[^i],var/l[^i]}** r,
+// /{usr/li[^b],var/li[^b]}** r,
+// /{usr/lib[^/],var/lib[^/]}** r,
+// /{usr/lib/[^s],var/lib/[^s]}** r,
+// /{usr/lib/s[^n],var/lib/s[^n]}** r,
+// /{usr/lib/sn[^a],var/lib/sn[^a]}** r,
+// /{usr/lib/sna[^p],var/lib/sna[^p]}** r,
+// /{usr/lib/snap[^d],var/lib/snap[^d]}** r,
+//
+// Note that with the previous rules, /usr/lib/snapdaemon would not match any rule
+//
+// This function has the following limitations:
+// The first character after a subpattern common to two or more excludePatterns cannot
+// be '*' on any of the excludePatterns that share the common prefix.
+// Eg. ["/snap/core/**", "/snap/*/core/**"] where '/snap/' would be the subpattern common
+// to both excludePattern, and '*' would be the first character after the common subpattern
+// in the second excludePattern.
+// This is because there are no apparmor rules that can fulfill both requirements.
+// For /snap/[^c]** -> It will also match /snap/a/core/** which should be excluded by
+// the second pattern.
+// For /snap/[^c*]** -> It will also exclude access to /snap/a/a that should be allowed
+// as it is not explicitly excluded by any pattern
+//
+// When the '*' is used to exclude suffixes, like in ["/*.bin"], rules should be generated
+// in a reverse way:
+//
+// /*[^n]{,/**} rw,
+// /*[^i]n{,/**} rw,
+// /*[^b]in{,/**} rw,
+// /*[^.]bin{,/**} rw,
+//
+// While generating those rules is technically possible, it will make the logic way more
+// complex, thus the function would just return an error if a pattern of this kind is found.
+// This functionality can be added in a subsequent PR if needed in the future
+func GenerateAAREExclusionPatterns(excludePatterns []string, opts *AAREExclusionPatternsOptions) (string, error) {
+	if len(excludePatterns) == 0 {
+		return "", errors.New("no patterns provided")
+	}
+	seen := map[string]bool{}
+	for _, patt := range excludePatterns {
+		// check for duplicates
+		if seen[patt] {
+			return "", errors.New("exclude patterns contain duplicates")
+		}
+		seen[patt] = true
+
+		// check if it is at least legnth 2
+		if len(patt) < 2 {
+			return "", errors.New("exclude patterns must be at least length 2")
+		}
+
+		// check that it starts as an absolute path
+		if patt[0] != '/' {
+			return "", errors.New("exclude patterns must be absolute filepaths")
+		}
+
+		// TODO: should we also validate that the only character in the pattern
+		// from AARE is "*" ?
+	}
+	if opts == nil {
+		opts = &AAREExclusionPatternsOptions{}
+	}
+	return generateAAREExclusionPatternsGenericImpl(excludePatterns, opts)
+
+}
+
+func generateAAREExclusionPatternsGenericImpl(excludePatterns []string, opts *AAREExclusionPatternsOptions) (string, error) {
+	// Find the length of longest pattern (size)
+	size := 0
+	for _, pattern := range excludePatterns {
+		if len(pattern) > size {
+			size = len(pattern)
+		}
+	}
+
+	// Find the longest prefix common to ALL patterns.
+	commonPrefix, _ := strutil.FindCommonPrefix(excludePatterns)
+
+	// This loop will iterate over the length of the longest excludePattern
+	// (charInd = 1..size), generating an apparmor rule on each iteration
+	// for the corresponding subpatterns, understanding as such, the first
+	// (charInd+1) characters of the excludePatterns.
+	var builder strings.Builder
+	for charInd := 1; charInd < size; charInd++ {
+		// This loop will group the subpatterns properly, generating the subpatterns map, where:
+		//     - the key would be the subpatternPrefix, considering as such the subpattern except
+		//       its last character (pattern[0:charInd]).
+		//     - the value would be the charset, which would be the subpattern last character
+		//       (pattern[charInd]). If several subpatterns share the same subpatternPrefix, the
+		//       charset would be a string including the last character of all those subpatterns.
+		subpatternPrefix := ""
+		subpatterns := map[string]string{}
+		for _, pattern := range excludePatterns {
+			// Handle unsupported cases
+			if (charInd < len(pattern)) && pattern[charInd] == '*' {
+				// Check if the excludePattern has a character different from '/' after a wildcard
+				if ((charInd + 1) < len(pattern)) && (pattern[charInd+1] != '/') {
+					return "", errors.New("exclude patterns does not support suffixes for now")
+				}
+				// Check if '*' is the first character after a common subpattern
+				for _, patt := range excludePatterns {
+					if (patt != pattern) && ((charInd) < len(patt)) && (pattern[:charInd] == patt[:charInd]) && (patt[charInd] != '*') {
+						return "", errors.New("first character after a common subpattern cannot be a wildcard")
+					}
+				}
+			}
+
+			// Skip patterns that are already finished, wildcards and slashes preceded by wildcards.
+			if (charInd >= len(pattern)) ||
+				(pattern[charInd] == '*') ||
+				((pattern[charInd] == '/') && (pattern[charInd-1] == '*')) {
+				continue
+			}
+			// Group subpatterns
+			subpatternPrefix = pattern[:charInd]
+			if charset, exists := subpatterns[subpatternPrefix]; !exists {
+				// Add the pattern if it didn't exist yet
+				subpatterns[subpatternPrefix] = string(pattern[charInd])
+			} else {
+				if !strings.Contains(charset, string(pattern[charInd])) {
+					// Two patterns only differ on the last character
+					subpatterns[subpatternPrefix] = charset + string(pattern[charInd])
+				}
+			}
+		}
+
+		// Write patterns
+		if len(subpatterns) > 0 {
+			// First order keys to ensure profiles are always the same
+			// Sort key in map to ensure consistency in results
+			prefixes := make([]string, 0, len(subpatterns))
+			for prefix := range subpatterns {
+				prefixes = append(prefixes, prefix)
+			}
+			sort.Strings(prefixes)
+
+			// <prefix><common-prefix><exp><suffix>
+			// eg. /squashfs-root/usr/lib/[^a]**			if len(subpatterns) == 1
+			// eg. /squashfs-root/usr/lib/{[^a],[^b]}**		if len(subpatterns) > 1
+			builder.WriteString(opts.Prefix)
+			if charInd < len(commonPrefix) {
+				builder.WriteString(commonPrefix[:charInd])
+			} else {
+				builder.WriteString(commonPrefix)
+			}
+			if len(subpatterns) > 1 {
+				builder.WriteRune('{')
+			}
+			for i := range prefixes {
+				if i > 0 {
+					builder.WriteRune(',')
+				}
+				if len(commonPrefix) < len(prefixes[i]) {
+					builder.WriteString(prefixes[i][len(commonPrefix):])
+				}
+				builder.WriteString("[^" + subpatterns[prefixes[i]] + "]")
+			}
+			if len(subpatterns) > 1 {
+				builder.WriteRune('}')
+			}
+			builder.WriteString("**")
+			builder.WriteString(opts.Suffix)
+			builder.WriteRune('\n')
+		}
+	}
+	return builder.String(), nil
+}
+
 // LevelType encodes the kind of support for apparmor
 // found on this system.
 type LevelType int