Commit Graph

15051 Commits

Author SHA1 Message Date
Daniel P. Berrange
9cd6a57db6 Block all use of libvirt.so in setuid programs
Avoid people introducing security flaws in their apps by
forbidding the use of libvirt.so in setuid programs, with
a check in virInitialize.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
9b0af09240 Remove (nearly) all use of getuid()/getgid()
Most of the usage of getuid()/getgid() is in cases where we are
considering what privileges we have. As such the code should be
using the effective IDs, not real IDs.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
c566fa1ad0 Add stub getegid impl for platforms lacking it
We already have stubs for getuid, geteuid, getgid but
not for getegid. Something in gnulib already does a
check for it during configure, so we already have the
HAVE_GETEGID macro defined.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
171bb12911 Don't allow remote driver daemon autostart when running setuid
We don't want setuid programs automatically spawning libvirtd,
so disable any use of autostart when setuid.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
e22b0232c7 Only allow the UNIX transport in remote driver when setuid
We don't know enough about quality of external libraries used
for non-UNIX transports, nor do we want to spawn external
commands when setuid. Restrict to the bare minimum which is
UNIX transport for local usage. Users shouldn't need to be
running setuid if connecting to remote hypervisors in any
case.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
71b21f12be Block all use of getenv with syntax-check
The use of getenv is typically insecure, and we want people
to use our wrappers, to force them to think about setuid
needs.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
1e4a02bdfe Remove all direct use of getenv
Unconditional use of getenv is not secure in setuid env.
While not all libvirt code runs in a setuid env (since
much of it only exists inside libvirtd) this is not always
clear to developers. So make all the code paranoid, even
if it only ever runs inside libvirtd.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
9b8f307c6a Make virCommand env handling robust in setuid env
When running setuid, we must be careful about what env vars
we allow commands to inherit from us. Replace the
virCommandAddEnvPass function with two new ones which do
filtering

  virCommandAddEnvPassAllowSUID
  virCommandAddEnvPassBlockSUID

