Commit Graph

218 Commits

Author SHA1 Message Date
Ján Tomko
d6b8838dd8 security: dac: also label listen UNIX sockets
We switched to opening mode='bind' sockets ourselves:
commit 30fb2276d8
    qemu: support passing pre-opened UNIX socket listen FD
in v4.5.0-rc1~251

Then fixed qemuBuildChrChardevStr to change libvirtd's label
while creating the socket:
commit b0c6300fc4
    qemu: ensure FDs passed to QEMU for chardevs have correct SELinux labels
v4.5.0-rc1~52

Also add labeling of these sockets to the DAC driver.
Instead of duplicating the logic which decides whether libvirt should
pre-create the socket, assume an existing path meaning that it was created
by libvirt.

https://bugzilla.redhat.com/show_bug.cgi?id=1633389

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2018-10-02 12:44:44 +02:00
Michal Privoznik
cd33eaa251 security: Don't try to lock NULL paths
It may happen that in the list of paths/disk sources to relabel
there is a disk source. If that is the case, the path is NULL. In
that case, we shouldn't try to lock the path. It's likely a
network disk anyway and therefore there is nothing to lock.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2018-09-26 12:01:48 +02:00
Michal Privoznik
f4d7ca62a4 security: Grab a reference to virSecurityManager for transactions
This shouldn't be needed per-se. Security manager shouldn't
disappear during transactions - it's immutable. However, it
doesn't hurt to grab a reference either - transaction code uses
it after all.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2018-09-26 12:01:36 +02:00
Michal Privoznik
7a9ca0fae9 security_dac: Lock metadata when running transaction
Lock all the paths we want to relabel to mutually exclude other
libvirt daemons.

The only hitch here is that directories can't be locked.
Therefore, when relabeling a directory do not lock it (this
happens only when setting up some domain private paths anyway,
e.g. huge pages directory).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2018-09-18 17:12:53 +02:00
Michal Privoznik
6e22ab2701 security_dac: Fix info messages when chown()-ing
Firstly, the message that says we're setting uid:gid shouldn't be
called from virSecurityDACSetOwnershipInternal() because
virSecurityDACRestoreFileLabelInternal() is calling it too.
Secondly, there are places between us reporting label restore and
us actually doing it where we can quit. Don't say we're doing
something until we are actually about to do it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2018-09-18 17:12:53 +02:00
Michal Privoznik
c0b68c6e19 security_dac: Move transaction handling up one level
So far the whole transaction handling is done
virSecurityDACSetOwnershipInternal(). This needs to change for
the sake of security label remembering and locking. Otherwise we
would be locking a path when only appending it to transaction
list and not when actually relabeling it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2018-09-18 17:12:53 +02:00
Michal Privoznik
d41c162177 virSecurityManagerTransactionCommit: Accept pid == -1
It will be desirable to run transactions more often than we
currently do. Even if the domain we're relabeling the paths for
does not run in a namespace. If that's the case, there is no need
to fork() as we are already running in the right namespace. To
differentiate whether transaction code should fork() or not the
@pid argument now accepts -1 (which means do not fork).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2018-09-17 10:58:17 +02:00
Michal Privoznik
ca010e9d76 security_dac: Fix const correctness
These two functions (virSecurityDACSetOwnership and
virSecurityDACRestoreFileLabelInternal) do not really change
@src. Make it const.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2018-09-07 09:57:39 +02:00
Michal Privoznik
8a5713e235 security_dac: Pass virSecurityManagerPtr to virSecurityDACRestoreFileLabelInternal
This function is going call security manager APIs and therefore
it needs pointer to it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2018-09-07 09:57:28 +02:00
Michal Privoznik
3ac7793ad1 security_dac: Pass virSecurityManagerPtr to virSecurityDACSetOwnership
This function is going call security manager APIs and therefore
it needs pointer to it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2018-09-07 09:57:22 +02:00
Stefan Berger
a21c45c41d security: Add DAC and SELinux security for tpm-emulator
Extend the DAC and SELinux modules with support for the tpm-emulator.
We label the Unix socket that QEMU connects to after starting swtmp
with DAC and SELinux labels. We do not have to restore the labels in
this case since the tpm-emulator will remove the Unix socket when it
terminates.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2018-06-06 10:48:41 -04:00
Stefan Berger
33af0b2b7c conf: Add support for external swtpm TPM emulator to domain XML
This patch adds support for an external swtpm TPM emulator. The XML for
this type of TPM looks as follows:

 <tpm model='tpm-tis'>
   <backend type='emulator'/>
 </tpm>

The XML will currently only define a TPM 1.2.

Extend the documentation.

Add a test case testing the XML parser and formatter.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2018-06-06 10:48:41 -04:00
Martin Kletzander
156f03ec59 security/: Remove spaces after casts
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2018-05-03 22:31:37 +02:00
Michal Privoznik
ca250269b0 virSecurityDACChownListFree: Don't leak list->items array
We're freeing individual items in it but not the array itself.

