All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael J Gruber <git@drmicha.warpmail.net>
To: Johannes Schindelin <johannes.schindelin@gmx.de>, git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] Unbreak interactive GPG prompt upon signing
Date: Tue, 6 Sep 2016 14:32:40 +0200	[thread overview]
Message-ID: <ced7502d-0095-bd90-19e3-c14d0e4d4f07@drmicha.warpmail.net> (raw)
In-Reply-To: <0b8196564ac9f1db7c59b42d7e0973ba1399ec52.1473148900.git.johannes.schindelin@gmx.de>

Johannes Schindelin venit, vidit, dixit 06.09.2016 10:01:
> With the recent update in efee955 (gpg-interface: check gpg signature
> creation status, 2016-06-17), we ask GPG to send all status updates to
> stderr, and then catch the stderr in an strbuf.
> 
> But GPG might fail, and send error messages to stderr. And we simply
> do not show them to the user.
> 
> Even worse: this swallows any interactive prompt for a passphrase. And
> detaches stderr from the tty so that the passphrase cannot be read.
> 
> So while the first problem could be fixed (by printing the captured
> stderr upon error), the second problem cannot be easily fixed, and
> presents a major regression.

My Git has that commit and does ask me for the passphrase on the tty.
Also, I do get error messages:

git tag -u pebcak -s testt -m m
error: gpg failed to sign the data
error: unable to sign the tag

which we could (maybe should) amend by gpg's stderr.

> So let's just revert commit efee9553a4f97b2ecd8f49be19606dd4cf7d9c28.

That "just" reintroduces the problem that the orignal patch solves.

The passphrase/tty issue must be Windows specific - or the non-issue
Linux-specific, if you prefer.

> This fixes https://github.com/git-for-windows/git/issues/871
> 
> Cc: Michael J Gruber <git@drmicha.warpmail.net>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Published-As: https://github.com/dscho/git/releases/tag/fix-gpg-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git fix-gpg-v1
> 
>  gpg-interface.c | 8 ++------
>  t/t7004-tag.sh  | 9 +--------
>  2 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 8672eda..3f3a3f7 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -153,11 +153,9 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
>  	struct child_process gpg = CHILD_PROCESS_INIT;
>  	int ret;
>  	size_t i, j, bottom;
> -	struct strbuf gpg_status = STRBUF_INIT;
>  
>  	argv_array_pushl(&gpg.args,
>  			 gpg_program,
> -			 "--status-fd=2",
>  			 "-bsau", signing_key,
>  			 NULL);
>  
> @@ -169,12 +167,10 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
>  	 */
>  	sigchain_push(SIGPIPE, SIG_IGN);
>  	ret = pipe_command(&gpg, buffer->buf, buffer->len,
> -			   signature, 1024, &gpg_status, 0);
> +			   signature, 1024, NULL, 0);
>  	sigchain_pop(SIGPIPE);
>  
> -	ret |= !strstr(gpg_status.buf, "\n[GNUPG:] SIG_CREATED ");
> -	strbuf_release(&gpg_status);
> -	if (ret)
> +	if (ret || signature->len == bottom)
>  		return error(_("gpg failed to sign the data"));
>  
>  	/* Strip CR from the line endings, in case we are on Windows. */
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 8b0f71a..f9b7d79 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1202,17 +1202,10 @@ test_expect_success GPG,RFC1991 \
>  # try to sign with bad user.signingkey
>  git config user.signingkey BobTheMouse
>  test_expect_success GPG \
> -	'git tag -s fails if gpg is misconfigured (bad key)' \
> +	'git tag -s fails if gpg is misconfigured' \
>  	'test_must_fail git tag -s -m tail tag-gpg-failure'
>  git config --unset user.signingkey
>  
> -# try to produce invalid signature
> -test_expect_success GPG \
> -	'git tag -s fails if gpg is misconfigured (bad signature format)' \
> -	'test_config gpg.program echo &&
> -	 test_must_fail git tag -s -m tail tag-gpg-failure'
> -
> -
>  # try to verify without gpg:
>  
>  rm -rf gpghome
> 


  reply	other threads:[~2016-09-06 12:32 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 [this message]
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
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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ced7502d-0095-bd90-19e3-c14d0e4d4f07@drmicha.warpmail.net \
    --to=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.