Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

many: update apparmor to 4.0.1 #14150

Merged
merged 17 commits into from
Jul 11, 2024
Merged
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion sandbox/apparmor/apparmor.go
Copy link
Collaborator

@ernestl ernestl Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ func probeParserFeatures() ([]string, error) {
probe: "prompt /foo r,",
},
}
_, internal, err := AppArmorParser()
cmd, internal, err := AppArmorParser()
if err != nil {
return []string{}, err
}
Expand All @@ -716,6 +716,7 @@ func probeParserFeatures() ([]string, error) {
features = append(features, "snapd-internal")
}
sort.Strings(features)
logger.Debugf("probed apparmor parser features for version %s (internal=%v): %v", appArmorParserVersion(cmd), internal, features)
return features, nil
}

Expand Down Expand Up @@ -806,6 +807,20 @@ func AppArmorParser() (cmd *exec.Cmd, internal bool, err error) {
return nil, false, os.ErrNotExist
}

func appArmorParserVersion(cmd *exec.Cmd) string {
cmd.Args = append(cmd.Args, "--version")
output, err := cmd.CombinedOutput()
if err != nil {
return ""
}
logger.Debugf("apparmor_parser --version\n%s", output)
// output is like "AppArmor parser version 2.13.4\n"
ernestl marked this conversation as resolved.
Show resolved Hide resolved
// "Copyright ..."
// get the version number from the first line
parts := strings.Split(strings.Split(string(output), "\n")[0], " ")
return parts[len(parts)-1]
}

// tryAppArmorParserFeature attempts to pre-process a bit of apparmor syntax with a given parser.
func tryAppArmorParserFeature(cmd *exec.Cmd, flags []string, rule string) error {
cmd.Args = append(cmd.Args, "--preprocess")
Expand Down