[libvirt] [SECURITY] PATCH: Fix missing read-only access checks (CVE-2008-5086)
James Morris
jmorris at namei.org
Wed Dec 17 22:23:27 UTC 2008
On Wed, 17 Dec 2008, Daniel P. Berrange wrote:
> 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.
Dan (Walsh or otherwise),
Any idea if the standard SELinux policy helps here?
We'll need to assess policy for these sockets for sVirt, in any case.
>
> 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 :|
>
> --
> Libvir-list mailing list
> Libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
James Morris
<jmorris at namei.org>
More information about the libvir-list
mailing list