[libvirt] [PATCH 0/4] Couple of apparmor related fixes
Firstly, the virt-aa-helper was never installed under /usr/lib or /usr/lib64. Since it's introduction it lives under /usr/libexec and therefore the profile name should reflect that. Secondly, the profile misses some binaries we use. And lastly, some other internal binaries are misplaced in the profile. Michal Prívozník (4): apparmor: Fix parthelper, iohelper and virt-aa-helper paths in profiles apparmor: Allow libvirt to spawn virt-aa-helper and libvirt_lxc docs: Fix virt-aa-helper location apparmor: Rename virt-aa-helper profile docs/drvqemu.html.in | 2 +- src/security/Makefile.inc.am | 10 +++++----- ...bvirt.virt-aa-helper => usr.libexec.virt-aa-helper} | 6 +++--- src/security/apparmor/usr.sbin.libvirtd | 6 ++++-- 4 files changed, 13 insertions(+), 11 deletions(-) rename src/security/apparmor/{usr.lib.libvirt.virt-aa-helper => usr.libexec.virt-aa-helper} (90%) -- 2.19.2
These helper binaries are installed under libexec dir not lib dir. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.lib.libvirt.virt-aa-helper | 2 +- src/security/apparmor/usr.sbin.libvirtd | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper index de9436872c..e2c336fca0 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper @@ -33,7 +33,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { deny /dev/mapper/ r, deny /dev/mapper/* r, - /usr/{lib,lib64}/libvirt/virt-aa-helper mr, + /usr/libexec/virt-aa-helper mr, /{usr/,}sbin/apparmor_parser Ux, /etc/apparmor.d/libvirt/* r, diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index f0ffc53008..660d72abc1 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -98,8 +98,8 @@ audit deny /sys/kernel/security/apparmor/.* rwxl, /sys/kernel/security/apparmor/profiles r, /usr/{lib,lib64}/libvirt/* PUxr, - /usr/{lib,lib64}/libvirt/libvirt_parthelper ix, - /usr/{lib,lib64}/libvirt/libvirt_iohelper ix, + /usr/libexec/libvirt_parthelper ix, + /usr/libexec/libvirt_iohelper ix, /etc/libvirt/hooks/** rmix, /etc/xen/scripts/** rmix, -- 2.19.2
On Tue, Jan 22, 2019 at 2:40 PM Michal Privoznik <mprivozn@redhat.com> wrote:
These helper binaries are installed under libexec dir not lib dir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.lib.libvirt.virt-aa-helper | 2 +- src/security/apparmor/usr.sbin.libvirtd | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper index de9436872c..e2c336fca0 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper @@ -33,7 +33,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { deny /dev/mapper/ r, deny /dev/mapper/* r,
- /usr/{lib,lib64}/libvirt/virt-aa-helper mr, + /usr/libexec/virt-aa-helper mr,
In a common Debian/Ubuntu installation those are in fact in /usr/lib/libvirt/ So this change would break us. To me it seems the current content matches the distro's with apparmor in place. Not sure about Suse here atm. But if we are changing that we should consider making this dependent on --libexecdir as this is where this path really comes from. And Debian/Ubuntu are setting --libexecdir=\${prefix}/lib/libvirt at config time.
/{usr/,}sbin/apparmor_parser Ux,
/etc/apparmor.d/libvirt/* r, diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index f0ffc53008..660d72abc1 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -98,8 +98,8 @@ audit deny /sys/kernel/security/apparmor/.* rwxl, /sys/kernel/security/apparmor/profiles r, /usr/{lib,lib64}/libvirt/* PUxr, - /usr/{lib,lib64}/libvirt/libvirt_parthelper ix, - /usr/{lib,lib64}/libvirt/libvirt_iohelper ix, + /usr/libexec/libvirt_parthelper ix, + /usr/libexec/libvirt_iohelper ix, /etc/libvirt/hooks/** rmix, /etc/xen/scripts/** rmix,
-- 2.19.2
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
On Tue, Jan 22, 2019 at 03:09:17PM +0100, Christian Ehrhardt wrote:
On Tue, Jan 22, 2019 at 2:40 PM Michal Privoznik <mprivozn@redhat.com> wrote:
These helper binaries are installed under libexec dir not lib dir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.lib.libvirt.virt-aa-helper | 2 +- src/security/apparmor/usr.sbin.libvirtd | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper index de9436872c..e2c336fca0 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper @@ -33,7 +33,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { deny /dev/mapper/ r, deny /dev/mapper/* r,
- /usr/{lib,lib64}/libvirt/virt-aa-helper mr, + /usr/libexec/virt-aa-helper mr,
In a common Debian/Ubuntu installation those are in fact in /usr/lib/libvirt/ So this change would break us. To me it seems the current content matches the distro's with apparmor in place. Not sure about Suse here atm.
But if we are changing that we should consider making this dependent on --libexecdir as this is where this path really comes from. And Debian/Ubuntu are setting --libexecdir=\${prefix}/lib/libvirt at config time.
Agreed any path in the apparmour profile that is related to libvirt should be a variable that is substited in at build time to take account of possible distro differences. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 1/22/19 3:12 PM, Daniel P. Berrangé wrote:
On Tue, Jan 22, 2019 at 03:09:17PM +0100, Christian Ehrhardt wrote:
On Tue, Jan 22, 2019 at 2:40 PM Michal Privoznik <mprivozn@redhat.com> wrote:
These helper binaries are installed under libexec dir not lib dir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.lib.libvirt.virt-aa-helper | 2 +- src/security/apparmor/usr.sbin.libvirtd | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper index de9436872c..e2c336fca0 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper @@ -33,7 +33,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { deny /dev/mapper/ r, deny /dev/mapper/* r,
- /usr/{lib,lib64}/libvirt/virt-aa-helper mr, + /usr/libexec/virt-aa-helper mr,
In a common Debian/Ubuntu installation those are in fact in /usr/lib/libvirt/ So this change would break us. To me it seems the current content matches the distro's with apparmor in place. Not sure about Suse here atm.
But if we are changing that we should consider making this dependent on --libexecdir as this is where this path really comes from. And Debian/Ubuntu are setting --libexecdir=\${prefix}/lib/libvirt at config time.
Agreed any path in the apparmour profile that is related to libvirt should be a variable that is substited in at build time to take account of possible distro differences.
Okay, looks like we have an agreement here. We can have .in file from which the configure will generate the actual file with correct paths. Plus it will have to deal with renaming so that the file name matches its path. Just a side note, the rationale behind these patches is that gentoo has currently three patches it applies on the top of libvirt git. I'd like to get rid of them as I had to rebase them quite often recently. Michal
Both of these binaries are spawn by libvirt. Add a rule to the default profile to allow that. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.sbin.libvirtd | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index 660d72abc1..8a402bd6ec 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -98,6 +98,8 @@ audit deny /sys/kernel/security/apparmor/.* rwxl, /sys/kernel/security/apparmor/profiles r, /usr/{lib,lib64}/libvirt/* PUxr, + /usr/libexec/virt-aa-helper PUxr, + /usr/libexec/libvirt_lxc PUxr, /usr/libexec/libvirt_parthelper ix, /usr/libexec/libvirt_iohelper ix, /etc/libvirt/hooks/** rmix, -- 2.19.2
On Tue, Jan 22, 2019 at 2:40 PM Michal Privoznik <mprivozn@redhat.com> wrote:
Both of these binaries are spawn by libvirt. Add a rule to the default profile to allow that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.sbin.libvirtd | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index 660d72abc1..8a402bd6ec 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -98,6 +98,8 @@ audit deny /sys/kernel/security/apparmor/.* rwxl, /sys/kernel/security/apparmor/profiles r,
/usr/{lib,lib64}/libvirt/* PUxr, + /usr/libexec/virt-aa-helper PUxr, + /usr/libexec/libvirt_lxc PUxr, /usr/libexec/libvirt_parthelper ix, /usr/libexec/libvirt_iohelper ix,
In this case this would not have been that bad, as the rule above would have covered the Debian/Ubuntu case. But as in my former reply, now that you have made me thinking about it I'd think we'd actually want $(get --libexecdir )/* PUxr, instead of all 5 lines above
/etc/libvirt/hooks/** rmix,
-- 2.19.2
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
On 1/22/19 3:14 PM, Christian Ehrhardt wrote:
On Tue, Jan 22, 2019 at 2:40 PM Michal Privoznik <mprivozn@redhat.com> wrote:
Both of these binaries are spawn by libvirt. Add a rule to the default profile to allow that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/apparmor/usr.sbin.libvirtd | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/security/apparmor/usr.sbin.libvirtd b/src/security/apparmor/usr.sbin.libvirtd index 660d72abc1..8a402bd6ec 100644 --- a/src/security/apparmor/usr.sbin.libvirtd +++ b/src/security/apparmor/usr.sbin.libvirtd @@ -98,6 +98,8 @@ audit deny /sys/kernel/security/apparmor/.* rwxl, /sys/kernel/security/apparmor/profiles r,
/usr/{lib,lib64}/libvirt/* PUxr, + /usr/libexec/virt-aa-helper PUxr, + /usr/libexec/libvirt_lxc PUxr, /usr/libexec/libvirt_parthelper ix, /usr/libexec/libvirt_iohelper ix,
In this case this would not have been that bad, as the rule above would have covered the Debian/Ubuntu case. But as in my former reply, now that you have made me thinking about it I'd think we'd actually want $(get --libexecdir )/* PUxr, instead of all 5 lines above
This will work if all the binaries are placed in a separate folder. However, if they are not and live right under libexec/ dir I don't think it would be safe to allow just any binary from the dir. Michal
The location of virt-aa-helper shown in the docs is incorrect. The helper binary is installed under libexec dir. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/drvqemu.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index 0d14027646..bc9c2b78de 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -352,7 +352,7 @@ chmod o+x /path/to/directory <p> While users can define their own AppArmor profile scheme, a typical configuration will include a profile for <code>/usr/sbin/libvirtd</code>, - <code>/usr/lib/libvirt/virt-aa-helper</code> (a helper program which the + <code>/usr/libexec/virt-aa-helper</code> (a helper program which the libvirtd daemon uses instead of manipulating AppArmor directly), and an abstraction to be included by <code>/etc/apparmor.d/libvirt/TEMPLATE</code> (typically <code>/etc/apparmor.d/abstractions/libvirt-qemu</code>). -- 2.19.2
On Tue, Jan 22, 2019 at 2:40 PM Michal Privoznik <mprivozn@redhat.com> wrote:
The location of virt-aa-helper shown in the docs is incorrect. The helper binary is installed under libexec dir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/drvqemu.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index 0d14027646..bc9c2b78de 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -352,7 +352,7 @@ chmod o+x /path/to/directory <p> While users can define their own AppArmor profile scheme, a typical configuration will include a profile for <code>/usr/sbin/libvirtd</code>, - <code>/usr/lib/libvirt/virt-aa-helper</code> (a helper program which the + <code>/usr/libexec/virt-aa-helper</code> (a helper program which the
As the others, this depends on --libexecdir from the configure step
libvirtd daemon uses instead of manipulating AppArmor directly), and an abstraction to be included by <code>/etc/apparmor.d/libvirt/TEMPLATE</code> (typically <code>/etc/apparmor.d/abstractions/libvirt-qemu</code>). -- 2.19.2
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
The profile name should reflect the path under which the binary it describes is installed. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/Makefile.inc.am | 10 +++++----- ...bvirt.virt-aa-helper => usr.libexec.virt-aa-helper} | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) rename src/security/apparmor/{usr.lib.libvirt.virt-aa-helper => usr.libexec.virt-aa-helper} (92%) diff --git a/src/security/Makefile.inc.am b/src/security/Makefile.inc.am index b24cdfd083..ae8e979b84 100644 --- a/src/security/Makefile.inc.am +++ b/src/security/Makefile.inc.am @@ -36,7 +36,7 @@ EXTRA_DIST += \ security/apparmor/TEMPLATE.lxc \ security/apparmor/libvirt-qemu \ security/apparmor/libvirt-lxc \ - security/apparmor/usr.lib.libvirt.virt-aa-helper \ + security/apparmor/usr.libexec.virt-aa-helper \ security/apparmor/usr.sbin.libvirtd \ $(NULL) @@ -90,7 +90,7 @@ endif WITH_SECDRIVER_APPARMOR if WITH_APPARMOR_PROFILES apparmordir = $(sysconfdir)/apparmor.d/ apparmor_DATA = \ - security/apparmor/usr.lib.libvirt.virt-aa-helper \ + security/apparmor/usr.libexec.virt-aa-helper \ security/apparmor/usr.sbin.libvirtd \ $(NULL) @@ -110,11 +110,11 @@ APPARMOR_LOCAL_DIR = "$(DESTDIR)$(apparmordir)/local" install-apparmor-local: $(MKDIR_P) "$(APPARMOR_LOCAL_DIR)" echo "# Site-specific additions and overrides for \ - 'usr.lib.libvirt.virt-aa-helper'" \ - >"$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper" + 'usr.libexec.virt-aa-helper'" \ + >"$(APPARMOR_LOCAL_DIR)/usr.libexec.virt-aa-helper" uninstall-apparmor-local: - rm -f "$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper" + rm -f "$(APPARMOR_LOCAL_DIR)/usr.libexec.virt-aa-helper" rmdir "$(APPARMOR_LOCAL_DIR)" || : INSTALL_DATA_LOCAL += install-apparmor-local diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.libexec.virt-aa-helper similarity index 92% rename from src/security/apparmor/usr.lib.libvirt.virt-aa-helper rename to src/security/apparmor/usr.libexec.virt-aa-helper index e2c336fca0..f24095ef89 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.libexec.virt-aa-helper @@ -1,7 +1,7 @@ # Last Modified: Mon Apr 5 15:10:27 2010 #include <tunables/global> -profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { +profile virt-aa-helper /usr/libexec/virt-aa-helper { #include <abstractions/base> # needed for searching directories @@ -63,5 +63,5 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { /**.[iI][sS][oO] r, /**/disk{,.*} r, - #include <local/usr.lib.libvirt.virt-aa-helper> + #include <local/usr.libexec.virt-aa-helper> } -- 2.19.2
On Tue, Jan 22, 2019 at 2:40 PM Michal Privoznik <mprivozn@redhat.com> wrote:
The profile name should reflect the path under which the binary it describes is installed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/Makefile.inc.am | 10 +++++----- ...bvirt.virt-aa-helper => usr.libexec.virt-aa-helper} | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) rename src/security/apparmor/{usr.lib.libvirt.virt-aa-helper => usr.libexec.virt-aa-helper} (92%)
diff --git a/src/security/Makefile.inc.am b/src/security/Makefile.inc.am index b24cdfd083..ae8e979b84 100644 --- a/src/security/Makefile.inc.am +++ b/src/security/Makefile.inc.am @@ -36,7 +36,7 @@ EXTRA_DIST += \ security/apparmor/TEMPLATE.lxc \ security/apparmor/libvirt-qemu \ security/apparmor/libvirt-lxc \ - security/apparmor/usr.lib.libvirt.virt-aa-helper \ + security/apparmor/usr.libexec.virt-aa-helper \
And a final reply with essentially the same content: this depends on --libexecdir from the configure step.
security/apparmor/usr.sbin.libvirtd \ $(NULL)
@@ -90,7 +90,7 @@ endif WITH_SECDRIVER_APPARMOR if WITH_APPARMOR_PROFILES apparmordir = $(sysconfdir)/apparmor.d/ apparmor_DATA = \ - security/apparmor/usr.lib.libvirt.virt-aa-helper \ + security/apparmor/usr.libexec.virt-aa-helper \ security/apparmor/usr.sbin.libvirtd \ $(NULL)
@@ -110,11 +110,11 @@ APPARMOR_LOCAL_DIR = "$(DESTDIR)$(apparmordir)/local" install-apparmor-local: $(MKDIR_P) "$(APPARMOR_LOCAL_DIR)" echo "# Site-specific additions and overrides for \ - 'usr.lib.libvirt.virt-aa-helper'" \ - >"$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper" + 'usr.libexec.virt-aa-helper'" \ + >"$(APPARMOR_LOCAL_DIR)/usr.libexec.virt-aa-helper"
uninstall-apparmor-local: - rm -f "$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper" + rm -f "$(APPARMOR_LOCAL_DIR)/usr.libexec.virt-aa-helper" rmdir "$(APPARMOR_LOCAL_DIR)" || :
INSTALL_DATA_LOCAL += install-apparmor-local diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.libexec.virt-aa-helper similarity index 92% rename from src/security/apparmor/usr.lib.libvirt.virt-aa-helper rename to src/security/apparmor/usr.libexec.virt-aa-helper index e2c336fca0..f24095ef89 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.libexec.virt-aa-helper @@ -1,7 +1,7 @@ # Last Modified: Mon Apr 5 15:10:27 2010 #include <tunables/global>
-profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { +profile virt-aa-helper /usr/libexec/virt-aa-helper { #include <abstractions/base>
# needed for searching directories @@ -63,5 +63,5 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { /**.[iI][sS][oO] r, /**/disk{,.*} r,
- #include <local/usr.lib.libvirt.virt-aa-helper> + #include <local/usr.libexec.virt-aa-helper> } -- 2.19.2
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
participants (3)
-
Christian Ehrhardt -
Daniel P. Berrangé -
Michal Privoznik