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

* 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>
This commit is contained in:
Daniel Kahn Gillmor 2017-02-05 02:12:25 -05:00
parent 995c46ea77
commit 28aa689058
4 changed files with 38 additions and 13 deletions

View 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;

View 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);

View 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+");

View 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)