172 lines
6.5 KiB
Diff
172 lines
6.5 KiB
Diff
|
commit 922b7fda77b094dbf022d625238262ea05335666
|
||
|
Author: Daniel P. Berrange <berrange@redhat.com>
|
||
|
Date: Wed Aug 28 15:25:40 2013 +0100
|
||
|
|
||
|
Add support for using 3-arg pkcheck syntax for process (CVE-2013-4311)
|
||
|
|
||
|
With the existing pkcheck (pid, start time) tuple for identifying
|
||
|
the process, there is a race condition, where a process can make
|
||
|
a libvirt RPC call and in another thread exec a setuid application,
|
||
|
causing it to change to effective UID 0. This in turn causes polkit
|
||
|
to do its permission check based on the wrong UID.
|
||
|
|
||
|
To address this, libvirt must get the UID the caller had at time
|
||
|
of connect() (from SO_PEERCRED) and pass a (pid, start time, uid)
|
||
|
triple to the pkcheck program.
|
||
|
|
||
|
This fix requires that libvirt is re-built against a version of
|
||
|
polkit that has the fix for its CVE-2013-4288, so that libvirt
|
||
|
can see 'pkg-config --variable pkcheck_supports_uid polkit-gobject-1'
|
||
|
|
||
|
Signed-off-by: Colin Walters <walters@redhat.com>
|
||
|
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
|
||
|
|
||
|
Index: libvirt-1.1.2/configure.ac
|
||
|
===================================================================
|
||
|
--- libvirt-1.1.2.orig/configure.ac
|
||
|
+++ libvirt-1.1.2/configure.ac
|
||
|
@@ -1184,6 +1184,14 @@ if test "x$with_polkit" = "xyes" || test
|
||
|
AC_PATH_PROG([PKCHECK_PATH],[pkcheck], [], [/usr/sbin:$PATH])
|
||
|
if test "x$PKCHECK_PATH" != "x" ; then
|
||
|
AC_DEFINE_UNQUOTED([PKCHECK_PATH],["$PKCHECK_PATH"],[Location of pkcheck program])
|
||
|
+ AC_MSG_CHECKING([whether pkcheck supports uid value])
|
||
|
+ pkcheck_supports_uid=`$PKG_CONFIG --variable pkcheck_supports_uid polkit-gobject-1`
|
||
|
+ if test "x$pkcheck_supports_uid" = "xtrue"; then
|
||
|
+ AC_MSG_RESULT([yes])
|
||
|
+ AC_DEFINE_UNQUOTED([PKCHECK_SUPPORTS_UID], 1, [Pass uid to pkcheck])
|
||
|
+ else
|
||
|
+ AC_MSG_RESULT([no])
|
||
|
+ fi
|
||
|
AC_DEFINE_UNQUOTED([WITH_POLKIT], 1,
|
||
|
[use PolicyKit for UNIX socket access checks])
|
||
|
AC_DEFINE_UNQUOTED([WITH_POLKIT1], 1,
|
||
|
Index: libvirt-1.1.2/daemon/remote.c
|
||
|
===================================================================
|
||
|
--- libvirt-1.1.2.orig/daemon/remote.c
|
||
|
+++ libvirt-1.1.2/daemon/remote.c
|
||
|
@@ -2738,10 +2738,12 @@ remoteDispatchAuthPolkit(virNetServerPtr
|
||
|
int status = -1;
|
||
|
char *ident = NULL;
|
||
|
bool authdismissed = 0;
|
||
|
+ bool supportsuid = false;
|
||
|
char *pkout = NULL;
|
||
|
struct daemonClientPrivate *priv =
|
||
|
virNetServerClientGetPrivateData(client);
|
||
|
virCommandPtr cmd = NULL;
|
||
|
+ static bool polkitInsecureWarned;
|
||
|
|
||
|
virMutexLock(&priv->lock);
|
||
|
action = virNetServerClientGetReadonly(client) ?
|
||
|
@@ -2763,14 +2765,28 @@ remoteDispatchAuthPolkit(virNetServerPtr
|
||
|
goto authfail;
|
||
|
}
|
||
|
|
||
|
+ if (timestamp == 0) {
|
||
|
+ VIR_WARN("Failing polkit auth due to missing client (pid=%lld) start time",
|
||
|
+ (long long)callerPid);
|
||
|
+ goto authfail;
|
||
|
+ }
|
||
|
+
|
||
|
VIR_INFO("Checking PID %lld running as %d",
|
||
|
(long long) callerPid, callerUid);
|
||
|
|
||
|
virCommandAddArg(cmd, "--process");
|
||
|
- if (timestamp != 0) {
|
||
|
- virCommandAddArgFormat(cmd, "%lld,%llu", (long long) callerPid, timestamp);
|
||
|
+# ifdef PKCHECK_SUPPORTS_UID
|
||
|
+ supportsuid = true;
|
||
|
+# endif
|
||
|
+ if (supportsuid) {
|
||
|
+ virCommandAddArgFormat(cmd, "%lld,%llu,%lu",
|
||
|
+ (long long) callerPid, timestamp, (unsigned long) callerUid);
|
||
|
} else {
|
||
|
- virCommandAddArgFormat(cmd, "%lld", (long long) callerPid);
|
||
|
+ if (!polkitInsecureWarned) {
|
||
|
+ VIR_WARN("No support for caller UID with pkcheck. This deployment is known to be insecure.");
|
||
|
+ polkitInsecureWarned = true;
|
||
|
+ }
|
||
|
+ virCommandAddArgFormat(cmd, "%lld,%llu", (long long) callerPid, timestamp);
|
||
|
}
|
||
|
virCommandAddArg(cmd, "--allow-user-interaction");
|
||
|
|
||
|
Index: libvirt-1.1.2/libvirt.spec.in
|
||
|
===================================================================
|
||
|
--- libvirt-1.1.2.orig/libvirt.spec.in
|
||
|
+++ libvirt-1.1.2/libvirt.spec.in
|
||
|
@@ -508,8 +508,7 @@ BuildRequires: cyrus-sasl-devel
|
||
|
%endif
|
||
|
%if %{with_polkit}
|
||
|
%if 0%{?fedora} >= 12 || 0%{?rhel} >= 6
|
||
|
-# Only need the binary, not -devel
|
||
|
-BuildRequires: polkit >= 0.93
|
||
|
+BuildRequires: polkit-devel >= 0.93
|
||
|
%else
|
||
|
BuildRequires: PolicyKit-devel >= 0.6
|
||
|
%endif
|
||
|
Index: libvirt-1.1.2/src/access/viraccessdriverpolkit.c
|
||
|
===================================================================
|
||
|
--- libvirt-1.1.2.orig/src/access/viraccessdriverpolkit.c
|
||
|
+++ libvirt-1.1.2/src/access/viraccessdriverpolkit.c
|
||
|
@@ -72,8 +72,12 @@ static char *
|
||
|
virAccessDriverPolkitFormatProcess(const char *actionid)
|
||
|
{
|
||
|
virIdentityPtr identity = virIdentityGetCurrent();
|
||
|
- const char *process = NULL;
|
||
|
+ const char *callerPid = NULL;
|
||
|
+ const char *callerTime = NULL;
|
||
|
+ const char *callerUid = NULL;
|
||
|
char *ret = NULL;
|
||
|
+ bool supportsuid = false;
|
||
|
+ static bool polkitInsecureWarned;
|
||
|
|
||
|
if (!identity) {
|
||
|
virAccessError(VIR_ERR_ACCESS_DENIED,
|
||
|
@@ -81,17 +85,43 @@ virAccessDriverPolkitFormatProcess(const
|
||
|
actionid);
|
||
|
return NULL;
|
||
|
}
|
||
|
- if (virIdentityGetAttr(identity, VIR_IDENTITY_ATTR_UNIX_PROCESS_ID, &process) < 0)
|
||
|
+ if (virIdentityGetAttr(identity, VIR_IDENTITY_ATTR_UNIX_PROCESS_ID, &callerPid) < 0)
|
||
|
+ goto cleanup;
|
||
|
+ if (virIdentityGetAttr(identity, VIR_IDENTITY_ATTR_UNIX_PROCESS_TIME, &callerTime) < 0)
|
||
|
+ goto cleanup;
|
||
|
+ if (virIdentityGetAttr(identity, VIR_IDENTITY_ATTR_UNIX_USER_ID, &callerUid) < 0)
|
||
|
goto cleanup;
|
||
|
|
||
|
- if (!process) {
|
||
|
+ if (!callerPid) {
|
||
|
virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||
|
_("No UNIX process ID available"));
|
||
|
goto cleanup;
|
||
|
}
|
||
|
-
|
||
|
- if (VIR_STRDUP(ret, process) < 0)
|
||
|
+ if (!callerTime) {
|
||
|
+ virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||
|
+ _("No UNIX process start time available"));
|
||
|
+ goto cleanup;
|
||
|
+ }
|
||
|
+ if (!callerUid) {
|
||
|
+ virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||
|
+ _("No UNIX caller UID available"));
|
||
|
goto cleanup;
|
||
|
+ }
|
||
|
+
|
||
|
+#ifdef PKCHECK_SUPPORTS_UID
|
||
|
+ supportsuid = true;
|
||
|
+#endif
|
||
|
+ if (supportsuid) {
|
||
|
+ if (virAsprintf(&ret, "%s,%s,%s", callerPid, callerTime, callerUid) < 0)
|
||
|
+ goto cleanup;
|
||
|
+ } else {
|
||
|
+ if (!polkitInsecureWarned) {
|
||
|
+ VIR_WARN("No support for caller UID with pkcheck. This deployment is known to be insecure.");
|
||
|
+ polkitInsecureWarned = true;
|
||
|
+ }
|
||
|
+ if (virAsprintf(&ret, "%s,%s", callerPid, callerTime) < 0)
|
||
|
+ goto cleanup;
|
||
|
+ }
|
||
|
|
||
|
cleanup:
|
||
|
virObjectUnref(identity);
|