Apparmor mount propagation #4295

mihalicyn · 2023-03-31T11:43:58Z

Long story behind this. Many years ago, Stéphane Graber
discovered an issue with apparmor mount rules.

Since
7f2b132
commit ("apparmor: Update mount states handling") it was prohibited
to change mount propagation flags, just because adding rules which
allow mount propagation user inside the container gets an ability
to mount everything [1].

Now with modern systemd versions this problem become more critical than
before. For instance, ArchLinux containers fail to start without
nesting apparmor profile enabled (because nesting profile effectively
just allow all mounts). Of course, that's a security issue.

We've also enabled sharing on the container rootfs:
#4229

Now for many workloads it's needed to change propagation flag to
private (see canonical/craft-parts#400).

Issue:
$ lxc-start -F archlinux-test

systemd 253-1-arch running in system mode (+PAM +AUDIT -SELINUX -APPARMOR -IMA +SMACK +SECCOMP +GCRYPT +GNUTLS +OPENSSL +ACL +BLKID +CURL +ELFUTILS +FIDO2 +IDN2 -IDN +IPTC +KMOD +LIBCRYPTSETUP +LIBFDISK +PCRE2 -PWQUALITY +P11KIT -QRENCODE +TPM2 +BZIP2 +LZ4 +XZ +ZLIB +ZSTD +BPF_FRAMEWORK +XKBCOMMON +UTMP -SYSVINIT default-hierarchy=unified)
Detected virtualization lxc.
Detected architecture x86-64.

Welcome to Arch Linux!

bpf-lsm: BPF LSM hook not enabled in the kernel, BPF LSM not supported
Failed to remount root directory as MS_SLAVE: Permission denied
(sd-gens) failed with exit status 1.
[!!!!!!] Failed to start up manager.
Exiting PID 1...

Workaround (unsafe):
$ lxc-start -s lxc.apparmor.allow_nesting=1 -s lxc.apparmor.profile=generated -F arch-test

John Johansen (Apparmor maintainer) and LXD team worked on fix [2].
It was merged to stable AppArmor 3.0 and 3.1 branches already.
There is no stable AppArmor version tag for that, but I think it will
be in the AppArmor version 3.0.10.

See also:
[1] https://bugs.launchpad.net/apparmor/+bug/1597017
[2] https://gitlab.com/apparmor/apparmor/-/merge_requests/333

Fixes: #4280

lxc-jenkins · 2023-03-31T11:45:14Z

Testsuite passed

lxc-jenkins · 2023-03-31T11:53:03Z

Testsuite passed

mihalicyn · 2023-03-31T11:55:31Z

We need to be careful with merging this, because this change will cause wider privileges on the old AppArmor versions...
Currently we have no stable AppArmor version with required fix.

Only privileged LXC containers are affected because their security model relies on the AppArmor heavily.

brauner

Over 4 or 5 years later.

Quoting from the commit message:

Now with modern systemd versions this problem become more critical than
before. For instance, ArchLinux containers fail to start without
nesting apparmor profile enabled (because nesting profile effectively
just allow all mounts). Of course, that's a security issue.

This claims that this becomes more critical but doesn't explain why e.g., Archlinux containers failing to start are caused by this. I'd like to have that. What exactly is failing suddenly?

config/apparmor/abstractions/container-base.in

config/apparmor/abstractions/container-base

Blub · 2023-03-31T12:14:23Z

Should we wait for a release and check the version via apparmor_parser --version?
We should also remember that once we can assume a new-enough apparmor we should be able to replace append_all_remount_rules with using options in (...)

mihalicyn · 2023-03-31T12:22:57Z

Should we wait for a release and check the version via apparmor_parser --version? We should also remember that once we can assume a new-enough apparmor we should be able to replace append_all_remount_rules with using options in (...)

That's a good point. I thought about that in the context of LXD. Because in LXD we have a template for AppArmor profile (https://github.com/lxc/lxd/blob/bb09d3a6c071ac8c38861e3fc0595b7a5550bf09/lxd/apparmor/instance_lxc.go#L7), it's easy to check AppArmor version in runtime and generate an appropriate AppArmor profile for that.

mihalicyn · 2023-03-31T12:29:17Z

Over 4 or 5 years later.

Quoting from the commit message:

Now with modern systemd versions this problem become more critical than
before. For instance, ArchLinux containers fail to start without
nesting apparmor profile enabled (because nesting profile effectively
just allow all mounts). Of course, that's a security issue.

This claims that this becomes more critical but doesn't explain why e.g., Archlinux containers failing to start are caused by this. I'd like to have that. What exactly is failing suddenly?

Fixed.

Long story behind this. Many years ago, Stéphane Graber discovered an issue with apparmor mount rules. Since lxc@7f2b132 commit ("apparmor: Update mount states handling") it was prohibited to change mount propagation flags, just because adding rules which allow mount propagation user inside the container gets an ability to mount everything [1]. Now with modern systemd versions this problem become more critical than before. For instance, ArchLinux containers fail to start without nesting apparmor profile enabled (because nesting profile effectively just allow all mounts). Of course, that's a security issue. We've also enabled sharing on the container rootfs: lxc#4229 Now for many workloads it's needed to change propagation flag to private (see canonical/craft-parts#400). Issue: $ lxc-start -F archlinux-test systemd 253-1-arch running in system mode (+PAM +AUDIT -SELINUX -APPARMOR -IMA +SMACK +SECCOMP +GCRYPT +GNUTLS +OPENSSL +ACL +BLKID +CURL +ELFUTILS +FIDO2 +IDN2 -IDN +IPTC +KMOD +LIBCRYPTSETUP +LIBFDISK +PCRE2 -PWQUALITY +P11KIT -QRENCODE +TPM2 +BZIP2 +LZ4 +XZ +ZLIB +ZSTD +BPF_FRAMEWORK +XKBCOMMON +UTMP -SYSVINIT default-hierarchy=unified) Detected virtualization lxc. Detected architecture x86-64. Welcome to Arch Linux! bpf-lsm: BPF LSM hook not enabled in the kernel, BPF LSM not supported Failed to remount root directory as MS_SLAVE: Permission denied (sd-gens) failed with exit status 1. [!!!!!!] Failed to start up manager. Exiting PID 1... Workaround (unsafe): $ lxc-start -s lxc.apparmor.allow_nesting=1 -s lxc.apparmor.profile=generated -F arch-test John Johansen (Apparmor maintainer) and LXD team worked on fix [2]. It was merged to stable AppArmor 3.0 and 3.1 branches already. There is no stable AppArmor version tag for that, but I think it will be in the AppArmor version 3.0.10. See also: [1] https://bugs.launchpad.net/apparmor/+bug/1597017 [2] https://gitlab.com/apparmor/apparmor/-/merge_requests/333 Fixes: lxc#4280 Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>

lxc-jenkins · 2023-03-31T12:30:54Z

Testsuite passed

lxc-jenkins · 2023-03-31T12:35:02Z

Testsuite passed

brauner · 2023-03-31T12:51:34Z

Over 4 or 5 years later.
Quoting from the commit message:

Now with modern systemd versions this problem become more critical than
before. For instance, ArchLinux containers fail to start without
nesting apparmor profile enabled (because nesting profile effectively
just allow all mounts). Of course, that's a security issue.

This claims that this becomes more critical but doesn't explain why e.g., Archlinux containers failing to start are caused by this. I'd like to have that. What exactly is failing suddenly?

Fixed.

Funny:

Failed to remount root directory as MS_SLAVE: Permission denied.

And that is why I asked. :)