==19200== 40 bytes in 1 blocks are definitely lost in loss record 847 of 1,059
==19200==    at 0x4C2D12F: realloc (vg_replace_malloc.c:785)
==19200==    by 0x52C5532: virReallocN (viralloc.c:245)
==19200==    by 0x52C5628: virExpandN (viralloc.c:294)
==19200==    by 0x52C58FC: virInsertElementsN (viralloc.c:436)
==19200==    by 0x542856B: virSecurityDACChownListAppend (security_dac.c:115)
==19200==    by 0x54286B4: virSecurityDACTransactionAppend (security_dac.c:167)
==19200==    by 0x542902F: virSecurityDACSetOwnershipInternal (security_dac.c:560)
==19200==    by 0x54295D6: virSecurityDACSetOwnership (security_dac.c:650)
==19200==    by 0x542AEE0: virSecurityDACSetInputLabel (security_dac.c:1472)
==19200==    by 0x542B61D: virSecurityDACSetAllLabel (security_dac.c:1693)
==19200==    by 0x542DD67: virSecurityManagerSetAllLabel (security_manager.c:869)
==19200==    by 0x54279C2: virSecurityStackSetAllLabel (security_stack.c:361)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2018-03-13 15:38:17 +01:00
Christian Ehrhardt
a5486e57f5 security: full path option for DomainSetPathLabel
virSecurityManagerDomainSetPathLabel is used to make a path known
to the security modules, but today is used interchangably for
 - paths to files/dirs to be accessed directly
 - paths to a dir, but the access will actually be to files therein

Depending on the security module it is important to know which of
these types it will be.

The argument allowSubtree augments the call to the implementations of
DomainSetPathLabel that can - per security module - decide if extra
actions shall be taken.

For now dac/selinux handle this as before, but apparmor will make
use of it to add a wildcard to the path that was passed.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2018-01-09 17:29:52 +01:00
Pavel Hrdina
1b4f66ec80 security: introduce virSecurityManager(Set|Restore)ChardevLabel
SELinux and DAC drivers already have both functions but they were not
exported as public API of security manager.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-12-05 13:54:48 +01:00
Ján Tomko
d8116b5a0a security: Introduce functions for input device hot(un)plug
Export the existing DAC and SELinux for separate use and introduce
functions for stack, nop and the security manager.
2017-11-24 17:38:51 +01:00
Peter Krempa
2742dfee28 security: dac: Take parent security label into account
Until now we ignored user-provided backing chains and while detecting
the code inherited labels of the parent device. With user provided
chains we should keep this functionality, so label of the parent image
in the backing chain will be applied if an image-specific label is not
present.
2017-11-03 09:15:41 +01:00
Peter Krempa
0a294a8e28 util: storagefile: Add helpers to check presence of backing store
Add helpers that will simplify checking if a backing file is valid or
whether it has backing store. The helper virStorageSourceIsBacking
returns true if the given virStorageSource is a valid backing store
member. virStorageSourceHasBacking returns true if the virStorageSource
has a backing store child.

Adding these functions creates a central points for further refactors.
2017-10-17 06:19:18 +02:00
Cole Robinson
98931187ee security: dac: relabel spice rendernode
For a logged in user this a path like /dev/dri/renderD128 will have
default ownership root:video which won't work for the qemu:qemu user,
so we need to chown it.

We only do this when mount namespaces are enabled in the qemu driver,
so the chown'ing doesn't interfere with other users of the shared
render node path

https://bugzilla.redhat.com/show_bug.cgi?id=1460804
2017-09-12 12:27:42 -04:00
Cole Robinson
321031e482 security: add MANAGER_MOUNT_NAMESPACE flag
The VIR_SECURITY_MANAGER_MOUNT_NAMESPACE flag informs the DAC driver
if mount namespaces are in use for the VM. Will be used for future
changes.

Wire it up in the qemu driver
2017-09-12 12:27:42 -04:00
Michal Privoznik
e0d1a378dc security: Don't skip relabel for all chardevs
Our commit e13e8808f9 was way too generic. Currently, virtlogd is
used only for chardevs type of file and nothing else. True, we
must not relabel the path in this case, but we have to in all
other cases. For instance, if you want to have a physical console
attached to your guest:

    <console type='dev'>
      <source path='/dev/ttyS0'/>
      <target type='virtio' port='1'/>
    </console>

