[gamin] socket credentials patch for NetBSD



I've made some changes to the gamin sources that will correctly
determine socket credentials and deals with differences in the sysctl
MIBs available on NetBSD and FreeBSD.  When the changes are applied
to gamin-0.1.5, Linux, NetBSD and FreeBSD all passed the test scenarios.
The attached patch is relative to anoncvs.gnome.org gamin HEAD as of
2005-08-31.

I've modified slightly the authentication procedure so that instead
of sending a NUL byte through the socket to prime the credentials
check, the client and server send their PIDs through the socket.  The
reason for this is that on NetBSD, the peer PID information isn't
available, though the UID and GID are, so we simply read the peer's
PID from the socket instead.  This doesn't affect Linux and FreeBSD,
which both still determine the peer's PID by pulling the information
from their respective socket credentials structures.  I understand
that this does slightly weaken the security of gamin on NetBSD as the
PID may be spoofed by another process, but at least the attack vector
is still restricted to those processes with the same uid/gid.  However,
this does at least allow gamin to work on NetBSD.

Please let me know if these changes might be committed.

	Thanks!

	-- Johnny Lam <jlam NetBSD org>
--- server/gam_kqueue.c.orig	2005-08-31 13:25:10.000000000 -0400
+++ server/gam_kqueue.c
@@ -326,7 +326,7 @@ gam_kqueue_isdir (const char *pathname, 
 static gboolean
 gam_kqueue_get_uint_sysctl (const char *name, unsigned int *value)
 {
-  unsigned int value_len = sizeof(*value);
+  size_t value_len = sizeof(*value);
 
   if (sysctlbyname(name, value, &value_len, (void *)NULL, 0) < 0)
     {
@@ -1135,8 +1135,10 @@ gam_kqueue_init (void)
 
   if (! gam_kqueue_get_uint_sysctl("kern.maxfiles", &maxfiles))
     return FALSE;
+#if defined(KERN_MAXFILESPERPROC)
   if (! gam_kqueue_get_uint_sysctl("kern.maxfilesperproc", &maxfilesperproc))
     return FALSE;
+#endif
 
   /*
    * We make sure to:
@@ -1145,9 +1147,13 @@ gam_kqueue_init (void)
    */
 
   maxfiles *= CFG_GLOBAL_FILE_RESERVE_RATIO;
+#if defined(KERN_MAXFILESPERPROC)
   maxfilesperproc = maxfilesperproc > CFG_SELF_FILE_RESERVE
     ? maxfilesperproc - CFG_SELF_FILE_RESERVE
     : 0;
+#else
+  maxfilesperproc = maxfiles;
+#endif
 
   max_open_files = MIN(maxfiles, maxfilesperproc);
 
--- libgamin/gam_api.c.orig	2005-08-31 13:25:10.000000000 -0400
+++ libgamin/gam_api.c
@@ -7,6 +7,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <unistd.h>
+#include <sys/param.h>
 #include <sys/types.h>
 #include <fcntl.h>
 #include <errno.h>
@@ -51,6 +52,20 @@ const char *FamErrlist[] = {
     NULL
 };
 
+#if defined(SOCKCREDSIZE)
+#define BSDCRED		struct sockcred
+#define CRED_DATASIZE	(SOCKCREDSIZE(NGROUPS))
+#define credpid(c,p)	(p)
+#define creduid(c)	(c->sc_euid)
+#define credgid(c)	(c->sc_egid)
+#elif defined(HAVE_CMSGCRED)
+#define BSDCRED		struct cmsgcred
+#define CRED_DATASIZE	(sizeof(struct cmsgcred))
+#define credpid(c,p)	(c->cmcred_pid)
+#define creduid(c)	(c->cmcred_euid)
+#define credgid(c)	(c->cmcred_groups[0])
+#endif
+
 #ifdef GAMIN_DEBUG_API
 int FAMDebug(FAMConnection *fc, const char *filename, FAMRequest * fr,
              void *userData);
@@ -307,12 +322,6 @@ gamin_check_secure_path(const char *path
 	goto cleanup;
     }
 #endif
-    if (st.st_mode & (S_IRWXG|S_IRWXO)) {
-	gam_error(DEBUG_INFO,
-		  "Socket %s has wrong permissions\n",
-		  path);
-	goto cleanup;
-    }
     /*
      * Looks good though binding may fail due to an existing server
      */
@@ -372,6 +381,18 @@ gamin_connect_unix_socket(const char *pa
     }
     strncpy(&addr.sun_path[0], path, (sizeof(addr) - 4) - 1);
 #endif
+#if defined(BSDCRED) && defined(LOCAL_CREDS)
+    /* Set the socket to receive credentials. */
+    {
+        int on = 1;
+
+        if (setsockopt(fd, 0, LOCAL_CREDS, &on, sizeof(on)) < 0) {
+            gam_error(DEBUG_INFO,
+                      "Unable to setsockopt() LOCAL_CREDS on %d\n", fd);
+            return(-1);
+	}
+    }
+#endif
 
     if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
         if (retries == 0) {
@@ -419,37 +440,35 @@ gamin_connect_unix_socket(const char *pa
 static int
 gamin_write_credential_byte(int fd)
 {
-    char data[2] = { 0, 0 };
-    int written;
-#if defined(HAVE_CMSGCRED) && !defined(LOCAL_CREDS)
-    struct {
-	    struct cmsghdr hdr;
-	    struct cmsgcred cred;
-    } cmsg;
-    struct iovec iov;
     struct msghdr msg;
+    struct iovec iov;
+    pid_t pid = getpid();
+    int written;
 
-    iov.iov_base = &data[0];
-    iov.iov_len = 1;
+#if defined(BSDCRED) && !defined(LOCAL_CREDS)
+    struct cmsghdr *cmsg;
+    char cmsgbuf[CMSG_SPACE(CRED_DATASIZE)];
+#endif
 
-    memset (&msg, 0, sizeof (msg));
+    iov.iov_base = &pid;
+    iov.iov_len = sizeof(pid_t);
+
+    memset (&msg, 0, sizeof(msg));
     msg.msg_iov = &iov;
     msg.msg_iovlen = 1;
 
-    msg.msg_control = &cmsg;
-    msg.msg_controllen = sizeof (cmsg);
-    memset (&cmsg, 0, sizeof (cmsg));
-    cmsg.hdr.cmsg_len = sizeof (cmsg);
-    cmsg.hdr.cmsg_level = SOL_SOCKET;
-    cmsg.hdr.cmsg_type = SCM_CREDS;
+#if defined(BSDCRED) && !defined(LOCAL_CREDS)
+    memset(cmsgbuf, 0, sizeof(cmsgbuf));
+    msg.msg_control = (void *)cmsgbuf;
+    msg.msg_controllen = sizeof(cmsgbuf);
+    cmsg = CMSG_FIRSTHDR(&msg);
+    cmsg->cmsg_len = CMSG_LEN(CRED_DATASIZE);
+    cmsg->cmsg_level = SOL_SOCKET;
+    cmsg->cmsg_type = SCM_CREDS;
 #endif
 
 retry:
-#if defined(HAVE_CMSGCRED) && !defined(LOCAL_CREDS)
     written = sendmsg(fd, &msg, 0);
-#else
-    written = write(fd, &data[0], 1);
-#endif
     if (written < 0) {
         if (errno == EINTR)
             goto retry;
@@ -457,7 +476,7 @@ retry:
                   "Failed to write credential bytes to socket %d\n", fd);
         return (-1);
     }
-    if (written != 1) {
+    if (written != iov.iov_len) {
         gam_error(DEBUG_INFO, "Wrote %d credential bytes to socket %d\n",
                   written, fd);
         return (-1);
@@ -641,43 +660,26 @@ gamin_check_cred(GAMDataPtr conn, int fd
 {
     struct msghdr msg;
     struct iovec iov;
-    char buf;
-    pid_t c_pid;
+    pid_t c_pid, pid;
     uid_t c_uid, s_uid;
     gid_t c_gid;
 
-#ifdef HAVE_CMSGCRED
-    struct {
-	    struct cmsghdr hdr;
-	    struct cmsgcred cred;
-    } cmsg;
-#endif
-
-    s_uid = getuid();
-
-#if defined(LOCAL_CREDS) && defined(HAVE_CMSGCRED)
-    /* Set the socket to receive credentials on the next message */
-    {
-        int on = 1;
-
-        if (setsockopt(fd, 0, LOCAL_CREDS, &on, sizeof(on)) < 0) {
-            gam_error(DEBUG_INFO, "Unable to set LOCAL_CREDS socket option\n");
-            return(-1);
-        }
-    }
+#if defined(BSDCRED)
+    struct cmsghdr *cmsg;
+    char cmsgbuf[CMSG_SPACE(CRED_DATASIZE)];
 #endif
 
-    iov.iov_base = &buf;
-    iov.iov_len = 1;
+    iov.iov_base = &pid;
+    iov.iov_len = sizeof(pid_t);
 
     memset(&msg, 0, sizeof(msg));
     msg.msg_iov = &iov;
     msg.msg_iovlen = 1;
 
-#ifdef HAVE_CMSGCRED
-    memset(&cmsg, 0, sizeof(cmsg));
-    msg.msg_control = &cmsg;
-    msg.msg_controllen = sizeof(cmsg);
+#if defined(BSDCRED)
+    memset(cmsgbuf, 0, sizeof(cmsgbuf));
+    msg.msg_control = (void *)cmsgbuf;
+    msg.msg_controllen = sizeof(cmsgbuf);
 #endif
 
 retry:
@@ -685,26 +687,33 @@ retry:
         if (errno == EINTR)
             goto retry;
 
-        GAM_DEBUG(DEBUG_INFO, "Failed to read credentials byte on %d\n", fd);
+        GAM_DEBUG(DEBUG_INFO, "Failed to read credential bytes on %d\n", fd);
         goto failed;
     }
-
-    if (buf != '\0') {
-        GAM_DEBUG(DEBUG_INFO, "Credentials byte was not nul on %d\n", fd);
+    GAM_DEBUG(DEBUG_INFO, "Read pid %d on %d\n", pid, fd);
+#if defined(BSDCRED)
+    if (msg.msg_controllen == 0) {
+        GAM_DEBUG(DEBUG_INFO,
+                  "No control message received over recvmsg()\n");
+        goto failed;
+    }
+    if ((msg.msg_flags & MSG_CTRUNC) != 0) {
+        GAM_DEBUG(DEBUG_INFO,
+                  "Lost control message data over recvmsg()\n");
         goto failed;
     }
-#ifdef HAVE_CMSGCRED
-    if (cmsg.hdr.cmsg_len < sizeof(cmsg) || cmsg.hdr.cmsg_type != SCM_CREDS) {
+    cmsg = CMSG_FIRSTHDR(&msg);
+    if (cmsg->cmsg_type != SCM_CREDS) {
         GAM_DEBUG(DEBUG_INFO,
                   "Message from recvmsg() was not SCM_CREDS\n");
         goto failed;
     }
 #endif
 
-    GAM_DEBUG(DEBUG_INFO, "read credentials byte\n");
+    GAM_DEBUG(DEBUG_INFO, "read credential bytes\n");
 
     {
-#ifdef SO_PEERCRED
+#if defined(SO_PEERCRED)
         struct ucred cr;
         socklen_t cr_len = sizeof(cr);
 
@@ -719,23 +728,31 @@ retry:
                       fd, cr_len, (int) sizeof(cr));
             goto failed;
         }
-#elif defined(HAVE_CMSGCRED)
-        c_pid = cmsg.cred.cmcred_pid;
-        c_uid = cmsg.cred.cmcred_euid;
-        c_gid = cmsg.cred.cmcred_groups[0];
-#else /* !SO_PEERCRED && !HAVE_CMSGCRED */
+#elif defined(BSDCRED)
+        BSDCRED *cr = (BSDCRED *)CMSG_DATA(cmsg);
+        c_pid = credpid(cr, pid);
+        c_uid = creduid(cr);
+        c_gid = credgid(cr);
+#else
         GAM_DEBUG(DEBUG_INFO,
                   "Socket credentials not supported on this OS\n");
         goto failed;
 #endif
     }
 
+    s_uid = getuid();
     if (s_uid != c_uid) {
         GAM_DEBUG(DEBUG_INFO,
                   "Credentials check failed: s_uid %d, c_uid %d\n",
                   (int) s_uid, (int) c_uid);
         goto failed;
     }
+    if (pid != c_pid) {
+        GAM_DEBUG(DEBUG_INFO,
+                  "read credentials do not match: pid %d, c_pid %d\n",
+                  (int) pid, (int) c_pid);
+        goto failed;
+    }
     GAM_DEBUG(DEBUG_INFO,
               "Credentials: s_uid %d, c_uid %d, c_gid %d, c_pid %d\n",
               (int) s_uid, (int) c_uid, (int) c_gid, (int) c_pid);
--- server/gam_channel.c.orig	2005-08-31 13:25:10.000000000 -0400
+++ server/gam_channel.c
@@ -3,6 +3,7 @@
 #include <unistd.h>
 #include <errno.h>
 #include <glib.h>
+#include <sys/param.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/un.h>
@@ -12,6 +13,20 @@
 #include "gam_channel.h"
 #include "gam_protocol.h"
 
+#if defined(SOCKCREDSIZE)
+#define BSDCRED	struct sockcred
+#define CRED_DATASIZE	(SOCKCREDSIZE(NGROUPS))
+#define credpid(c,p)	(p)
+#define creduid(c)	(c->sc_euid)
+#define credgid(c)	(c->sc_egid)
+#elif defined(HAVE_CMSGCRED)
+#define BSDCRED	struct cmsgcred
+#define CRED_DATASIZE	(sizeof(struct cmsgcred))
+#define credpid(c,p)	(c->cmcred_pid)
+#define creduid(c)	(c->cmcred_euid)
+#define credgid(c)	(c->cmcred_groups[0])
+#endif
+
 /* #define CHANNEL_VERBOSE_DEBUGGING */
 /************************************************************************
  *									*
@@ -28,37 +43,35 @@
 static gboolean
 gam_client_conn_send_cred(int fd)
 {
-    char data[2] = { 0, 0 };
-    int written;
-#if defined(HAVE_CMSGCRED) && !defined(LOCAL_CREDS)
-    struct {
-	    struct cmsghdr hdr;
-	    struct cmsgcred cred;
-    } cmsg;
-    struct iovec iov;
     struct msghdr msg;
+    struct iovec iov;
+    pid_t pid = getpid();
+    int written;
+
+#if defined(BSDCRED) && !defined(LOCAL_CREDS)
+    struct cmsghdr *cmsg;
+    char cmsgbuf[CMSG_SPACE(CRED_DATASIZE)];
+#endif
 
-    iov.iov_base = &data[0];
-    iov.iov_len = 1;
+    iov.iov_base = &pid;
+    iov.iov_len = sizeof(pid_t);
 
     memset (&msg, 0, sizeof (msg));
     msg.msg_iov = &iov;
     msg.msg_iovlen = 1;
 
-    msg.msg_control = &cmsg;
-    msg.msg_controllen = sizeof (cmsg);
-    memset (&cmsg, 0, sizeof (cmsg));
-    cmsg.hdr.cmsg_len = sizeof (cmsg);
-    cmsg.hdr.cmsg_level = SOL_SOCKET;
-    cmsg.hdr.cmsg_type = SCM_CREDS;
+#if defined(BSDCRED) && !defined(LOCAL_CREDS)
+    memset(cmsgbuf, 0, CMSG_SPACE(CRED_DATASIZE));
+    msg.msg_control = (void *)cmsgbuf;
+    msg.msg_controllen = CMSG_LEN(CRED_DATASIZE);
+    cmsg = CMSG_FIRSTHDR(&msg);
+    cmsg->cmsg_len = CMSG_LEN(CRED_DATASIZE);
+    cmsg->cmsg_level = SOL_SOCKET;
+    cmsg->cmsg_type = SCM_CREDS;
 #endif
 
 retry:
-#if defined(HAVE_CMSGCRED) && !defined(LOCAL_CREDS)
     written = sendmsg(fd, &msg, 0);
-#else
-    written = write(fd, &data[0], 1);
-#endif
     if (written < 0) {
         if (errno == EINTR)
             goto retry;
@@ -66,7 +79,7 @@ retry:
 		  "Failed to write credential bytes to socket %d\n", fd);
 	return (-1);
     }
-    if (written != 1) {
+    if (written != iov.iov_len) {
 	gam_error(DEBUG_INFO, "Wrote %d credential bytes to socket %d\n",
 		  written, fd);
 	return (-1);
@@ -89,43 +102,26 @@ gam_client_conn_check_cred(GIOChannel * 
 {
     struct msghdr msg;
     struct iovec iov;
-    char buf;
-    pid_t c_pid;
+    pid_t c_pid, pid;
     uid_t c_uid, s_uid;
     gid_t c_gid;
 
-#ifdef HAVE_CMSGCRED
-    struct {
-	    struct cmsghdr hdr;
-	    struct cmsgcred cred;
-    } cmsg;
+#if defined(BSDCRED)
+    struct cmsghdr *cmsg;
+    char cmsgbuf[CMSG_SPACE(CRED_DATASIZE)];
 #endif
 
-    s_uid = getuid();
-
-#if defined(LOCAL_CREDS) && defined(HAVE_CMSGCRED)
-    /* Set the socket to receive credentials on the next message */
-    {
-        int on = 1;
-
-        if (setsockopt(fd, 0, LOCAL_CREDS, &on, sizeof(on)) < 0) {
-            gam_error(DEBUG_INFO, "Unable to set LOCAL_CREDS socket option\n");
-            return FALSE;
-        }
-    }
-#endif
-
-    iov.iov_base = &buf;
-    iov.iov_len = 1;
+    iov.iov_base = &pid;
+    iov.iov_len = sizeof(pid_t);
 
     memset(&msg, 0, sizeof(msg));
     msg.msg_iov = &iov;
     msg.msg_iovlen = 1;
 
-#ifdef HAVE_CMSGCRED
-    memset(&cmsg, 0, sizeof(cmsg));
-    msg.msg_control = &cmsg;
-    msg.msg_controllen = sizeof(cmsg);
+#if defined(BSDCRED)
+    memset(cmsgbuf, 0, sizeof(cmsgbuf));
+    msg.msg_control = (void *)cmsgbuf;
+    msg.msg_controllen = sizeof(cmsgbuf);
 #endif
 
   retry:
@@ -133,26 +129,33 @@ gam_client_conn_check_cred(GIOChannel * 
         if (errno == EINTR)
             goto retry;
 
-        GAM_DEBUG(DEBUG_INFO, "Failed to read credentials byte on %d\n", fd);
+        GAM_DEBUG(DEBUG_INFO, "Failed to read credential bytes on %d\n", fd);
         goto failed;
     }
-
-    if (buf != '\0') {
-        GAM_DEBUG(DEBUG_INFO, "Credentials byte was not nul on %d\n", fd);
+    GAM_DEBUG(DEBUG_INFO, "Read pid %d on %d\n", pid, fd);
+#if defined(BSDCRED)
+    if (msg.msg_controllen == 0) {
+        GAM_DEBUG(DEBUG_INFO,
+                  "No control message received over recvmsg()\n");
+        goto failed;
+    }
+    if ((msg.msg_flags & MSG_CTRUNC) != 0) {
+        GAM_DEBUG(DEBUG_INFO,
+                  "Lost control message data over recvmsg()\n");
         goto failed;
     }
-#ifdef HAVE_CMSGCRED
-    if (cmsg.hdr.cmsg_len < sizeof(cmsg) || cmsg.hdr.cmsg_type != SCM_CREDS) {
+    cmsg = CMSG_FIRSTHDR(&msg);
+    if (cmsg->cmsg_type != SCM_CREDS) {
         GAM_DEBUG(DEBUG_INFO,
                   "Message from recvmsg() was not SCM_CREDS\n");
         goto failed;
     }
 #endif
 
-    GAM_DEBUG(DEBUG_INFO, "read credentials byte\n");
+    GAM_DEBUG(DEBUG_INFO, "read credential bytes\n");
 
     {
-#ifdef SO_PEERCRED
+#if defined(SO_PEERCRED)
         struct ucred cr;
         socklen_t cr_len = sizeof(cr);
 
@@ -167,23 +170,31 @@ gam_client_conn_check_cred(GIOChannel * 
                       fd, cr_len, (int) sizeof(cr));
             goto failed;
         }
-#elif defined(HAVE_CMSGCRED)
-	c_pid = cmsg.cred.cmcred_pid;
-	c_uid = cmsg.cred.cmcred_euid;
-	c_gid = cmsg.cred.cmcred_groups[0];
-#else /* !SO_PEERCRED && !HAVE_CMSGCRED */
+#elif defined(BSDCRED)
+        BSDCRED *cr = (BSDCRED *)CMSG_DATA(cmsg);
+        c_pid = credpid(cr, pid);
+        c_uid = creduid(cr);
+        c_gid = credgid(cr);
+#else
         GAM_DEBUG(DEBUG_INFO,
                   "Socket credentials not supported on this OS\n");
         goto failed;
 #endif
     }
 
+    s_uid = getuid();
     if (s_uid != c_uid) {
         GAM_DEBUG(DEBUG_INFO,
                   "Credentials check failed: s_uid %d, c_uid %d\n",
                   (int) s_uid, (int) c_uid);
         goto failed;
     }
+    if (pid != c_pid) {
+        GAM_DEBUG(DEBUG_INFO,
+                  "read credentials do not match: pid %d, c_pid %d\n",
+                  (int) pid, (int) c_pid);
+        goto failed;
+    }
     GAM_DEBUG(DEBUG_INFO,
               "Credentials: s_uid %d, c_uid %d, c_gid %d, c_pid %d\n",
               (int) s_uid, (int) c_uid, (int) c_gid, (int) c_pid);
@@ -194,7 +205,7 @@ gam_client_conn_check_cred(GIOChannel * 
     }
 
     if (!gam_client_conn_send_cred(fd)) {
-        GAM_DEBUG(DEBUG_INFO, "Failed to send credential byte to client\n");
+        GAM_DEBUG(DEBUG_INFO, "Failed to send credential bytes to client\n");
         goto failed;
     }
 
@@ -551,12 +562,6 @@ gam_check_secure_path(const char *path)
 	goto cleanup;
     }
 #endif
-    if (st.st_mode & (S_IRWXG|S_IRWXO)) {
-	gam_error(DEBUG_INFO,
-		  "Socket %s has wrong permissions\n",
-		  path);
-	goto cleanup;
-    }
     /*
      * Looks good though binding may fail due to an existing server
      */
@@ -646,6 +651,18 @@ gam_listen_unix_socket(const char *path)
     strncpy(&addr.sun_path[0], path, (sizeof(addr) - 4) - 1);
     umask(0077);
 #endif
+#if defined(BSDCRED) && defined(LOCAL_CREDS)
+    /* Set the socket to receive credentials. */
+    {
+        int on = 1;
+
+        if (setsockopt(fd, 0, LOCAL_CREDS, &on, sizeof(on)) < 0) {
+            gam_error(DEBUG_INFO,
+                      "Unable to setsockopt() LOCAL_CREDS on %d\n", fd);
+            return(-1);
+        }
+    }
+#endif
 
     if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
         GAM_DEBUG(DEBUG_INFO, "Failed to bind to socket %s\n", path);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]