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@redhat.com>

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index e8cdfa6435020d1b9472913795a9a6cf5079ff38..2e50f8ccafaa1322d532d2ac667a6af657f801a9 100644 (file)
--- 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,