From 03d89991f24f67ec5900bd8bbe0b09dbd0f536e4 Mon Sep 17 00:00:00 2001 From: Jamie Strandboge Date: Wed, 28 Sep 2011 15:43:39 +0800 Subject: [PATCH] fix AppArmor driver for pipe character devices The AppArmor security driver adds only the path specified in the domain XML for character devices of type 'pipe'. It should be using .in and .out. We do this by creating a new vah_add_file_chardev() and use it for char devices instead of vah_add_file(). Also adjust valid_path() to accept S_FIFO (since qemu chardevs of type 'pipe' use fifos). This is https://launchpad.net/bugs/832507 --- src/security/virt-aa-helper.c | 76 ++++++++++++++++++++++++++++------- tests/virt-aa-helper-test | 18 +++++++++ 2 files changed, 80 insertions(+), 14 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 66608f8013..ad4b8a7cfd 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -3,7 +3,7 @@ * virt-aa-helper: wrapper program used by AppArmor security driver. * * Copyright (C) 2010-2011 Red Hat, Inc. - * Copyright (C) 2009-2010 Canonical Ltd. + * Copyright (C) 2009-2011 Canonical Ltd. * * See COPYING.LIB for the License of this software * @@ -568,9 +568,6 @@ valid_path(const char *path, const bool readonly) case S_IFDIR: return 1; break; - case S_IFIFO: - return 1; - break; case S_IFSOCK: return 1; break; @@ -795,6 +792,51 @@ vah_add_file(virBufferPtr buf, const char *path, const char *perms) return rc; } +static int +vah_add_file_chardev(virBufferPtr buf, + const char *path, + const char *perms, + const int type) +{ + char *pipe_in; + char *pipe_out; + int rc = -1; + + if (type == VIR_DOMAIN_CHR_TYPE_PIPE) { + /* add the pipe input */ + if (virAsprintf(&pipe_in, "%s.in", path) == -1) { + vah_error(NULL, 0, _("could not allocate memory")); + goto clean; + } + + if (vah_add_file(buf, pipe_in, perms) != 0) + goto clean_pipe_in; + + /* add the pipe output */ + if (virAsprintf(&pipe_out, "%s.out", path) == -1) { + vah_error(NULL, 0, _("could not allocate memory")); + goto clean_pipe_in; + } + + if (vah_add_file(buf, pipe_out, perms) != 0) + goto clean_pipe_out; + + rc = 0; + clean_pipe_out: + VIR_FREE(pipe_out); + clean_pipe_in: + VIR_FREE(pipe_in); + } else { + /* add the file */ + if (vah_add_file(buf, path, perms) != 0) + goto clean; + rc = 0; + } + + clean: + return rc; +} + static int file_iterate_hostdev_cb(usbDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) @@ -835,7 +877,6 @@ add_file_path(virDomainDiskDefPtr disk, return ret; } - static int get_files(vahControl * ctl) { @@ -878,12 +919,17 @@ get_files(vahControl * ctl) ctl->def->serials[i]->source.type == VIR_DOMAIN_CHR_TYPE_FILE || ctl->def->serials[i]->source.type == VIR_DOMAIN_CHR_TYPE_PIPE) && ctl->def->serials[i]->source.data.file.path) - if (vah_add_file(&buf, - ctl->def->serials[i]->source.data.file.path, "rw") != 0) + if (vah_add_file_chardev(&buf, + ctl->def->serials[i]->source.data.file.path, + "rw", + ctl->def->serials[i]->source.type) != 0) goto clean; if (ctl->def->console && ctl->def->console->source.data.file.path) - if (vah_add_file(&buf, ctl->def->console->source.data.file.path, "rw") != 0) + if (vah_add_file_chardev(&buf, + ctl->def->console->source.data.file.path, + "rw", + ctl->def->console->source.type) != 0) goto clean; for (i = 0 ; i < ctl->def->nparallels; i++) @@ -893,9 +939,10 @@ get_files(vahControl * ctl) ctl->def->parallels[i]->source.type == VIR_DOMAIN_CHR_TYPE_FILE || ctl->def->parallels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PIPE) && ctl->def->parallels[i]->source.data.file.path) - if (vah_add_file(&buf, - ctl->def->parallels[i]->source.data.file.path, - "rw") != 0) + if (vah_add_file_chardev(&buf, + ctl->def->parallels[i]->source.data.file.path, + "rw", + ctl->def->parallels[i]->source.type) != 0) goto clean; for (i = 0 ; i < ctl->def->nchannels; i++) @@ -905,9 +952,10 @@ get_files(vahControl * ctl) ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_FILE || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PIPE) && ctl->def->channels[i]->source.data.file.path) - if (vah_add_file(&buf, - ctl->def->channels[i]->source.data.file.path, - "rw") != 0) + if (vah_add_file_chardev(&buf, + ctl->def->channels[i]->source.data.file.path, + "rw", + ctl->def->channels[i]->source.type) != 0) goto clean; if (ctl->def->os.kernel) diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index c8bf398de9..21a27666b5 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -1,4 +1,10 @@ #!/bin/sh +# +# virt-aa-helper needs a working locale system. If testing this in a chroot +# system, need to make sure these are setup properly. On Debian-based systems +# this can be done with something like (as root): +# locale-gen en_US.UTF-8 + set -e test_hostdev="no" @@ -255,6 +261,10 @@ testme "0" "serial (pty)" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,,,g" "$template_xml" > "$test_xml" testme "0" "serial (dev)" "-r -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,,,g" "$template_xml" > "$test_xml" +mkfifo "$tmpdir/serial.pipe.in" "$tmpdir/serial.pipe.out" +testme "0" "serial (pipe)" "-r -u $valid_uuid" "$test_xml" + sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,,,g" "$template_xml" > "$test_xml" touch "$tmpdir/console.log" testme "0" "console" "-r -u $valid_uuid" "$test_xml" @@ -262,9 +272,17 @@ testme "0" "console" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,,,g" "$template_xml" > "$test_xml" testme "0" "console (pty)" "-r -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,,,g" "$template_xml" > "$test_xml" +mkfifo "$tmpdir/console.pipe.in" "$tmpdir/console.pipe.out" +testme "0" "console (pipe)" "-r -u $valid_uuid" "$test_xml" + sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,,,g" "$template_xml" > "$test_xml" testme "0" "parallel (pty)" "-r -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,,,g" "$template_xml" > "$test_xml" +mkfifo "$tmpdir/parallel.pipe.in" "$tmpdir/parallel.pipe.out" +testme "0" "parallel (pipe)" "-r -u $valid_uuid" "$test_xml" + sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,,,g" "$template_xml" > "$test_xml" touch "$tmpdir/guestfwd" testme "0" "channel (unix)" "-r -u $valid_uuid" "$test_xml"