Because AppArmor is the proximal cause but not the distal cause. The distal cause is that I made the rootfs MS_SHARED a little while ago. So when systemd forks of new services in a new mount namespace it sets MS_SLAVE and has done so since forever. Now it starts failing because it actually does need to change to MS_SLAVE whereas before it would've been a nop.

mihalicyn · 2023-03-31T12:57:39Z

The distal cause is that I made the rootfs MS_SHARED a little while ago. So when systemd forks of new services in a new mount namespace it sets MS_SLAVE and has done so since forever. Now it starts failing because it actually does need to change to MS_SLAVE whereas before it would've been a nop.

agree!

hallyn · 2023-03-31T14:37:33Z

Should we wait for a release and check the version via apparmor_parser --version? We should also remember that once we can assume a new-enough apparmor we should be able to replace append_all_remount_rules with using options in (...)

That's a good point. I thought about that in the context of LXD. Because in LXD we have a template for AppArmor profile (https://github.com/lxc/lxd/blob/bb09d3a6c071ac8c38861e3fc0595b7a5550bf09/lxd/apparmor/instance_lxc.go#L7), it's easy to check AppArmor version in runtime and generate an appropriate AppArmor profile for that.

That's good for the run-time generated profiles. Is there a sort of #ifdef we can put in config/apparmor/abstractions/container-base{,.in} to gate policy on the apparmor version?

mihalicyn · 2023-03-31T14:57:52Z

Should we wait for a release and check the version via apparmor_parser --version? We should also remember that once we can assume a new-enough apparmor we should be able to replace append_all_remount_rules with using options in (...)

That's a good point. I thought about that in the context of LXD. Because in LXD we have a template for AppArmor profile (https://github.com/lxc/lxd/blob/bb09d3a6c071ac8c38861e3fc0595b7a5550bf09/lxd/apparmor/instance_lxc.go#L7), it's easy to check AppArmor version in runtime and generate an appropriate AppArmor profile for that.

