[libvirt] PATCH: better error checking for LOCAL_PEERCRED

[Eric Blake]

On 10/16/2013 10:08 AM, Brian Candler wrote:
> On 15/10/2013 12:16, Daniel P. Berrange wrote:
>> Unfortunately your patch does not apply since your mail client has
>> messed up line wrapping. Also there have been conflicting changes
>> to the code since your patch. I would fix it myself, but I don't
>> have ability to compile test code on BSD platforms.
>>
>> Can you update your patch & re-send.
> Sorry about that. Updated patch uploaded to
> https://bugzilla.redhat.com/show_bug.cgi?id=1019929 where Thunderbird
> can't mangle it.

On the other hand, making someone chase a URL is less convenient than
attaching the patch (ideally, sending inline the way 'git send-email'
does things is preferred, but since 'git am' can also handle
attachments, it's still easier on the maintainer to send through the
list).  That said, I went ahead and did the work for you this time around.

Here's what I pushed:

From aa0f09929d02ccdbf3ca9502a1fd39d90db0c690 Mon Sep 17 00:00:00 2001
From: Brian Candler <b.candler at pobox.com>
Date: Thu, 17 Oct 2013 06:21:57 -0600
Subject: [PATCH] better error checking for LOCAL_PEERCRED

This patch improves the error checking in the LOCAL_PEERCRED version
of virNetSocketGetUNIXIdentity, used by FreeBSD and Mac OSX.

1. The error return paths now correctly unlock the socket. This is
implemented in exactly the same way as the SO_PEERCRED version,
using "goto cleanup"

2. cr.cr_ngroups is initialised to -1, and cr.cr_ngroups is checked
for negative and overlarge values.

This means that if the getsockopt() call returns success but doesn't
actually update the xucred structure, this is now caught. This
happened previously when getsockopt was called with SOL_SOCKET
instead of SOL_LOCAL, prior to commit 5a468b3, and resulted in
random uids being accepted.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 src/rpc/virnetsocket.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index e8cdfa6..2e50f8c 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1174,25 +1174,27 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr
sock,
 {
     struct xucred cr;
     socklen_t cr_len = sizeof(cr);
+    int ret = -1;
+
     virObjectLock(sock);

+    cr.cr_ngroups = -1;
     if (getsockopt(sock->fd, VIR_SOL_PEERCRED, LOCAL_PEERCRED, &cr,
&cr_len) < 0) {
         virReportSystemError(errno, "%s",
                              _("Failed to get client socket identity"));
-        virObjectUnlock(sock);
-        return -1;
+        goto cleanup;
     }

     if (cr.cr_version != XUCRED_VERSION) {
         virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
                        _("Failed to get valid client socket identity"));
-        return -1;
+        goto cleanup;
     }

-    if (cr.cr_ngroups == 0) {
+    if (cr.cr_ngroups <= 0 || cr.cr_ngroups > NGROUPS) {
         virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
                        _("Failed to get valid client socket identity
groups"));
-        return -1;
+        goto cleanup;
     }

     /* PID and process creation time are not supported on BSDs */
@@ -1201,8 +1203,11 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
     *uid = cr.cr_uid;
     *gid = cr.cr_gid;

+    ret = 0;
+
+cleanup:
     virObjectUnlock(sock);
-    return 0;
+    return ret;
 }
 #else
 int virNetSocketGetUNIXIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED,
-- 
1.8.3.1


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131017/6fcfe333/attachment-0001.sig>