Allow rootless AppArmor

[kernelmethod]

Related issues:

• Allow rootless containers to use AppArmor profiles #958 (closed in Allow rootless containers to use AppArmor profiles #960, reverted by Revert "Allow rootless containers to use AppArmor profiles" #969)
• Default AppArmor profile never loaded & apparmor.IsEnabled() always returns false when rootless podman#15874

This PR re-implements #960, allowing containers to execute under a specified AppArmor profile when run rootlessly. In addition, it changes the default AppArmor profile used by containers to unconfined.

The current implementation of Podman compiles and loads the default AppArmor profile at runtime. However, this is incompatible with providing rootless AppArmor compatibility, as loading an AppArmor profile requires root privileges. As discussed in containers/podman#15874, the ideal mechanism by which a default profile should be loaded is to add a profile to /etc/apparmor.d at installation time, by individual distributions. This profile should then be specified in a default containers.conf provided by the distribution.

[kernelmethod]

As noted in containers/podman#15874, as it currently stands, all rootless containers currently run unconfined anyways, which limits the downstream security impacts of this change. Nonetheless, it would be nice if we could coordinate with Debian and OpenSUSE package maintainers to try to get in a default profile sooner rather than later.

I've also opened containers/podman#19303, which will need to be merged in after this.

[rhatdan]

I am fine with this change, although it seems to be contradictory. You are allowing rootless podman to use apparmor but you comment above the rootless processes can not set apparmor profiles.

[kernelmethod]

I am fine with this change, although it seems to be contradictory. You are allowing rootless podman to use apparmor but you comment above the rootless processes can not set apparmor profiles.

To clarify, I meant that rootless processes cannot load AppArmor profiles into the kernel; i.e., you can't run apparmor_parser -r ... without root privileges. But with this change (and containers/podman#19303) it's now possible for rootless users to specify --security-opt $profile_name with an actual effect (for profiles that are already loaded into the kernel).

[XSpielinbox]

In addition, it changes the default AppArmor profile used by containers to unconfined.

Why this change? This seems like a step backwards to me. Also: If you want to encourage distributions to ship a different default profile, why not ship it directly?

[Luap99]

I know nothing apparmor but I agree changing the default to unconfined seems like a regression in the rootful case.
If you want distros to ship a profile by default I think it is best to have a profile file here then the packagers can just ship it and modify if needed.

[kernelmethod]

I'm happy to provide a default profile (although it seems that it would be better suited to the other PR containers/podman#15874 than here), but actually getting it installed will need some additional cooperation with whoever is in charge of the Debian and OpenSUSE repos.

In any case, it doesn't make sense for the default profile to be specified inline in this library; that job should be deferred to containers.conf. Moreover, there isn't any good way (as best as I can tell) to specified a non-unconfined AppArmor profile by default here that won't subsequently break Podman's CI/CD pipeline, which was exactly the reason why my original attempt to fix this issue was reverted.

[Luap99]

Having a containers.conf option like apparmor_profile sounds good to me, this could work the same as seccomp_profile for example.

As far as breaking podman CI goes we can always add something in our CI setup where we load the profile at the start as root. The important things is to not cause regressions for users who you use a released version so it must be clear for effected distro packagers that they have to ship this profile when in order to not cause regressions.

[XSpielinbox]

There is the template in this repository from which the current default for the rootfull podman gets generated: https://github.com/containers/common/blob/main/pkg/apparmor/apparmor_linux_template.go

This default should be a good compromise between security and convenience.
Assuming that it is so, one could just write the result of this template to disk, from where loading it automatically should not be an issue as far as I understand it - all the other applications also get their profiles loaded without difficulties.
If then this makes your CI/CD pipeline fail, you either have a special case, where you need a special profile, that is not worth integrating in the default or your pipelines is doings things, it should not do and you need to fix your CI/CD pipeline to adhere to the profile.

If the default profile isn't a good compromise at the moment, it would need adjusting anyway. It may be, that one has to make a separate default for rootless, but it should work the same in principal, right?

Having a containers.conf option like apparmor_profile sounds good to me, this could work the same as seccomp_profile for example.

According to the containers.conf (5) man page, apparmor_profile already exists as an config option with the default value container-default. So, if one default profile for rootfull and rootless podman is enough/feasible, no changes would be needed, if separate defaults exist, one should of course be able to configure them separately.

[rhatdan]

@kernelmethod @XSpielinbox @saschagrunert @vrothberg @siretart PTAL

I don't think this should be decided by engineers who do not/have not worked with AppArmor

[kernelmethod]

I'll follow @XSpielinbox 's suggestion and use the existing template, and see where that takes us.

[rhatdan]

@kernelmethod @XSpielinbox @saschagrunert @vrothberg @siretart any update on this?

[XSpielinbox]

I have nothing to add to what I said before. I am looking forward to an implementation using the template.
Is there any way I can help with that?

[rhatdan]

/approve
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kernelmethod, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[XSpielinbox]

It was now merged without usage of the template, but rather weakening it to unconfined.

What was the reasoning behind that?

[kernelmethod]

@XSpielinbox Go is unfortunately not a language I'm terribly good with, and I've had other obligations take priority over this so I've fallen a bit behind on getting this PR and containers/podman#19303 ready (particularly the latter). I'm still working on it; however, if it's high-priority for you and you want to help out, feel free to make a PR to https://github.com/kernelmethod/podman and I'll merge your changes into mine.

@rhatdan -- unfortunately as @XSpielinbox said I don't think that this PR was ready to be merged, at least per the previous discussion. Are we now okay with defaulting to making Podman containers unconfined? (Technically already the default for rootless containers, but not for rootful ones.)

[vrothberg]

I'll revert the commit. I am not sure it was on purpose since merging requires ACKs from at least two maintainers.

[vrothberg]

#1620

[XSpielinbox]

@XSpielinbox Go is unfortunately not a language I'm terribly good with, and I've had other obligations take priority over this so I've fallen a bit behind on getting this PR and containers/podman#19303 ready (particularly the latter). I'm still working on it; however, if it's high-priority for you and you want to help out, feel free to make a PR to https://github.com/kernelmethod/podman and I'll merge your changes into mine.

Unfortunately, it is more or less the same for me. I will see what I can do. I would really like to see this feature implemented some day. I wanted to learn Go programming for month now, but haven't found the time yet.

[XSpielinbox]

@kernelmethod What OS are you using to test the AppArmor support?

[kernelmethod]

@kernelmethod What OS are you using to test the AppArmor support?

Debian (Bookworm)

[kernelmethod]

I'll revert the commit. I am not sure it was on purpose since merging requires ACKs from at least two maintainers.

Thanks @vrothberg; I'll open a new PR once everything's ready.