That's good for the run-time generated profiles. Is there a sort of #ifdef we can put in config/apparmor/abstractions/container-base{,.in} to gate policy on the apparmor version?

As far as I know AppArmor doesn't support that. The only thing that we can do for non-runtime-generated profile is to put some warning to container logs if the AppArmor version is affected and privileged container is used.

stgraber · 2023-03-31T15:19:39Z

Can we hold off merging this until we get a CVE filed against AppArmor?

I'd like to have some assurance that distros that care about security have the parser bug resolved before anyone gets LXC with the new rule enabled.

mihalicyn · 2023-03-31T15:28:22Z

Sure, friends, I'm not insisting on merging this right now. But just want to be prepared and discuss everything in advance.

(we can convert this to a "draft" PR)

brauner · 2023-03-31T15:30:00Z

Sure, friends, I'm not insisting on merging this right now. But just want to be prepared and discuss everything in advance.

(we can convert this to a "draft" PR)

@stgraber can also just set the Blocked label.

brauner · 2023-03-31T15:31:01Z

Can we hold off merging this until we get a CVE filed against AppArmor?

I have to say that I'm puzzled why that never happened before. I mean that bug is older than 4 years and @Blub has even proposed patches to fix this before. :)

mihalicyn · 2023-03-31T15:31:57Z

Looks like I have no permission to convert PR to a draft.

mihalicyn · 2023-03-31T15:33:36Z

There is some CVE: https://bugs.launchpad.net/bugs/cve/2016-1585 (https://cve.mitre.org/cgi-bin/cvename.cgi?name=2016-1585)

mihalicyn · 2023-06-09T14:56:05Z

https://gitlab.com/apparmor/apparmor/-/wikis/Release_Notes_2.13.8
https://gitlab.com/apparmor/apparmor/-/wikis/Release_Notes_3.0.10

https://alioth-lists.debian.net/pipermail/pkg-apparmor-team/2023-May/003994.html

mihalicyn · 2023-08-30T15:19:28Z

cc @alexmurray @jrjohansen

mihalicyn force-pushed the apparmor_mount_propagation branch from f933d4b to 0e2868b Compare March 31, 2023 11:51

mihalicyn requested review from Blub, stgraber and hallyn March 31, 2023 11:53

brauner requested changes Mar 31, 2023

View reviewed changes

config/apparmor/abstractions/container-base.in Outdated Show resolved Hide resolved

config/apparmor/abstractions/container-base Outdated Show resolved Hide resolved

mihalicyn force-pushed the apparmor_mount_propagation branch from 0e2868b to 12a662b Compare March 31, 2023 12:27

mihalicyn force-pushed the apparmor_mount_propagation branch from 12a662b to 890de07 Compare March 31, 2023 12:32

brauner marked this pull request as draft March 31, 2023 15:48

mihalicyn marked this pull request as ready for review August 17, 2023 12:34

stgraber merged commit c1d7302 into lxc:main Aug 17, 2023

stgraber mentioned this pull request Oct 17, 2023

Ubuntu 23.10 (mantic) containers fail to start lxc/lxc-ci#788

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apparmor mount propagation #4295

Apparmor mount propagation #4295

mihalicyn commented Mar 31, 2023 •

edited

Loading

lxc-jenkins commented Mar 31, 2023

lxc-jenkins commented Mar 31, 2023

mihalicyn commented Mar 31, 2023 •

edited

Loading

brauner left a comment

Blub commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

lxc-jenkins commented Mar 31, 2023

lxc-jenkins commented Mar 31, 2023

brauner commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

hallyn commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

stgraber commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

brauner commented Mar 31, 2023

brauner commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

mihalicyn commented Mar 31, 2023 •

edited

Loading

mihalicyn commented Jun 9, 2023

mihalicyn commented Aug 30, 2023

Apparmor mount propagation #4295

Apparmor mount propagation #4295

Conversation

mihalicyn commented Mar 31, 2023 • edited Loading

lxc-jenkins commented Mar 31, 2023

lxc-jenkins commented Mar 31, 2023

mihalicyn commented Mar 31, 2023 • edited Loading

brauner left a comment

Choose a reason for hiding this comment

Blub commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

lxc-jenkins commented Mar 31, 2023

lxc-jenkins commented Mar 31, 2023

brauner commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

hallyn commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

stgraber commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

brauner commented Mar 31, 2023

brauner commented Mar 31, 2023

mihalicyn commented Mar 31, 2023

mihalicyn commented Mar 31, 2023 • edited Loading

mihalicyn commented Jun 9, 2023

mihalicyn commented Aug 30, 2023

mihalicyn commented Mar 31, 2023 •

edited

Loading

mihalicyn commented Mar 31, 2023 •

edited

Loading

mihalicyn commented Mar 31, 2023 •

edited

Loading