Don't do polkit auth as root

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

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
 

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;

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,25 +2603,6 @@ 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);
@@ -2706,7 +2675,6 @@ remoteDispatchAuthPolkit (struct qemud_server *server ATTRIBUTE_UNUSED,
              polkit_result_to_string_representation(pkresult));
     ret->complete = 1;
     client->auth = REMOTE_AUTH_NONE;
-    }
 
     return 0;
 }

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 = {