Commit Graph

204 Commits

Author SHA1 Message Date
Eric Blake
0d166c6b7c build: detect potentential uninitialized variables
Even with -Wuninitialized (which is part of autobuild.sh
--enable-compile-warnings=error), gcc does NOT catch this
use of an uninitialized variable:

{
  if (cond)
    goto error;
  int a = 1;
error:
  printf("%d", a);
}

which prints 0 (supposing the stack started life wiped) if
cond was true.  Clang will catch it, but we don't use clang
as often.  Using gcc -Wjump-misses-init catches it, but also
gives false positives:

{
  if (cond)
    goto error;
  int a = 1;
  return a;
error:
  return 0;
}

Here, a was never used in the scope of the error block, so
declaring it after goto is technically fine (and clang agrees).
However, given that our HACKING already documents a preference
to C89 decl-before-statement, the false positive warning is
enough of a prod to comply with HACKING.

[Personally, I'd _really_ rather use C99 decl-after-statement
to minimize scope, but until gcc can efficiently and reliably
catch scoping and uninitialized usage bugs, I'll settle with
the compromise of enforcing a coding standard that happens to
reject false positives if it can also detect real bugs.]

* acinclude.m4 (LIBVIRT_COMPILE_WARNINGS): Add -Wjump-misses-init.
* src/util/util.c (__virExec): Adjust offenders.
* src/conf/domain_conf.c (virDomainTimerDefParseXML): Likewise.
* src/remote/remote_driver.c (doRemoteOpen): Likewise.
* src/phyp/phyp_driver.c (phypGetLparNAME, phypGetLparProfile)
(phypGetVIOSFreeSCSIAdapter, phypVolumeGetKey)
(phypGetStoragePoolDevice)
(phypVolumeGetPhysicalVolumeByStoragePool)
(phypVolumeGetPath): Likewise.
* src/vbox/vbox_tmpl.c (vboxNetworkUndefineDestroy)
(vboxNetworkCreate, vboxNetworkDumpXML)
(vboxNetworkDefineCreateXML): Likewise.
* src/xenapi/xenapi_driver.c (getCapsObject)
(xenapiDomainDumpXML): Likewise.
* src/xenapi/xenapi_utils.c (createVMRecordFromXml): Likewise.
* src/security/security_selinux.c (SELinuxGenNewContext):
Likewise.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Likewise.
* src/qemu/qemu_hotplug.c (qemuDomainChangeEjectableMedia):
Likewise.
* src/qemu/qemu_process.c (qemuProcessWaitForMonitor): Likewise.
* src/qemu/qemu_monitor_text.c (qemuMonitorTextGetPtyPaths):
Likewise.
* src/qemu/qemu_driver.c (qemudDomainShutdown)
(qemudDomainBlockStats, qemudDomainMemoryPeek): Likewise.
* src/storage/storage_backend_iscsi.c
(virStorageBackendCreateIfaceIQN): Likewise.
* src/node_device/node_device_udev.c (udevProcessPCI): Likewise.
2011-04-04 11:26:29 -06:00
Eric Blake
32e52134ff smartcard: enable SELinux support
* src/security/security_selinux.c
(SELinuxRestoreSecuritySmartcardCallback)
(SELinuxSetSecuritySmartcardCallback): New helper functions.
(SELinuxRestoreSecurityAllLabel, SELinuxSetSecurityAllLabel): Use
them.
2011-02-03 19:28:53 -07:00
Laine Stump
d89608f994 Add a function to the security driver API that sets the label of an open fd.
A need was found to set the SELinux context label on an open fd (a
pipe, as a matter of fact). This patch adds a function to the security
driver API that will set the label on an open fd to secdef.label. For
all drivers other than the SELinux driver, it's a NOP. For the SElinux
driver, it calls fsetfilecon().

If the return is a failure, it only returns error up to the caller if
1) the desired label is different from the existing label, 2) the
destination fd is of a type that supports setting the selinux context,
and 3) selinux is in enforcing mode. Otherwise it will return
success. This follows the pattern of the existing function
SELinuxSetFilecon().
2011-01-26 09:03:11 -05:00
Eric Blake
98334e7c3a domain_conf: split source data out from ChrDef
This opens up the possibility of reusing the smaller ChrSourceDef
for both qemu monitor and a passthrough smartcard device.

