[PATCH 7/7] gpg-interface: check gpg signature creation status
- From
Jeff King <peff@peff.net>
- Date
- Jun 16, 2016, 09:42 UTC
- Message-ID
- <20160616094239.GG15851@sigill.intra.peff.net>
- In-Reply-To
- <20160616093248.GA15130@sigill.intra.peff.net>
When we create a signature, it may happen that gpg returns with "success" but not with an actual detached signature on stdout.
Check for the correct signature creation status to catch these cases better. Really, --status-fd parsing is the only way to check gpg status reliably. We do the same for verify already.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> Signed-off-by: Jeff King <peff@peff.net> --- Obviously the big change from your v3 is that this just passes the buffer to pipe_command() instead of doing the read itself.
I changed the name of your "err" strbuf to "gpg_status" to be more descriptive, and to match the similar usage on the verification side.
In the test, I bumped the config-setting inside the test, and used test_config to get the automatic cleanup (using "git -c" would also have worked).
I left the test description as-is, though it is a bit misleading. We are not really checking for bad input anymore, but rather "did gpg send us the special stats". Your fake "echo" fails on both counts, so the test is still effective. A more robust test would be to output something that _looked_ like real gpg output, but not to output the correct status token.
gpg-interface.c | 8 ++++++-- t/t7004-tag.sh | 9 ++++++++- 2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/gpg-interface.c b/gpg-interface.c index 74f08a2..08356f9 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -153,9 +153,11 @@ 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); @@ -167,10 +169,12 @@ 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, NULL, 0); + signature, 1024, &gpg_status, 0); sigchain_pop(SIGPIPE); - if (ret || signature->len == bottom) + ret |= !strstr(gpg_status.buf, "\n[GNUPG:] SIG_CREATED "); + strbuf_release(&gpg_status); + if (ret) 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 f9b7d79..8b0f71a 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -1202,10 +1202,17 @@ 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' \ + 'git tag -s fails if gpg is misconfigured (bad key)' \ '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
-- 2.9.0.160.g4984cba