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

[Werner Koch]

On Wed, 18 Oct 2017 01:45, dkg at fifthhorseman.net said:

> 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

Do you have an example on how this can happen?  Two users on the same
X-server pose a problem anyway.  However, adding a user ID does not harm.

> 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.

Well, that breaks the API between gpg-agent and Pinentry.  Becuase the
hostname is only additionl information which can't be intrepreted any
further I think this break is acceptable.  However, to be future proof I
would suggest to change it this way:

-      if ((optstr = xtryasprintf ("OPTION owner=%lu %d %s",
-                                  ctrl->client_pid, ctrl->client_uid,
-                                  nodename)))
+      if ((optstr = xtryasprintf ("OPTION owner=%lu/%d %s",
+                                  ctrl->client_pid, ctrl->client_uid,
+                                  nodename)))

The existing Pinentries would then use "/NNN HOSTNAME" for the
hostname because of the its too simple parser.  If we ever need the gid
or whatever, it can be added by using another slash as delimiter.

We also need to define how to indicate "UID not available". "-1" might
be okay but an emtpy string like in "/PID// HOSTNAME" avoids the need to
parse negative numbers.


> -static unsigned long
> -get_client_pid (int fd)
> +static struct peer_info_s
> +get_client_info (int fd)
>  {

Please don't return a structure but pass it by reference.


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

assuan_get_peercred may actually fail (e.g. "gpg-agent --server" or
certain platforms).  There should be a fallback to  assuan_get_pid in
this case.

I have not looked closely at the Pinentry patch but adjusted to the here
discussed changes it seems okay.


Salam-Shalom,

   Werner

-- 
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.gnupg.org/pipermail/gnupg-devel/attachments/20171018/9c846e17/attachment.sig>