[PATCH] agent: Send pinentry the uid of connecting process where possible.

Daniel Kahn Gillmor dkg at fifthhorseman.net
Wed Oct 18 01:45:36 CEST 2017


* agent/agent.h (server_control_s): Add field 'client_uid'.
* agent/call-pinentry.c (start_pinentry): Add uid field to assuan
option "owner" sent to pinentry.
* agent/command-ssh.c (peer_info_s): New static struct.
(get_client_pid): Rename to...
(get_client_info): Here, and extract uid in addition to pid.
(start_command_handler_ssh): Use get_client_info() instead of
get_client_pid().
* agent/command.c (start_command_handler): Use assuan_get_peercred
instead of assuan_get_pid.

--

This also requires an update to pinentry to handle the new uid field.
Distributing the uid as well as the pid makes it harder for a
different user on the same machine to take advantage of any race
conditions between when a requesting process might ask for something
that needs pinentry, and when pinentry gets around to inspecting the
state of that process.

We put the uid before the nodename because the uid is guaranteed to be
a integer (represented in decimal), which makes it much simpler to
parse past than the potentially arbitrarily structured nodename.

If we were willing to depend on the nodename being
whitespace-delimited (as the current, unreleased pinentry code does),
then we could add the uid after the nodename.  But since no released
pinentry depends on this option anyway, i think we should make the
more conservative, easily-parseable choice and put the user ID first.

Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
---
 agent/agent.h         |  1 +
 agent/call-pinentry.c |  5 +++--
 agent/command-ssh.c   | 29 +++++++++++++++++++++++------
 agent/command.c       | 23 +++++++++++++++++------
 4 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/agent/agent.h b/agent/agent.h
index f5df75e6e..af64f335e 100644
--- a/agent/agent.h
+++ b/agent/agent.h
@@ -226,6 +226,7 @@ struct server_control_s
   char *lc_ctype;
   char *lc_messages;
   unsigned long client_pid;
+  int client_uid;
 
   /* The current pinentry mode.  */
   pinentry_mode_t pinentry_mode;
diff --git a/agent/call-pinentry.c b/agent/call-pinentry.c
index 6a5c1fe1e..35766b6b1 100644
--- a/agent/call-pinentry.c
+++ b/agent/call-pinentry.c
@@ -593,8 +593,9 @@ start_pinentry (ctrl_t ctrl)
         nodename = utsbuf.nodename;
 #endif /*!HAVE_W32_SYSTEM*/
 
