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

RFC: [PATCH] SCM_CREDS support



Hi,

New patch, now receiver centric, as requested for SCM_CREDS support for
GNU/Hurd: scm_creds_sendmsg.c.diff and scm_cresd_recvmsg.c.

(previous second patch, (updated scm_creds-sendmsg.c_2.diff) for the
-n option, might not be used, in that case the first attached
(scm_creds-sendmsg.c.diff) patch will change accordingly.

We are now checking authorization on the receive side. There are still
some FIXMEs that need to be resolved.  Maybe som more cleanup and
locking, etc is needed. Additionally, maybe sending two ports to
the receiver with the corresponding check on the receiver side is not
enough security, therefore this RFC. 

I've been running two kvm-images with eglibc built with this patch for
a few days now, and haven't seen any problems so far. On the list of
things running properly with SCM_CREDS support are:

- All 11 gamin tests OK
- All 15+2 dbus tests OK (with upstream patches)
- 171+ of 182 glib2.0 tests OK (with upstream patches)
- Many more window managers, web browsers and X applications work
- emacs works in X (when built with patched eglibc)
- gdm3 builds (to be tested) - ...

This patch implements the first cases in the test code sent to the list
in September 2013, options <no flag> (and -z), see
http://lists.debian.org/debian-hurd/2013/09/msg00034.html 

Option -z (used by dbus) is no longer a special case, same behaviour as
without.

Options:
no flag: 
-z: send a zero credentials byte as data (used by dbus)
(no difference with w/o in the current implementation)

1) Sent credentials and PID are correct:
a) ./scm_cred_recv &
./scm_cred_send
Linux: OK, only sent IDs
kFreeBSD: OK, sent IDs + groups (irrespective of sent IDs)
Hurd: OK,same as kfreebsd
see /usr/include/i386-gnu/bits/socket.h:
/* Structure delivered by SCM_CREDS.  This describes the identity of the
   sender of the data simultaneously received on the socket.  By BSD
   convention, this is included only when a sender on a AF_LOCAL socket
   sends cmsg data of this type and size; the sender's structure is
   ignored, and the system fills in the various IDs of the sender process.  */
struct cmsgcred
...

b)./scm_cred_recv -z&
./scm_cred_send -z
Linux: OK, same as above
kFreeBSD: OK, same as above
Hurd: OK, same as above

2) When wrong credentials are sent the behaviour is different:
Linux:
no flag: ERROR [EPERM Operation not permitted] sendmsg
-z: ERROR [EPERM Operation not permitted] sendmsg

kFreeBSD: 
no flag: sent credentials are not honoured, received ones are created
-z: sent credentials are not honoured, received ones are created

Hurd:
same as kfreebsd

--- a/sysdeps/mach/hurd/sendmsg.c
+++ b/sysdeps/mach/hurd/sendmsg.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2001,2002,2004,2010 Free Software Foundation, Inc.
+/* Copyright (C) 2001,2002,2004,2010,2013 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -17,6 +17,7 @@
 
 #include <errno.h>
 #include <string.h>
+#include <unistd.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 
@@ -38,6 +39,9 @@
   int *fds, nfds;
   struct sockaddr_un *addr = message->msg_name;
   socklen_t addr_len = message->msg_namelen;
+  struct cmsgcred *ucredp = NULL;
+  void *control = message->msg_control;
+  socklen_t controllen = message->msg_controllen;
   addr_port_t aport = MACH_PORT_NULL;
   union
   {
@@ -106,6 +110,33 @@
 	}
     }
 
+  /* SCM_CREDS support: Create and send the ancillary data  */
+  cmsg = CMSG_FIRSTHDR (message);
+  if (cmsg != NULL && cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_CREDS)
+    {
+      for (; cmsg; cmsg = CMSG_NXTHDR (message, cmsg))
+	{
+	  ucredp = (struct cmsgcred *) CMSG_DATA(cmsg);
+	  process_t proc = getproc ();
+	  auth_t auth = getauth ();
+	  nports = 2;
+	  ports = __alloca (nports * sizeof (mach_port_t));
+	  ports[0] = proc;
+	  ports[1] = auth;
+
+	  /* Fill in credentials data */
+	  ucredp->cmcred_pid = __getpid();
+	  ucredp->cmcred_uid = __getuid();
+	  ucredp->cmcred_euid = __geteuid();
+	  ucredp->cmcred_gid = __getgid();
+	  /* egid not in struct csmgcred */
+	  ucredp->cmcred_ngroups =
+	    __getgroups (sizeof (ucredp->cmcred_groups) / sizeof (gid_t),
+			 ucredp->cmcred_groups);
+	}
+      goto label;
+    }
+
   /* SCM_RIGHTS support: get the number of fds to send.  */
   cmsg = CMSG_FIRSTHDR (message);
   for (; cmsg; cmsg = CMSG_NXTHDR (message, cmsg))
@@ -147,6 +178,7 @@
 	}
     }
 