* src/conf/domain_conf.h (_virDomainChrDef): Factor host
details...
(_virDomainChrSourceDef): ...into new struct.
(virDomainChrSourceDefFree): New prototype.
* src/conf/domain_conf.c (virDomainChrDefFree)
(virDomainChrDefParseXML, virDomainChrDefFormat): Split...
(virDomainChrSourceDefClear, virDomainChrSourceDefFree)
(virDomainChrSourceDefParseXML, virDomainChrSourceDefFormat):
...into new functions.
(virDomainChrDefParseTargetXML): Update clients to reflect type
split.
* src/vmx/vmx.c (virVMXParseSerial, virVMXParseParallel)
(virVMXFormatSerial, virVMXFormatParallel): Likewise.
* src/xen/xen_driver.c (xenUnifiedDomainOpenConsole): Likewise.
* src/xen/xend_internal.c (xenDaemonParseSxprChar)
(xenDaemonFormatSxprChr): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainDumpXML, vboxAttachSerial)
(vboxAttachParallel): Likewise.
* src/security/security_dac.c (virSecurityDACSetChardevLabel)
(virSecurityDACSetChardevCallback)
(virSecurityDACRestoreChardevLabel)
(virSecurityDACRestoreChardevCallback): Likewise.
* src/security/security_selinux.c (SELinuxSetSecurityChardevLabel)
(SELinuxSetSecurityChardevCallback)
(SELinuxRestoreSecurityChardevLabel)
(SELinuxSetSecurityChardevCallback): Likewise.
* src/security/virt-aa-helper.c (get_files): Likewise.
* src/lxc/lxc_driver.c (lxcVmStart, lxcDomainOpenConsole):
Likewise.
* src/uml/uml_conf.c (umlBuildCommandLineChr): Likewise.
* src/uml/uml_driver.c (umlIdentifyOneChrPTY, umlIdentifyChrPTY)
(umlDomainOpenConsole): Likewise.
* src/qemu/qemu_command.c (qemuBuildChrChardevStr)
(qemuBuildChrArgStr, qemuBuildCommandLine)
(qemuParseCommandLineChr): Likewise.
* src/qemu/qemu_domain.c (qemuDomainObjPrivateXMLFormat)
(qemuDomainObjPrivateXMLParse): Likewise.
* src/qemu/qemu_cgroup.c (qemuSetupChardevCgroup): Likewise.
* src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Likewise.
* src/qemu/qemu_driver.c (qemudFindCharDevicePTYsMonitor)
(qemudFindCharDevicePTYs, qemuPrepareChardevDevice)
(qemuPrepareMonitorChr, qemudShutdownVMDaemon)
(qemuDomainOpenConsole): Likewise.
* src/qemu/qemu_command.h (qemuBuildChrChardevStr)
(qemuBuildChrArgStr): Delete, now that they are static.
* src/libvirt_private.syms (domain_conf.h): New exports.
* cfg.mk (useless_free_options): Update list.
* tests/qemuxml2argvtest.c (testCompareXMLToArgvFiles): Update
tests.
2011-01-14 09:54:26 -07:00
Daniel P. Berrange
d6623003c6 Refactor the security drivers to simplify usage
The current security driver usage requires horrible code like

    if (driver->securityDriver &&
        driver->securityDriver->domainSetSecurityHostdevLabel &&
        driver->securityDriver->domainSetSecurityHostdevLabel(driver->securityDriver,
                                                              vm, hostdev) < 0)

This pair of checks for NULL clutters up the code, making the driver
calls 2 lines longer than they really need to be. The goal of the
patchset is to change the calling convention to simply

  if (virSecurityManagerSetHostdevLabel(driver->securityDriver,
                                        vm, hostdev) < 0)

The first check for 'driver->securityDriver' being NULL is removed
by introducing a 'no op' security driver that will always be present
if no real driver is enabled. This guarentees driver->securityDriver
!= NULL.

The second check for 'driver->securityDriver->domainSetSecurityHostdevLabel'
being non-NULL is hidden in a new abstraction called virSecurityManager.
This separates the driver callbacks, from main internal API. The addition
of a virSecurityManager object, that is separate from the virSecurityDriver
struct also allows for security drivers to carry state / configuration
information directly. Thus the DAC/Stack drivers from src/qemu which
used to pull config from 'struct qemud_driver' can now be moved into
the 'src/security' directory and store their config directly.

* src/qemu/qemu_conf.h, src/qemu/qemu_driver.c: Update to
  use new virSecurityManager APIs
* src/qemu/qemu_security_dac.c,  src/qemu/qemu_security_dac.h
  src/qemu/qemu_security_stacked.c, src/qemu/qemu_security_stacked.h:
  Move into src/security directory
* src/security/security_stack.c, src/security/security_stack.h,
  src/security/security_dac.c, src/security/security_dac.h: Generic
  versions of previous QEMU specific drivers
* src/security/security_apparmor.c, src/security/security_apparmor.h,
  src/security/security_driver.c, src/security/security_driver.h,
  src/security/security_selinux.c, src/security/security_selinux.h:
  Update to take virSecurityManagerPtr object as the first param
  in all callbacks
* src/security/security_nop.c, src/security/security_nop.h: Stub
  implementation of all security driver APIs.
