[libvirt] [PATCH 1/1] apparmor: allow tunnelled migrations.

The pathname for the pipe for tunnelled migration is unresolvable. The libvirt apparmor driver therefore refuses access, causing migration to fail. If we can't resolve the path, the worst that can happen is that we should have given permission to the file but didn't. Otherwise (especially since this is a /proc/$$/fd/N file) the file is already open and libvirt won't be refused access by apparmor anyway. Also adjust virt-aa-helper to allow access to the *.tunnelmigrate.dest.name files. Changelog: Dec 2: per jdstrand comment, also change the Error to a VIR_WARN. For more information, see https://launchpad.net/bugs/869553. Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com> --- src/security/security_apparmor.c | 6 +++--- src/security/virt-aa-helper.c | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 299dcc6..5e68da8 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -791,9 +791,9 @@ AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, } if (virFileResolveLink(proc, &fd_path) < 0) { - virSecurityReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("could not find path for descriptor")); - return rc; + /* it's a deleted file, presumably. Ignore? */ + VIR_WARN("could not find path for descriptor %s, skipping", proc); + return 0; } return reload_profile(mgr, vm, fd_path, true); diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 14399cc..4561bb9 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1220,6 +1220,10 @@ main(int argc, char **argv) LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, " \"/run/libvirt/**/%s.pid\" rwk,\n", ctl->def->name); + virBufferAsprintf(&buf, " \"%s/run/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", + LOCALSTATEDIR, ctl->def->name); + virBufferAsprintf(&buf, " \"/run/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", + ctl->def->name); if (ctl->files) virBufferAdd(&buf, ctl->files, -1); } -- 1.7.5.4

On 12/02/2011 12:10 PM, Serge Hallyn wrote:
The pathname for the pipe for tunnelled migration is unresolvable. The libvirt apparmor driver therefore refuses access, causing migration to fail. If we can't resolve the path, the worst that can happen is that we should have given permission to the file but didn't. Otherwise (especially since this is a /proc/$$/fd/N file) the file is already open and libvirt won't be refused access by apparmor anyway.
Also adjust virt-aa-helper to allow access to the *.tunnelmigrate.dest.name files.
Changelog: Dec 2: per jdstrand comment, also change the Error to a VIR_WARN.
I tend to put comments like the above after the ---; they are nice during patch review for comparing how the patch has evolved compared to prior reviews, but the history of how a patch was created is no longer important once you have the patch itself in libvirt.git.
For more information, see https://launchpad.net/bugs/869553.
Whereas this definitely belongs in the commit message.
Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com> --- src/security/security_apparmor.c | 6 +++--- src/security/virt-aa-helper.c | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-)
ACK and pushed, with the compilation actually fixed by squashing this in: diff --git i/src/security/security_apparmor.c w/src/security/security_apparmor.c index 5e68da8..db7e7dc 100644 --- i/src/security/security_apparmor.c +++ w/src/security/security_apparmor.c @@ -38,6 +38,7 @@ #include "virfile.h" #include "configmake.h" #include "command.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_SECURITY #define SECURITY_APPARMOR_VOID_DOI "0" -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Fri, 2011-12-02 at 13:10 -0600, Serge Hallyn wrote:
The pathname for the pipe for tunnelled migration is unresolvable. The libvirt apparmor driver therefore refuses access, causing migration to fail. If we can't resolve the path, the worst that can happen is that we should have given permission to the file but didn't. Otherwise (especially since this is a /proc/$$/fd/N file) the file is already open and libvirt won't be refused access by apparmor anyway.
Also adjust virt-aa-helper to allow access to the *.tunnelmigrate.dest.name files.
Changelog: Dec 2: per jdstrand comment, also change the Error to a VIR_WARN.
For more information, see https://launchpad.net/bugs/869553.
Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com> --- src/security/security_apparmor.c | 6 +++--- src/security/virt-aa-helper.c | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 299dcc6..5e68da8 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -791,9 +791,9 @@ AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, }
if (virFileResolveLink(proc, &fd_path) < 0) { - virSecurityReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("could not find path for descriptor")); - return rc; + /* it's a deleted file, presumably. Ignore? */ + VIR_WARN("could not find path for descriptor %s, skipping", proc); + return 0; }
return reload_profile(mgr, vm, fd_path, true);
ACK
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 14399cc..4561bb9 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1220,6 +1220,10 @@ main(int argc, char **argv) LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, " \"/run/libvirt/**/%s.pid\" rwk,\n", ctl->def->name); + virBufferAsprintf(&buf, " \"%s/run/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", + LOCALSTATEDIR, ctl->def->name); + virBufferAsprintf(&buf, " \"/run/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", + ctl->def->name); if (ctl->files) virBufferAdd(&buf, ctl->files, -1); }
ACK -- Jamie Strandboge | http://www.canonical.com
participants (3)
-
Eric Blake
-
Jamie Strandboge
-
Serge Hallyn