Re: [PATCH] gpg-interface: reflect stderr to stderr

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Sep 7, 2016, 08:27 UTC
Message-ID: <655b42d8-baa9-e649-2b3c-5b7bfc914bc5@drmicha.warpmail.net>
In-Reply-To: <alpine.DEB.2.20.1609061843120.129229@virtualbox>

Johannes Schindelin venit, vidit, dixit 06.09.2016 18:43:

Show 21 quoted lines

> Hi Michael,
> 
> okay, final mail on this issue today:
> 
> On Tue, 6 Sep 2016, Johannes Schindelin wrote:
> 
>> Your original issue seemed to be that the gpg command could succeed, but
>> still no signature be seen. There *must* be a way to test whether the
>> called program added a signature, simply by testing whether *any*
>> characters were written.
>>
>> And if characters were written that were not actually a GPG signature,
>> maybe the enterprisey user who configured the gpg command to be her magic
>> script actually meant something else than a GPG signature to be added?
> 
> I actually just saw that this is *precisely* what the code does already:
> 
>         if (ret || signature->len == bottom)
>                 return error(_("gpg failed to sign the data"));
> 
> Why is this not good enough?

Assuming "this" refers to error():

You said it's not good enough - because gpg's stderr is not displayed.

And I agree with you on that point.

Assuming "this" refers to the exit code check:

gpg documentation says so over and over again: do not rely on exit codes, parse status-fd instead. (An often ignored advice, oh well.)

As for most of your other remarks, I would appreciate if you could take
a breath and reread what I wrote and what you wrote - before you send it
- and curb your remarks about who is working on resolving issues.

So, trying again to structure the issues and solutions, there are three issues:

A) relying on gpg's exit code (and stdout) is not enough. Secure use of gpg requires checking status-fd.

This is what my old patch solved.

B) "gpg --status-fd=2" and swallowing stderr hides usual stderr from the user.

This is what my old patch introduced and a Windows user reported. It is solved by my new additional patch.

C) With that old patch, that Windows user is not asked for a passphrase on tty any more.

Reverting my patches appears to solve C on Windows and reintroduces A on all platforms, obviously. C is not present on Linux. B is solved either way.

Now, I can't reproduce C on Linux[*], so there is more involved. It could be that my patch just exposes a problem in our start_command() etc.: run-command.c contains a lot of ifdefing, so possibly quite different code is run on different platforms.

It would be great if someone with a Windows environment could help our efforts in resolving issue C, by checking what is actually behind[**]: I can't believe that capturing stderr keeps gpg from reading stdin, but who knows. Maybe Jeff of pipe_command() fame? I'll put him on cc.

Michael

[*] Maybe that even depends on Linux environments (terminal emulator), so input from others would be helpful, too:

Without a passphrase-agent/wallet etc, does "git tag -s -m test test" ask you for a passphrase on the terminal?

I does for me with this stack:

X11->i3->st->tmux->bash->git->gpg

[**] "--status-fd=3" instead of "--status-fd=2" in my old patch would be a check whether our capturing of stderr is creating problems on Windows or gpg's writing status to stderr (which --status-fd=3 would change, at the expense of breaking the final check): Does gpg ask for the passphrase now?