Re: [PATCH] daemon: fix logic bug with virAsprintf

[Jim Fehlig <jfehlig@xxxxxxxx>]

Eric Blake wrote:
> Regression introduced in commit 7033c5f2, due to improper conversion
> from snprintf to virAsprintf.
>
> * daemon/remote.c (remoteDispatchAuthList): Check return value
> correctly.
> ---
>
> This one's embarrassing.  I think I broke polkit authorization in
> 0.9.10.  :(
>   

Yes, you did :).

>  daemon/remote.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 9c61306..724db23 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -2052,7 +2052,7 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED,
>          } else if (callerUid == 0) {
>              char *ident;
>              if (virAsprintf(&ident, "pid:%lld,uid:%d",
> -                            (long long) callerPid, callerUid) == 0) {
> +                            (long long) callerPid, callerUid) >= 0) {
>                  VIR_INFO("Bypass polkit auth for privileged client %s",
>                           ident);
>                  if (virNetServerClientSetIdentity(client, ident) < 0)
>   

I also found a segfault and was about to post the attached patch.

Regards,
Jim

>From a06fab953f99e778883618dd0aeaef8da5d5b32a Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@xxxxxxxx>
Date: Wed, 15 Feb 2012 10:01:50 -0700
Subject: [PATCH] Fix polkit0 authentication

Commit 7033c5f2 introduced some bugs in polkit0 authentication.

Fix libvirtd segfault in remoteDispatchAuthPolkit().

Fix polkit authentication bypass when caller UID = 0.
---
 daemon/remote.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 1cea942..a57656c 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2052,16 +2052,16 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED,
         } else if (callerUid == 0) {
             char *ident;
             if (virAsprintf(&ident, "pid:%lld,uid:%d",
-                            (long long) callerPid, callerUid) == 0) {
-                VIR_INFO("Bypass polkit auth for privileged client %s",
-                         ident);
-                if (virNetServerClientSetIdentity(client, ident) < 0)
-                    virResetLastError();
-                else
-                    auth = VIR_NET_SERVER_SERVICE_AUTH_NONE;
-                VIR_FREE(ident);
+                            (long long) callerPid, callerUid) < 0) {
+                virReportOOMError();
+                goto cleanup;
             }
-            rv = -1;
+            VIR_INFO("Bypass polkit auth for privileged client %s", ident);
+            if (virNetServerClientSetIdentity(client, ident) < 0)
+                virResetLastError();
+            else
+                auth = VIR_NET_SERVER_SERVICE_AUTH_NONE;
+            VIR_FREE(ident);
         }
     }
 
@@ -2593,8 +2593,6 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
     struct daemonClientPrivate *priv =
         virNetServerClientGetPrivateData(client);
 
-    memset(ident, 0, sizeof ident);
-
     virMutexLock(&priv->lock);
 
     action = virNetServerClientGetReadonly(client) ?
-- 
1.7.7

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list