On Tue, Sep 27, 2011 at 01:44:59PM -0500, Jamie Strandboge wrote: > The AppArmor security driver adds only the path specified in the domain > XML for character devices of type 'pipe'. It should be using <path>.in > and <path>.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 > > -- > Jamie Strandboge | http://www.canonical.com > Author: Jamie Strandboge <jamie@xxxxxxxxxxxxx> > Description: 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 <path>.in and > <path>.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). > Bug-Ubuntu: https://launchpad.net/bugs/832507 > > diff -Naurp libvirt.orig/src/security/virt-aa-helper.c libvirt/src/security/virt-aa-helper.c > --- libvirt.orig/src/security/virt-aa-helper.c 2011-09-27 11:24:27.000000000 -0500 > +++ libvirt/src/security/virt-aa-helper.c 2011-09-27 13:23:00.000000000 -0500 > @@ -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 > case S_IFDIR: > return 1; > break; > - case S_IFIFO: > - return 1; > - break; > case S_IFSOCK: > return 1; > break; > @@ -796,6 +793,51 @@ vah_add_file(virBufferPtr buf, const cha > } > > 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 -Naurp libvirt.orig/tests/virt-aa-helper-test libvirt/tests/virt-aa-helper-test > --- libvirt.orig/tests/virt-aa-helper-test 2010-10-23 10:10:47.000000000 -0500 > +++ libvirt/tests/virt-aa-helper-test 2011-09-27 13:23:00.000000000 -0500 > @@ -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_ > sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='dev'><source path='/dev/ttyS0'/><target port='0'/></serial></devices>,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,</devices>,<serial type='pipe'><source path='$tmpdir/serial.pipe'/><target port='0'/></serial></devices>,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,</devices>,<console type='file'><source path='$tmpdir/console.log'/><target port='0'/></console></devices>,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" > sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<console type='pty'><target port='0'/></console></devices>,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,</devices>,<console type='pipe'><source path='$tmpdir/console.pipe'/><target port='0'/></console></devices>,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,</devices>,<parallel type='pty'><source path='/dev/pts/0'/><target port='0'/></parallel></devices>,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,</devices>,<parallel type='pipe'><source path='$tmpdir/parallel.pipe'/><target port='0'/></parallel></devices>,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,</devices>,<channel type='unix'><source mode='bind' path='$tmpdir/guestfwd'/><target type='guestfwd' address='10.0.2.1' port='4600'/></channel></devices>,g" "$template_xml" > "$test_xml" > touch "$tmpdir/guestfwd" > testme "0" "channel (unix)" "-r -u $valid_uuid" "$test_xml" Sounds familiar, ACK ... But the whole .in/.out magical semantic is really ugly, it would be better to get this fixed one way or another in qemu. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list