Support /proc/PID/attr/apparmor/exec in addition to /proc/PID/attr/exec

[AkihiroSuda]

Requested in docker/for-linux#1199 (comment)

use /proc/self/attr/apparmor/exec instead of /proc/self/attr/exec.

[AkihiroSuda]

This is kinda enhancement, but probably we should have this in rc94 to rescue Arch Linux users.

[AkihiroSuda]

PR: #2803

[cyphar]

Feels like this is a kernel regression (or rather, a regression by the Arch Linux maintainers). Why does CONFIG_LSM=...,bpf cause the AppArmor interface to disappear? We can support the new one but it feels like the upstream kernel devs (or Arch devs) should know this is breaking userspace.

[AkihiroSuda]

Feels like this is a kernel regression (or rather, a regression by the Arch Linux maintainers).

Not a kernel regression, this is a configuration issue on Arch side: archlinux/svntogit-packages@69cb8c2#diff-6503b30d816e211d1909eed3e1eff5f9105fdc48f5735921a59f4906f99415beR9406

Why does CONFIG_LSM=...,bpf cause the AppArmor interface to disappear?

Because it prioritize bpf over apparmor.
A workaround is to prepend apparmor before bpf in the kernel cmdline: e.g., apparmor=1 lsm=capability,lockdown,yama,apparmor,bpf

[cyphar]

Sorry, that's what I meant by "regression by the Arch Linux maintainers". But yeah, I already LGTM'd the patch, just seems like the Arch folks should test config changes before releasing them. 🤷

[anthraxx]

@cyphar In Arch we do test kernels and configuration adjustments to some degree, our general purpose kernel always goes through the testing repository which requires approvals. However, the side effects of such a change are not trivial to recognize especially as Apparmor is not something we support out of the box and by default in our kernel. It seems the testing audience overlooked implications causing by this or the coverage of our testing group did not have a subset of "custom enabled apparmor as non primary LSM + using docker".
However strictly speaking, this is not really a configuration issue. It surely may create interference and uncovers a problem, hence is a regression introduced by a change n our side, but the issue is clearly (as you pointed out) a certain LSM specific expectations for the interface in runc. 🐱
Unfortunately this seems to be known for quite a while and even some reddit users wrote about this, but it didn't reach any official channels as of today 😿

[Foxboron]

@AkihiroSuda and fwiw, I do backport patches for runc/containerd/docker if I'm made aware of them :)

[Maryse47]

@anthraxx this issue is much beyond apparmor use with runc or docker. In fact Arch change of appending bpf to lsm list broke the legacy way of enabling major lsm with security=<lsm_name>. This affects apparmor, selinux and perhaps smack too. This breaks user configs that worked for years and tons of docs must be rewritten including Arch wiki. I have no idea why Arch needs to enable bpf lsm by default while it doesn't provide any bpf program that uses it but I don't think it was good idea and should be reconsidered.

[anthraxx]

@Maryse47 I understood that, but its the legacy interface after all. Arch Linux is a rolling release, at a certain point in time users are expected to adapt to more modern ways to declare this. As I've tried to point out, the implications of this didn't initially arise during the testing phase. Documentation and wiki pages are open to be contributed to, especially if they lack non legacy ways to configure this. I'm gratefully inviting you to contribute to the wiki and extend documentation, but I believe discussing Arch Linux specifics at this level starts to really be off-topic for this thread.

[Maryse47]

I understood that, but its the legacy interface after all. Arch Linux is a rolling release, at a certain point in time users are expected to adapt to more modern ways to declare this.

I would understand if this change brought any benefit for Arch users but you just trade legacy interface to enable some idle code that has zero support from distro side (even wiki has zero documentation about it), not sure if there is a single person who really uses bpf lsm on Arch but then they can enable it themselves just fine. AFAIK the logic behind enabling bpf was - it doesn't hurt - but now it was proved it did hurt some users so the assumptions were clearly wrong.

As I've tried to point out, the implications of this didn't initially arise during the testing phase

I don't understand why this matters now while they arised after testing phase with real users. It only shows that testing phase was unfortunately insufficient but it isn't excuse for ignoring the problem.

I believe discussing Arch Linux specifics at this level starts to really be off-topic for this thread.

The problem I see is Arch broke stuff without a reason and puts the burden of fixing it on someone else because our testing didn't uncover the problem. Such attitude doesn't build faith in Arch maintainership and is generally bad for relations with upstream projects.

[cyphar]

Okay, this thread isn't the best place for arguments about Arch maintainership.

@anthraxx The reason I said it was a configuration bug is that /proc/self/attr/exec is a very long-standing interface, and the reason why the kernel didn't remove it when they added the subdirectory version is precisely because removing it will break userspace programs. I just took a look, and this bug is not only present in runc -- LXC has the exact same bug and LXC is usually more up-to-date than us when it comes to kernel developments (in runc, afaik I'm the only semi-regular kernel developer while LXC has several full-time kernel developers).

So while you may view this as a bug in userland, that's not historically how backwards-incompatible UAPI changes are treated in Linux. If a kernel change breaks userspace, it's never userspace's fault -- and in my view that includes changes made by downstream kernel maintainers. The fact that an interface is described as legacy in the kernel documentation does not indicate that it's okay for downstream maintainers to break userspace by changing it.

Personally I still find it strange that LSMs now have this (new) behaviour where kernel configuration options change parts of the UAPI. My guess is that most LSM developers assume everyone uses libapparmor or libselinux -- which simply isn't the case. (And AppArmor has caused backwards-incompatible changes in the past -- the most notable example being signal mediation, so it's hardly a new issue.)

But yeah my comment was a bit flippant -- sorry for how I phrased it. We've fixed the issue now, so let's move on.

[cyphar]

(Also it should be noted this interface was only added in Linux 5.8 -- I would argue that "was the only way of doing this operation two kernel releases ago" really doesn't count as a legacy interface!)