interfaces/apparmor: fix incorrect apparmor profile glob #5109
Conversation
interfaces/apparmor/backend.go
Outdated
| // profileGlobs returns a pair of globs that describe the apparmor profiles of | ||
| // a given snap. The first glob describes profiles for all apps and hooks while | ||
| // the second profile describes the snap-update-ns profile for the whole snap. | ||
| func profileGlobs(snapName string) (string, string) { |
There was a problem hiding this comment.
I would return a []string
interfaces/apparmor/backend.go
Outdated
| @@ -304,8 +311,7 @@ func (b *Backend) Setup(snapInfo *snap.Info, opts interfaces.ConfinementOptions, | |||
| return fmt.Errorf("cannot obtain expected security files for snap %q: %s", snapName, err) | |||
| } | |||
| dir := dirs.SnapAppArmorDir | |||
| glob1 := fmt.Sprintf("snap*.%s*", snapInfo.Name()) | |||
| glob2 := fmt.Sprintf("snap-update-ns.%s", snapInfo.Name()) | |||
| glob1, glob2 := profileGlobs(snapInfo.Name()) | |||
There was a problem hiding this comment.
can we agree that glob1, glob2 is very obscure? It was saved before by being right next to the glob itself, but now that it's separate I think it's worth finding a better name.
Or, continuing @pedronis's comment, you could have it return a struct{ forsnap, forSun } (but not actually sun), and then glob := profileGlobs(...) and then glob.forSnap etc.
There was a problem hiding this comment.
afaict they are used just to build a []string to be consumed by EnsureDirStateGlobs, that why my suggestion
There was a problem hiding this comment.
Not tested, how about something like this:
type profileGlobs strucft { snapName string }
// Globs returns a list of globs that describe apparmor profiles for a given snap.
func (pg profileGlobs) Globs() []string {
return []string{...}
}
There was a problem hiding this comment.
I will just go for a []string since that's easier and captures the intent better.
This patch fixes a bug where a glob describing apparmor profiles for a snap would match unrelated snap that happens to be a proper prefix of the name of the snap in question. What was missing was a dot between the snap name and the glob that describes all applications and hooks. In the broken version it was snap.$SNAP_NAME*. For example, setting up security for "snapcraft" would nuke the profiles of "snapcraft-pr". This patch also includes small helper computes the name of the snap-update-ns profile for a given snap. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
zyga
force-pushed
the
fix/fix-wrong-glob-in-appamror-profiles
branch
from
19d4983
to
cd79d87
Compare
Codecov Report
@@ Coverage Diff @@
## master #5109 +/- ##
=========================================
- Coverage 79.21% 79.2% -0.01%
=========================================
Files 485 485
Lines 35959 35961 +2
=========================================
Hits 28484 28484
- Misses 5230 5231 +1
- Partials 2245 2246 +1
Continue to review full report at Codecov.
|
This patch fixes a bug where a glob describing apparmor profiles for a
snap would match unrelated snap that happens to be a proper prefix of
the name of the snap in question.
What was missing was a dot between the snap name and the glob that
describes all applications and hooks. In the broken version it was
snap.$SNAP_NAME*.
For example, setting up security for "snapcraft" would nuke the profiles
of "snapcraft-pr".
This patch also includes small helper computes the name of the
snap-update-ns profile for a given snap.
Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com