Starting such domain fails because qemu doesn't have access to
/dev/ttyS0 because we haven't relabelled the path.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2017-06-22 14:28:15 +02:00
Pavel Hrdina
e13e8808f9 security: don't relabel chardev source if virtlogd is used as stdio handler
In the case that virtlogd is used as stdio handler we pass to QEMU
only FD to a PIPE connected to virtlogd instead of the file itself.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1430988

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2017-06-16 16:00:10 +02:00
Pavel Hrdina
1763a2e7e2 conf: move seclabel for chardev source to the correct sturcture
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2017-06-16 15:42:59 +02:00
Erik Skultety
3a2a2a7401 mdev: Pass a uuidstr rather than an mdev object to some util functions
Namely, this patch is about virMediatedDeviceGetIOMMUGroup{Dev,Num}
functions. There's no compelling reason why these functions should take
an object, on the contrary, having to create an object every time one
needs to query the IOMMU group number, discarding the object afterwards,
seems odd.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2017-05-18 12:20:15 +02:00
Erik Skultety
606afafba4 security: Enable labeling of vfio mediated devices
This patch updates all of our security driver to start labeling the
VFIO IOMMU devices under /dev/vfio/ as well.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2017-03-27 15:39:35 +02:00
Erik Skultety
ec783d7c77 conf: Introduce new hostdev device type mdev
A mediated device will be identified by a UUID (with 'model' now being
a mandatory <hostdev> attribute to represent the mediated device API) of
the user pre-created mediated device. We also need to make sure that if
user explicitly provides a guest address for a mdev device, the address
type will be matching the device API supported on that specific mediated
device and error out with an incorrect XML message.

The resulting device XML:
<devices>
  <hostdev mode='subsystem' type='mdev' model='vfio-pci'>
    <source>
      <address uuid='c2177883-f1bb-47f0-914d-32a22e3a8804'>
    </source>
  </hostdev>
</devices>

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2017-03-27 15:39:35 +02:00
Michal Privoznik
679bf50120 secdrivers: Implement memdev relabel APIs
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-03-15 16:48:24 +01:00
Michal Privoznik
ea416faf74 security_dac: Label host side of NVDIMM
When domain is being started up, we ought to relabel the host
side of NVDIMM so qemu has access to it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-03-15 14:39:53 +01:00
Erik Skultety
df7f42d5be security: DAC: fix the transaction model's list append
The problem is in the way how the list item is created prior to
appending it to the transaction list - the @path attribute is just a
shallow copy instead of deep copy of the hostdev device's path.
Unfortunately, the hostdev devices from which the @path is extracted, in
order to add them into the transaction list, are only temporary and
freed before the buildup of the qemu namespace, thus making the @path
attribute in the transaction list NULL, causing 'permission denied' or
'double free' or 'unknown cause' errors.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1413773

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2017-01-17 15:49:57 +01:00
Michal Privoznik
67232478db security_dac: Implement transaction APIs
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-01-10 12:50:00 +01:00
Michal Privoznik
39779eb195 security_dac: Resolve virSecurityDACSetOwnershipInternal const correctness
The code at the very bottom of the DAC secdriver that calls
chown() should be fine with read-only data. If something needs to
be prepared it should have been done beforehand.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-01-10 12:49:59 +01:00
Eric Farman
81a206f52b security: Include vhost-scsi in security labels
Ensure that the vhost-scsi wwpn information is passed to the
different security policies.

Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
2016-11-24 12:16:26 -05:00
Eric Farman
fc0e627bac Introduce framework for a hostdev SCSI_host subsystem type
We already have a "scsi" hostdev subsys type, which refers to a single
LUN that is passed through to a guest.  But what of things where
multiple LUNs are passed through via a single SCSI HBA, such as with
the vhost-scsi target?  Create a new hostdev subsys type that will
carry this.

Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
2016-11-24 12:15:26 -05:00
John Ferlan
77a12987a4 Introduce virDomainChrSourceDefNew for virDomainChrDefPtr
Change the virDomainChrDef to use a pointer to 'source' and allocate
that pointer during virDomainChrDefNew.

This has tremendous "fallout" in the rest of the code which mainly
has to change source.$field to source->$field.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2016-10-21 14:03:36 -04:00
Ján Tomko
5da23bbedf security: label the slic_table
Add support for the slic_table to the security drivers.
2016-05-25 17:15:21 +02:00
Martin Kletzander
32f3f0835e security: Rename DomainSetDirLabel to DomainSetPathLabel
It already labels abritrary paths, so it's just the naming that was
wrong.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2016-04-18 20:34:30 +02:00
Jiri Denemark
8f0a15727f security: Do not restore labels on device tree binary
A device tree binary file specified by /domain/os/dtb element is a
read-only resource similar to kernel and initrd files. We shouldn't
restore its label when destroying a domain to avoid breaking other
domains configure with the same device tree.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2016-01-15 16:34:37 +01:00
Jiri Denemark
68acc701bd security: Do not restore kernel and initrd labels
Kernel/initrd files are essentially read-only shareable images and thus
should be handled in the same way. We already use the appropriate label
for kernel/initrd files when starting a domain, but when a domain gets
destroyed we would remove the labels which would make other running
domains using the same files very unhappy.

https://bugzilla.redhat.com/show_bug.cgi?id=921135

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2016-01-15 10:55:58 +01:00
Ján Tomko
be33e96533 security_dac: remove extra Security from function names
Many of the functions follow the pattern:
virSecurity.*Security.*Label

