From: Michael J Gruber <git@drmicha.warpmail.net>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Jeff King <peff@peff.net>
Subject: Re: [PATCH] gpg-interface: reflect stderr to stderr
Date: Wed, 7 Sep 2016 10:27:34 +0200 [thread overview]
Message-ID: <655b42d8-baa9-e649-2b3c-5b7bfc914bc5@drmicha.warpmail.net> (raw)
In-Reply-To: <alpine.DEB.2.20.1609061843120.129229@virtualbox>
Johannes Schindelin venit, vidit, dixit 06.09.2016 18:43:
> 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?
next prev parent reply other threads:[~2016-09-07 8:27 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-06 8:01 [PATCH] Unbreak interactive GPG prompt upon signing Johannes Schindelin
2016-09-06 12:32 ` Michael J Gruber
2016-09-06 13:13 ` [PATCH] gpg-interface: reflect stderr to stderr Michael J Gruber
2016-09-06 16:30 ` Johannes Schindelin
2016-09-06 16:42 ` Johannes Schindelin
2016-09-06 16:43 ` Johannes Schindelin
2016-09-07 8:27 ` Michael J Gruber [this message]
2016-09-07 8:39 ` Jeff King
2016-09-07 9:32 ` Michael J Gruber
2016-09-08 18:20 ` Junio C Hamano
2016-09-08 20:03 ` Jeff King
2016-09-08 21:36 ` Junio C Hamano
2016-09-12 13:39 ` Michael J Gruber
2018-10-02 13:02 ` Johannes Schindelin
2016-09-09 7:28 ` Johannes Schindelin
2016-09-06 16:39 ` [PATCH] Unbreak interactive GPG prompt upon signing Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=655b42d8-baa9-e649-2b3c-5b7bfc914bc5@drmicha.warpmail.net \
--to=git@drmicha.warpmail.net \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).