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

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. :( 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) -- 1.7.7.6

At 02/15/2012 07:03 AM, 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. :(
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)
ACK Wen Congyang

On 02/15/2012 12:21 AM, Wen Congyang wrote:
At 02/15/2012 07:03 AM, 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. :(
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)
ACK
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

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

On 02/15/2012 10:52 AM, Jim Fehlig wrote:
This one's embarrassing. I think I broke polkit authorization in 0.9.10. :(
Yes, you did :).
I also found a segfault and was about to post the attached patch.
Regards, Jim
0001-Fix-polkit0-authentication.patch
From a06fab953f99e778883618dd0aeaef8da5d5b32a Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig@suse.com> 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(-)
Aargh - I pushed my shorter fix before reviewing your more complete fix. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Eric Blake wrote:
From a06fab953f99e778883618dd0aeaef8da5d5b32a Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig@suse.com> 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(-)
Aargh - I pushed my shorter fix before reviewing your more complete fix. ACK.
Thanks. I rebased the patch and pushed it. Regards, Jim
participants (3)
-
Eric Blake
-
Jim Fehlig
-
Wen Congyang