Don't do polkit auth as root

diff --git a/ChangeLog b/ChangeLog
index 4679180395be0acae547e3a0d0b3614a540c6ae2..44b5066c52d8c1fbaa3700736e6c583db7be73d5 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+Fri Apr  4 11:00:08 EDT 2008 Daniel P. Berrange <berrange@redhat.com>
+
+       * src/libvirt.c: Don't run polkit-auth if running as root
+       * qemud/qemud.c, qemud/remote.c, qemud/internal.h: Don't
+       ask client for polkit auth if they are running as root
+
 Fri Apr  4 13:19:08 CEST 2008 Daniel Veillard <veillard@redhat.com>
 
        * src/virsh.c: patch from Shigeki Sakamoto adding message on vcpupin

diff --git a/qemud/internal.h b/qemud/internal.h
index 19339a8c0b82de7425ddceb348f6736c71d0454e..da556752b30a8fd66caf271ed13656fdded56a02 100644 (file)
--- a/qemud/internal.h
+++ b/qemud/internal.h
@@ -179,6 +179,9 @@ void qemudLog(int priority, const char *fmt, ...)
 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
 

diff --git a/qemud/qemud.c b/qemud/qemud.c
index 4db9d32d7cdb9dc2a820844ba621cbf28e56a78b..6534ec362ebd776ad5f141d5780f284270466ee9 100644 (file)
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -1040,6 +1040,28 @@ remoteCheckAccess (struct qemud_client *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 qemud_server *server, struct qemud_socket
     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;

diff --git a/qemud/remote.c b/qemud/remote.c
index 82b8dcedaa1c86cbeb5c9ef23b84d89b38576841..787146eab60f6c669a0cc43f354bcc16fb3cd0f5 100644 (file)
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -2570,27 +2570,6 @@ remoteDispatchAuthSaslStep (struct qemud_server *server ATTRIBUTE_UNUSED,
 
 
 #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_server *server ATTRIBUTE_UNUSED,
 {
     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_server *server ATTRIBUTE_UNUSED,
         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;
-        }
+    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 (!(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;
 }

diff --git a/src/libvirt.c b/src/libvirt.c
index e19e5fd1753a1d7d4010f31898aad50bd0c171b9..4bda97cb6e5b829c048ae24ff017cef59e1fc8cc 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -116,17 +116,23 @@ static int virConnectAuthCallbackDefault(virConnectCredentialPtr cred,
         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 = {