[libvirt PATCH 0/8] apparmor: Improve overrides, fix 2.x compatibility

An alternative to Jim's attempt[1]. See [2] for the discussion leading up to these changes. [1] https://listman.redhat.com/archives/libvir-list/2023-June/240531.html [2] https://listman.redhat.com/archives/libvir-list/2023-June/240251.html Andrea Bolognani (8): meson: Detect AppArmor 3.x apparmor: Allow version-specific bits in profiles apparmor: Allow version-specific bits in abstractions too apparmor: Only support passt on 3.x apparmor: Make abstractions extensible apparmor: Improve virt-aa-helper include apparmor: Make all profiles extensible NEWS: Mention overrides for AppArmor profiles and abstractions NEWS.rst | 8 +++ meson.build | 3 + .../apparmor/{libvirt-lxc => libvirt-lxc.in} | 4 ++ .../{libvirt-qemu => libvirt-qemu.in} | 6 ++ src/security/apparmor/meson.build | 68 ++++++++++++++++--- .../usr.lib.libvirt.virt-aa-helper.in | 5 ++ src/security/apparmor/usr.sbin.libvirtd.in | 4 ++ src/security/apparmor/usr.sbin.virtqemud.in | 4 ++ src/security/apparmor/usr.sbin.virtxend.in | 4 ++ 9 files changed, 96 insertions(+), 10 deletions(-) rename src/security/apparmor/{libvirt-lxc => libvirt-lxc.in} (98%) rename src/security/apparmor/{libvirt-qemu => libvirt-qemu.in} (98%) -- 2.41.0

We will soon need to base some decisions on whether AppArmor 3.x or 2.x is present on the system. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 3 +++ 1 file changed, 3 insertions(+) diff --git a/meson.build b/meson.build index aa391e7178..060eafc344 100644 --- a/meson.build +++ b/meson.build @@ -879,6 +879,9 @@ endif apparmor_dep = dependency('libapparmor', required: get_option('apparmor')) if apparmor_dep.found() conf.set('WITH_APPARMOR', 1) + if apparmor_dep.version().version_compare('>=3.0.0') + conf.set('WITH_APPARMOR_3', 1) + endif conf.set_quoted('APPARMOR_DIR', sysconfdir / 'apparmor.d') conf.set_quoted('APPARMOR_PROFILES_PATH', '/sys/kernel/security/apparmor/profiles') endif -- 2.41.0