* src/security/security_manager.h, src/security/security_manager.c:
  New internal API for invoking security drivers
* src/libvirt.c: Add missing debug for security APIs
2011-01-10 18:10:52 +00:00
Josh Durgin
ee3efc41e6 Skip file-based security checks for network disks
Network disks are accessed by qemu directly, and have no
associated file on the host, so checking for file ownership etc.
is unnecessary.

Signed-off-by: Josh Durgin <joshd@hq.newdream.net>
2010-12-21 07:30:25 -07:00
Eric Blake
8e2b26db94 build: allow older libselinux again
* configure.ac (with_selinux): Check for <selinux/label.h>.
* src/security/security_selinux.c (getContext): New function.
(SELinuxRestoreSecurityFileLabel): Use it to restore compilation
when using older libselinux.
2010-12-20 10:26:15 -07:00
Eric Blake
6679943f94 selinux: avoid memory overhead of matchpathcon
https://bugzilla.redhat.com/show_bug.cgi?id=658657

* src/security/security_selinux.c
(SELinuxRestoreSecurityFileLabel): Use selabel_lookup instead of
matchpathcon.
Suggested by Daniel Walsh.
2010-12-13 17:01:57 -07:00
Eric Blake
d95488dce5 security, storage: plug memory leaks for security_context_t
security_context_t happens to be a typedef for char*, and happens to
begin with a string usable as a raw context string.  But in reality,
it is an opaque type that may or may not have additional information
after the first NUL byte, where that additional information can
include pointers that can only be freed via freecon().

Proof is from this valgrind run of daemon/libvirtd:

==6028== 839,169 (40 direct, 839,129 indirect) bytes in 1 blocks are definitely lost in loss record 274 of 274
==6028==    at 0x4A0515D: malloc (vg_replace_malloc.c:195)
==6028==    by 0x3022E0D48C: selabel_open (label.c:165)
==6028==    by 0x3022E11646: matchpathcon_init_prefix (matchpathcon.c:296)
==6028==    by 0x3022E1190D: matchpathcon (matchpathcon.c:317)
==6028==    by 0x4F9D842: SELinuxRestoreSecurityFileLabel (security_selinux.c:382)

800k is a lot of memory to be leaking.