+ label:
   if (addr)
     {
       if (addr->sun_family == AF_LOCAL)
@@ -188,8 +220,8 @@
 						   ports,
 						   MACH_MSG_TYPE_COPY_SEND,
 						   nports,
-						   message->msg_control,
-						   message->msg_controllen,
+						   control,
+						   controllen,
 						   &amount);
 			      __mach_port_deallocate (__mach_task_self (),
 						      aport);
--- a/sysdeps/mach/hurd/recvmsg.c
+++ b/sysdeps/mach/hurd/recvmsg.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2001, 2002, 2010 Free Software Foundation, Inc.
+/* Copyright (C) 2001, 2002, 2010, 2013 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -18,10 +18,12 @@
 #include <errno.h>
 #include <string.h>
 #include <sys/socket.h>
+#include <unistd.h>
 
 #include <hurd.h>
 #include <hurd/fd.h>
 #include <hurd/socket.h>
+#include <hurd/id.h>
 
 /* Receive a message as described by MESSAGE from socket FD.
    Returns the number of bytes read or -1 for errors.  */
@@ -155,6 +157,104 @@
     message->msg_controllen = clen;
   memcpy (message->msg_control, cdata, message->msg_controllen);
 
+  error_t check_auth (process_t proc, auth_t auth, __pid_t pid,
+		      __uid_t euid, __uid_t auid,
+		      __gid_t egid, __gid_t agid,
+		      int ngroups, __gid_t *groups)
+  {
+    error_t err;
+    pid_t rpid, rppid;
+    int orphaned;
+    uid_t euids_buf[CMGROUP_MAX], auids_buf[CMGROUP_MAX];
+    uid_t *euids = euids_buf, *auids = auids_buf;
+    gid_t egids_buf[CMGROUP_MAX], agids_buf[CMGROUP_MAX];
+    gid_t *egids = egids_buf, *agids = agids_buf;
+    size_t neuids = CMGROUP_MAX, nauids = CMGROUP_MAX;
+    size_t negids = CMGROUP_MAX, nagids = CMGROUP_MAX;
+
+    /* FIXME: In this the right lock? */
+    /* FIXME: EPERM and/or EACCESS? */
+    HURD_CRITICAL_BEGIN;
+    __mutex_lock (&_hurd_id.lock);
+    err = _hurd_check_ids();
+    if (err)
+      goto finish;
+
+    err = __USEPORT (PROC, __proc_getpids (proc, &rpid, &rppid, &orphaned));
+    if (err)
+      goto finish;
+    if (rpid != pid )
+      {
+	err = EPERM;
+	goto finish;
+      }
+
+    err = __USEPORT (AUTH, __auth_getids
+		     (auth,
+		      &euids, &neuids, &auids, &nauids,
+		      &egids, &negids, &agids, &nagids));
+    if (err)
+      goto finish;
+
+    /* Check ids */
+    if (euid != euids_buf[0] || auid != auids_buf[0] ||
+        egid != egids_buf[0] || agid != agids_buf[0])
+      {
+	err = EPERM;
+	goto finish;
+      }
+    /* Check groups */
+    for (int i = 0; i < negids; i++)
+      {
+	/* FIXME: is sorted check needed? */
+	if (groups[i] != egids_buf[i])
+	  {
+	    err = EPERM;
+	    goto finish;
+	  }
+      }
+
+  finish:
+    __mutex_unlock (&_hurd_id.lock);
+    HURD_CRITICAL_END;
+    return err;
+  }
+
+  /* SCM_CREDS support: Fill in the credentials data  */
+  struct cmsgcred *ucredp = NULL;
+  __pid_t pid = 0;
+  __uid_t euid = 0, auid = 0;
+  __gid_t egid = 0, agid = 0;
+  int ngroups;
+  __gid_t groups[CMGROUP_MAX];
+
+  cmsg = CMSG_FIRSTHDR (message);
+  /* Read received credentials */
+  if (cmsg != NULL && cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_CREDS)
+    {
+      for (; cmsg; cmsg = CMSG_NXTHDR (message, cmsg))
+	{
+	  ucredp = (struct cmsgcred *) CMSG_DATA(cmsg);
+	  pid = ucredp->cmcred_pid;
+	  auid = ucredp->cmcred_uid;
+	  euid = ucredp->cmcred_euid;
+	  agid = ucredp->cmcred_gid;
+	  egid = agid; /* Not in struct cmsgcred */
+	  ngroups = ucredp->cmcred_ngroups;
+	  for (int i = 0; i < ngroups ; i++)
+	    groups[i] = ucredp->cmcred_groups[i];
+	}
+
+      /* Check recieved credentials */
+      err = check_auth (ports[0], ports[1], pid,
+			euid, auid, egid, agid, ngroups, groups);
+      __mach_port_deallocate (mach_task_self (), ports[0]);
+      __mach_port_deallocate (mach_task_self (), ports[1]);
+      if (err)
+	return __hurd_fail (err);
+      goto label;
+    }
+
   /* SCM_RIGHTS ports.  */
   if (nports > 0)
     {
@@ -234,6 +334,7 @@
 	}
     }
 
+ label:
   __vm_deallocate (__mach_task_self (), (vm_address_t) cdata, clen);
 
   return (buf - data);

Reply to: