Skip to content

Commit

Permalink
Unbreak interactive GPG prompt upon signing
Browse files Browse the repository at this point in the history
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.

So let's just revert commit efee955.

This fixes #871

Cc: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
  • Loading branch information
dscho committed Sep 18, 2024
1 parent 3dab29b commit 08d520d
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 49 deletions.
21 changes: 3 additions & 18 deletions gpg-interface.c
Original file line number Diff line number Diff line change
Expand Up @@ -972,12 +972,9 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
struct child_process gpg = CHILD_PROCESS_INIT;
int ret;
size_t bottom;
const char *cp;
struct strbuf gpg_status = STRBUF_INIT;

strvec_pushl(&gpg.args,
use_format->program,
"--status-fd=2",
"-bsau", signing_key,
NULL);

Expand All @@ -989,23 +986,11 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
*/
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);

for (cp = gpg_status.buf;
cp && (cp = strstr(cp, "[GNUPG:] SIG_CREATED "));
cp++) {
if (cp == gpg_status.buf || cp[-1] == '\n')
break; /* found */
}
ret |= !cp;
if (ret) {
error(_("gpg failed to sign the data:\n%s"),
gpg_status.len ? gpg_status.buf : "(no gpg output)");
strbuf_release(&gpg_status);
return -1;
}
strbuf_release(&gpg_status);
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. */
remove_cr_after(signature, bottom);
Expand Down
31 changes: 0 additions & 31 deletions t/t7004-tag.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1517,44 +1517,13 @@ test_expect_success GPG \
'test_config user.signingkey BobTheMouse &&
test_must_fail git tag -s -m tail tag-gpg-failure'

# 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 produce invalid signature
test_expect_success GPG 'git verifies tag is valid with double signature' '
git tag -s -m tail tag-gpg-double-sig &&
git cat-file tag tag-gpg-double-sig >tag &&
othersigheader=$(test_oid othersigheader) &&
sed -ne "/^\$/q;p" tag >new-tag &&
cat <<-EOM >>new-tag &&
$othersigheader -----BEGIN PGP SIGNATURE-----
someinvaliddata
-----END PGP SIGNATURE-----
EOM
sed -e "1,/^tagger/d" tag >>new-tag &&
new_tag=$(git hash-object -t tag -w new-tag) &&
git update-ref refs/tags/tag-gpg-double-sig $new_tag &&
git verify-tag tag-gpg-double-sig &&
git fsck
'

# try to sign with bad user.signingkey
test_expect_success GPGSM \
'git tag -s fails if gpgsm is misconfigured (bad key)' \
'test_config user.signingkey BobTheMouse &&
test_config gpg.format x509 &&
test_must_fail git tag -s -m tail tag-gpg-failure'

# try to produce invalid signature
test_expect_success GPGSM \
'git tag -s fails if gpgsm is misconfigured (bad signature format)' \
'test_config gpg.x509.program echo &&
test_config gpg.format x509 &&
test_must_fail git tag -s -m tail tag-gpg-failure'

# try to verify without gpg:

rm -rf gpghome
Expand Down

0 comments on commit 08d520d

Please sign in to comment.