Remove the second 'Security' from the names, it should be obvious
that the virSecurity* functions deal with security labels even
without it.
2015-12-15 16:06:08 +01:00
Ján Tomko
63cc969a84 security_dac: check if virSecurityDACGetIds returns negative
Use the customary check '< 0' instead of checking for non-zero.

No functional change.
2015-12-09 10:44:26 +01:00
Ján Tomko
d5aba1a4d9 security: label the evdev for input device passthrough
Add functions for setting and restoring the label of input devices
to DAC and SELinux drivers.

https://bugzilla.redhat.com/show_bug.cgi?id=1231114
2015-11-30 12:57:43 +01:00
Jiri Denemark
c6172260a5 security: Cleanup DAC driver
Fixes several style issues and removes "DEF" (what is it supposed to
mean anyway?) from debug messages.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2015-11-11 17:02:53 +01:00
Michal Privoznik
6222a6fee3 security_dac: Introduce remember/recall APIs
Even though the APIs are not implemented yet, they create a
skeleton that can be filled in later.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Michal Privoznik
ec04c18bc5 security_dac: Limit usage of virSecurityDACSetOwnershipInternal
This function should really be called only when we want to change
ownership of a file (or disk source). Lets switch to calling a
wrapper function which will eventually record the current owner
of the file and call virSecurityDACSetOwnershipInternal
subsequently.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Michal Privoznik
fdf44d5b47 virSecurityDACRestoreSecurityFileLabel: Pass virSecurityDACDataPtr
This is pure code adjustment. The structure is going to be needed
later as it will hold a reference that will be used to talk to
virtlockd. However, so far this is no functional change just code
preparation.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Michal Privoznik
607f34319d virSecurityDACSetOwnership: Pass virSecurityDACDataPtr
This is pure code adjustment. The structure is going to be needed
later as it will hold a reference that will be used to talk to
virtlockd. However, so far this is no functional change just code
preparation.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Michal Privoznik
a0f43d820d virSecurityDACSetOwnershipInternal: Don't chown so often
It's better if we stat() file that we are about to chown() at
first and check if there's something we need to change. Not that
it would make much difference, but for the upcoming patches we
need to be doing stat() anyway. Moreover, if we do things this
way, we can drop @chown_errno variable which will become
redundant.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Michal Privoznik
d37d8f78c0 security_dac: Fix TODO marks
Correctly mark the places where we need to remember and recall
file ownership. We don't want to mislead any potential developer.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-16 16:51:41 +02:00
Martin Kletzander
f4c60dfbf2 security_dac: Add SetDirLabel support
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-08-24 11:53:17 +02:00
Martin Kletzander
7b6953bc22 security_dac: Label non-listening sockets
SELinux security driver already does that, but DAC driver somehow missed
the memo.  Let's fix it so it works the same way.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-08-24 11:53:17 +02:00
Ján Tomko
2764977314 Fix build on mingw
Add missing ATTRIBUTE_UNUSED markers.
2014-12-11 11:13:43 +01:00
Luyao Huang
c7c96647e9 dac: Add a new func to get DAC label of a running process
When using qemuProcessAttach to attach a qemu process,
the DAC label is not filled correctly.

Introduce a new function to get the uid:gid from the system
and fill the label.

This fixes the daemon crash when 'virsh screenshot' is called:
https://bugzilla.redhat.com/show_bug.cgi?id=1161831

It also fixes qemu-attach after the prerequisite of this patch
(commit f8c1fb3) was pushed out of order.

Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2014-12-11 10:29:43 +01:00
Cole Robinson
d3489548b5 security: Fix labelling host devices (bz 1145968)
The check for ISCSI devices was missing a check of subsys type, which
meant we could skip labelling of other host devices as well. This fixes
USB hotplug on F21

https://bugzilla.redhat.com/show_bug.cgi?id=1145968
2014-09-24 17:04:28 -04:00
Michal Privoznik
542899168c qemu: Implement extended loader and nvram
QEMU now supports UEFI with the following command line:

  -drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
  -drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \

where the first line reflects <loader> and the second one <nvram>.
Moreover, these two lines obsolete the -bios argument.