-      if ((optstr = xtryasprintf ("OPTION owner=%lu %s",
-                                  ctrl->client_pid, nodename)))
+      if ((optstr = xtryasprintf ("OPTION owner=%lu %d %s",
+                                  ctrl->client_pid, ctrl->client_uid,
+                                  nodename)))
         {
           assuan_transact (entry_ctx, optstr, NULL, NULL, NULL, NULL, NULL,
                            NULL);
diff --git a/agent/command-ssh.c b/agent/command-ssh.c
index 9d45a1864..71f311845 100644
--- a/agent/command-ssh.c
+++ b/agent/command-ssh.c
@@ -255,6 +255,11 @@ static gpg_error_t ssh_signature_encoder_eddsa (ssh_key_type_spec_t *spec,
 static gpg_error_t ssh_key_extract_comment (gcry_sexp_t key, char **comment);
 
 
+struct peer_info_s
+{
+  unsigned long pid;
+  int uid;
+};
 
 /* Global variables.  */
 
@@ -3581,10 +3586,12 @@ ssh_request_process (ctrl_t ctrl, estream_t stream_sock)
 
 
 /* Return the peer's pid.  */
-static unsigned long
-get_client_pid (int fd)
+static struct peer_info_s
+get_client_info (int fd)
 {
-  pid_t client_pid = (pid_t)0;
+  struct peer_info_s out;
+  pid_t client_pid = (pid_t)(-1);
+  uid_t client_uid = (uid_t)-1;
 
 #ifdef SO_PEERCRED
   {
@@ -3599,8 +3606,10 @@ get_client_pid (int fd)
       {
 #if defined (HAVE_STRUCT_SOCKPEERCRED_PID) || defined (HAVE_STRUCT_UCRED_PID)
         client_pid = cr.pid;
+        client_uid = cr.uid;
 #elif defined (HAVE_STRUCT_UCRED_CR_PID)
         client_pid = cr.cr_pid;
+        client_pid = cr.cr_uid;
 #else
 #error "Unknown SO_PEERCRED struct"
 #endif
@@ -3611,6 +3620,7 @@ get_client_pid (int fd)
     socklen_t len = sizeof (pid_t);
 
     getsockopt (fd, SOL_LOCAL, LOCAL_PEERPID, &client_pid, &len);
+    getsockopt (fd, SOL_LOCAL, LOCAL_PEERUID, &client_uid, &len);
   }
 #elif defined (LOCAL_PEEREID)
   {
@@ -3619,6 +3629,7 @@ get_client_pid (int fd)
 
     if (getsockopt (fd, 0, LOCAL_PEEREID, &unp, &unpl) != -1)
       client_pid = unp.unp_pid;
+      client_uid = unp.unp_euid;
   }
 #elif defined (HAVE_GETPEERUCRED)
   {
@@ -3626,7 +3637,8 @@ get_client_pid (int fd)
 
     if (getpeerucred (fd, &ucred) != -1)
       {
-        client_pid= ucred_getpid (ucred);
+        client_pid = ucred_getpid (ucred);
+	client_uid = ucred_geteuid (ucred);
         ucred_free (ucred);
       }
   }
@@ -3634,7 +3646,9 @@ get_client_pid (int fd)
   (void)fd;
 #endif
 
-  return (unsigned long)client_pid;
+  out.pid = (client_pid == (pid_t)(-1)? 0 : (unsigned long)client_pid);
+  out.uid = (int)client_uid;
+  return out;
 }
 
 
@@ -3645,12 +3659,15 @@ start_command_handler_ssh (ctrl_t ctrl, gnupg_fd_t sock_client)
   estream_t stream_sock = NULL;
   gpg_error_t err;
   int ret;
+  struct peer_info_s peer_info;
 
   err = agent_copy_startup_env (ctrl);
   if (err)
     goto out;
 
-  ctrl->client_pid = get_client_pid (FD2INT(sock_client));
+  peer_info = get_client_info (FD2INT(sock_client));
+  ctrl->client_pid = peer_info.pid;
+  ctrl->client_uid = peer_info.uid;
 
   /* Create stream from socket.  */
   stream_sock = es_fdopen (FD2INT(sock_client), "r+");
diff --git a/agent/command.c b/agent/command.c
index fd39c680f..de7ba2051 100644
--- a/agent/command.c
+++ b/agent/command.c
@@ -3328,7 +3328,7 @@ start_command_handler (ctrl_t ctrl, gnupg_fd_t listen_fd, gnupg_fd_t fd)
 
   for (;;)
     {
-      pid_t client_pid;
+      assuan_peercred_t client_creds;
 
       rc = assuan_accept (ctx);
       if (gpg_err_code (rc) == GPG_ERR_EOF || rc == -1)
@@ -3341,12 +3341,23 @@ start_command_handler (ctrl_t ctrl, gnupg_fd_t listen_fd, gnupg_fd_t fd)
           break;
         }
 
-      client_pid = assuan_get_pid (ctx);
-      ctrl->server_local->connect_from_self = (client_pid == getpid ());
-      if (client_pid != ASSUAN_INVALID_PID)
-        ctrl->client_pid = (unsigned long)client_pid;
+      rc = assuan_get_peercred (ctx, &client_creds);
+      if (rc)
+        {
+          log_info ("Assuan get_peercred failed: %s\n", gpg_strerror (rc));
+          ctrl->client_pid = 0;
+          ctrl->client_uid = -1;
+        }
       else
-        ctrl->client_pid = 0;
+        {
+          ctrl->server_local->connect_from_self =
+            (client_creds->pid == getpid ());
+          if (client_creds->pid != ASSUAN_INVALID_PID)
+            ctrl->client_pid = (unsigned long)client_creds->pid;
+          else
+            ctrl->client_pid = 0;
+          ctrl->client_uid = client_creds->uid;
+        }
 
       rc = assuan_process (ctx);
       if (rc)
-- 
2.14.2




More information about the Gnupg-devel mailing list