[libvirt] [SECURITY] PATCH: Fix missing read-only access checks (CVE-2008-5086)
by Daniel P. Berrange
The following methods in libvirt.c are missing a check against the
read-only connection flag:
virDomainMigrate
virDomainMigratePrepare
virDomainMigratePerform
virDomainMigrateFinish
virDomainMigratePrepare2
virDomainMigrateFinish2
virDomainBlockPeek
virDomainMemoryPeek
virDomainSetAutostart
virNetworkSetAutostart
virConnectFindStoragePoolSources
virStoragePoolSetAutostart
If using PolicyKit auth, the default policy will allow any local user
to make a read-only connection to the libvirtd daemon without needing
authentication.
If not using PolicyKit, the default libvirtd.conf configuration settings
will allow an unprivileged user to make a read-only connection to the
libvirtd daemon without needing authentication.
Thus out of the box unprivileged local users may be able to migrate VMs,
set or unset the autostart flag for domains, networks & storage pools,
and access privileged data in the VM memory, or disks.
All TCP remote connections are read-write, and default settings require
full authentication, thus remote access is not impacted by this flaw.
Administrators can apply a workaround by editting /etc/libvirt/libvirtd.conf
to explicitly set 'unix_sock_ro_perms' parameter to '0700'. Restart the
libvirtd daemon after making this change.
The first vulnerable release was 0.3.2, where the virDomainMigrate API
was added for the Xen driver. Other APIs were added in various subsequent
releases depending on the hypervisor driver in question.
The attached patch has been committed to CVS, and OS distributors are
recommended to apply this patch to all existing releases shipped. It
was diff'd against current CVS head, and applies against 0.5.1, and
is trivially re-diffable for all earlier releases.
This flaw has been assigned the identifier CVE-2008-5086
Daniel
diff --git a/src/libvirt.c b/src/libvirt.c
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2296,6 +2296,16 @@ virDomainMigrate (virDomainPtr domain,
conn = domain->conn; /* Source connection. */
if (!VIR_IS_CONNECT (dconn)) {
virLibConnError (conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return NULL;
+ }
+
+ if (domain->conn->flags & VIR_CONNECT_RO) {
+ virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return NULL;
+ }
+ if (dconn->flags & VIR_CONNECT_RO) {
+ /* NB, delibrately report error against source object, not dest here */
+ virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
return NULL;
}
@@ -2426,6 +2436,11 @@ virDomainMigratePrepare (virConnectPtr d
return -1;
}
+ if (dconn->flags & VIR_CONNECT_RO) {
+ virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return -1;
+ }
+
if (dconn->driver->domainMigratePrepare)
return dconn->driver->domainMigratePrepare (dconn, cookie, cookielen,
uri_in, uri_out,
@@ -2457,6 +2472,11 @@ virDomainMigratePerform (virDomainPtr do
}
conn = domain->conn;
+ if (domain->conn->flags & VIR_CONNECT_RO) {
+ virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return -1;
+ }
+
if (conn->driver->domainMigratePerform)
return conn->driver->domainMigratePerform (domain, cookie, cookielen,
uri,
@@ -2482,6 +2502,11 @@ virDomainMigrateFinish (virConnectPtr dc
if (!VIR_IS_CONNECT (dconn)) {
virLibConnError (NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return NULL;
+ }
+
+ if (dconn->flags & VIR_CONNECT_RO) {
+ virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
return NULL;
}
@@ -2517,6 +2542,11 @@ virDomainMigratePrepare2 (virConnectPtr
return -1;
}
+ if (dconn->flags & VIR_CONNECT_RO) {
+ virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return -1;
+ }
+
if (dconn->driver->domainMigratePrepare2)
return dconn->driver->domainMigratePrepare2 (dconn, cookie, cookielen,
uri_in, uri_out,
@@ -2547,6 +2577,11 @@ virDomainMigrateFinish2 (virConnectPtr d
return NULL;
}
+ if (dconn->flags & VIR_CONNECT_RO) {
+ virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return NULL;
+ }
+
if (dconn->driver->domainMigrateFinish2)
return dconn->driver->domainMigrateFinish2 (dconn, dname,
cookie, cookielen,
@@ -2905,6 +2940,11 @@ virDomainBlockPeek (virDomainPtr dom,
}
conn = dom->conn;
+ if (dom->conn->flags & VIR_CONNECT_RO) {
+ virLibDomainError(dom, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (-1);
+ }
+
if (!path) {
virLibDomainError (dom, VIR_ERR_INVALID_ARG,
_("path is NULL"));
@@ -2980,6 +3020,11 @@ virDomainMemoryPeek (virDomainPtr dom,
}
conn = dom->conn;
+ if (dom->conn->flags & VIR_CONNECT_RO) {
+ virLibDomainError(dom, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (-1);
+ }
+
/* Flags must be VIR_MEMORY_VIRTUAL at the moment.
*
* Note on access to physical memory: A VIR_MEMORY_PHYSICAL flag is
@@ -3246,6 +3291,11 @@ virDomainSetAutostart(virDomainPtr domai
}
conn = domain->conn;
+
+ if (domain->conn->flags & VIR_CONNECT_RO) {
+ virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (-1);
+ }
if (conn->driver->domainSetAutostart)
return conn->driver->domainSetAutostart (domain, autostart);
@@ -4197,6 +4247,11 @@ virNetworkSetAutostart(virNetworkPtr net
return (-1);
}
+ if (network->conn->flags & VIR_CONNECT_RO) {
+ virLibNetworkError(network, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (-1);
+ }
+
conn = network->conn;
if (conn->networkDriver && conn->networkDriver->networkSetAutostart)
@@ -4395,6 +4450,11 @@ virConnectFindStoragePoolSources(virConn
return NULL;
}
+ if (conn->flags & VIR_CONNECT_RO) {
+ virLibConnError(conn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return NULL;
+ }
+
if (conn->storageDriver && conn->storageDriver->findPoolSources)
return conn->storageDriver->findPoolSources(conn, type, srcSpec, flags);
@@ -5068,6 +5128,11 @@ virStoragePoolSetAutostart(virStoragePoo
return (-1);
}
+ if (pool->conn->flags & VIR_CONNECT_RO) {
+ virLibStoragePoolError(pool, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (-1);
+ }
+
conn = pool->conn;
if (conn->storageDriver && conn->storageDriver->poolSetAutostart)
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
15 years, 11 months
[libvirt] libvirt and 192.168.122.0/24
by H. Peter Anvin
I'm kind of wondering why libvirt defaults to 192.168.122.0/24 by
default. It is a piece of address space which is relatively likely to
conflict with address space used in the environment surrounding the
machine. Since libvirtd is on by default if installed, this is
particularly problematic.
I would like to suggest either one of the following as default address
space, unless the user has explicitly configured otherwise:
192.0.2.0/24 - reserved as "test and example network"
198.18.0.0/15 - reserved as "benchmark test network"
See RFC 3330 and RFC 2544 for the definitions of these networks. Both
of them are "should not appear on the public Internet" address blocks,
and much less likely than the RFC 1918 address block (10.0.0.0/8,
172.16.0.0/12, and 192.168.0.0/16) to be encountered "in the wild" by a
user.
-hpa
15 years, 11 months
[libvirt] [PATCH] [OpenVZ] Segmentation fault
by Anton Protopopov
2008/12/13 Ivan Vovk <ivovk(a)intermedia.net>
> Hello,
>
>
>
> after updating to the last official release 0.5.1 my application stopped
> working. No errors to the log file though i raised exceptions where it was
> possible.
>
> i checked with virsh xml description used for creating OpenVZ container and
> got the following:
>
>
>
> virsh # create ovz.xml
> Segmentation fault
>
> Attached patch, fixes that.
15 years, 11 months
[libvirt] [PATCH] Portability fixes for directory storage backend
by john.levon＠sun.com
# HG changeset patch
# User john.levon(a)sun.com
# Date 1229399267 28800
# Node ID 53a18080ea210168c044d7ade8dd8db0881d5c85
# Parent cd2fb266824178c4d63296b587eeedb2360f9f1c
Portability fixes for directory storage backend
Signed-off-by: John Levon <john.levon(a)sun.com>
diff --git a/configure.in b/configure.in
--- a/configure.in
+++ b/configure.in
@@ -75,7 +75,7 @@ AC_CHECK_FUNCS([cfmakeraw regexec uname
AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid])
dnl Availability of various common headers (non-fatal if missing).
-AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h])
+AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h endian.h])
dnl Where are the XDR functions?
dnl If portablexdr is installed, prefer that.
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -31,10 +31,14 @@
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>
+#include <string.h>
+
+#ifdef HAVE_ENDIAN_H
#include <endian.h>
-#include <byteswap.h>
-#include <mntent.h>
-#include <string.h>
+#else
+#define __LITTLE_ENDIAN 1234
+#define __BIG_ENDIAN 4321
+#endif
#include <libxml/parser.h>
#include <libxml/tree.h>
@@ -240,6 +244,9 @@ static int virStorageBackendProbeFile(vi
}
#if WITH_STORAGE_FS
+
+#include <mntent.h>
+
struct _virNetfsDiscoverState {
const char *host;
virStoragePoolSourceList list;
15 years, 11 months
[libvirt] [PATCH] let gcc's -Wformat do its job; avoid "make syntax-check" failure
by Jim Meyering
I first noticed that "make syntax-check" failed.
Then I saw that virAsprintf's prototype lacked ATTRIBUTE_FORMAT.
This fixes both and updates HACKING.
>From 5bae37c505738ed4223625f8e3cc88ad6f4bf68c Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Wed, 17 Dec 2008 08:54:46 +0100
Subject: [PATCH] let gcc's -Wformat do its job; avoid "make syntax-check" failure
* src/util.c (virAsprintf): Remove trailing space.
* src/util.h (virAsprintf): Use ATTRIBUTE_FORMAT.
* HACKING (Printf-style functions): New section.
---
HACKING | 16 ++++++++++++++++
src/util.c | 2 +-
src/util.h | 3 ++-
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/HACKING b/HACKING
index 3945833..e088da8 100644
--- a/HACKING
+++ b/HACKING
@@ -247,6 +247,22 @@ are some special reasons why you cannot include these files
explicitly.
+Printf-style functions
+======================
+
+Whenever you add a new printf-style function, i.e., one with a format
+string argument and following "..." in its prototype, be sure to use
+gcc's printf attribute directive in the prototype. For example, here's
+the one for virAsprintf, in util.h:
+
+ int virAsprintf(char **strp, const char *fmt, ...)
+ ATTRIBUTE_FORMAT(printf, 2, 3);
+
+This makes it so gcc's -Wformat and -Wformat-security options can do
+their jobs and cross-check format strings with the number and types
+of arguments.
+
+
Libvirt commiters guidelines
============================
diff --git a/src/util.c b/src/util.c
index 12097d4..9eda378 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1158,7 +1158,7 @@ virParseNumber(const char **str)
*
* like asprintf but makes sure *strp == NULL on failure
*/
-int
+int
virAsprintf(char **strp, const char *fmt, ...)
{
va_list ap;
diff --git a/src/util.h b/src/util.h
index 3d603dc..0475bd3 100644
--- a/src/util.h
+++ b/src/util.h
@@ -112,7 +112,8 @@ int virMacAddrCompare (const char *mac1, const char *mac2);
void virSkipSpaces(const char **str);
int virParseNumber(const char **str);
-int virAsprintf(char **strp, const char *fmt, ...);
+int virAsprintf(char **strp, const char *fmt, ...)
+ ATTRIBUTE_FORMAT(printf, 2, 3);
#define VIR_MAC_BUFLEN 6
#define VIR_MAC_PREFIX_BUFLEN 3
--
1.6.1.rc2.316.geb2f0
15 years, 11 months
[libvirt] [PATCH] Fix _FILE_OFFSET_BITS re-definition
by john.levon＠sun.com
# HG changeset patch
# User john.levon(a)sun.com
# Date 1229399267 28800
# Node ID db36391b739c117f5887388f65f31e6a9d2d361b
# Parent 020f8b8e9340287a6ab3d869b359e39b905cd0ff
Fix _FILE_OFFSET_BITS re-definition
Since config.h contains the _FILE_OFFSET_BITS setting (a little dubious
in itself), it must be the first header included, otherwise system
headers can define _FILE_OFFSET_BITS differently themselves.
Signed-off-by: John Levon <john.levon(a)sun.com>
diff --git a/src/node_device_hal.c b/src/node_device_hal.c
--- a/src/node_device_hal.c
+++ b/src/node_device_hal.c
@@ -21,10 +21,11 @@
* Author: David F. Lively <dlively(a)virtualiron.com>
*/
+#include <config.h>
+
#include <stdio.h>
#include <stdlib.h>
-#include <config.h>
#include <libhal.h>
#include "node_device_conf.h"
15 years, 11 months