And make virCommandAddEnvPassCommon use the appropriate
ones

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
d665003da1 Set a sane $PATH for virt-login-shell
The virt-login-shell binary shouldn't need to execute programs
relying on $PATH, but just in case set a fixed $PATH value
of /bin:/usr/bin

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
3e2f27e13b Don't link virt-login-shell against libvirt.so (CVE-2013-4400)
The libvirt.so library has far too many library deps to allow
linking against it from setuid programs. Those libraries can
do stuff in __attribute__((constructor) functions which is
not setuid safe.

The virt-login-shell needs to link directly against individual
files that it uses, with all library deps turned off except
for libxml2 and libselinux.

Create a libvirt-setuid-rpc-client.la library which is linked
to by virt-login-shell. A config-post.h file allows this library
to disable all external deps except libselinux and libxml2.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
b7fcc799ad Close all non-stdio FDs in virt-login-shell (CVE-2013-4400)
We don't want to inherit any FDs in the new namespace
except for the stdio FDs. Explicitly close them all,
just in case some do not have the close-on-exec flag
set.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
8c3586ea75 Only allow 'stderr' log output when running setuid (CVE-2013-4400)
We must not allow file/syslog/journald log outputs when running
setuid since they can be abused to do bad things. In particular
the 'file' output can be used to overwrite files.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
ae53e5d10e Add helpers for getting env vars in a setuid environment
Care must be taken accessing env variables when running
setuid. Introduce a virGetEnvAllowSUID for env vars which
are safe to use in a setuid environment, and another
virGetEnvBlockSUID for vars which are not safe. Also add
a virIsSUID helper method for any other non-env var code
to use.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
57687fd6bf Fix perms for virConnectDomainXML{To,From}Native (CVE-2013-4401)
The virConnectDomainXMLToNative API should require 'connect:write'
not 'connect:read', since it will trigger execution of the QEMU
binaries listed in the XML.

Also make virConnectDomainXMLFromNative API require a full
read-write connection and 'connect:write' permission. Although the
current impl doesn't trigger execution of QEMU, we should not
rely on that impl detail from an API permissioning POV.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 13:58:40 +01:00
Chen Hanxiao
8d35f9dbad docs: fix a typo in formatdomain
s/effect/affect

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
2013-10-21 10:25:29 +02:00
Geoff Hickey
2de2458efa Fix a problem introduced by commit 99889012
The meaning of one line of code was accidentally inverted.

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-10-18 12:44:05 -06:00
Michal Privoznik
d9be5a7157 qemu: Fix augeas support for migration ports
Commit e3ef20d7 allows user to configure migration ports range via
qemu.conf. However, it forgot to update augeas definition file and
even the test data was malicious.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2013-10-18 18:30:13 +02:00
Jiri Denemark
34adf622a3 docs: Expand description of host-model CPU mode
host-model is a nice idea but it's current implementation make it
useless on some hosts so it should be used with care.
2013-10-18 16:35:57 +02:00
Jiri Denemark
e3ef20d7f7 qemu: Make migration port range configurable
https://bugzilla.redhat.com/show_bug.cgi?id=1019053
2013-10-18 16:35:38 +02:00
Wang Yufei
0196845d3a qemu: Avoid assigning unavailable migration ports
https://bugzilla.redhat.com/show_bug.cgi?id=1019053

When we migrate vms concurrently, there's a chance that libvirtd on
destination assigns the same port for different migrations, which will
lead to migration failure during prepare phase on destination. So we use
virPortAllocator here to solve the problem.

Signed-off-by: Wang Yufei <james.wangyufei@huawei.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2013-10-18 16:34:09 +02:00
Michal Privoznik
9b3c8bd4f8 viralloc.h: Fix typo in VIR_APPEND_ELEMENT_COPY_QUIT
In fact, the suffix should be _QUIET not _QUIT to stress the
fact, that no OOM error is reported on error.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2013-10-18 15:50:31 +02:00
Christophe Fergeau
68eb3709a1 netcf: Don't complain when cleanup is called before init
netcfStateInitialize() initializes the driverState variable,
and when netcfStateCleanup is called, it will call virReportError()
if driverState is NULL.
This is not consistent with what other state objects are doing,
they return -1 without reporting an error in such cases.

See also
https://www.redhat.com/archives/libvir-list/2013-October/msg00809.html:

On Thu, Oct 17, 2013 at 01:40:19PM +0100, Daniel P. Berrange wrote:
> We don't want virStateCleanup to skip execution if virStateInitialize
> has failed though - every callback in virStateCleanup should be written
> to be safe if its corresponding init function hasn't run.
2013-10-18 14:31:51 +02:00
Zhou Yimin
9712c2510e remote: fix regression in event deregistration
Introduced by 7b87a3
When I quit the process which only register VIR_DOMAIN_EVENT_ID_REBOOT,
I got error like:
"libvirt: XML-RPC error : internal error: domain event 0 not registered".
Then I add the following code, it fixed.

Signed-off-by: Zhou Yimin <zhouyimin@huawei.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2013-10-18 06:21:29 -06:00
Chen Hanxiao
8ebd3d8892 daemon: don't free domain if it's null
If we fail to get domain, we had to judge whether
it's null or not when doing 'cleanup'.

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
2013-10-18 07:41:34 +02:00
John Ferlan
0cacffac64 Remove ATTRIBUTE_NONNULL(3) from qemuMonitorJSONDrivePivot
The header definition didn't match the function declaration, so adjusted
header to reflect the definition.

Found during a Coverity build where STATIC_ANALYSIS is enabled resulting
in the internal.h adding __nonnull__ handling to arguments.

Commit '6d264c91' added support for the qemuMonitorJSONDrivePivot() and
commit 'fbc3adc9' added a corresponding test which ended up triggering
the build failure which I didn't notice until today!
2013-10-17 19:36:42 -04:00
Chen Hanxiao
21f2d80b0c virsh: improve usability of '--print-xml' flag for attach-disk command
'--print-xml' option is very useful for doing some test.
But we had to specify a real domain for it.
This patch could enable us to specify a fake domain
when using --print-xml option.

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2013-10-17 15:43:57 -06:00
Geoff Hickey
7ab40c5d09 esx: Remove unnecessary NULL comparisons (3/3)
Code cleanup: remove explicit NULL comparisons like ptr == NULL and
ptr != NULL from the ESX code, replacing them with the simpler ptr
and !ptr.

Part three of three.
2013-10-17 11:27:08 -06:00
Geoff Hickey
42c20d4bef esx: Remove unnecessary NULL comparisons (2/3)
Code cleanup: remove explicit NULL comparisons like ptr == NULL and
ptr != NULL from the ESX code, replacing them with the simpler ptr
and !ptr.

Part two of three.
2013-10-17 11:27:08 -06:00
Geoff Hickey
9988901267 esx: Remove unnecessary NULL comparisons (1/3)
Code cleanup: remove explicit NULL comparisons like ptr == NULL and
ptr != NULL from the ESX code, replacing them with the simpler ptr
and !ptr.

Part one of three.
2013-10-17 11:27:08 -06:00
Daniel P. Berrange
291a6ef3e4 Add support for enabling SASL for SPICE guests
QEMU has support for SASL auth for SPICE guests, but libvirt
has no way to enable it. Following the example from VNC where
it is globally enabled via qemu.conf

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-17 16:02:43 +01:00
Michal Privoznik
ac5f3f292b qemuDomainCleanupRemove: s/memmove/VIR_DELETE_ELEMENT_INPLACE/
The last argument of memmove is the amount of bytes to be moved. The
amount is in Bytes. We are moving some void pointers around. However,
since sizeof(void *) is not Byte on any architecture, we've got the
arithmetic wrong.
2013-10-17 15:24:05 +02:00
Brian Candler
aa0f09929d better error checking for LOCAL_PEERCRED
This patch improves the error checking in the LOCAL_PEERCRED version
of virNetSocketGetUNIXIdentity, used by FreeBSD and Mac OSX.

1. The error return paths now correctly unlock the socket. This is
implemented in exactly the same way as the SO_PEERCRED version,
using "goto cleanup"

2. cr.cr_ngroups is initialised to -1, and cr.cr_ngroups is checked
for negative and overlarge values.

This means that if the getsockopt() call returns success but doesn't
actually update the xucred structure, this is now caught. This
happened previously when getsockopt was called with SOL_SOCKET
instead of SOL_LOCAL, prior to commit 5a468b3, and resulted in
random uids being accepted.

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-10-17 06:24:49 -06:00
Chen Hanxiao
55da09933f virsh: fix a typo in virsh-domain
s/it's/its

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
2013-10-17 09:08:25 +02:00
Giuseppe Scrivano
70dadfa7e7 build: use the gnulib version of the .m4 files when present
prevent aclocal from preferring .m4 files under m4/ over the version
provided by gnulib, by using only one directory.

I have noticed this after './configure --help' gave me two different
versions of "--enable-threads".  This was caused by aclocal that
preferred the version of lock.m4 provided by autopoint instead of
using the newer version distributed with gnulib.

Having two different directories made sense back when we checked
gnulib files into libvirt.git, but that was ages ago.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2013-10-16 11:07:24 -06:00
Eric Blake
caf516db51 storage: allow interleave in pool XML
The RNG grammar did not allow arbitrary interleaving, which makes
it harder than necessary to create a new pool from handwritten XML.

* docs/schemas/storagepool.rng: Allow interleaving.
* tests/storagepoolxml2xmlin/pool-sheepdog.xml: Test interleave.
* tests/storagepoolxml2xmlin/pool-iscsi-auth.xml: Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-10-16 10:15:44 -06:00
Eric Blake
59dce8d278 storage: document existing pools
We forgot to document several pool types.

* docs/formatstorage.html.in: Add docs for scsi, mpath, rbd, and
sheepdog.

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-10-16 09:57:31 -06:00
Christophe Fergeau
6340c7dda0 remote-driver: Fix 'leav' typo in comment 2013-10-16 17:27:19 +02:00
Osier Yang
0959785d3b rpc: Correct the wrong payload size checking
<...>
/* Size of message length field. Not counted in VIR_NET_MESSAGE_MAX
 * and VIR_NET_MESSAGE_INITIAL.
 */
const VIR_NET_MESSAGE_LEN_MAX = 4;
</...>

However, msg->bufferLength includes the length word. The wrong checking
was introduced by commit e914dcfd.

* src/rpc/virnetmessage.c:
  - Correct the checking in virNetMessageEncodePayloadRaw
  - Use a new variable to track the new payload length in
    virNetMessageEncodePayloadRaw
2013-10-16 20:36:46 +08:00
Daniel P. Berrange
8757d0abbc Add support for detecting PPC little endian arches
The recent patch series proposing the addition of PPC little endian
arch support to Linux defines new arch names 'ppcle' and 'ppc64le':

https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-August/109908.html

This just makes libvirt know about these arch names, so it doesn't
immediately trip up if it seems these new names from uname.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-16 12:40:35 +01:00
Daniel P. Berrange
0894ce863f Fix typo breaking cgroups for NBD backed filesystems
A typo in the setup of NBD backed filesystems meant the
/dev/nbdN device would not be added to the cgroups device
ACL.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-16 12:22:40 +01:00
Daniel P. Berrange
8f132ef1b1 Add some logging to LXC disk/fs nbd/loop setup
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-16 12:22:40 +01:00
Daniel P. Berrange
1d8afffecd Add logging to LXC cgroup devices setup
To facilitate debugging, add some more logging to LXC cgroup
devices ACL setup.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-16 12:22:40 +01:00
Daniel P. Berrange
64d4eb6a50 Add log statement when NBD device is setup
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-16 12:22:40 +01:00
Pranavkumar Sawargaonkar
9f53ffcb81 AArch64: Add qemu capabilities schemeta for test.
Add qemu AArch64 capabilities schemeta in caps-qemu-kvm.xml.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>

(crobinso: add aarch64 to schema arch list)
2013-10-15 17:02:17 -04:00
Pranavkumar Sawargaonkar
2f064f35d5 Implement minimal sysinfo for AArch64 platforms.
Implement the bare minimal sysinfo for AArch64 platforms by
reading the CPU models from /proc/cpuinfo.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
2013-10-15 16:59:43 -04:00
Pranavkumar Sawargaonkar
950127620d AArch64: Parse cputopology from /proc/cpuinfo.
CPU "parser" for AArch64.
Showing cputopology in arm64 linux is work-in-progress so for now
all AArch64 cpus belong to same socket (like PPC).

Also we parse BogoMIPS same like arm 32bit.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
2013-10-15 16:59:43 -04:00
Pranavkumar Sawargaonkar
5cb6816715 AArch64: CPU Support for AArch64 (ARMv8 64bit).
Adding CPU encoder/decoder for AArch64.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>

(crobinso: fix for recent libvirt Ptr refactoring)
2013-10-15 16:59:26 -04:00
Pranavkumar Sawargaonkar
670c08afd4 AArch64: Add AArch64 architecture to list of valid arches.
Adding AArch64(ARMv8 64bit) to the current list of valid architectures.

For now, AArch64 name would imply AArch64 LE mode only. In future,
we might have separate names for AArch64 LE and BE.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
2013-10-15 16:55:11 -04:00
Dusty Mabe
4132dede06 Ignore thin pool LVM devices.
This should resolve:

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

For BZ 924672 the problem stems from the fact that thin pool logical
volume devices show up in /sbin/lvs output just like normal logical
volumes do. Libvirt incorrectly assumes they are just normal logical
volumes and that they will have a corresponding /dev/vgname/lvname
device that has been created by udev and tries to use this device.

To illustrate here is an example of the /dev/vgname/ directory and
the lvs output for a normal lv, thin lv, and thin pool:

    LV     VG       Attr      LSize  Pool Origin Data%  Move Log Copy%  Convert
    lv     vgguests -wi-a----  1.00g
    pool   vgguests twi-a-tz- 11.00g               0.00
    thinlv vgguests Vwi-a-tz-  1.00g pool          0.00
total 0
lrwxrwxrwx. 1 root root 7 Oct  8 19:35 lv -> ../dm-7
lrwxrwxrwx. 1 root root 7 Oct  8 19:37 thinlv -> ../dm-6

This patch modifies virStorageBackendLogicalMakeVol() to ignore thin pool
devices.
2013-10-15 16:52:57 -04:00
Michal Privoznik
da716da3e4 domain_conf.c: Initialize arrVar and cntVar
Some ancient gcc fails to see the variables are initialized in a
separate function and a false positive is produced:

cc1: warnings being treated as errors
conf/domain_conf.c: In function 'virDomainChrGetDomainPtrs':
conf/domain_conf.c:10342: error: 'arrVar' may be used uninitialized in this function [-Wuninitialized]
conf/domain_conf.c:10343: error: 'cntVar' may be used uninitialized in this function [-Wuninitialized]
conf/domain_conf.c: In function 'virDomainChrInsert':
conf/domain_conf.c:10362: error: 'arrPtr' may be used uninitialized in this function [-Wuninitialized]
conf/domain_conf.c:10363: error: 'cntPtr' may be used uninitialized in this function [-Wuninitialized]
conf/domain_conf.c: In function 'virDomainChrRemove':
conf/domain_conf.c:10374: error: 'arrPtr' may be used uninitialized in this function [-Wuninitialized]
conf/domain_conf.c:10375: error: 'cntPtr' may be used uninitialized in this function [-Wuninitialized]

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2013-10-15 19:34:10 +02:00