* 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 <ian.johnson@canonical.com>
* interfaces/docker-support: generate AARE exclusion patterns with helper func
Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
* 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 <ian.johnson@canonical.com>
* sandbox/apparmor: fix bug mis-sorting capitalized letters in AARE exclude patt
Thanks to Alberto for spotting this :-)
Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
* 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 277fbc266e (many: add components to interfaces.SnapAppSet (#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 <ian.johnson@canonical.com>
Co-authored-by: Ian Johnson <ian.johnson@canonical.com>
This new function is useful for parsing `url.Values` where there may be
multiple values for the same key, and those values may themselves be
comma-separated values.
This is a port of a commit from the `x-go` package, which was originally
intended to be used in pebble. The commit can be found here:
32684ae6fb
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
JoinNonEmpty concatenates non-empty strings with the specified
separator. This overcomes a problem with strings.Join, which
introduces separators for empty strings.
Go 1.19 includes some changes to gofmt which intend to make lists and
heading clearer when rendered (https://go.dev/doc/go1.19). This commit
is the result of running the new gofmt and manually fixing some of it.
This was necessary because the new gofmt assumed lines beginning w/ tabs
to start lists or examples. While this is often true in our codebase,
we occasionally also use tabs to indent the lines after a TODO or FIXME
prefix or in yaml (e.g., excerpts of a snap.yaml). This meant that a lot of the
reformatted comments were broken and had to be fixed manually.
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
We should be careful not to concatenate variable strings into the first
argument of Sprintf/Printf/Errorf: if these variable strings end up
containing a percent character, it will break the way that the printfs
arguments are interpreted. Luckily, golang is smarter than C and is able
to detect mismatches between the number of '%' and the number of
arguments, but it still can lead to unexpected results:
$ sudo snap set core refresh.rate-limit='%[2]#v'
error: cannot perform the following tasks:
- Run configure hook of "core" snap (run hook "configure": cannot parse "%!#(BADINDEX)v": no numerical prefix)
also:
$ sudo snap set core refresh.rate-limit='%#v'
error: cannot perform the following tasks:
- Run configure hook of "core" snap (run hook "configure": cannot parse "&errors.errorString{s:"no numerical prefix"}": %!s(MISSING))
Moreover, it appears that all the occurrences of such pattern in our
code are situated either on unprivileged processes (like the `snap`
client), or, when in snapd, can only be triggered by the root user
(notice the `sudo` in the commands above).
Nevertheless, let's be defensive and fix these.
There are also other occurrences of concatenations in formatting
strings, but those are only constants so they don't pose a problem. But
to avoid the risk of these strings getting updated in the future with a
mutable version, let's explicitly mark these format prefixes as `const`.
The current code in strutil.VersionCompare() is not fully
compatible with what a debian version allows. Specifically
the version number of `libgcc-s1` in 22.04 is valid
(`12-20220319-1ubuntu1`) but it's rejected right now.
The debian policy [1] says:
```
The upstream_version may contain only alphanumerics[14] and the
characters . + - : (full stop, plus, hyphen, colon) and should
start with a digit. If there is no debian_revision then hyphens
are not allowed; if there is no epoch then colons are not allowed.
```
and python-apt/apt/dpkg are all happy with the version. Given that
we generally follow the debian policy for this we should fix this.
Thanks to Gustavo for pointing this out.
[1] https://www.debian.org/doc/debian-policy/ch-controlfields.html#version
* Group 3rd party imports
* Remove unused vars
* Move var only used in tests into test
* Add 'nolint' to ignore warning on unused iotas (might've been
left to improve readability).
