[PATCH] Add inside-Emacs mode to GUI pinentry programs

[Daiki Ueno]

Hello Neal,

Thanks for the review, and sorry for late response.  I was a bit
occupied with other tasks.

"Neal H. Walfield" <neal at walfield.org> writes:

> At Sun, 14 Jun 2015 10:24:04 +0900,
> Daiki Ueno wrote:
>> - the UID value check is now replaced with simple cast to uint32_t,
>>   using configure time check
>
> Is this really safe?  Before you caught the error, now we silently
> take the value mod 2^32.

Right, I added a check: uid == (uint32_t) uid.

The intention of the above change was to suppress a compiler warning
saying that a check like uid <= ULONG_MAX always evaluates to true.

>> However, perhaps the above scenario might be actually possible by
>> explicitly unsetenv'ing INSIDE_EMACS in gpg-agent, when
>> allow-emacs-pinentry is not set in gpg-agent.conf (like we do for
>> DISPLAY, when keep-display is not set).
>
> I seem to remember Werner suggesting that this would require a large
> change, but if it is easy, I would welcome this approach.

I leave this as is for now.  I think we could remove the Assuan option
later if restricting the envvar is sufficient for our purpose.

>> pinentry_cmd_handler_t fallback_cmd_handler;
>
> Is there any reason this isn't marked static?

Sure, fixed.

> If set_socket is successful, but read_from_emacs fails, then
> emacs_socket will be valid (>= 0) and the test in
> initial_emacs_cmd_handler will fail.  I think
> initial_emacs_cmd_handler either needs to test the return result of
> pinentry_emacs_init or pinentry_emacs_init needs to deinit the socket
> if emacs doesn't have pinentry support.

I took the latter approach and added a code to deinit emacs_socket in
other appropriate places as well.

> pinentry_enable_emacs_cmd_handler can be called multiple times, but
> this function isn't designed for this.
>
>   void
>   pinentry_enable_emacs_cmd_handler (void)
>   {
>     const char *envvar;

Good point.  I added a check if pinentry_cmd_handler is already set to
initial_emacs_cmd_handler or emacs_cmd_handler.

> Also, since pinentry_enable_emacs_cmd_handler is only called from the
> option handler, I don't think we need the field allow_emacs_prompt in
> the pinentry structure.

Yes, removed the field.

Other changes from the previous version are:

- Removed allocation code from read_from_emacs and simplified the
  interface
- Fixed minor errors detected by clang-analyzer

Regards,
-- 
Daiki Ueno
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-inside-Emacs-mode-to-GUI-pinentry-programs.patch
Type: text/x-patch
Size: 27367 bytes
Desc: not available
URL: </pipermail/attachments/20150617/324c6bdf/attachment-0001.bin>