Note that UEFI is unusable without ACPI. This is handled properly now.
Among with this extension, the variable file is expected to be
writable and hence we need security drivers to label it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
2014-09-10 09:38:07 +02:00
John Ferlan
17bddc46f4 hostdev: Introduce virDomainHostdevSubsysSCSIiSCSI
Create the structures and API's to hold and manage the iSCSI host device.
This extends the 'scsi_host' definitions added in commit id '5c811dce'.
A future patch will add the XML parsing, but that code requires some
infrastructure to be in place first in order to handle the differences
between a 'scsi_host' and an 'iSCSI host' device.
2014-07-24 07:04:44 -04:00
John Ferlan
42957661dc hostdev: Introduce virDomainHostdevSubsysSCSIHost
Split virDomainHostdevSubsysSCSI further. In preparation for having
either SCSI or iSCSI data, create a union in virDomainHostdevSubsysSCSI
to contain just a virDomainHostdevSubsysSCSIHost to describe the
'scsi_host' host device
2014-07-24 06:39:28 -04:00
John Ferlan
5805621cd9 hostdev: Introduce virDomainHostdevSubsysSCSI
Create a separate typedef for the hostdev union data describing SCSI
Then adjust the code to use the new pointer
2014-07-24 06:39:27 -04:00
John Ferlan
1c8da0d44e hostdev: Introduce virDomainHostdevSubsysPCI
Create a separate typedef for the hostdev union data describing PCI.
Then adjust the code to use the new pointer
2014-07-24 06:39:27 -04:00
John Ferlan
7540d07f09 hostdev: Introduce virDomainHostdevSubsysUSB
Create a separate typedef for the hostdev union data describing USB.
Then adjust the code to use the new pointer
2014-07-24 06:39:27 -04:00
Peter Krempa
0a515a3ba3 security: DAC: Plumb usage of chown callback
Use the callback to set disk and storage image labels by modifying the
existing functions and adding wrappers to avoid refactoring a lot of the
code.
2014-07-24 09:59:00 +02:00
Peter Krempa
7490a6d272 security: DAC: Introduce callback to perform image chown
To integrate the security driver with the storage driver we need to
pass a callback for a function that will chown storage volumes.

Introduce and document the callback prototype.
2014-07-24 09:58:59 +02:00
Peter Krempa
9f28599d51 security: DAC: Remove superfluous link resolution
When restoring security labels in the dac driver the code would resolve
the file path and use the resolved one to be chown-ed. The setting code
doesn't do that. Remove the unnecessary code.
2014-07-24 09:58:59 +02:00
Michal Privoznik
0b22a16d7e virSecurityDeviceLabelDef: substitute 'norelabel' with 'relabel'
Similarly to the previous commit, boolean variables should not start
with 'no-' prefix.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-07-11 10:36:02 +02:00
Michal Privoznik
13adf1b2ce virSecurityLabelDef: substitute 'norelabel' with 'relabel'
This negation in names of boolean variables is driving me insane. The
code is much more readable if we drop the 'no-' prefix. Well, at least
for me.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-07-11 10:35:58 +02:00
Peter Krempa
7077cfeff4 security: DAC: Implement per-image seclabel set
Refactor the code and reuse it to implement the functionality.
2014-07-09 11:11:43 +02:00
Peter Krempa
4f2170dc24 security: DAC: Implement per-image seclabel restore
Refactor the existing code to allow re-using it for the per-image label
restore too.
2014-07-09 11:10:08 +02:00
Peter Krempa
63834faadb storage: Move readonly and shared flags to disk source from disk def
In the future we might need to track state of individual images. Move
the readonly and shared flags to the virStorageSource struct so that we
can keep them in a per-image basis.
2014-07-08 14:27:19 +02:00
Peter Krempa
23a8646a89 security: Rename virSecurityManagerRestoreImageLabel to *Disk*
I'm going to add functions that will deal with individual image files
rather than whole disks. Rename the security function to make room for
the new one.
2014-06-26 10:18:47 +02:00
Peter Krempa
c8e4220515 security: Rename virSecurityManagerSetImageLabel to *Disk*
I'm going to add functions that will deal with individual image files
rather than whole disks. Rename the security function to make room for
the new one.
2014-06-20 09:27:19 +02:00
Peter Krempa
7c6dae0418 security: Sanitize type of @migrated in virSecurityManagerRestoreAllLabel
Also remove one spurious ATTRIBUTE_UNUSED guarding the @migrated
argument.
2014-06-20 09:27:19 +02:00
Peter Krempa
83c896c859 util: Don't require full disk definition when getting imagelabels
The image labels are stored in the virStorageSource struct. Convert the
virDomainDiskDefGetSecurityLabelDef helper not to use the full disk def
and move it appropriately.
2014-06-20 09:27:15 +02:00
Peter Krempa
cc6484d486 security: Don't skip labelling for network disks
A network disk might actually be backed by local storage. Also the path
iterator actually handles networked disks well now so remove the code
that skips the labelling in dac and selinux security driver.
2014-06-12 10:32:12 +02:00
Julio Faracco
5a2bd4c917 conf: more enum cleanups in "src/conf/domain_conf.h"
In "src/conf/domain_conf.h" there are many enum declarations. The
cleanup in this header filer was started, but it wasn't enough and
there are many other files that has enum variables declared. So, the
commit was starting to be big. This commit finish the cleanup in this
header file and in other files that has enum variables, parameters,
or functions declared.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-06-02 15:32:58 -06:00
Julio Faracco
d4dad16204 conf: enum cleanups in "src/conf/domain_conf.h"
In "src/conf/domain_conf.h" there are many enumerations (enum)
declarations to be converted as a typedef too. As mentioned before,
it's better to use a typedef for variable types, function types and
other usages. I think this file has most of those enum declarations
at "src/conf/". So, me and Eric Blake plan to keep the cleanups all
over the source code. This time, most of the files changed in this
commit are related to part of one file: "src/conf/domain_conf.h".

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
2014-06-02 15:20:22 -06:00
Ján Tomko
88e36a69de Fix crash in DAC driver with no seclabels
With dynamic_ownership = 1 but no seclabels, RestoreChardevLabel
dereferences the NULL seclabel when checking if norelabel is set.

