many: update apparmor to 4.0.1 #14150

zyga · 2024-07-02T08:17:24Z

This is a rebase of #13354

This is replacement for older attempt by Alex M: #13354

Bug reports and fixes to consider: https://ubuntu-archive-team.ubuntu.com/pending-sru.html

alexmurray · 2024-07-04T20:49:35Z

Thanks @zyga - so like the original PR, we still see failures in tests/main/snapd-homedirs-vendored which I also explained back in the original PR - #13354 (comment) - I am not sure there is a good fix for this - every time we update to a newer vendored AppArmor which has a new default ABI, this test will likely fail in the same way due to the mismatch between the old and new ABIs. But once this is merged to master then the test will pass 🙃

Signed-off-by: Alex Murray <[email protected]>

Unlike the Launchpad tarball, the one from apparmor gitlab tarball requires this to be present as it is just a snapshot of the git tree, not a release tarball like those provided by Launchpad. Signed-off-by: Alex Murray <[email protected]>

This was already included upstream as part of the 3.1.0 release and hence is included in the 4.0.1 release which we are now vendoring. Signed-off-by: Alex Murray <[email protected]>

They are already included in apparmor 4.x release. Signed-off-by: Zygmunt Krynicki <[email protected]>

All local patches are now merged in the 4.x release. Signed-off-by: Zygmunt Krynicki <[email protected]>

Signed-off-by: Alex Murray <[email protected]>

This is helpful when trying to debug why certain features may not be supported. Signed-off-by: Alex Murray <[email protected]>

Signed-off-by: Alex Murray <[email protected]>

…)" (canonical#13765) This reverts commit ce29886.

valentindavid

LGTM

valentindavid · 2024-07-08T08:21:08Z

build-aux/snap/local/apparmor/af_names.h