* src/storage/storage_backend.c
(virStorageBackendUpdateVolTargetInfoFD): Avoid leak on error.
* src/security/security_selinux.c
(SELinuxReserveSecurityLabel, SELinuxGetSecurityProcessLabel)
(SELinuxRestoreSecurityFileLabel): Use correct function to free
security_context_t.
2010-11-24 15:23:43 -07:00
Daniel Veillard
83fa118458 Fix a failure to restore SELinux label for character devices
The code in SELinuxRestoreSecurityChardevLabel() was trying to
use SELinuxSetFilecon directly for devices or file types while
it should really use SELinuxRestoreSecurityFileLabel encapsulating
routine, which avoid various problems like resolving symlinks,
making sure he file exists and work around NFS problems
2010-11-18 17:55:14 +01:00
Eric Blake
e8aba782e7 maint: avoid remaining sprintf uses
* cfg.mk (sc_prohibit_sprintf): New rule.
(sc_prohibit_asprintf): Avoid false positives.
* docs/hacking.html.in (Printf-style functions): Document the
policy.
* HACKING: Regenerate.
* .x-sc_prohibit_sprintf: New exemptions.
* Makefile.am (syntax_check_exceptions): Ship new file.
* src/vbox/vbox_tmpl.c (vboxStartMachine, vboxAttachUSB): Use
virAsprintf instead.
* src/uml/uml_driver.c (umlOpenMonitor): Use snprintf instead.
* tools/virsh.c (cmdDetachInterface): Likewise.
* src/security/security_selinux.c (SELinuxGenSecurityLabel):
Likewise.
* src/openvz/openvz_driver.c (openvzDomainDefineCmd): Likewise,
and ensure large enough buffer.
2010-11-17 10:13:12 -07:00
Laine Stump
5b04f42c6f ignore SELinuxSetFilecon error in SELinuxSetSecurityFileLabel if on nfs
If virDomainAttachDevice() was called with an image that was located
on a root-squashed NFS server, and in a directory that was unreadable
by root on the machine running libvirtd, the attach would fail due to
an attempt to change the selinux label of the image with EACCES (which
isn't covered as an ignore case in SELinuxSetFilecon())

NFS doesn't support SELinux labelling anyway, so we mimic the failure
handling of commit 93a18bbafa, which
just ignores the errors if the target is on an NFS filesystem (in
SELinuxSetSecurityAllLabel() only, though.)

This can be seen as a follow-on to commit
347d266c51, which ignores file open
failures of files on NFS that occur directly in
virDomainDiskDefForeachPath() (also necessary), but does not ignore
failures in functions that are called from there (eg
SELinuxSetSecurityFileLabel()).
2010-11-11 11:43:06 -05:00
Stefan Berger
60ae1c34ad bye to close(), welcome to VIR_(FORCE_)CLOSE()
Using automated replacement with sed and editing I have now replaced all
occurrences of close() with VIR_(FORCE_)CLOSE() except for one, of
course. Some replacements were straight forward, others I needed to pay
attention. I hope I payed attention in all the right places... Please
have a look. This should have at least solved one more double-close
error.
2010-11-09 15:48:48 -05:00
Daniel P. Berrange
347d266c51 Ignore open failures when relabelling disk images
NFS in root squash mode may prevent opening disk images to
determine backing store. Ignore errors in this scenario.

* src/security/security_selinux.c: Ignore open failures on disk
  images
2010-11-02 14:38:23 +00:00
Daniel P. Berrange
93a18bbafa Treat failure to relabel stdin_path as non-fatal on NFS
NFS does not support file labelling, so ignore this error
for stdin_path when on NFS.

* src/security/security_selinux.c: Ignore failures on labelling
  stdin_path on NFS
* src/util/storage_file.c, src/util/storage_file.h: Refine
  virStorageFileIsSharedFS() to allow it to check for a
  specific FS type.
2010-11-02 14:38:19 +00:00
Daniel P. Berrange
e06772f006 Remove audit hooks from the selinux security driver code
This reverts commit b8e2de8899
The hooks will be re-added in the QEMU driver itself.

* src/security/security_selinux.c: Remove audit hooks
2010-10-27 17:03:21 +01:00
Miloslav Trmač
b8e2de8899 Audit SELinux label assignment.
A more natural auditing point would perhaps be
SELinuxSetSecurityProcessLabel, but this happens in the child after root
permissions are dropped, so the kernel would refuse the audit record.
2010-10-19 17:31:31 +01:00
Daniel P. Berrange
68719c4bdd Disable all disk probing in QEMU driver & add config option to re-enable
Disk format probing is now disabled by default. A new config
option in /etc/qemu/qemu.conf will re-enable it for existing
deployments where this causes trouble
2010-07-19 18:25:13 +01:00
Daniel P. Berrange
f70e080962 Pass security driver object into all security driver callbacks
The implementation of security driver callbacks often needs
to access the security driver object. Currently only a handful
of callbacks include the driver object as a parameter. Later
patches require this is many more places.

* src/qemu/qemu_driver.c: Pass in the security driver object
  to all callbacks
* src/qemu/qemu_security_dac.c, src/qemu/qemu_security_stacked.c,
  src/security/security_apparmor.c, src/security/security_driver.h,
  src/security/security_selinux.c: Add a virSecurityDriverPtr
  param to all security callbacks
2010-07-19 18:25:13 +01:00
Daniel P. Berrange
a885334499 Convert all disk backing store loops to shared helper API
Update the QEMU cgroups code, QEMU DAC security driver, SELinux
and AppArmour security drivers over to use the shared helper API
virDomainDiskDefForeachPath().

* src/qemu/qemu_driver.c, src/qemu/qemu_security_dac.c,
  src/security/security_selinux.c, src/security/virt-aa-helper.c:
  Convert over to use virDomainDiskDefForeachPath()
2010-07-19 18:25:13 +01:00
Daniel P. Berrange
bf80fc68ca Require format to be passed into virStorageFileGetMetadata
Require the disk image to be passed into virStorageFileGetMetadata.
If this is set to VIR_STORAGE_FILE_AUTO, then the format will be
resolved using probing. This makes it easier to control when
probing will be used

* src/qemu/qemu_driver.c, src/qemu/qemu_security_dac.c,
  src/security/security_selinux.c, src/security/virt-aa-helper.c:
  Set VIR_STORAGE_FILE_AUTO when calling virStorageFileGetMetadata.
* src/storage/storage_backend_fs.c: Probe for disk format before
  calling virStorageFileGetMetadata.
* src/util/storage_file.h, src/util/storage_file.c: Remove format
  from virStorageFileMeta struct & require it to be passed into
  method.
2010-07-19 18:25:04 +01:00
Laine Stump
590c9c9acf Set proper selinux label on image file during qemu domain restore
Also restore the label to its original value after qemu is finished
with the file.

Prior to this patch, qemu domain restore did not function properly if
selinux was set to enforce.
2010-06-28 11:55:32 -04:00
Daniel P. Berrange
2bad82f71e Set labelling for character devices in security drivers
When configuring serial, parallel, console or channel devices
with a file, dev or pipe backend type, it is necessary to label
the file path in the security drivers. For char devices of type
file, it is neccessary to pre-create (touch) the file if it does
not already exist since QEMU won't be allowed todo so itself.
dev/pipe configs already require the admin to pre-create before
starting the guest.

* src/qemu/qemu_security_dac.c: set file ownership for character
  devices
* src/security/security_selinux.c: Set file labeling for character
  devices
* src/qemu/qemu_driver.c: Add character devices to cgroup ACL
2010-06-25 14:39:54 +01:00
Daniel P. Berrange
f43dc26af7 Don't raise errors in the selinux restore code
The restore code is done in places where errors cannot be
raised, since they will overwrite over pre-existing errors.

* src/security/security_selinux.c: Only warn about failures
  in label restore, don't report errors
2010-06-08 15:21:07 +01:00
Daniel J Walsh
e72cc3c11d Add support for setting socket MLS level in SELinux driver
When SELinux is running in MLS mode, libvirtd will have a
different security level to the VMs. For libvirtd to be
able to connect to the monitor console, the client end of
the UNIX domain socket needs a different label. This adds
infrastructure to set the socket label via the security
driver framework

* src/qemu/qemu_driver.c: Call out to socket label APIs in
  security driver
* src/qemu/qemu_security_stacked.c: Wire up socket label
  drivers
* src/security/security_driver.h: Define security driver
  entry points for socket labelling
* src/security/security_selinux.c: Set socket label based on
  VM label
2010-06-08 15:14:57 +01:00
Jamie Strandboge
2b57478ef0 Add stdin_path to qemudStartVMDaemon() args.
Adjust args to qemudStartVMDaemon() to also specify path to stdin_fd,
so this can be passed to the AppArmor driver via SetSecurityAllLabel().

This updates all calls to qemudStartVMDaemon() as well as setting up
the non-AppArmor security driver *SetSecurityAllLabel() declarations
for the above. This is required for the following
"apparmor-fix-save-restore" patch since AppArmor resolves the passed
file descriptor to the pathname given to open().
2010-06-07 14:41:09 -04:00
Jim Meyering
dcf30d9c25 initialize "meta" in virStorageFileGetMetadata, not in each caller
Do not require each caller of virStorageFileGetMetadata and
virStorageFileGetMetadataFromFD to first clear the storage of the
"meta" buffer.  Instead, initialize that storage in
virStorageFileGetMetadataFromFD.
* src/util/storage_file.c (virStorageFileGetMetadataFromFD): Clear
"meta" here, not before each of the following callers.
* src/qemu/qemu_driver.c (qemuSetupDiskCgroup): Don't clear "meta" here.
(qemuTeardownDiskCgroup): Likewise.
* src/qemu/qemu_security_dac.c (qemuSecurityDACSetSecurityImageLabel):
Likewise.
* src/security/security_selinux.c (SELinuxSetSecurityImageLabel):
Likewise.
* src/security/virt-aa-helper.c (get_files): Likewise.
2010-05-19 17:07:08 +02:00
Eric Blake
d533a98ed6 virFileResolveLink: fix return value
virFileResolveLink was returning a positive value on error,
thus confusing callers that assumed failure was < 0.  The
confusion is further evidenced by callers that would have
ended up calling virReportSystemError with a negative value
instead of a valid errno.

Fixes Red Hat BZ #591363.

* src/util/util.c (virFileResolveLink): Live up to documentation.
* src/qemu/qemu_security_dac.c
(qemuSecurityDACRestoreSecurityFileLabel): Adjust callers.
* src/security/security_selinux.c
(SELinuxRestoreSecurityFileLabel): Likewise.
* src/storage/storage_backend_disk.c
(virStorageBackendDiskDeleteVol): Likewise.
2010-05-17 14:48:27 -06:00
Daniel P. Berrange
02ddaddfa8 Don't reset user/group/security label on shared filesystems during migrate
When QEMU runs with its disk on NFS, and as a non-root user, the
disk is chownd to that non-root user. When migration completes
the last step is shutting down the QEMU on the source host. THis
normally resets user/group/security label. This is bad when the
VM was just migrated because the file is still in use on the dest
host. It is thus neccessary to skip the reset step for any files
found to be on a shared filesystem

* src/libvirt_private.syms: Export virStorageFileIsSharedFS
* src/util/storage_file.c, src/util/storage_file.h: Add a new
  method virStorageFileIsSharedFS() to determine if a file is
  on a shared filesystem (NFS, GFS, OCFS2, etc)
* src/qemu/qemu_driver.c: Tell security driver not to reset
  disk labels on migration completion
* src/qemu/qemu_security_dac.c, src/qemu/qemu_security_stacked.c,
  src/security/security_selinux.c, src/security/security_driver.h,
  src/security/security_apparmor.c: Add ability to skip disk
  restore step for files on shared filesystems.
2010-05-14 09:21:24 -04:00
Jiri Denemark
c90a1ad7d0 Report all errors in SELinuxRestoreSecurityFileLabel 2010-04-30 14:27:42 +02:00
Laine Stump
8a7b4be5ab Allow domain disk images on root-squash NFS to coexist with security driver.
(suggested by Daniel Berrange, tested by Dan Kenigsberg)

virStorageFileGetMetadata will fail for disk images that are stored on
a root-squash NFS share that isn't world-readable.
SELinuxSetSecurityImageLabel is called during the startup of every
domain (as long as security_driver != "none"), and it will propogate
the error from virStorageFileGetMetadata, causing the domain startup
to fail. This is, however, a common scenario when qemu is run as a
non-root user and the disk image is stored on NFS.

Ignoring this failure (which doesn't matter in this case, since the
next thing done by SELinuxSetSecurityImageLabel - setting the file
context - will also fail (and that function already ignores failures
due to root-squash NFS) will allow us to continue bringing up the
domain. The result is that we don't need to disable the entire
security driver just because a domain's disk image is stored on
root-squashed NFS.
2010-04-02 09:39:20 -04:00
Cole Robinson
65e97240e6 security: selinux: Fix crash when releasing non-existent label
This can be triggered by the qemuStartVMDaemon cleanup path if a
VM references a non-existent USB device (by product) in the XML.
2010-03-22 10:45:36 -04:00
Cole Robinson
3f1aa08af6 security: Set permissions for kernel/initrd
Fixes URL installs when running virt-install as root on Fedora.
2010-03-15 12:36:50 -04:00
Daniel P. Berrange
09ed07293f Fix USB passthrough based on product/vendor
Changeset

  commit 5073aa994a
  Author: Cole Robinson <crobinso@redhat.com>
  Date:   Mon Jan 11 11:40:46 2010 -0500

Added support for product/vendor based passthrough, but it only
worked at the security driver layer. The main guest XML config
was not updated with the resolved bus/device ID. When the QEMU
argv refactoring removed use of product/vendor, this then broke
launching guests.

THe solution is to move the product/vendor resolution up a layer
into the QEMU driver. So the first thing QEMU does is resolve
the product/vendor to a bus/device and updates the XML config
with this info. The rest of the code, including security drivers
and QEMU argv generated can now rely on bus/device always being
set.

* src/util/hostusb.c, src/util/hostusb.h: Split vendor/product
  resolution code out of usbGetDevice and into usbFindDevice.
  Add accessors for bus/device ID
* src/security/virt-aa-helper.c, src/security/security_selinux.c,
  src/qemu/qemu_security_dac.c: Remove vendor/product from the
  usbGetDevice() calls
* src/qemu/qemu_driver.c: Use usbFindDevice to resolve vendor/product
  into a bus/device ID
2010-03-05 15:05:14 +00:00
Daniel P. Berrange
ce71b8654d Remove virConnectPtr from USB/PCI device iterators
All callers now pass a NULL virConnectPtr into the USB/PCi device
iterator functions. Therefore the virConnectPtr arg can now be
removed from these functions

* src/util/hostusb.h, src/util/hostusb.c: Remove virConnectPtr
  from usbDeviceFileIterate
* src/util/pci.c, src/util/pci.h: Remove virConnectPtr arg from
  pciDeviceFileIterate
* src/qemu/qemu_security_dac.c, src/security/security_selinux.c: Update
  to drop redundant virConnectPtr arg
2010-02-10 13:29:57 +00:00
Daniel P. Berrange
d6126f764f Remove use of virConnectPtr from security driver APIs
The virConnectPtr is no longer required for error reporting since
that is recorded in a thread local. Remove use of virConnectPtr
from all APIs in security_driver.{h,c} and update all callers to
match
2010-02-10 11:49:24 +00:00
Matthias Bolte
314ced3951 Remove conn parameter from USB functions
It was used for error reporting only.
2010-02-09 01:04:55 +01:00
Matthias Bolte
9140756d30 Remove conn parameter from PCI functions
It was used for error reporting only.
2010-02-09 01:04:55 +01:00
Matthias Bolte
fe46a7e686 Remove conn parameter from storage file functions
It was used for error reporting only.
2010-02-09 01:04:55 +01:00
Matthias Bolte
a5ab900d26 Remove conn parameter from virReportSystemError 2010-02-09 01:04:54 +01:00
Matthias Bolte
8ce5e2c1ab Remove conn parameter from virReportOOMError 2010-02-09 01:04:54 +01:00
Daniel P. Berrange
0c0e0d0263 Refactor setup & cleanup of security labels in security driver
The current security driver architecture has the following
split of logic

 * domainGenSecurityLabel

    Allocate the unique label for the domain about to be started

 * domainGetSecurityLabel

    Retrieve the current live security label for a process

 * domainSetSecurityLabel

    Apply the previously allocated label to the current process
    Setup all disk image / device labelling

 * domainRestoreSecurityLabel

    Restore the original disk image / device labelling.
    Release the unique label for the domain

The 'domainSetSecurityLabel' method is special because it runs
in the context of the child process between the fork + exec.

This is require in order to set the process label. It is not
required in order to label disks/devices though. Having the
disk labelling code run in the child process limits what it
can do.

In particularly libvirtd would like to remember the current
disk image label, and only change shared image labels for the
first VM to start. This requires use & update of global state
in the libvirtd daemon, and thus cannot run in the child
process context.

The solution is to split domainSetSecurityLabel into two parts,
one applies process label, and the other handles disk image
labelling. At the same time domainRestoreSecurityLabel is
similarly split, just so that it matches the style. Thus the
previous 4 methods are replaced by the following 6 new methods

 * domainGenSecurityLabel

    Allocate the unique label for the domain about to be started
    No actual change here.

 * domainReleaseSecurityLabel

   Release the unique label for the domain

 * domainGetSecurityProcessLabel

   Retrieve the current live security label for a process
   Merely renamed for clarity.

 * domainSetSecurityProcessLabel

   Apply the previously allocated label to the current process

 * domainRestoreSecurityAllLabel

    Restore the original disk image / device labelling.

 * domainSetSecurityAllLabel

    Setup all disk image / device labelling

The SELinux and AppArmour drivers are then updated to comply with
this new spec. Notice that the AppArmour driver was actually a
little different. It was creating its profile for the disk image
and device labels in the 'domainGenSecurityLabel' method, where as
the SELinux driver did it in 'domainSetSecurityLabel'. With the
new method split, we can have consistency, with both drivers doing
that in the domainSetSecurityAllLabel method.

NB, the AppArmour changes here haven't been compiled so may not
build.
2010-01-21 14:00:16 +00:00
Daniel P. Berrange
81fbb4cb23 Make security drivers responsible for checking dynamic vs static labelling
The QEMU driver is doing 90% of the calls to check for static vs
dynamic labelling. Except it is forgetting todo so in many places,
in particular hotplug is mistakenly assigning disk labels. Move
all this logic into the security drivers themselves, so the HV
drivers don't have to think about it.

* src/security/security_driver.h: Add virDomainObjPtr parameter
  to virSecurityDomainRestoreHostdevLabel and to
  virSecurityDomainRestoreSavedStateLabel
* src/security/security_selinux.c, src/security/security_apparmor.c:
  Add explicit checks for VIR_DOMAIN_SECLABEL_STATIC and skip all
  chcon() code in those cases
* src/qemu/qemu_driver.c: Remove all checks for VIR_DOMAIN_SECLABEL_STATIC
  or VIR_DOMAIN_SECLABEL_DYNAMIC. Add missing checks for possibly NULL
  driver entry points.
2010-01-21 14:00:16 +00:00
Cole Robinson
5073aa994a Implement path lookup for USB by vendor:product
Based off how QEMU does it, look through /sys/bus/usb/devices/* for
matching vendor:product info, and if found, use info from the surrounding
files to build the device's /dev/bus/usb path.

This fixes USB device assignment by vendor:product when running qemu
as non-root (well, it should, but for some reason I couldn't reproduce
the failure people are seeing in [1], but it appears to work properly)

[1] https://bugzilla.redhat.com/show_bug.cgi?id=542450
2010-01-13 15:24:41 -05:00
Matthias Bolte
1b9d074493 Add virBufferFreeAndReset() and replace free()
Replace free(virBufferContentAndReset()) with virBufferFreeAndReset().
Update documentation and replace all remaining calls to free() with
calls to VIR_FREE(). Also add missing calls to virBufferFreeAndReset()
and virReportOOMError() in OOM error cases.
2009-12-10 00:00:50 +01:00
Daniel P. Berrange
e7c78b0a94 Support QEMU's virtual FAT block device driver
Introduce a new type="dir"  mode for <disks> that allows use of
QEMU's  virtual FAT block device driver. eg

    <disk type='dir' device='floppy'>
      <source dir='/tmp/test'/>
      <target dev='fda' bus='fdc'/>
      <readonly/>
    </disk>

gets turned into

  -drive file=fat:floppy:/tmp/test,if=floppy,index=0

Only read-only disks are supported with virtual FAT mode

* src/conf/domain_conf.c, src/conf/domain_conf.h: Add type="dir"
* docs/schemas/domain.rng: Document new disk type
* src/xen/xend_internal.c, src/xen/xm_internal.c: Raise error for
  unsupported disk types
* tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-empty.args: Fix
  empty disk file handling
* tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.args,
  tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fat.xml,
  tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.args,
  tests/qemuxml2argvdata/qemuxml2argv-floppy-drive-fat.xml
  tests/qemuxml2argvtest.c: Test QEMU vitual FAT driver
* src/qemu/qemu_conf.c: Support generating fat:/some/dir type
  disk args
* src/security/security_selinux.c: Temporarily skip labelling
  of directory based disks
2009-11-23 12:17:16 +00:00
Daniel P. Berrange
bc0010b3d1 Fix save and restore with non-privileged guests and SELinux
When running qemu:///system instance, libvirtd runs as root,
but QEMU may optionally be configured to run non-root. When
then saving a guest to a state file, the file is initially
created as root, and thus QEMU cannot write to it. It is also
missing labelling required to allow access via SELinux.

* src/qemu/qemu_driver.c: Set ownership on save image before
  running migrate command in virDomainSave impl. Call out to
  security driver to set save image labelling
* src/security/security_driver.h: Add driver APIs for setting
  and restoring saved state file labelling
* src/security/security_selinux.c: Implement saved state file
  labelling for SELinux
2009-11-11 15:24:32 +00:00
Cole Robinson
4c44cdcce3 Various error reporting fixes
- Don't duplicate SystemError
- Use proper error code in domain_conf
- Fix a broken error call in qemu_conf
- Don't use VIR_ERR_ERROR in security driver (isn't a valid code in this case)
2009-11-03 16:46:54 -05:00
Daniel P. Berrange
46992453b9 Annotate many methods with ATTRIBUTE_RETURN_CHECK & fix problems
Nearly all of the methods in src/util/util.h have error codes that
must be checked by the caller to correct detect & report failure.
Add ATTRIBUTE_RETURN_CHECK to ensure compile time validation of
this

* daemon/libvirtd.c: Add explicit check on return value of virAsprintf
* src/conf/domain_conf.c: Add missing check on virParseMacAddr return
  value status & report error
* src/network/bridge_driver.c: Add missing OOM check on virAsprintf
  and report error
* src/qemu/qemu_conf.c: Add missing check on virParseMacAddr return
  value status & report error
* src/security/security_selinux.c: Remove call to virRandomInitialize
  that's done in libvirt.c already
* src/storage/storage_backend_logical.c: Add check & log on virRun
  return status
* src/util/util.c: Add missing checks on virAsprintf/Run status
* src/util/util.h: Annotate all methods with ATTRIBUTE_RETURN_CHECK
  if they return an error status code
* src/vbox/vbox_tmpl.c: Add missing check on virParseMacAddr
* src/xen/xm_internal.c: Add missing checks on virAsprintf
* tests/qemuargv2xmltest.c: Remove bogus call to virRandomInitialize()
2009-11-02 11:09:03 -05:00
Jamie Strandboge
709c37e932 Add a domain argument to SVirt *RestoreImageLabel
When James Morris originally submitted his sVirt patches (as seen in
libvirt 0.6.1), he did not require on disk labelling for
virSecurityDomainRestoreImageLabel. A later commit[2] changed this
behavior to assume on disk labelling, which halts implementations for
path-based MAC systems such as AppArmor and TOMOYO where
vm->def->seclabel is required to obtain the label.

* src/security/security_driver.h src/qemu/qemu_driver.c
  src/security/security_selinux.c: adds the 'virDomainObjPtr vm'
  argument back to *RestoreImageLabel
2009-10-07 12:40:01 +02:00
Mark McLoughlin
1daea0c59d Fix USB device re-labelling
A simple misplaced break out of a switch results in:

  libvir: error : Failed to open file '/sys/bus/pci/devices/0000:00:54c./vendor': No such file or directory
  libvir: error : Failed to open file '/sys/bus/pci/devices/0000:00:54c./device': No such file or directory
  libvir: error : this function is not supported by the hypervisor: Failed to read product/vendor ID for 0000:00:54c.

when trying to passthrough a USB host device to qemu.

* src/security_selinux.c: fix a switch/break thinko
2009-10-01 11:56:09 +01:00
Chris Lalancette
47c8709564 Fix up a few typos in the tree.
Signed-off-by: Chris Lalancette <clalance@redhat.com>
2009-09-30 14:05:24 +02:00
Mark McLoughlin
fe627697a3 Re-label image file backing stores
Use virStorageFileGetMetadata() to find any backing stores for images
and re-label them

Without this, qemu cannot access qcow2 backing files, see:

  https://bugzilla.redhat.com/497131

* src/security/security_selinux.c: re-label backing store files in
  SELinuxSetSecurityImageLabel()
2009-09-30 10:37:00 +01:00
Daniel P. Berrange
e56c6a83b4 Move security drivers to src/security/
* src/Makefile.am, src/qemu/qemu_conf.h, src/qemu/qemu_driver.c,
  tests/seclabeltest.c: Adapt for changed paths
* src/security.c: Rename to src/security/security_driver.c
* src/security.h: Rename to src/security/security_driver.h
* src/security_selinux.c, src/security_selinux.h: Move to src/security/
2009-09-21 14:41:44 +01:00