Remove this check, since it is already done in RestoreSecurityAllLabel
and if norelabel is set, RestoreChardevLabel is never called.
2014-05-19 15:31:42 +02:00
Michal Privoznik
64c1e1ead1 virSecurityDACRestoreSecurityHostdevLabel: Unmark @def as unused
The domain definition is clearly used a few lines
below so there's no need to mark @def as unused.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-05-19 11:15:52 +02:00
Jim Fehlig
a0f82fd2bd security_dac: honor relabel='no' in chardev config
The DAC driver ignores the relabel='no' attribute in chardev config

  <serial type='file'>
    <source path='/tmp/jim/test.file'>
      <seclabel model='dac' relabel='no'/>
    </source>
    <target port='0'/>
  </serial>

This patch avoids labeling chardevs when relabel='no' is specified.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
2014-05-16 15:32:15 -06:00
Jim Fehlig
bb917a90b1 security_dac: avoid relabeling hostdevs when relabel='no'
When relabel='no' at the domain level, there is no need to call
the hostdev relabeling functions.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
2014-05-16 15:32:14 -06:00
Jim Fehlig
3c2487ab0a security_dac: honor relabel='no' in disk config
https://bugzilla.redhat.com/show_bug.cgi?id=999301

The DAC driver ignores the relabel='no' attribute in disk config

  <disk type='file' device='floppy'>
    <driver name='qemu' type='raw'/>
    <source file='/some/path/floppy.img'>
      <seclabel model='dac' relabel='no'/>
    </source>
    <target dev='fda' bus='fdc'/>
    <readonly/>
  </disk>

This patch avoid labeling disks when relabel='no' is specified.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
2014-05-16 15:32:14 -06:00
Jim Fehlig
9369a56244 security_dac: avoid relabeling when relabel='no'
If relabel='no' at the domain level, no need to attempt relabeling
in virSecurityDAC{Set,Restore}SecurityAllLabel().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
2014-05-16 15:32:14 -06:00
Jim Fehlig
3de7e4ec5e security_dac: rework callback parameter passing
Currently, the DAC security driver passes callback data as

    void params[2];
    params[0] = mgr;
    params[1] = def;

Clean this up by defining a structure for passing the callback
data.  Moreover, there's no need to pass the whole virDomainDef
in the callback as the only thing needed in the callbacks is
virSecurityLabelDefPtr.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
2014-05-16 15:32:14 -06:00
Jim Fehlig
1d98e713d5 security_dac: cleanup use of enum types
In switch statements, use enum types since it is safer when
adding new items to the enum.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
2014-05-16 15:32:14 -06:00
Jim Fehlig
e1ce6d836e security_dac: annotate some functions with ATTRIBUTE_NONNULL
Annotate some static function parameters with ATTRIBUTE_NONNULL
and remove checks for NULL inputs.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
2014-05-16 15:32:14 -06:00
Jim Fehlig
5099084eb3 security_dac: Fix indentation 2014-05-14 11:43:00 -06:00
Jim Fehlig
d77fa847c3 security_dac: Remove unnecessary curly braces 2014-05-14 11:43:00 -06:00
Jim Fehlig
fcf5ea7fc0 security_dac: Remove unnecessary ATTRIBUTE_UNUSED
Remove ATTRIBUTE_UNUSED on several function parameters that are
actually used.
2014-05-14 11:43:00 -06:00
Eric Blake
5160ab795a util: move detection of shared filesystems
The code in virstoragefile.c is getting more complex as I
consolidate backing chain handling code.  But for the setuid
virt-login-shell, we don't need to crawl backing chains.  It's
easier to audit things for setuid security if there are fewer
files involved, so this patch moves the one function that
virFileOpen() was actually relying on to also live in virfile.c.

* src/util/virstoragefile.c (virStorageFileIsSharedFS)
(virStorageFileIsSharedFSType): Move...
* src/util/virfile.c (virFileIsSharedFS, virFileIsSharedFSType):
...to here, and rename.
(virFileOpenAs): Update caller.
* src/security/security_selinux.c
(virSecuritySELinuxSetFileconHelper)
(virSecuritySELinuxSetSecurityAllLabel)
(virSecuritySELinuxRestoreSecurityImageLabelInt): Likewise.
* src/security/security_dac.c
(virSecurityDACRestoreSecurityImageLabelInt): Likewise.
* src/qemu/qemu_driver.c (qemuOpenFileAs): Likewise.
* src/qemu/qemu_migration.c (qemuMigrationIsSafe): Likewise.
* src/util/virstoragefile.h: Adjust declarations.
* src/util/virfile.h: Likewise.
* src/libvirt_private.syms (virfile.h, virstoragefile.h): Move
symbols as appropriate.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-04-01 10:38:14 -06:00
Eric Blake
16ac4c9d64 conf: move host disk type to util/
A continuation of the migration of disk details to virstoragefile.
This patch moves a single enum, but converting the name has quite
a bit of fallout.