Perform an additional preprocessing step before the existing variable substitution. This is the same approach that we already use to customize systemd unit files based on whether the service supports TCP connections. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/meson.build | 34 ++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/security/apparmor/meson.build b/src/security/apparmor/meson.build index 58b4024b85..c4745acdb9 100644 --- a/src/security/apparmor/meson.build +++ b/src/security/apparmor/meson.build @@ -14,9 +14,41 @@ apparmor_gen_profiles_conf = configuration_data({ apparmor_dir = sysconfdir / 'apparmor.d' +# Our profiles use some features that only work well on AppArmor 3.x, +# specifically the 'include if exists' directive. In order to keep +# supporting AppArmor 2.x, the bits that are version-specific are +# enclosed in special markers and we decide which ones to include +# based on the AppArmor version detected on the host. +# +# TODO: drop the additional complexity once we no longer target +# distros that ship AppArmor 2.x (Debian 11, Ubuntu 20.04) +if conf.has('WITH_APPARMOR_3') + apparmor_gen_cmd = [ + 'sed', + '-e', '/[@]BEGIN_APPARMOR_3[@]/d', + '-e', '/[@]END_APPARMOR_3[@]/d', + '-e', '/[@]BEGIN_APPARMOR_2[@]/,/[@]END_APPARMOR_2[@]/d', + '@INPUT@' + ] +else + apparmor_gen_cmd = [ + 'sed', + '-e', '/[@]BEGIN_APPARMOR_3[@]/,/[@]END_APPARMOR_3[@]/d', + '-e', '/[@]BEGIN_APPARMOR_2[@]/d', + '-e', '/[@]END_APPARMOR_2[@]/d', + '@INPUT@' + ] +endif + foreach name : apparmor_gen_profiles - configure_file( + tmp = configure_file( input: '@0@.in'.format(name), + output: '@0@.tmp'.format(name), + command: apparmor_gen_cmd, + capture: true, + ) + configure_file( + input: tmp, output: name, configuration: apparmor_gen_profiles_conf, install: true, -- 2.41.0

Compared to profiles, we only need a single preprocessing step here, as there is no variable substitution happening. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .../apparmor/{libvirt-lxc => libvirt-lxc.in} | 0 .../{libvirt-qemu => libvirt-qemu.in} | 0 src/security/apparmor/meson.build | 19 +++++++++++++++---- 3 files changed, 15 insertions(+), 4 deletions(-) rename src/security/apparmor/{libvirt-lxc => libvirt-lxc.in} (100%) rename src/security/apparmor/{libvirt-qemu => libvirt-qemu.in} (100%) diff --git a/src/security/apparmor/libvirt-lxc b/src/security/apparmor/libvirt-lxc.in similarity index 100% rename from src/security/apparmor/libvirt-lxc rename to src/security/apparmor/libvirt-lxc.in diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu.in similarity index 100% rename from src/security/apparmor/libvirt-qemu rename to src/security/apparmor/libvirt-qemu.in diff --git a/src/security/apparmor/meson.build b/src/security/apparmor/meson.build index c4745acdb9..8bc2405f88 100644 --- a/src/security/apparmor/meson.build +++ b/src/security/apparmor/meson.build @@ -5,6 +5,11 @@ apparmor_gen_profiles = [ 'usr.sbin.virtxend', ] +apparmor_gen_abstractions = [ + 'libvirt-qemu', + 'libvirt-lxc', +] + apparmor_gen_profiles_conf = configuration_data({ 'sysconfdir': sysconfdir, 'sbindir': sbindir, @@ -56,10 +61,16 @@ foreach name : apparmor_gen_profiles ) endforeach -install_data( - [ 'libvirt-qemu', 'libvirt-lxc' ], - install_dir: apparmor_dir / 'abstractions', -) +foreach name : apparmor_gen_abstractions + configure_file( + input: '@0@.in'.format(name), + output: name, + command: apparmor_gen_cmd, + capture: true, + install: true, + install_dir: apparmor_dir / 'abstractions', + ) +endforeach install_data( [ 'TEMPLATE.qemu', 'TEMPLATE.lxc' ], -- 2.41.0

The subprofile can only work by including the abstraction shipped in the passt package, which we can't assume is present, and 'include if exists' doesn't work well on 2.x. No distro that's stuck on AppArmor 2.x is likely to be shipping passt anyway. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/libvirt-qemu.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/security/apparmor/libvirt-qemu.in b/src/security/apparmor/libvirt-qemu.in index 44056b5f14..1548cf23bf 100644 --- a/src/security/apparmor/libvirt-qemu.in +++ b/src/security/apparmor/libvirt-qemu.in @@ -185,6 +185,7 @@ /usr/{lib,lib64}/libswtpm_libtpms.so mr, /usr/lib/@{multiarch}/libswtpm_libtpms.so mr, +@BEGIN_APPARMOR_3@ # support for passt network back-end /usr/bin/passt Cx -> passt, @@ -199,6 +200,7 @@ include if exists <abstractions/passt> } +@END_APPARMOR_3@ # for save and resume /{usr/,}bin/dash rmix, -- 2.41.0

Implement the standard AppArmor 3.x abstraction extension approach. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/libvirt-lxc.in | 4 ++++ src/security/apparmor/libvirt-qemu.in | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/security/apparmor/libvirt-lxc.in b/src/security/apparmor/libvirt-lxc.in index 0c8b812743..ffe4d8f21f 100644 --- a/src/security/apparmor/libvirt-lxc.in +++ b/src/security/apparmor/libvirt-lxc.in @@ -116,3 +116,7 @@ deny /sys/fs/cgrou[^p]*{,/**} wklx, deny /sys/fs/cgroup?*{,/**} wklx, deny /sys/fs?*{,/**} wklx, + +@BEGIN_APPARMOR_3@ + include if exists <abstractions/libvirt-lxc.d> +@END_APPARMOR_3@ diff --git a/src/security/apparmor/libvirt-qemu.in b/src/security/apparmor/libvirt-qemu.in index 1548cf23bf..53f45c3a28 100644 --- a/src/security/apparmor/libvirt-qemu.in +++ b/src/security/apparmor/libvirt-qemu.in @@ -271,3 +271,7 @@ # required for QEMU accessing UEFI nvram variables owner /var/lib/libvirt/qemu/nvram/*_VARS.fd rwk, owner /var/lib/libvirt/qemu/nvram/*_VARS.ms.fd rwk, + +@BEGIN_APPARMOR_3@ + include if exists <abstractions/libvirt-qemu.d> +@END_APPARMOR_3@ -- 2.41.0

For AppArmor 3.x we can use 'include if exists', which frees us from having to create a dummy override. For AppArmor 2.x we keep things as they are to avoid introducing regressions. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/meson.build | 15 ++++++++++----- .../apparmor/usr.lib.libvirt.virt-aa-helper.in | 5 +++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/security/apparmor/meson.build b/src/security/apparmor/meson.build index 8bc2405f88..b9257c816d 100644 --- a/src/security/apparmor/meson.build +++ b/src/security/apparmor/meson.build @@ -77,8 +77,13 @@ install_data( install_dir: apparmor_dir / 'libvirt', ) -install_data( - 'usr.lib.libvirt.virt-aa-helper.local', - install_dir: apparmor_dir / 'local', - rename: 'usr.lib.libvirt.virt-aa-helper', -) +if not conf.has('WITH_APPARMOR_3') + # We only install the empty local override for AppArmor 2.x. For + # AppArmor 3.x, upstream's preference is to avoid creating these + # files in order to limit the amount of filesystem clutter. + install_data( + 'usr.lib.libvirt.virt-aa-helper.local', + install_dir: apparmor_dir / 'local', + rename: 'usr.lib.libvirt.virt-aa-helper', + ) +endif diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in index ff1d46bebe..26ee20a17d 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in @@ -71,5 +71,10 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper { /**.[iI][sS][oO] r, /**/disk{,.*} r, +@BEGIN_APPARMOR_3@ + include if exists <local/usr.lib.libvirt.virt-aa-helper> +@END_APPARMOR_3@ +@BEGIN_APPARMOR_2@ #include <local/usr.lib.libvirt.virt-aa-helper> +@END_APPARMOR_2@ } -- 2.41.0

On 6/29/23 07:14, Andrea Bolognani wrote:
For AppArmor 3.x we can use 'include if exists', which frees us from having to create a dummy override. For AppArmor 2.x we keep things as they are to avoid introducing regressions.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/meson.build | 15 ++++++++++----- .../apparmor/usr.lib.libvirt.virt-aa-helper.in | 5 +++++ 2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/security/apparmor/meson.build b/src/security/apparmor/meson.build index 8bc2405f88..b9257c816d 100644 --- a/src/security/apparmor/meson.build +++ b/src/security/apparmor/meson.build @@ -77,8 +77,13 @@ install_data( install_dir: apparmor_dir / 'libvirt', )
-install_data( - 'usr.lib.libvirt.virt-aa-helper.local', - install_dir: apparmor_dir / 'local', - rename: 'usr.lib.libvirt.virt-aa-helper', -) +if not conf.has('WITH_APPARMOR_3') + # We only install the empty local override for AppArmor 2.x. For + # AppArmor 3.x, upstream's preference is to avoid creating these + # files in order to limit the amount of filesystem clutter. + install_data( + 'usr.lib.libvirt.virt-aa-helper.local', + install_dir: apparmor_dir / 'local', + rename: 'usr.lib.libvirt.virt-aa-helper', + ) +endif diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in index ff1d46bebe..26ee20a17d 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in @@ -71,5 +71,10 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper { /**.[iI][sS][oO] r, /**/disk{,.*} r,
+@BEGIN_APPARMOR_3@ + include if exists <local/usr.lib.libvirt.virt-aa-helper> +@END_APPARMOR_3@ +@BEGIN_APPARMOR_2@ #include <local/usr.lib.libvirt.virt-aa-helper> +@END_APPARMOR_2@ }
The markers have the added benefit of reminding us to nuke this file when we remove 2.x support. Regards, Jim

Do for all other profiles what we already do for the virt-aa-helper one. In this case we limit the feature to AppArmor 3.x, as it was never implemented for 2.x. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/usr.sbin.libvirtd.in | 4 ++++ src/security/apparmor/usr.sbin.virtqemud.in | 4 ++++ src/security/apparmor/usr.sbin.virtxend.in | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/src/security/apparmor/usr.sbin.libvirtd.in b/src/security/apparmor/usr.sbin.libvirtd.in index edb8dd8e26..1601d73d47 100644 --- a/src/security/apparmor/usr.sbin.libvirtd.in +++ b/src/security/apparmor/usr.sbin.libvirtd.in @@ -139,4 +139,8 @@ profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) { /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix, } + +@BEGIN_APPARMOR_3@ + include if exists <local/usr.sbin.libvirtd> +@END_APPARMOR_3@ } diff --git a/src/security/apparmor/usr.sbin.virtqemud.in b/src/security/apparmor/usr.sbin.virtqemud.in index f269c60809..6b9c5d32d9 100644 --- a/src/security/apparmor/usr.sbin.virtqemud.in +++ b/src/security/apparmor/usr.sbin.virtqemud.in @@ -132,4 +132,8 @@ profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) { /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix, } + +@BEGIN_APPARMOR_3@ + include if exists <local/usr.sbin.virtqemud> +@END_APPARMOR_3@ } diff --git a/src/security/apparmor/usr.sbin.virtxend.in b/src/security/apparmor/usr.sbin.virtxend.in index 72e0d801e5..78a11305f5 100644 --- a/src/security/apparmor/usr.sbin.virtxend.in +++ b/src/security/apparmor/usr.sbin.virtxend.in @@ -52,4 +52,8 @@ profile virtxend @sbindir@/virtxend flags=(attach_disconnected) { @libexecdir@/libvirt_iohelper ix, /etc/libvirt/hooks/** rmix, /etc/xen/scripts/** rmix, + +@BEGIN_APPARMOR_3@ + include if exists <local/usr.sbin.virtxend> +@END_APPARMOR_3@ } -- 2.41.0

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- NEWS.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 950b188a8b..92596d6088 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -33,6 +33,14 @@ v9.5.0 (unreleased) image on discard requests. Disabling cluster unrefing decreases fragmentation of the image. + * apparmor: All profiles and abstractions now support local overrides + + This has long been the case for the ``virt-aa-helper`` profile, but has + now been extended to all other profiles and abstractions. The mechanism + used is the standard AppArmor 3.x one, where the contents of ``foo`` and + ``abstractions/foo`` can be overridden by creating ``local/foo`` and + ``abstractions/foo.d`` respectively. + * **Bug fixes** -- 2.41.0

On 6/29/23 07:14, Andrea Bolognani wrote:
An alternative to Jim's attempt[1]. See [2] for the discussion leading up to these changes.
[1] https://listman.redhat.com/archives/libvir-list/2023-June/240531.html [2] https://listman.redhat.com/archives/libvir-list/2023-June/240251.html
Andrea Bolognani (8): meson: Detect AppArmor 3.x apparmor: Allow version-specific bits in profiles apparmor: Allow version-specific bits in abstractions too apparmor: Only support passt on 3.x apparmor: Make abstractions extensible apparmor: Improve virt-aa-helper include apparmor: Make all profiles extensible NEWS: Mention overrides for AppArmor profiles and abstractions
NEWS.rst | 8 +++ meson.build | 3 + .../apparmor/{libvirt-lxc => libvirt-lxc.in} | 4 ++ .../{libvirt-qemu => libvirt-qemu.in} | 6 ++ src/security/apparmor/meson.build | 68 ++++++++++++++++--- .../usr.lib.libvirt.virt-aa-helper.in | 5 ++ src/security/apparmor/usr.sbin.libvirtd.in | 4 ++ src/security/apparmor/usr.sbin.virtqemud.in | 4 ++ src/security/apparmor/usr.sbin.virtxend.in | 4 ++ 9 files changed, 96 insertions(+), 10 deletions(-) rename src/security/apparmor/{libvirt-lxc => libvirt-lxc.in} (98%) rename src/security/apparmor/{libvirt-qemu => libvirt-qemu.in} (98%)
Nice work! Much better than the profile duplication, although I still think zapping 2.x support is easier with my hack :-P. Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
participants (2)
-
Andrea Bolognani
-
Jim Fehlig