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

Daniel Kahn Gillmor dkg at fifthhorseman.net
Thu Oct 19 09:43:10 CEST 2017


On Wed 2017-10-18 09:45:36 +0200, Werner Koch wrote:
> 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.

I don't think that's the case because of:

              if (pinentry.owner_host)
                {
                  for (endp=pinentry.owner_host; *endp && *endp != ' '; endp++)
                    ;
                  *endp = 0;
                }

So it would just take the hostname as "/NNN", not "/NNN HOSTNAME".

That said, it's easy to use a / instead of whitespace as a delimiter, so
i've adopted that suggestion.

I just don't think it's worth fretting about it too much either way
because those versions of pinentry haven't had an official release yet.

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

ok.

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

Gotcha, thanks for catching that.

I've pushed the variant of the patch that addresses all of your
suggestions here to master, as 28aa6890588cc108639951bb4bef03ac17743046

And i've pushed the adjusted pinentry patch to
6aafa61b199ab9091d6c9e190129f2ead2a647c7.

Hope that's helpful,

     --dkg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.gnupg.org/pipermail/gnupg-devel/attachments/20171019/d0dbe182/attachment.sig>


More information about the Gnupg-devel mailing list