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-10 13:09:08 +01:00 · 2013-10-10 13:09:08 +01:00 · 3e2f27e13b
parent b7fcc799ad
commit 3e2f27e13b
12 changed files with 153 additions and 21 deletions
--- a/Makefile.am
+++ b/Makefile.am
@ -31,6 +31,7 @@ XML_EXAMPLES = \
 					test/*.xml storage/*.xml)))

 EXTRA_DIST = \
+  config-post.h \
  ChangeLog-old \
  libvirt.spec libvirt.spec.in \
  mingw-libvirt.spec.in \
--- a/config-post.h
+++ b/config-post.h
@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2013 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Since virt-login-shell will be setuid, we must do everything
+ * we can to avoid linking to other libraries. Many of them do
+ * unsafe things in functions marked __atttribute__((constructor)).
+ * The only way avoid to avoid such deps is to re-compile the
+ * functions with the code in question disabled, and for that we
+ * must override the main config.h rules. Hence this file :-(
+ */
+
+#ifdef LIBVIRT_SETUID_RPC_CLIENT
+# undef HAVE_LIBDEVMAPPER_H
+# undef HAVE_LIBNL
+# undef HAVE_LIBNL3
+# undef HAVE_LIBSASL2
+# undef WITH_CAPNG
+# undef WITH_CURL
+# undef WITH_DTRACE_PROBES
+# undef WITH_GNUTLS
+# undef WITH_MACVTAP
+# undef WITH_NUMACTL
+# undef WITH_SASL
+# undef WITH_SSH2
+# undef WITH_VIRTUALPORT
+# undef WITH_YAJL
+# undef WITH_YAJL2
+#endif
--- a/configure.ac
+++ b/configure.ac
@ -20,6 +20,7 @@ AC_INIT([libvirt], [1.1.3], [libvir-list@redhat.com], [], [http://libvirt.org])
 AC_CONFIG_SRCDIR([src/libvirt.c])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
+AH_BOTTOM([#include <config-post.h>])
 AC_CONFIG_MACRO_DIR([m4])
 dnl Make automake keep quiet about wildcards & other GNUmake-isms; also keep
 dnl quiet about the fact that we intentionally cater to automake 1.9
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@ -18,6 +18,7 @@

 INCLUDES = \
 	-I$(top_builddir)/gnulib/lib -I$(top_srcdir)/gnulib/lib \
+	-I$(top_srcdir) \
 	-I$(top_builddir)/include -I$(top_srcdir)/include \
 	-I$(top_builddir)/src -I$(top_srcdir)/src \
 	-I$(top_srcdir)/src/util \
--- a/examples/domain-events/events-c/Makefile.am
+++ b/examples/domain-events/events-c/Makefile.am
@ -15,7 +15,8 @@
 ## <http://www.gnu.org/licenses/>.

 INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include \
-	   -I$(top_builddir)/gnulib/lib -I$(top_srcdir)/gnulib/lib
+	   -I$(top_builddir)/gnulib/lib -I$(top_srcdir)/gnulib/lib \
+	   -I$(top_srcdir)
 noinst_PROGRAMS = event-test
 event_test_CFLAGS = $(WARN_CFLAGS)
 event_test_SOURCES = event-test.c
--- a/examples/hellolibvirt/Makefile.am
+++ b/examples/hellolibvirt/Makefile.am
@ -14,7 +14,7 @@
 ## License along with this library.  If not, see
 ## <http://www.gnu.org/licenses/>.

-INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include
+INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include -I$(top_srcdir)
 noinst_PROGRAMS = hellolibvirt
 hellolibvirt_CFLAGS = $(WARN_CFLAGS)
 hellolibvirt_SOURCES = hellolibvirt.c
--- a/examples/openauth/Makefile.am
+++ b/examples/openauth/Makefile.am
@ -14,7 +14,7 @@
 ## License along with this library.  If not, see
 ## <http://www.gnu.org/licenses/>.

-INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include
+INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include -I$(top_srcdir)
 noinst_PROGRAMS = openauth
 openauth_CFLAGS = $(WARN_CFLAGS)
 openauth_SOURCES = openauth.c
--- a/gnulib/lib/Makefile.am
+++ b/gnulib/lib/Makefile.am
@ -27,4 +27,4 @@ noinst_LTLIBRARIES =

 include gnulib.mk

-INCLUDES = $(GETTEXT_CPPFLAGS)
+INCLUDES = -I$(top_srcdir) $(GETTEXT_CPPFLAGS)
--- a/python/Makefile.am
+++ b/python/Makefile.am
@ -20,6 +20,7 @@ INCLUDES = \
 	$(PYTHON_INCLUDES) \
 	-I$(top_builddir)/gnulib/lib \
 	-I$(top_srcdir)/gnulib/lib \
+	-I$(top_srcdir) \
 	-I$(top_builddir)/src \
 	-I$(top_srcdir)/src \
 	-I$(top_srcdir)/src/util \
--- a/src/Makefile.am
+++ b/src/Makefile.am
@ -21,6 +21,7 @@
 # that actually use them. Also keep GETTEXT_CPPFLAGS at the end.
 INCLUDES =	-I../gnulib/lib					\
 		-I$(top_srcdir)/gnulib/lib			\
+		-I$(top_srcdir)					\
 		-I../include					\
 		-I$(top_srcdir)/include				\
 		-I$(top_srcdir)/src/util			\
@ -1966,6 +1967,77 @@ libvirt_lxc_la_LDFLAGS = \
 libvirt_lxc_la_CFLAGS = $(AM_CFLAGS)
 libvirt_lxc_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD)

+# Since virt-login-shell will be setuid, we must do everything
+# we can to avoid linking to other libraries. Many of them do
+# unsafe things in functions marked __atttribute__((constructor)).
+# This library is built to include the bare minimum required to
+# have a RPC client for local UNIX socket access only. We use
+# the ../config-post.h header to disable all external deps that
+# we don't want
+if WITH_LXC
+noinst_LTLIBRARIES += libvirt-setuid-rpc-client.la
+
+libvirt_setuid_rpc_client_la_SOURCES = 		\
+		util/viralloc.c			\
+		util/virbitmap.c		\
+		util/virbuffer.c		\
+		util/vircommand.c		\
+		util/virconf.c			\
+		util/virerror.c			\
+		util/virevent.c			\
+		util/vireventpoll.c		\
+		util/virfile.c			\
+		util/virhash.c			\
+		util/virhashcode.c		\
+		util/virjson.c			\
+		util/virlog.c			\
+		util/virobject.c		\
+		util/virpidfile.c		\
+		util/virprocess.c		\
+		util/virrandom.c		\
+		util/virsocketaddr.c		\
+		util/virstoragefile.c		\
+		util/virstring.c		\
+		util/virtime.c			\
+		util/virthread.c		\
+		util/virtypedparam.c		\
+		util/viruri.c			\
+		util/virutil.c			\
+		util/viruuid.c			\
+		conf/domain_event.c		\
+		rpc/virnetsocket.c		\
+		rpc/virnetsocket.h		\
+		rpc/virnetmessage.h		\
+		rpc/virnetmessage.c		\
+		rpc/virkeepalive.c		\
+		rpc/virkeepalive.h		\
+		rpc/virnetclient.c		\
+		rpc/virnetclientprogram.c	\
+		rpc/virnetclientstream.c	\
+		rpc/virnetprotocol.c		\
+		remote/remote_driver.c		\
+		remote/remote_protocol.c	\
+		remote/qemu_protocol.c		\
+		remote/lxc_protocol.c		\
+		datatypes.c			\
+		libvirt.c			\
+		libvirt-lxc.c			\
+		$(NULL)
+
+libvirt_setuid_rpc_client_la_LDFLAGS =		\
+		$(AM_LDFLAGS)			\
+		$(LIBXML_LIBS)			\
+		$(SELINUX_LIBS)			\
+		$(NULL)
+libvirt_setuid_rpc_client_la_CFLAGS =		\
+		-DLIBVIRT_SETUID_RPC_CLIENT	\
+		-I$(top_srcdir)/src/conf	\
+		-I$(top_srcdir)/src/rpc		\
+		$(AM_CFLAGS)			\
+		$(SELINUX_CFLAGS)		\
+		$(NULL)
+endif WITH_LXC
+
 lockdriverdir = $(libdir)/libvirt/lock-driver
 lockdriver_LTLIBRARIES =

--- a/src/libvirt.c
+++ b/src/libvirt.c
@ -445,41 +445,47 @@ virGlobalInit(void)
    if (!bindtextdomain(PACKAGE, LOCALEDIR))
        goto error;

+    /*
+     * Note we must avoid everything except 'remote' driver
+     * for virt-login-shell usage
+     */
+#ifndef LIBVIRT_SETUID_RPC_CLIENT
    /*
     * Note that the order is important: the first ones have a higher
     * priority when calling virConnectOpen.
     */
-#ifdef WITH_TEST
+# ifdef WITH_TEST
    if (testRegister() == -1)
        goto error;
-#endif
-#ifdef WITH_OPENVZ
+# endif
+# ifdef WITH_OPENVZ
    if (openvzRegister() == -1)
        goto error;
-#endif
-#ifdef WITH_VMWARE
+# endif
+# ifdef WITH_VMWARE
    if (vmwareRegister() == -1)
        goto error;
-#endif
-#ifdef WITH_PHYP
+# endif
+# ifdef WITH_PHYP
    if (phypRegister() == -1)
        goto error;
-#endif
-#ifdef WITH_ESX
+# endif
+# ifdef WITH_ESX
    if (esxRegister() == -1)
        goto error;
-#endif
-#ifdef WITH_HYPERV
+# endif
+# ifdef WITH_HYPERV
    if (hypervRegister() == -1)
        goto error;
-#endif
-#ifdef WITH_XENAPI
+# endif
+# ifdef WITH_XENAPI
    if (xenapiRegister() == -1)
        goto error;
-#endif
-#ifdef WITH_PARALLELS
+# endif
+# ifdef WITH_PARALLELS
    if (parallelsRegister() == -1)
        goto error;
+# endif
 #endif
 #ifdef WITH_REMOTE
    if (remoteRegister() == -1)
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@ -153,6 +153,11 @@ virt_host_validate_CFLAGS = \
 		$(COVERAGE_CFLAGS)				\
 		$(NULL)

+# Since virt-login-shell will be setuid, we must do everything
+# we can to avoid linking to other libraries. Many of them do
+# unsafe things in functions marked __atttribute__((constructor)).
+# This we statically link to a library containing only the minimal
+# libvirt client code, not libvirt.so itself.
 virt_login_shell_SOURCES =					\
 		virt-login-shell.c

@ -163,11 +168,11 @@ virt_login_shell_LDFLAGS = \
 virt_login_shell_LDADD =					\
 		$(STATIC_BINARIES)				\
 		$(PIE_LDFLAGS)					\
-		../src/libvirt.la				\
-		../src/libvirt-lxc.la				\
+		../src/libvirt-setuid-rpc-client.la		\
 		../gnulib/lib/libgnu.la

 virt_login_shell_CFLAGS =					\
+		-DLIBVIRT_SETUID_RPC_CLIENT			\
 		$(WARN_CFLAGS)					\
 		$(PIE_CFLAGS)					\
 		$(COVERAGE_CFLAGS)