This patch makes two adjustments to the way policy kit authentication is done. - Currently the server unconditionally ask the client to do policykit authentication. This is unnecessary if the remote client is running as root, which we can check via UNIX socket credentials. Unconditionally asking plays havoc with SSH tunneling, so this patch makes it check the socket credentials &not ask for auth if the client is UID==0 - The virsh client will unconditionally call polkit-auth to request credentials. This is also unneccessary if the client is running as root, so this patch makes it skip that step as root. The patch is bigger than it seems because removing an if() conditional made a huge chunk be re-indented. Dan. Index: qemud/internal.h =================================================================== RCS file: /data/cvs/libvirt/qemud/internal.h,v retrieving revision 1.42 diff -u -p -r1.42 internal.h --- qemud/internal.h 23 Jan 2008 14:54:41 -0000 1.42 +++ qemud/internal.h 3 Apr 2008 20:04:53 -0000 @@ -179,6 +179,9 @@ void qemudLog(int priority, const char * void remoteDispatchClientRequest (struct qemud_server *server, struct qemud_client *client); +#if HAVE_POLKIT +int qemudGetSocketIdentity(int fd, uid_t *uid, pid_t *pid); +#endif #endif Index: qemud/qemud.c =================================================================== RCS file: /data/cvs/libvirt/qemud/qemud.c,v retrieving revision 1.91 diff -u -p -r1.91 qemud.c --- qemud/qemud.c 14 Mar 2008 15:21:15 -0000 1.91 +++ qemud/qemud.c 3 Apr 2008 20:04:54 -0000 @@ -1040,6 +1040,28 @@ remoteCheckAccess (struct qemud_client * return 0; } +#if HAVE_POLKIT +int qemudGetSocketIdentity(int fd, uid_t *uid, pid_t *pid) { +#ifdef SO_PEERCRED + struct ucred cr; + unsigned int cr_len = sizeof (cr); + + if (getsockopt (fd, SOL_SOCKET, SO_PEERCRED, &cr, &cr_len) < 0) { + qemudLog(QEMUD_ERR, _("Failed to verify client credentials: %s"), + strerror(errno)); + return -1; + } + + *pid = cr.pid; + *uid = cr.uid; +#else + /* XXX Many more OS support UNIX socket credentials we could port to. See dbus ....*/ +#error "UNIX socket credentials not supported/implemented on this platform yet..." +#endif + return 0; +} +#endif + static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket *sock) { int fd; struct sockaddr_storage addr; @@ -1075,6 +1097,26 @@ static int qemudDispatchServer(struct qe memcpy (&client->addr, &addr, sizeof addr); client->addrlen = addrlen; +#if HAVE_POLKIT + /* Only do policy checks for non-root - allow root user + through with no checks, as a fail-safe - root can easily + change policykit policy anyway, so its pointless trying + to restrict root */ + if (client->auth == REMOTE_AUTH_POLKIT) { + uid_t uid; + pid_t pid; + + if (qemudGetSocketIdentity(client->fd, &uid, &pid) < 0) + goto cleanup; + + /* Cient is running as root, so disable auth */ + if (uid == 0) { + qemudLog(QEMUD_INFO, _("Turn off polkit auth for privileged client %d"), pid); + client->auth = REMOTE_AUTH_NONE; + } + } +#endif + if (client->type != QEMUD_SOCK_TYPE_TLS) { client->mode = QEMUD_MODE_RX_HEADER; client->bufferLength = REMOTE_MESSAGE_HEADER_XDR_LEN; Index: qemud/remote.c =================================================================== RCS file: /data/cvs/libvirt/qemud/remote.c,v retrieving revision 1.27 diff -u -p -r1.27 remote.c --- qemud/remote.c 27 Mar 2008 13:43:01 -0000 1.27 +++ qemud/remote.c 3 Apr 2008 20:04:54 -0000 @@ -2570,27 +2570,6 @@ remoteDispatchAuthSaslStep (struct qemud #if HAVE_POLKIT -static int qemudGetSocketIdentity(int fd, uid_t *uid, pid_t *pid) { -#ifdef SO_PEERCRED - struct ucred cr; - unsigned int cr_len = sizeof (cr); - - if (getsockopt (fd, SOL_SOCKET, SO_PEERCRED, &cr, &cr_len) < 0) { - qemudLog(QEMUD_ERR, _("Failed to verify client credentials: %s"), - strerror(errno)); - return -1; - } - - *pid = cr.pid; - *uid = cr.uid; -#else - /* XXX Many more OS support UNIX socket credentials we could port to. See dbus ....*/ -#error "UNIX socket credentials not supported/implemented on this platform yet..." -#endif - return 0; -} - - static int remoteDispatchAuthPolkit (struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client, @@ -2600,6 +2579,15 @@ remoteDispatchAuthPolkit (struct qemud_s { pid_t callerPid; uid_t callerUid; + PolKitCaller *pkcaller = NULL; + PolKitAction *pkaction = NULL; + PolKitContext *pkcontext = NULL; + PolKitError *pkerr = NULL; + PolKitResult pkresult; + DBusError err; + const char *action = client->readonly ? + "org.libvirt.unix.monitor" : + "org.libvirt.unix.manage"; REMOTE_DEBUG("Start PolicyKit auth %d", client->fd); if (client->auth != REMOTE_AUTH_POLKIT) { @@ -2615,98 +2603,78 @@ remoteDispatchAuthPolkit (struct qemud_s return -2; } - /* Only do policy checks for non-root - allow root user - through with no checks, as a fail-safe - root can easily - change policykit policy anyway, so its pointless trying - to restrict root */ - if (callerUid == 0) { - qemudLog(QEMUD_INFO, _("Allowing PID %d running as root"), callerPid); - ret->complete = 1; - client->auth = REMOTE_AUTH_NONE; - } else { - PolKitCaller *pkcaller = NULL; - PolKitAction *pkaction = NULL; - PolKitContext *pkcontext = NULL; - PolKitError *pkerr = NULL; - PolKitResult pkresult; - DBusError err; - const char *action = client->readonly ? - "org.libvirt.unix.monitor" : - "org.libvirt.unix.manage"; - - qemudLog(QEMUD_INFO, _("Checking PID %d running as %d"), - callerPid, callerUid); - dbus_error_init(&err); - if (!(pkcaller = polkit_caller_new_from_pid(server->sysbus, - callerPid, &err))) { - qemudLog(QEMUD_ERR, _("Failed to lookup policy kit caller: %s"), - err.message); - dbus_error_free(&err); - remoteDispatchFailAuth(client, req); - return -2; - } - - if (!(pkaction = polkit_action_new())) { - qemudLog(QEMUD_ERR, _("Failed to create polkit action %s\n"), - strerror(errno)); - polkit_caller_unref(pkcaller); - remoteDispatchFailAuth(client, req); - return -2; - } - polkit_action_set_action_id(pkaction, action); - - if (!(pkcontext = polkit_context_new()) || - !polkit_context_init(pkcontext, &pkerr)) { - qemudLog(QEMUD_ERR, _("Failed to create polkit context %s\n"), - (pkerr ? polkit_error_get_error_message(pkerr) - : strerror(errno))); - if (pkerr) - polkit_error_free(pkerr); - polkit_caller_unref(pkcaller); - polkit_action_unref(pkaction); - dbus_error_free(&err); - remoteDispatchFailAuth(client, req); - return -2; - } + qemudLog(QEMUD_INFO, _("Checking PID %d running as %d"), + callerPid, callerUid); + dbus_error_init(&err); + if (!(pkcaller = polkit_caller_new_from_pid(server->sysbus, + callerPid, &err))) { + qemudLog(QEMUD_ERR, _("Failed to lookup policy kit caller: %s"), + err.message); + dbus_error_free(&err); + remoteDispatchFailAuth(client, req); + return -2; + } + + if (!(pkaction = polkit_action_new())) { + qemudLog(QEMUD_ERR, _("Failed to create polkit action %s\n"), + strerror(errno)); + polkit_caller_unref(pkcaller); + remoteDispatchFailAuth(client, req); + return -2; + } + polkit_action_set_action_id(pkaction, action); + + if (!(pkcontext = polkit_context_new()) || + !polkit_context_init(pkcontext, &pkerr)) { + qemudLog(QEMUD_ERR, _("Failed to create polkit context %s\n"), + (pkerr ? polkit_error_get_error_message(pkerr) + : strerror(errno))); + if (pkerr) + polkit_error_free(pkerr); + polkit_caller_unref(pkcaller); + polkit_action_unref(pkaction); + dbus_error_free(&err); + remoteDispatchFailAuth(client, req); + return -2; + } #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)) { - qemudLog(QEMUD_ERR, - _("Policy kit failed to check authorization %d %s"), - polkit_error_get_error_code(pkerr), - polkit_error_get_error_message(pkerr)); - remoteDispatchFailAuth(client, req); - return -2; - } + pkresult = polkit_context_is_caller_authorized(pkcontext, + pkaction, + pkcaller, + 0, + &pkerr); + if (pkerr && polkit_error_is_set(pkerr)) { + qemudLog(QEMUD_ERR, + _("Policy kit failed to check authorization %d %s"), + polkit_error_get_error_code(pkerr), + polkit_error_get_error_message(pkerr)); + remoteDispatchFailAuth(client, req); + return -2; + } #else - pkresult = polkit_context_can_caller_do_action(pkcontext, - pkaction, - pkcaller); + 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) { - qemudLog(QEMUD_ERR, - _("Policy kit denied action %s from pid %d, uid %d," - " result: %s\n"), - action, callerPid, callerUid, - polkit_result_to_string_representation(pkresult)); - remoteDispatchFailAuth(client, req); - return -2; - } - qemudLog(QEMUD_INFO, - _("Policy allowed action %s from pid %d, uid %d, result %s"), + polkit_context_unref(pkcontext); + polkit_caller_unref(pkcaller); + polkit_action_unref(pkaction); + if (pkresult != POLKIT_RESULT_YES) { + qemudLog(QEMUD_ERR, + _("Policy kit denied action %s from pid %d, uid %d," + " result: %s\n"), action, callerPid, callerUid, polkit_result_to_string_representation(pkresult)); - ret->complete = 1; - client->auth = REMOTE_AUTH_NONE; + remoteDispatchFailAuth(client, req); + return -2; } + qemudLog(QEMUD_INFO, + _("Policy allowed action %s from pid %d, uid %d, result %s"), + action, callerPid, callerUid, + polkit_result_to_string_representation(pkresult)); + ret->complete = 1; + client->auth = REMOTE_AUTH_NONE; return 0; } Index: src/libvirt.c =================================================================== RCS file: /data/cvs/libvirt/src/libvirt.c,v retrieving revision 1.132 diff -u -p -r1.132 libvirt.c --- src/libvirt.c 21 Mar 2008 15:03:37 -0000 1.132 +++ src/libvirt.c 3 Apr 2008 20:04:54 -0000 @@ -116,17 +116,23 @@ static int virConnectAuthCallbackDefault size_t len; switch (cred[i].type) { -#if defined(POLKIT_AUTH) case VIR_CRED_EXTERNAL: { if (STRNEQ(cred[i].challenge, "PolicyKit")) return -1; +#if defined(POLKIT_AUTH) if (virConnectAuthGainPolkit(cred[i].prompt) < 0) return -1; - +#else + /* + * Ignore & carry on. Although we can't auth + * directly, the user may have authenticated + * themselves already outside context of libvirt + */ +#endif break; } -#endif + case VIR_CRED_USERNAME: case VIR_CRED_AUTHNAME: case VIR_CRED_ECHOPROMPT: @@ -186,9 +192,7 @@ static int virConnectCredTypeDefault[] = VIR_CRED_REALM, VIR_CRED_PASSPHRASE, VIR_CRED_NOECHOPROMPT, -#if defined(POLKIT_AUTH) VIR_CRED_EXTERNAL, -#endif }; static virConnectAuth virConnectAuthDefault = { -- |: Red Hat, Engineering, Boston -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 :|