* src/conf/domain_conf.h (virDomainDiskType): Move...
* src/util/virstoragefile.h (virStorageType): ...and rename.
* src/bhyve/bhyve_command.c (bhyveBuildDiskArgStr)
(virBhyveProcessBuildLoadCmd): Update clients.
* src/conf/domain_conf.c (virDomainDiskSourceDefParse)
(virDomainDiskDefParseXML, virDomainDiskSourceDefFormatInternal)
(virDomainDiskDefFormat, virDomainDiskGetActualType)
(virDomainDiskDefForeachPath, virDomainDiskSourceIsBlockType):
Likewise.
* src/conf/snapshot_conf.h (_virDomainSnapshotDiskDef): Likewise.
* src/conf/snapshot_conf.c (virDomainSnapshotDiskDefParseXML)
(virDomainSnapshotAlignDisks, virDomainSnapshotDiskDefFormat):
Likewise.
* src/esx/esx_driver.c (esxAutodetectSCSIControllerModel)
(esxDomainDefineXML): Likewise.
* src/locking/domain_lock.c (virDomainLockManagerAddDisk):
Likewise.
* src/lxc/lxc_controller.c
(virLXCControllerSetupLoopDeviceDisk)
(virLXCControllerSetupNBDDeviceDisk)
(virLXCControllerSetupLoopDevices, virLXCControllerSetupDisk):
Likewise.
* src/parallels/parallels_driver.c (parallelsGetHddInfo):
Likewise.
* src/phyp/phyp_driver.c (phypDiskType): Likewise.
* src/qemu/qemu_command.c (qemuGetDriveSourceString)
(qemuDomainDiskGetSourceString, qemuBuildDriveStr)
(qemuBuildCommandLine, qemuParseCommandLineDisk)
(qemuParseCommandLine): Likewise.
* src/qemu/qemu_conf.c (qemuCheckSharedDevice)
(qemuTranslateDiskSourcePool)
(qemuTranslateSnapshotDiskSourcePool): Likewise.
* src/qemu/qemu_domain.c (qemuDomainDeviceDefPostParse)
(qemuDomainDetermineDiskChain): Likewise.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo)
(qemuDomainSnapshotPrepareDiskExternalBackingInactive)
(qemuDomainSnapshotPrepareDiskExternalBackingActive)
(qemuDomainSnapshotPrepareDiskExternalOverlayActive)
(qemuDomainSnapshotPrepareDiskExternalOverlayInactive)
(qemuDomainSnapshotPrepareDiskInternal)
(qemuDomainSnapshotPrepare)
(qemuDomainSnapshotCreateSingleDiskActive): Likewise.
* src/qemu/qemu_hotplug.c (qemuDomainChangeEjectableMedia):
Likewise.
* src/qemu/qemu_migration.c (qemuMigrationIsSafe): Likewise.
* src/security/security_apparmor.c
(AppArmorRestoreSecurityImageLabel)
(AppArmorSetSecurityImageLabel): Likewise.
* src/security/security_dac.c (virSecurityDACSetSecurityImageLabel)
(virSecurityDACRestoreSecurityImageLabelInt)
(virSecurityDACSetSecurityAllLabel): Likewise.
* src/security/security_selinux.c
(virSecuritySELinuxRestoreSecurityImageLabelInt)
(virSecuritySELinuxSetSecurityImageLabel)
(virSecuritySELinuxSetSecurityAllLabel): Likewise.
* src/storage/storage_backend.c (virStorageFileBackendForType):
Likewise.
* src/storage/storage_backend_fs.c (virStorageFileBackendFile)
(virStorageFileBackendBlock): Likewise.
* src/storage/storage_backend_gluster.c
(virStorageFileBackendGluster): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc, vboxAttachDrives)
(vboxDomainAttachDeviceImpl, vboxDomainDetachDevice): Likewise.
* src/vmware/vmware_conf.c (vmwareVmxPath): Likewise.
* src/vmx/vmx.c (virVMXParseDisk, virVMXFormatDisk)
(virVMXFormatFloppy): Likewise.
* src/xenxs/xen_sxpr.c (xenParseSxprDisks, xenParseSxpr)
(xenFormatSxprDisk): Likewise.
* src/xenxs/xen_xm.c (xenParseXM, xenFormatXMDisk): Likewise.
* tests/securityselinuxlabeltest.c (testSELinuxLoadDef):
Likewise.
* src/libvirt_private.syms (domain_conf.h): Move symbols...
(virstoragefile.h): ...as appropriate.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-04-01 10:38:13 -06:00
Ján Tomko
9e7ecabf94 Indent top-level labels by one space in the rest of src/ 2014-03-25 14:58:40 +01:00
Eric Blake
2aad0ebe2d conf: use disk source accessors in security/
Part of a series of cleanups to use new accessor methods.

