[libvirt] [PATCH 1/1] virt-aa-helper: add unix channels (esp for qemu-guest-agent)
The original bug report was at https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1393842 Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/security/virt-aa-helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e53779e..8ec95c1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -998,6 +998,7 @@ get_files(vahControl * ctl) (ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PTY || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_DEV || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_FILE || + ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_UNIX || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PIPE) && ctl->def->channels[i]->source.data.file.path) if (vah_add_file_chardev(&buf, -- 2.1.4
On Mon, Apr 06, 2015 at 04:12:03PM +0000, Serge Hallyn wrote:
The original bug report was at https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1393842
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/security/virt-aa-helper.c | 1 + 1 file changed, 1 insertion(+)
ACK Should virt-aa-helper-test be updated too? Jan
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e53779e..8ec95c1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -998,6 +998,7 @@ get_files(vahControl * ctl) (ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PTY || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_DEV || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_FILE || + ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_UNIX || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PIPE) && ctl->def->channels[i]->source.data.file.path) if (vah_add_file_chardev(&buf, -- 2.1.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Quoting Ján Tomko (jtomko@redhat.com):
On Mon, Apr 06, 2015 at 04:12:03PM +0000, Serge Hallyn wrote:
The original bug report was at https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1393842
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/security/virt-aa-helper.c | 1 + 1 file changed, 1 insertion(+)
ACK
Should virt-aa-helper-test be updated too?
I guess so. More importantly, are there cases in libvirt where there is a an abstract UNIX channel? If so then we'd have to avoid those cases. The current code will write out an empty string for the file path, which probably will result in a corrupt policy.
Jan
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e53779e..8ec95c1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -998,6 +998,7 @@ get_files(vahControl * ctl) (ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PTY || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_DEV || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_FILE || + ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_UNIX || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PIPE) && ctl->def->channels[i]->source.data.file.path) if (vah_add_file_chardev(&buf, -- 2.1.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Quoting Serge Hallyn (serge.hallyn@ubuntu.com):
Quoting Ján Tomko (jtomko@redhat.com):
On Mon, Apr 06, 2015 at 04:12:03PM +0000, Serge Hallyn wrote:
The original bug report was at https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1393842
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/security/virt-aa-helper.c | 1 + 1 file changed, 1 insertion(+)
ACK
Should virt-aa-helper-test be updated too?
I guess so.
no - actually that test doesn't check for actual paths being present in the result, and the test already checks that a unix channel doesn't make virt-aa-helper crash. So adding a test for this would be greatly extending the scope of the current testcase. Seems worth doing, but outside the scope of this patch.
More importantly, are there cases in libvirt where there is a an abstract UNIX channel? If so then we'd have to avoid those cases. The current code will write out an empty string for the file path, which probably will result in a corrupt policy.
I'm sending an updated patch in reply to this email.
Changelog (v2): * skip abstract unix sockets Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/security/virt-aa-helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e53779e..3f0b167 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -998,8 +998,10 @@ get_files(vahControl * ctl) (ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PTY || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_DEV || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_FILE || + ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_UNIX || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PIPE) && - ctl->def->channels[i]->source.data.file.path) + ctl->def->channels[i]->source.data.file.path && + *ctl->def->channels[i]->source.data.file.path != '\0') if (vah_add_file_chardev(&buf, ctl->def->channels[i]->source.data.file.path, "rw", -- 2.1.4
On Fri, Apr 10, 2015 at 08:21:03PM +0000, Serge Hallyn wrote:
Changelog (v2): * skip abstract unix sockets
The patch changelog does not need to be in git history.
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/security/virt-aa-helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
ACK, I have reused the commit message from v1 and pushed the patch Jan
participants (2)
-
Ján Tomko -
Serge Hallyn