@@ -1,13 +1,16 @@
 /*
-  this file was generated on a Ubuntu kinetic install from the upstream


We should probably remove this file once we have moved to using core24 as build base. Right?

This is a bit of an annoying part. The parser looks at the host's socket.h file to determine what kind of sockets one can create and to know their names.

If the snap is built on an old system, where the definition of old is always inaccurate. In theory we could drop it but we'd alwayd be at the risk of compiling but silently failing to understand specific network types.

Signed-off-by: Zygmunt Krynicki <[email protected]>

jslarraz

LGTM

ernestl

Some comments/questions for consideration

ernestl · 2024-07-08T12:33:42Z

sandbox/apparmor/apparmor.go

Have this been considered: https://github.com/snapcore/snapd/blob/master/sandbox/apparmor/apparmor.go#L786-L788

Not covered by test: https://github.com/snapcore/snapd/blob/master/sandbox/apparmor/apparmor.go#L790

Maybe a test for external parser abi options: 4.0, 3.0, other?
cmd/snapd-apparmor TestLoadAppArmorProfiles is exercising 3.0 abi, is this still sufficient?

Can you clarify the 2nd link to a diff that's about not covered by test? Either the link is wonky or I don't understand what the question is.

I'll look into 4.0 testing in snapd-apparmor, thanks!

I've made a separate patch to enable apparmor 4.0 ABI when using the host compiler. I do not want to introduce it here as it's something we cannot easily synchronize with. It should be considered and reviewed separately. I also write some tests in the same pass.

The follow-up is: #14167

ernestl · 2024-07-08T12:36:39Z

sandbox/apparmor/apparmor.go

-		if tryAppArmorParserFeature(cmd, fp.flags, fp.probe) {
+		err := tryAppArmorParserFeature(cmd, fp.flags, fp.probe)
+		if err != nil {
+			logger.Debugf("cannot probe apparmor feature %q: %v", fp.feature, err)


nice addition

ernestl · 2024-07-08T12:38:58Z

sandbox/apparmor/apparmor.go

@@ -816,9 +819,9 @@ func tryAppArmorParserFeature(cmd *exec.Cmd, flags []string, rule string) bool {
 	// older versions of apparmor_parser can exit with success even
 	// though they fail to parse
 	if err != nil || strings.Contains(string(output), "parser error") {
-		return false
+		return fmt.Errorf("apparmor_parser failed: %v: %s", err, output)


nice addition

sandbox/apparmor/apparmor.go

pedronis

+1

ernestl

Thanks

zyga · 2024-07-09T10:49:07Z

There's a number of failures that need investigation. Please don't merge this yet @ernestl

zyga · 2024-07-09T12:35:29Z

This is hitting the /dev/mqueue issue with apparmor on the host.

zyga · 2024-07-10T08:33:33Z

AppArmor 4.0.1 SRU has been released to Noble.

It seems that mediation of mqueue is miscompiled by apparmor_parser 4.0.0~beta3 that was present in Ubuntu 24.04 until the 10th of July 2024. Detect this and mask the presence of mqueue unless apparmor parser 4.0.1, or newer, is used. Signed-off-by: Zygmunt Krynicki <[email protected]>

zyga · 2024-07-10T13:09:38Z

Various tests are now failing on:

+ snap debug sandbox-features --required apparmor:parser:snapd-internal
error: sandbox feature not available: "apparmor:parser:snapd-internal"

I have a hunch that this is because apparmor on the host and in snapd is the same version. Perhaps we need to fake the version to "+1" or fix the tests to cope?

zyga · 2024-07-10T13:41:12Z

The test google:ubuntu-18.04-64:tests/main/snapd-homedirs-vendored fails as in the original branch from Alex, due to the fact that when using apparmor re-execution we assume 4.0 is present and in that specific case, it is not as we repackage snapd snap from the store without injecting updated apparmor.

Mirror the logic used in apparmor-from-the-host to apparmor-from-snapd-snap. This mainly fixes tests that repackage old snapd snap without touching apparmor, but in general seems like the right thing to do. The logic is such, that abi 4 is preferred. Signed-off-by: Zygmunt Krynicki <[email protected]>

Signed-off-by: Zygmunt Krynicki <[email protected]>

zyga · 2024-07-10T14:25:34Z

I've improved the code a little so that tests might actually pass now.

zyga · 2024-07-10T14:29:21Z

If tests are happy this needs a re-review.

zyga · 2024-07-10T17:02:30Z

Dear reviewers, please see patches:

Those are non-test changes that have happened since the branch was mainly approved.

ernestl

Thanks, nice upgrade with ability to now manage version specific feature bugs and also the ability to use available internal ABI file!

Some nitpick comments for consideration.

Signed-off-by: Zygmunt Krynicki <[email protected]>

zyga mentioned this pull request Jul 2, 2024

many: update vendored apparmor to 4.0.1 #13354

Closed

zyga force-pushed the feature/apparmor-bump branch from d0f7415 to 6a9b038 Compare July 4, 2024 10:58

zyga marked this pull request as ready for review July 4, 2024 11:01

zyga force-pushed the feature/apparmor-bump branch from 6a9b038 to 0dd1f7b Compare July 4, 2024 11:05

alexmurray and others added 11 commits July 8, 2024 09:44

build-aux: update vendored apparmor to 4.0.1 release

70e1d20

Signed-off-by: Alex Murray <[email protected]>

build-aux: remove apparmor parser performance patch

3d7f8c1

This was already included upstream as part of the 3.1.0 release and hence is included in the 4.0.1 release which we are now vendoring. Signed-off-by: Alex Murray <[email protected]>

build-aux: remove remote patch application logic

9054d87

They are already included in apparmor 4.x release. Signed-off-by: Zygmunt Krynicki <[email protected]>

build-aux: remove local patch application logic

8e4d4bf

All local patches are now merged in the 4.x release. Signed-off-by: Zygmunt Krynicki <[email protected]>

cmd/configure.ac: expect apparmor 4.0.1 when building as a snap

e00e683

Signed-off-by: Alex Murray <[email protected]>

sandbox/apparmor: use apparmor 4.0 abi with vendored parser

1368576

Signed-off-by: Alex Murray <[email protected]>

sandbox/apparmor: add debug logging when probing parser features

4068f94

Signed-off-by: Alex Murray <[email protected]>

sandbox/apparmor: log apparmor_parser version when probing features

592758b

This is helpful when trying to debug why certain features may not be supported. Signed-off-by: Alex Murray <[email protected]>

tests/main: update for new vendored apparmor 4.0

3f176dd

Signed-off-by: Alex Murray <[email protected]>

Reapply "i/builtin: allow docker-support to use mqueue (canonical#13738…

c52a550

…)" (canonical#13765) This reverts commit ce29886.

zyga force-pushed the feature/apparmor-bump branch from cde583d to c52a550 Compare July 8, 2024 07:46

ernestl added this to the 2.64 milestone Jul 8, 2024

ernestl requested review from ernestl, valentindavid and alexmurray July 8, 2024 07:55

valentindavid approved these changes Jul 8, 2024

View reviewed changes

ernestl requested a review from pedronis July 8, 2024 09:54

interfaces: adjust docker-support test to handle mqueue

6ff315b

Signed-off-by: Zygmunt Krynicki <[email protected]>

ernestl requested a review from jslarraz July 8, 2024 11:42

jslarraz approved these changes Jul 8, 2024

View reviewed changes

ernestl reviewed Jul 8, 2024

View reviewed changes

zyga mentioned this pull request Jul 9, 2024

sandbox/apparmor: do not skip ABI 4.0 from host parser #14167

Merged

pedronis approved these changes Jul 9, 2024

View reviewed changes

ernestl approved these changes Jul 9, 2024

View reviewed changes

zyga added 2 commits July 10, 2024 16:05

sandbox/apparmor: unify test mocking logic

1ec2e54

Signed-off-by: Zygmunt Krynicki <[email protected]>

ernestl approved these changes Jul 11, 2024

View reviewed changes

ernestl changed the title ~~build-aux,sandbox: update apparmor to 4.0.1~~ many: update apparmor to 4.0.1 Jul 11, 2024

zyga added 2 commits July 11, 2024 14:57

sandbox/apparmor: refactor appArmorParserVersion not to clobber cmd

ae4c83e

Signed-off-by: Zygmunt Krynicki <[email protected]>

sandbox/apparmor: fix pair of typos

93867c1

Signed-off-by: Zygmunt Krynicki <[email protected]>

ernestl merged commit 0b52b0e into canonical:master Jul 11, 2024
49 of 53 checks passed

zyga deleted the feature/apparmor-bump branch July 12, 2024 07:48

ernestl mentioned this pull request Aug 19, 2024

release: 2.63.1 #14311

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

many: update apparmor to 4.0.1 #14150

many: update apparmor to 4.0.1 #14150

zyga commented Jul 2, 2024 •

edited by ernestl

Loading

alexmurray commented Jul 4, 2024

valentindavid left a comment

valentindavid Jul 8, 2024

zyga Jul 8, 2024

jslarraz left a comment

ernestl left a comment

ernestl Jul 8, 2024 •

edited

Loading

zyga Jul 8, 2024

zyga Jul 9, 2024

ernestl Jul 8, 2024

ernestl Jul 8, 2024

pedronis left a comment

ernestl left a comment

zyga commented Jul 9, 2024

zyga commented Jul 9, 2024

zyga commented Jul 10, 2024

zyga commented Jul 10, 2024

zyga commented Jul 10, 2024

zyga commented Jul 10, 2024

zyga commented Jul 10, 2024

zyga commented Jul 10, 2024

ernestl left a comment

		@@ -1,13 +1,16 @@
		/*
		this file was generated on a Ubuntu kinetic install from the upstream

many: update apparmor to 4.0.1 #14150

many: update apparmor to 4.0.1 #14150

Conversation

zyga commented Jul 2, 2024 • edited by ernestl Loading

alexmurray commented Jul 4, 2024

valentindavid left a comment

Choose a reason for hiding this comment

valentindavid Jul 8, 2024

Choose a reason for hiding this comment

zyga Jul 8, 2024

Choose a reason for hiding this comment

jslarraz left a comment

Choose a reason for hiding this comment

ernestl left a comment

Choose a reason for hiding this comment

ernestl Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

zyga Jul 8, 2024

Choose a reason for hiding this comment

zyga Jul 9, 2024

Choose a reason for hiding this comment

ernestl Jul 8, 2024

Choose a reason for hiding this comment

ernestl Jul 8, 2024

Choose a reason for hiding this comment

pedronis left a comment

Choose a reason for hiding this comment

ernestl left a comment

Choose a reason for hiding this comment

zyga commented Jul 9, 2024

zyga commented Jul 9, 2024

zyga commented Jul 10, 2024

zyga commented Jul 10, 2024

zyga commented Jul 10, 2024

zyga commented Jul 10, 2024

zyga commented Jul 10, 2024

zyga commented Jul 10, 2024

ernestl left a comment

Choose a reason for hiding this comment

zyga commented Jul 2, 2024 •

edited by ernestl

Loading

ernestl Jul 8, 2024 •

edited

Loading