* src/security/security_dac.c (virSecurityDACSetSecurityImageLabel)
(virSecurityDACRestoreSecurityImageLabelInt)
(virSecurityDACSetSecurityAllLabel): Use accessors.
* src/security/security_selinux.c
(virSecuritySELinuxRestoreSecurityImageLabelInt)
(virSecuritySELinuxSetSecurityImageLabel)
(virSecuritySELinuxSetSecurityAllLabel): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-03-24 11:59:50 -06:00
Daniel P. Berrange
2835c1e730 Add virLogSource variables to all source files
Any source file which calls the logging APIs now needs
to have a VIR_LOG_INIT("source.name") declaration at
the start of the file. This provides a static variable
of the virLogSource type.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2014-03-18 14:29:22 +00:00
Michal Privoznik
f5796b61cc virSecurityDACSetSecurityImageLabel: Unmark @def as unused
The @def is clearly used just a few lines below. There's no need to use
ATTRIBUTE_UNUSED for it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-03-11 11:18:06 +01:00
Osier Yang
10c9ceff6d util: Add one argument for several scsi utils
To support passing the path of the test data to the utils, one
more argument is added to virSCSIDeviceGetSgName,
virSCSIDeviceGetDevName, and virSCSIDeviceNew, and the related
code is changed accordingly.

Later tests for the scsi utils will be based on this patch.

Signed-off-by: Osier Yang <jyang@redhat.com>
2014-01-30 15:48:28 +08:00
Osier Yang
2b66504ded util: Add "shareable" field for virSCSIDevice struct
Unlike the host devices of other types, SCSI host device XML supports
"shareable" tag. This patch introduces it for the virSCSIDevice struct
for a later patch use (to detect if the SCSI device is shareable when
preparing the SCSI host device in QEMU driver).
2014-01-23 17:52:33 +08:00
John Ferlan
aa42bb1700 Add '+' to uid/gid printing for label processing
To ensure proper processing by virGetUserID() and virGetGroupID()
of a uid/gid add a "+" prior to the uid/gid to denote it's really
a uid/gid for the label.
2013-10-29 17:10:15 -04:00
Giuseppe Scrivano
64a68a4a09 security: add new internal function "virSecurityManagerGetBaseLabel"
virSecurityManagerGetBaseLabel queries the default settings used by
a security model.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2013-10-29 06:57:07 -06:00
Giuseppe Scrivano
4387132f31 security: use a single function to set DAC user and group
Merge the functions 'virSecurityDACSetUser' and
'virSecurityDACSetGroup' into 'virSecurityDACSetUserAndGroup'.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2013-10-29 06:55:09 -06:00
Eric Blake
745aa55fbf security: provide supplemental groups even when parsing label (CVE-2013-4291)
Commit 29fe5d7 (released in 1.1.1) introduced a latent problem
for any caller of virSecurityManagerSetProcessLabel and where
the domain already had a uid:gid label to be parsed.  Such a
setup would collect the list of supplementary groups during
virSecurityManagerPreFork, but then ignores that information,
and thus fails to call setgroups() to adjust the supplementary
groups of the process.

Upstream does not use virSecurityManagerSetProcessLabel for
qemu (it uses virSecurityManagerSetChildProcessLabel instead),
so this problem remained latent until backporting the initial
commit into v0.10.2-maint (commit c061ff5, released in 0.10.2.7),
where virSecurityManagerSetChildProcessLabel has not been
backported.  As a result of using a different code path in the
backport, attempts to start a qemu domain that runs as qemu:qemu
will end up with supplementary groups unchanged from the libvirtd
parent process, rather than the desired supplementary groups of
the qemu user.  This can lead to failure to start a domain
(typical Fedora setup assigns user 107 'qemu' to both group 107
'qemu' and group 36 'kvm', so a disk image that is only readable
under kvm group rights is locked out).  Worse, it is a security
hole (the qemu process will inherit supplemental group rights
from the parent libvirtd process, which means it has access
rights to files owned by group 0 even when such files should
not normally be visible to user qemu).

LXC does not use the DAC security driver, so it is not vulnerable
at this time.  Still, it is better to plug the latent hole on
the master branch first, before cherry-picking it to the only
vulnerable branch v0.10.2-maint.

* src/security/security_dac.c (virSecurityDACGetIds): Always populate
groups and ngroups, rather than only when no label is parsed.

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-08-29 08:43:03 -06:00
Martin Kletzander
bba579b6e0 Expose ownership ID parsing
Parsing 'user:group' is useful even outside the DAC security driver,
so expose the most abstract function which has no DAC security driver
bits in itself.
2013-07-24 14:29:11 +02:00