On Tue, Jan 21, 2014 at 10:47:07AM -0700, Eric Blake wrote: > I noticed that we allow virDomainGetVcpusFlags even for read-only > connections, but that with a flag, it can require guest agent > interaction. It is feasible that a malicious guest could > intentionally abuse the replies it sends over the guest agent > connection to possibly trigger a bug in libvirt's JSON parser, > or withhold an answer so as to prevent the use of the agent > in a later command such as a shutdown request. Although we > don't know of any such exploits now (and therefore don't mind > posting this patch publicly without trying to get a CVE assigned), > it is better to err on the side of caution and explicitly require > full access to any domain where the API requires guest interaction > to operate correctly. > > I audited all commands that are marked as conditionally using a > guest agent. Note that at least virDomainFSTrim is documented > as needing a guest agent, but that such use is unconditional > depending on the hypervisor (so the existing domain:fs_trim ACL > should be sufficient there, rather than also requirng domain:write). > But when designing future APIs, such as the plans for obtaining > a domain's IP addresses, we should copy the approach of this patch > in making interaction with the guest be specified via a flag, and > use that flag to also require stricter access checks. > > * src/libvirt.c (virDomainGetVcpusFlags): Forbid guest interaction > on read-only connection. > (virDomainShutdownFlags, virDomainReboot): Improve docs on agent > interaction. > * src/remote/remote_protocol.x > (REMOTE_PROC_DOMAIN_SNAPSHOT_CREATE_XML) > (REMOTE_PROC_DOMAIN_SET_VCPUS_FLAGS) > (REMOTE_PROC_DOMAIN_GET_VCPUS_FLAGS, REMOTE_PROC_DOMAIN_REBOOT) > (REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS): Require domain:write for any > conditional use of a guest agent. > * src/xen/xen_driver.c: Fix clients. > * src/libxl/libxl_driver.c: Likewise. > * src/uml/uml_driver.c: Likewise. > * src/qemu/qemu_driver.c: Likewise. > * src/lxc/lxc_driver.c: Likewise. > > Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> > --- > src/libvirt.c | 12 +++++++++--- > src/libxl/libxl_driver.c | 6 +++--- > src/lxc/lxc_driver.c | 4 ++-- > src/qemu/qemu_driver.c | 8 ++++---- > src/remote/remote_protocol.x | 5 +++++ > src/uml/uml_driver.c | 2 +- > src/xen/xen_driver.c | 6 +++--- > 7 files changed, 27 insertions(+), 16 deletions(-) ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list