Bug#988204: apparmor: AppArmor container behavior inappropriate under WSL

[Alistair Young]

Package: apparmor
Version: 2.13.6-10
Severity: normal
Tags: patch
X-Debbugs-Cc: ava...@arkane-systems.net

Dear Maintainer,

There is an issue with the apparmor package not functioning
appropriately when installed on Debian running under the Windows Subsystem
for Linux. Specifically, systemd-detect-virt detects WSL as a container,
technically accurately, but this then causes the apparmor.systemd script
to decline to start apparmor.

This is problematic, essentially, because while Debian-in-WSL is
technically a container, there is no accessible "host" above it that might
be setting AppArmor policies to be interfered with; just the WSL
distro-management foo. As such, in this case, it makes more sense to treat
Debian-in-WSL as a non-container for AppArmor purposes.

A patch to enable this follows:

--- apparmor.systemd.old 2021-05-07 11:54:24.786143397 -0500
+++ apparmor.systemd.new 2021-05-07 11:53:52.773126996 -0500
@@ -73,6 +73,7 @@
start)
if [ -x /usr/bin/systemd-detect-virt ] && \
systemd-detect-virt --quiet --container && \
+ [ $(systemd-detect-virt --container) != "wsl" ] \
! is_container_with_internal_policy; then
aa_log_daemon_msg "Not starting AppArmor in container"
aa_log_end_msg 0
@@ -88,6 +89,7 @@
restart|reload|force-reload)
if [ -x /usr/bin/systemd-detect-virt ] && \
systemd-detect-virt --quiet --container && \
+ [ $(systemd-detect-virt --container) != "wsl" ] \
! is_container_with_internal_policy; then
aa_log_daemon_msg "Not starting AppArmor in container"
aa_log_end_msg 0


-- System Information:
Debian Release: 11.0
APT prefers unstable
APT policy: (500, 'unstable')
Architecture: amd64 (x86_64)

Kernel: Linux 5.10.16.3-microsoft-custom-WSL2+ (SMP w/4 CPU threads)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US.UTF-8
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages apparmor depends on:
ii debconf [debconf-2.0] 1.5.76
ii libc6 2.31-12
ii lsb-base 11.1.0

apparmor recommends no packages.

Versions of packages apparmor suggests:
ii apparmor-profiles-extra 1.33
ii apparmor-utils 2.13.6-10

-- debconf information excluded

-- debsums errors found:
debsums: changed file /lib/apparmor/apparmor.systemd (from apparmor package)

[intrigeri]

Control: tag -1 + moreinfo

Hi,

Alistair Young (2021-05-07):

> Specifically, systemd-detect-virt detects WSL as a container,
> technically accurately, but this then causes the apparmor.systemd
> script to decline to start apparmor.

I'm trying to understand what's, at the end of the day, the desirable
behavior here, and why.

I understand you would like apparmor.service to start in a WSL
environment, i.e. you would like the AppArmor policy to be loaded.
Correct so far?

May I infer that a container run under WSL can actually load and
enforce AppArmor policy? In that case, IMO it would make much more
sense to have is_container_with_internal_policy return true (0) for
WSL containers, rather than tweaking apparmor.systemd to treat them as
non-containers.

Or is there any other reason why you want apparmor.service to start
under WSL?

Cheers!

[Christian Boltz]

Hello,

Am Montag, 8. November 2021, 20:53:01 CET schrieb Alistair J R Young:
> An improved patch for this issue follows, in accordance with the above
> thread:
>
> --- rc.apparmor.functions 2021-11-08 13:27:06.461249682 -0600
> +++ rc.apparmor.functions 2021-11-08 13:30:05.766141212 -0600

Your patch looks like something that should (also?) be fixed upstream.

May I ask you to submit a MR at https://gitlab.com/apparmor/apparmor/ ?
(If you don't want to do the upstreaming for whatever reason, I can
forward your patch - but usually it's better to avoid a "relay" ;-)


Regards,

Christian Boltz
--
Hmmm I think I hear steve yelling something about a unit test,
but he is on vacation so I'll just ignore him for now ;)
[John Johansen in apparmor]

[intrigeri]

Hi Christian,

Christian Boltz (2021-11-08):

> Your patch looks like something that should (also?) be fixed upstream.

My understanding is that the problem here is caused by a Debian patch:

https://salsa.debian.org/apparmor-team/apparmor/-/blob/debian/master/debian/patches/debian/Make-the-systemd-unit-a-no-op-in-containers-with-no-inter.patch

I could trace the history of that patch back to 2012 (2.7.102-0ubuntu3):

* debian/apparmor.init: do nothing in a container. This can be
removed once stacked profiles are supported and used by lxc.
(LP: #978297)

So I believe upstream is not affected, because it'll try to load the
AppArmor policy even inside the kind of containers where it will fail
(that is, most kinds).

I'm going to drop this patch in Debian, which should fix the problem
this bug report is about, because:

- As we can see here, this patch causes trouble in some environments.

- Most container technologies I'm aware of are closer to application
containers than system containers, and are not going to start the
whole pile of systemd units, so the patch does not matter there.
LXC is the only exception I'm aware of.

- The fact other distros did not need to apply such a patch suggests
it's not necessary for most use cases.

- I don't have any time/energy/motivation anymore to maintain or
upstream myself patches that were initially created as part of the
Ubuntu delta to meet Canonical's strategic goals & deadlines, and
never pushed upstream. I'm still immensely grateful by the work
done upstream by Canonical employees, though!

If removing the patch causes trouble for some sort of LXC containers
(there are multiple ways they can or cannot handle AppArmor, depending
on versions, system configuration, per-container configuration, I lost
track), I'll report this upstream and hopefully a LXC-friendly
solution will be implemented there by those of us who particularly
care about LXC :)

Cheers!

[intrigeri]

Control: tag -1 - moreinfo
Control: tag -1 + upstream
Control: found -1 3.0.3-6

Hi,

Sorry for the noise, I missed some context and got stuff wrong
in my previous message.

intrigeri (2022-02-12):

> Christian Boltz (2021-11-08):
>> Your patch looks like something that should (also?) be fixed upstream.
>
> My understanding is that the problem here is caused by a Debian patch:

True, but still, Christian is correct: I've already upstreamed some
years ago the function that's modified by Alistair's improved patch.

So yeah, Alistair, please submit your last patch a merge request
upstream, as Christian suggested :)

> I'm going to drop this patch in Debian

Not this time, at least. I'm going to propose it upstream and will
only drop it in Debian if it does not get picked up there.

Cheers!

[intrigeri]

Alistair J R Young (2022-02-12):

>> So yeah, Alistair, please submit your last patch a merge request upstream, as
>> Christian suggested :)
>

> I've done this now and it has been merged:
>
> https://gitlab.com/apparmor/apparmor/-/merge_requests/812

Awesome, thanks!

[intrigeri]

Hi,

intrigeri (2022-02-13):

FTR I've submitted a tiny follow-up MR:

https://gitlab.com/apparmor/apparmor/-/merge_requests/896

Since you authored the code I'm modifying, your review and test would
be welcome :)