[libvirt] [PATCH 0/2] Small apparmor fixes
This series fixes a few things I broke when changing the libvirtd apparmor profile to a named profile. See patches for details. Too bad I didn't make it for 5.1.0 release.... Jim Fehlig (2): apparmor: Check libvirtd profile status by name apparmor: Add ptrace and signal rules for named profile src/security/apparmor/libvirt-qemu | 2 ++ src/security/security_apparmor.c | 12 +++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) -- 2.20.1
Commit a3ab6d42 changed the libvirtd profile to a named profile, breaking the apparmor driver's ability to detect if the profile is active. When the apparmor driver loads it checks the status of the libvirtd profile using the full binary path, which fails since the profile is now referenced by name. If the apparmor driver is explicitly requested in /etc/libvirt/qemu.conf, then libvirtd fails to load too. Instead of only checking the profile status by full binary path, also check by profile name. The full path check is retained in case users have a customized libvirtd profile with full path. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/security/security_apparmor.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 4afdef065a..6d16b15c65 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -257,10 +257,16 @@ use_apparmor(void) if (access(APPARMOR_PROFILES_PATH, R_OK) != 0) goto cleanup; + /* First check profile status using full binary path. If that fails + * check using profile name. + */ rc = profile_status(libvirt_daemon, 1); - /* Error or unconfined should all result in -1*/ - if (rc < 0) - rc = -1; + if (rc < 0) { + rc = profile_status("libvirtd", 1); + /* Error or unconfined should all result in -1*/ + if (rc < 0) + rc = -1; + } cleanup: VIR_FREE(libvirt_daemon); -- 2.20.1
On Fri, 01 Mar 2019, Jim Fehlig wrote:
Commit a3ab6d42 changed the libvirtd profile to a named profile, breaking the apparmor driver's ability to detect if the profile is active. When the apparmor driver loads it checks the status of the libvirtd profile using the full binary path, which fails since the profile is now referenced by name. If the apparmor driver is explicitly requested in /etc/libvirt/qemu.conf, then libvirtd fails to load too.
Instead of only checking the profile status by full binary path, also check by profile name. The full path check is retained in case users have a customized libvirtd profile with full path.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/security/security_apparmor.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 4afdef065a..6d16b15c65 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -257,10 +257,16 @@ use_apparmor(void) if (access(APPARMOR_PROFILES_PATH, R_OK) != 0) goto cleanup;
+ /* First check profile status using full binary path. If that fails + * check using profile name. + */ rc = profile_status(libvirt_daemon, 1); - /* Error or unconfined should all result in -1*/ - if (rc < 0) - rc = -1; + if (rc < 0) { + rc = profile_status("libvirtd", 1); + /* Error or unconfined should all result in -1*/ + if (rc < 0) + rc = -1; + }
LGTM. +1 to apply. Thanks! -- Jamie Strandboge | http://www.canonical.com
Commit a3ab6d42 changed the libvirtd profile to a named profile but neglected to accommodate the change in the qemu profile ptrace and signal rules. As a result, libvirtd is unable to signal confined qemu processes and hence unable to shutdown or destroy VMs. Add ptrace and signal rules that reference the libvirtd profile by name in addition to full binary path. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/security/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 7d28faa163..474aaefdf8 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -16,8 +16,10 @@ network inet stream, network inet6 stream, + ptrace (readby, tracedby) peer=libvirtd, ptrace (readby, tracedby) peer=/usr/sbin/libvirtd, + signal (receive) peer=libvirtd, signal (receive) peer=/usr/sbin/libvirtd, /dev/net/tun rw, -- 2.20.1
On Fri, 01 Mar 2019, Jim Fehlig wrote:
Commit a3ab6d42 changed the libvirtd profile to a named profile but neglected to accommodate the change in the qemu profile ptrace and signal rules. As a result, libvirtd is unable to signal confined qemu processes and hence unable to shutdown or destroy VMs.
Add ptrace and signal rules that reference the libvirtd profile by name in addition to full binary path.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/security/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 7d28faa163..474aaefdf8 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -16,8 +16,10 @@ network inet stream, network inet6 stream,
+ ptrace (readby, tracedby) peer=libvirtd, ptrace (readby, tracedby) peer=/usr/sbin/libvirtd,
+ signal (receive) peer=libvirtd, signal (receive) peer=/usr/sbin/libvirtd,
/dev/net/tun rw,
+1 to commit -- Jamie Strandboge | http://www.canonical.com
On 3/2/19 7:20 AM, Jamie Strandboge wrote:
On Fri, 01 Mar 2019, Jim Fehlig wrote:
Commit a3ab6d42 changed the libvirtd profile to a named profile but neglected to accommodate the change in the qemu profile ptrace and signal rules. As a result, libvirtd is unable to signal confined qemu processes and hence unable to shutdown or destroy VMs.
Add ptrace and signal rules that reference the libvirtd profile by name in addition to full binary path.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/security/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 7d28faa163..474aaefdf8 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -16,8 +16,10 @@ network inet stream, network inet6 stream,
+ ptrace (readby, tracedby) peer=libvirtd, ptrace (readby, tracedby) peer=/usr/sbin/libvirtd,
+ signal (receive) peer=libvirtd, signal (receive) peer=/usr/sbin/libvirtd,
/dev/net/tun rw,
+1 to commit
Thanks! Any comment on 1/2? It fixes the rather nasty bug of libvirtd not starting when apparmor driver is explicitly enabled in qemu.conf. Regards, Jim
participants (2)
-
Jamie Strandboge -
Jim Fehlig