agent: Send pinentry the uid of connecting process where possible.
authorDaniel Kahn Gillmor <dkg@fifthhorseman.net>
Sun, 5 Feb 2017 07:12:25 +0000 (02:12 -0500)
committerDaniel Kahn Gillmor <dkg@fifthhorseman.net>
Thu, 19 Oct 2017 07:09:44 +0000 (03:09 -0400)
* 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): Try assuan_get_peercred,
and only fall back to assuan_get_pid when assuan_get_peercred fails.

--

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.

Use a / instead of whitespace to delimit pid/uid at Werner's request.

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@fifthhorseman.net>
agent/agent.h
agent/call-pinentry.c
agent/command-ssh.c
agent/command.c

index f5df75e..af64f33 100644 (file)
@@ -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;
index 6a5c1fe..98af95a 100644 (file)
@@ -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);
index 9d45a18..866f439 100644 (file)
@@ -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,11 @@ ssh_request_process (ctrl_t ctrl, estream_t stream_sock)
 
 
 /* Return the peer's pid.  */
-static unsigned long
-get_client_pid (int fd)
+static void
+get_client_info (int fd, struct peer_info_s *out)
 {
-  pid_t client_pid = (pid_t)0;
+  pid_t client_pid = (pid_t)(-1);
+  uid_t client_uid = (uid_t)-1;
 
 #ifdef SO_PEERCRED
   {
@@ -3599,8 +3605,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 +3619,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 +3628,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 +3636,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 +3645,8 @@ 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;
 }
 
 
@@ -3645,12 +3657,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));
+  get_client_info (FD2INT(sock_client), &peer_info);
+  ctrl->client_pid = peer_info.pid;
+  ctrl->client_uid = peer_info.uid;
 
   /* Create stream from socket.  */
   stream_sock = es_fdopen (FD2INT(sock_client), "r+");
index fd39c68..4016cc2 100644 (file)
@@ -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,20 @@ 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));
+          client_creds->pid = assuan_get_pid (ctx);
+          ctrl->client_uid = -1;
+        }
+      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)