From c75425734710181b19537c71da0c4d267f52f63c Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Thu, 22 Aug 2013 17:09:03 +0100 Subject: [PATCH] Convert remote daemon & acl code to use polkit API Convert the remote daemon auth check and the access control code to use the common polkit API for checking auth. Signed-off-by: Daniel P. Berrange --- daemon/Makefile.am | 3 +- daemon/remote.c | 235 ++--------------------------- src/Makefile.am | 4 +- src/access/viraccessdriverpolkit.c | 91 +++++------ 4 files changed, 47 insertions(+), 286 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 70b765585f..b95a79da7b 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -141,7 +141,7 @@ libvirtd_SOURCES = $(DAEMON_SOURCES) #-D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L libvirtd_CFLAGS = \ $(LIBXML_CFLAGS) $(GNUTLS_CFLAGS) $(SASL_CFLAGS) \ - $(XDR_CFLAGS) $(POLKIT_CFLAGS) $(DBUS_CFLAGS) $(LIBNL_CFLAGS) \ + $(XDR_CFLAGS) $(DBUS_CFLAGS) $(LIBNL_CFLAGS) \ $(WARN_CFLAGS) $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) \ -DQEMUD_PID_FILE="\"$(QEMUD_PID_FILE)\"" @@ -158,7 +158,6 @@ libvirtd_LDADD = \ $(GNUTLS_LIBS) \ $(SASL_LIBS) \ $(DBUS_LIBS) \ - $(POLKIT_LIBS) \ $(LIBNL_LIBS) if WITH_DTRACE_PROBES diff --git a/daemon/remote.c b/daemon/remote.c index ddd510c169..697413c92f 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -24,11 +24,6 @@ #include "virerror.h" -#if WITH_POLKIT0 -# include -# include -#endif - #include "remote.h" #include "libvirtd.h" #include "libvirt_internal.h" @@ -55,6 +50,7 @@ #include "virprobe.h" #include "viraccessapicheck.h" #include "viraccessapicheckqemu.h" +#include "virpolkit.h" #define VIR_FROM_THIS VIR_FROM_RPC @@ -3230,7 +3226,6 @@ remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, -#if WITH_POLKIT1 static int remoteDispatchAuthPolkit(virNetServerPtr server, virNetServerClientPtr client, @@ -3243,26 +3238,16 @@ remoteDispatchAuthPolkit(virNetServerPtr server, uid_t callerUid = -1; unsigned long long timestamp; const char *action; - int status = -1; char *ident = NULL; - bool authdismissed = 0; - char *pkout = NULL; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - virCommandPtr cmd = NULL; -# ifndef PKCHECK_SUPPORTS_UID - static bool polkitInsecureWarned; -# endif + int rv; virMutexLock(&priv->lock); action = virNetServerClientGetReadonly(client) ? "org.libvirt.unix.monitor" : "org.libvirt.unix.manage"; - cmd = virCommandNewArgList(PKCHECK_PATH, "--action-id", action, NULL); - virCommandSetOutputBuffer(cmd, &pkout); - virCommandSetErrorBuffer(cmd, &pkout); - VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client)); if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) { VIR_ERROR(_("client tried invalid PolicyKit init request")); @@ -3283,38 +3268,17 @@ remoteDispatchAuthPolkit(virNetServerPtr server, VIR_INFO("Checking PID %lld running as %d", (long long) callerPid, callerUid); - virCommandAddArg(cmd, "--process"); - -# ifdef PKCHECK_SUPPORTS_UID - virCommandAddArgFormat(cmd, "%lld,%llu,%lu", - (long long) callerPid, - timestamp, - (unsigned long) callerUid); -# else - 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); -# endif - - virCommandAddArg(cmd, "--allow-user-interaction"); - - if (virAsprintf(&ident, "pid:%lld,uid:%d", - (long long) callerPid, callerUid) < 0) + rv = virPolkitCheckAuth(action, + callerPid, + timestamp, + callerUid, + NULL, + true); + if (rv == -1) goto authfail; - - if (virCommandRun(cmd, &status) < 0) - goto authfail; - - authdismissed = (pkout && strstr(pkout, "dismissed=true")); - if (status != 0) { - VIR_ERROR(_("Policy kit denied action %s from pid %lld, uid %d " - "with status %d"), - action, (long long) callerPid, callerUid, status); + else if (rv == -2) goto authdeny; - } + PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW, "client=%p auth=%d identity=%s", client, REMOTE_AUTH_POLKIT, ident); @@ -3325,27 +3289,10 @@ remoteDispatchAuthPolkit(virNetServerPtr server, virNetServerClientSetAuth(client, 0); virNetServerTrackCompletedAuth(server); virMutexUnlock(&priv->lock); - virCommandFree(cmd); - VIR_FREE(pkout); - VIR_FREE(ident); return 0; error: - virCommandFree(cmd); - VIR_FREE(ident); - virResetLastError(); - - if (authdismissed) { - virReportError(VIR_ERR_AUTH_CANCELLED, "%s", - _("authentication cancelled by user")); - } else if (pkout && *pkout) { - virReportError(VIR_ERR_AUTH_FAILED, _("polkit: %s"), pkout); - } else { - virReportError(VIR_ERR_AUTH_FAILED, "%s", _("authentication failed")); - } - - VIR_FREE(pkout); virNetMessageSaveError(rerr); virMutexUnlock(&priv->lock); return -1; @@ -3362,166 +3309,6 @@ remoteDispatchAuthPolkit(virNetServerPtr server, client, REMOTE_AUTH_POLKIT, ident); goto error; } -#elif WITH_POLKIT0 -static int -remoteDispatchAuthPolkit(virNetServerPtr server, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_auth_polkit_ret *ret) -{ - pid_t callerPid; - gid_t callerGid; - uid_t callerUid; - PolKitCaller *pkcaller = NULL; - PolKitAction *pkaction = NULL; - PolKitContext *pkcontext = NULL; - PolKitError *pkerr = NULL; - PolKitResult pkresult; - DBusError err; - const char *action; - char *ident = NULL; - struct daemonClientPrivate *priv = - virNetServerClientGetPrivateData(client); - DBusConnection *sysbus; - unsigned long long timestamp; - - virMutexLock(&priv->lock); - - action = virNetServerClientGetReadonly(client) ? - "org.libvirt.unix.monitor" : - "org.libvirt.unix.manage"; - - VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client)); - if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) { - VIR_ERROR(_("client tried invalid PolicyKit init request")); - goto authfail; - } - - if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid, - &callerPid, ×tamp) < 0) { - VIR_ERROR(_("cannot get peer socket identity")); - goto authfail; - } - - if (virAsprintf(&ident, "pid:%lld,uid:%d", - (long long) callerPid, callerUid) < 0) - goto authfail; - - if (!(sysbus = virDBusGetSystemBus())) - goto authfail; - - VIR_INFO("Checking PID %lld running as %d", - (long long) callerPid, callerUid); - dbus_error_init(&err); - if (!(pkcaller = polkit_caller_new_from_pid(sysbus, - callerPid, &err))) { - VIR_ERROR(_("Failed to lookup policy kit caller: %s"), err.message); - dbus_error_free(&err); - goto authfail; - } - - if (!(pkaction = polkit_action_new())) { - char ebuf[1024]; - VIR_ERROR(_("Failed to create polkit action %s"), - virStrerror(errno, ebuf, sizeof(ebuf))); - polkit_caller_unref(pkcaller); - goto authfail; - } - polkit_action_set_action_id(pkaction, action); - - if (!(pkcontext = polkit_context_new()) || - !polkit_context_init(pkcontext, &pkerr)) { - char ebuf[1024]; - VIR_ERROR(_("Failed to create polkit context %s"), - (pkerr ? polkit_error_get_error_message(pkerr) - : virStrerror(errno, ebuf, sizeof(ebuf)))); - if (pkerr) - polkit_error_free(pkerr); - polkit_caller_unref(pkcaller); - polkit_action_unref(pkaction); - dbus_error_free(&err); - goto authfail; - } - -# if HAVE_POLKIT_CONTEXT_IS_CALLER_AUTHORIZED - pkresult = polkit_context_is_caller_authorized(pkcontext, - pkaction, - pkcaller, - 0, - &pkerr); - if (pkerr && polkit_error_is_set(pkerr)) { - VIR_ERROR(_("Policy kit failed to check authorization %d %s"), - polkit_error_get_error_code(pkerr), - polkit_error_get_error_message(pkerr)); - goto authfail; - } -# else - pkresult = polkit_context_can_caller_do_action(pkcontext, - pkaction, - pkcaller); -# endif - polkit_context_unref(pkcontext); - polkit_caller_unref(pkcaller); - polkit_action_unref(pkaction); - if (pkresult != POLKIT_RESULT_YES) { - VIR_ERROR(_("Policy kit denied action %s from pid %lld, uid %d, result: %s"), - action, (long long) callerPid, callerUid, - polkit_result_to_string_representation(pkresult)); - goto authdeny; - } - PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW, - "client=%p auth=%d identity=%s", - client, REMOTE_AUTH_POLKIT, ident); - VIR_INFO("Policy allowed action %s from pid %lld, uid %d, result %s", - action, (long long) callerPid, callerUid, - polkit_result_to_string_representation(pkresult)); - ret->complete = 1; - - virNetServerClientSetAuth(client, 0); - virNetServerTrackCompletedAuth(server); - virMutexUnlock(&priv->lock); - VIR_FREE(ident); - return 0; - - error: - VIR_FREE(ident); - virResetLastError(); - virReportError(VIR_ERR_AUTH_FAILED, "%s", - _("authentication failed")); - virNetMessageSaveError(rerr); - virMutexUnlock(&priv->lock); - return -1; - - authfail: - PROBE(RPC_SERVER_CLIENT_AUTH_FAIL, - "client=%p auth=%d", - client, REMOTE_AUTH_POLKIT); - goto error; - - authdeny: - PROBE(RPC_SERVER_CLIENT_AUTH_DENY, - "client=%p auth=%d identity=%s", - client, REMOTE_AUTH_POLKIT, ident); - goto error; -} - -#else /* !WITH_POLKIT0 & !HAVE_POLKIT1*/ - -static int -remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_auth_polkit_ret *ret ATTRIBUTE_UNUSED) -{ - VIR_ERROR(_("client tried unsupported PolicyKit init request")); - virReportError(VIR_ERR_AUTH_FAILED, "%s", - _("authentication failed")); - virNetMessageSaveError(rerr); - return -1; -} -#endif /* WITH_POLKIT1 */ /*************************************************************** diff --git a/src/Makefile.am b/src/Makefile.am index 3afae7b9d1..7680bae7dc 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2486,13 +2486,11 @@ libvirt_net_rpc_server_la_CFLAGS = \ $(AVAHI_CFLAGS) \ $(DBUS_CFLAGS) \ $(XDR_CFLAGS) \ - $(AM_CFLAGS) \ - $(POLKIT_CFLAGS) + $(AM_CFLAGS) libvirt_net_rpc_server_la_LDFLAGS = \ $(AM_LDFLAGS) \ $(AVAHI_LIBS) \ $(DBUS_LIBS) \ - $(POLKIT_LIBS) \ $(CYGWIN_EXTRA_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) libvirt_net_rpc_server_la_LIBADD = \ diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c index bdb35dbf79..2bc18429d6 100644 --- a/src/access/viraccessdriverpolkit.c +++ b/src/access/viraccessdriverpolkit.c @@ -26,6 +26,7 @@ #include "virlog.h" #include "virprocess.h" #include "virerror.h" +#include "virpolkit.h" #include "virstring.h" #define VIR_FROM_THIS VIR_FROM_ACCESS @@ -71,29 +72,26 @@ virAccessDriverPolkitFormatAction(const char *typename, } -static char * -virAccessDriverPolkitFormatProcess(const char *actionid) +static int +virAccessDriverPolkitGetCaller(const char *actionid, + pid_t *pid, + unsigned long long *startTime, + uid_t *uid) { virIdentityPtr identity = virIdentityGetCurrent(); - pid_t pid; - unsigned long long startTime; - uid_t uid; - char *ret = NULL; -#ifndef PKCHECK_SUPPORTS_UID - static bool polkitInsecureWarned; -#endif + int ret = -1; if (!identity) { virAccessError(VIR_ERR_ACCESS_DENIED, _("Policy kit denied action %s from "), actionid); - return NULL; + return -1; } - if (virIdentityGetUNIXProcessID(identity, &pid) < 0) + if (virIdentityGetUNIXProcessID(identity, pid) < 0) goto cleanup; - if (virIdentityGetUNIXProcessTime(identity, &startTime) < 0) + if (virIdentityGetUNIXProcessTime(identity, startTime) < 0) goto cleanup; - if (virIdentityGetUNIXUserID(identity, &uid) < 0) + if (virIdentityGetUNIXUserID(identity, uid) < 0) goto cleanup; if (!pid) { @@ -101,25 +99,14 @@ virAccessDriverPolkitFormatProcess(const char *actionid) _("No UNIX process ID available")); goto cleanup; } - if (!startTime) { - virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No UNIX process start time available")); - goto cleanup; - } -#ifdef PKCHECK_SUPPORTS_UID - if (virAsprintf(&ret, "%llu,%llu,%llu", - (unsigned long long)pid, startTime, (unsigned long long)uid) < 0) + if (virIdentityGetUNIXProcessTime(identity, startTime) < 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, "%llu,%llu", - (unsigned long long)pid, startTime) < 0) + + if (virIdentityGetUNIXUserID(identity, uid) < 0) goto cleanup; -#endif + + ret = 0; cleanup: virObjectUnref(identity); @@ -134,53 +121,43 @@ virAccessDriverPolkitCheck(virAccessManagerPtr manager ATTRIBUTE_UNUSED, const char **attrs) { char *actionid = NULL; - char *process = NULL; - virCommandPtr cmd = NULL; - int status; int ret = -1; + pid_t pid; + uid_t uid; + unsigned long long startTime; + int rv; if (!(actionid = virAccessDriverPolkitFormatAction(typename, permname))) goto cleanup; - if (!(process = virAccessDriverPolkitFormatProcess(actionid))) + if (virAccessDriverPolkitGetCaller(actionid, + &pid, + &startTime, + &uid) < 0) goto cleanup; - VIR_DEBUG("Check action '%s' for process '%s'", actionid, process); + VIR_DEBUG("Check action '%s' for process '%d' time %lld uid %d", + actionid, pid, startTime, uid); - cmd = virCommandNewArgList(PKCHECK_PATH, - "--action-id", actionid, - "--process", process, - NULL); + rv = virPolkitCheckAuth(actionid, + pid, + startTime, + uid, + attrs, + false); - while (attrs && attrs[0] && attrs[1]) { - virCommandAddArgList(cmd, "--detail", attrs[0], attrs[1], NULL); - attrs += 2; - } - - if (virCommandRun(cmd, &status) < 0) - goto cleanup; - - if (status == 0) { + if (rv == 0) { ret = 1; /* Allowed */ } else { - if (status == 1 || - status == 2 || - status == 3) { + if (rv == -2) { ret = 0; /* Denied */ } else { ret = -1; /* Error */ - virAccessError(VIR_ERR_ACCESS_DENIED, - _("Policy kit denied action %s from %s: " - "exit status %d"), - actionid, process, status); } - goto cleanup; } cleanup: - virCommandFree(cmd); VIR_FREE(actionid); - VIR_FREE(process); return ret; }