[PATCH] test verify-commit/tag to exit unsuccessfully
Hello There was a discussion in the mailing list with subject 'verify-tag/verify-commit should exit unsuccessfully when signature is not trusted' which leads to handling exit code of untrusted signatures in 4e5dc9ca1. git verify-commit and verify-tag should exit unsuccessfully when processing a signature by a gpg key with trust level set to 'never'. This commit introduce verify checks with trust-model set to direct in gpg.conf (to force trust level of the second key in the keychain to never). In these tests, 'git verify-tag/verify-commit eighth-signed-alt' must exit unsuccessfully and includes 'We do NOT trust this key!' on the stderr (gpg output). Formatted patch is attached. Vojtech Myslivec From 013678ac78ef42ef424a46da3b463ad96c2eb58d Mon Sep 17 00:00:00 2001 From: Vojtech Myslivec Date: Sat, 11 Aug 2018 22:59:49 +0200 Subject: [PATCH] test verify-commit/tag to exit unsuccessfully git verify-commit and verify-tag should exit unsuccessfully when processing a signature by a gpg key with trust level set to 'never'. This commit introduce verify checks with trust-model set to direct in gpg.conf (to force trust level of the second key in the keychain to never). In these tests, 'git verify-tag/verify-commit eighth-signed-alt' must exit unsuccessfully and includes 'We do NOT trust this key!' on the stderr (gpg output). Helped-by: Karel Koci --- t/t7030-verify-tag.sh| 34 ++ t/t7510-signed-commit.sh | 36 2 files changed, 70 insertions(+) diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 041e319e7..6bde65c9e 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -172,4 +172,38 @@ test_expect_success GPG 'verifying a forged tag with --format should fail silent test_must_be_empty actual-forged ' +test_expect_success GPG 'verify signatures with direct trust-model' ' + ( + echo "trust-model:0:\"direct" | gpgconf --change-options gpg + ) && + ( + for tag in initial second merge fourth-signed sixth-signed seventh-signed + do + git verify-tag $tag 2>actual && + grep "Good signature from" actual && + ! grep "BAD signature from" actual && + echo $tag OK || exit 1 + done + ) && + ( + for tag in fourth-unsigned fifth-unsigned sixth-unsigned + do + test_must_fail git verify-tag $tag 2>actual && + ! grep "Good signature from" actual && + ! grep "BAD signature from" actual && + echo $tag OK || exit 1 + done + ) && + ( + for tag in eighth-signed-alt + do + test_must_fail git verify-tag $tag 2>actual && + grep "Good signature from" actual && + ! grep "BAD signature from" actual && + grep "do NOT trust" actual && + echo $tag OK || exit 1 + done + ) +' + test_done diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 4e37ff8f1..6e34f98a6 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -234,4 +234,40 @@ test_expect_success GPG 'check config gpg.format values' ' test_must_fail git commit -S --amend -m "fail" ' +test_expect_success GPG 'verify signatures with direct trust-model' ' + ( + echo "trust-model:0:\"direct" | gpgconf --change-options gpg + ) && + ( + for commit in initial second merge fourth-signed \ + fifth-signed sixth-signed seventh-signed tenth-signed + do + git verify-commit $commit 2>actual && + grep "Good signature from" actual && + ! grep "BAD signature from" actual && + echo $commit OK || exit 1 + done + ) && + ( + for commit in merge^2 fourth-unsigned sixth-unsigned \ + seventh-unsigned ninth-unsigned + do + test_must_fail git verify-commit $commit 2>actual && + ! grep "Good signature from" actual && + ! grep "BAD signature from" actual && + echo $commit OK || exit 1 + done + ) && + ( + for commit in eighth-signed-alt + do + test_must_fail git verify-commit $commit 2>actual && + grep "Good signature from" actual && + ! grep "BAD signature from" actual && + grep "do NOT trust" actual && + echo $commit OK || exit 1 + done + ) +' + test_done -- 2.18.0
Re: [PATCH 1/1] verify-tag/verify-commit should exit unsuccessfully when signature is not trusted
On 9.8.2018 20:40, Junio C Hamano wrote: > Jeff King writes: > >> I guess leaving it serves as a sort of cross-check if gpg would return a >> zero exit code but indicate in the status result that the signature was >> not good. Sort of a belt-and-suspenders, I guess (which might not be >> that implausible if we think about somebody wrapping gpg with a sloppy >> bit of shell code that loses the exit code -- it's their fault, but it >> might be nice for us to err on the conservative side). > OK, this time a real log message. Now it is possible to achieve git verify-tag/verify-commit exits unsuccessfully when signatures are not trusted. I would like to introduce some tests for this behavior to prevent this problem in the future. Thank you all for discussion and for solving the issue. Vojtech and Karel
[PATCH 1/1] verify-tag/verify-commit should exit unsuccessfully when signature is not trusted
Hello, me and my colleague are struggling with automation of verifying git repositories and we have encountered that git verify-commit and verify-tag accepts untrusted signatures and exit successfully. We have done some investigation of the GPG verification changes in git repository which I includes in this patch message. GPG results `TRUST_NEVER` and `TRUST_UNDEFINED` in raw output is treated as untrusted in git (U) and should not be accepted in verify-commit and verify-tag command. In 434060ec6d verify-tag and verify-commit was centralized into check_signature function and good (G) and untrusted (U) signatures were marked as valid and exited successfully. In this commit it is incorrectly stated that this behavior is adopted from older verify-tag function however original verify-tag behavior was to return exit code from gpg process itself (removed in a4cc18f29). Also rejecting untrusted (U) signature is the pull/merge with --verify-signatures behavior (defined in builtin/merge.c cmd_merge function and presented in eb307ae7bb). The behavior of merge/pull --verify-signatures and verify-commit/verify-tag should be the same. With regards, Vojtech Myslivec and Karel Koci From c9c7b555da284c4f67fe36dc95d592644089544a Mon Sep 17 00:00:00 2001 From: Vojtech Myslivec Date: Tue, 31 Jul 2018 20:32:32 +0200 Subject: [PATCH] gpg-interface: Do not accept untrusted signatures In 434060ec6d verify-tag and verify-commit was centralized into check_signature function and good (G) and untrusted (U) signatures were marked as valid and exited successfully. In this commit it is incorrectly stated that this behavior is adopted from older verify-tag function however original verify-tag behavior was to return exit code from gpg process itself (removed in a4cc18f29). Also rejecting untrusted (U) signature is the pull/merge with --verify-signatures behavior (defined in builtin/merge.c cmd_merge function and presented in eb307ae7bb). The behavior of merge/pull --verify-signatures and verify-commit/verify-tag should be the same. --- gpg-interface.c | 2 +- t/t7030-verify-tag.sh| 4 ++-- t/t7510-signed-commit.sh | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gpg-interface.c b/gpg-interface.c index 09ddfbc26..83adc7d12 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -86,7 +86,7 @@ int check_signature(const char *payload, size_t plen, const char *signature, strbuf_release(_status); strbuf_release(_output); - return sigc->result != 'G' && sigc->result != 'U'; + return sigc->result != 'G'; } void print_signature_buffer(const struct signature_check *sigc, unsigned flags) diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 291a1e2b0..d6f77c443 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -63,7 +63,7 @@ test_expect_success GPG 'verify and show signatures' ' ( for tag in eighth-signed-alt do - git verify-tag $tag 2>actual && + test_must_fail git verify-tag $tag 2>actual && grep "Good signature from" actual && ! grep "BAD signature from" actual && grep "not certified" actual && @@ -103,7 +103,7 @@ test_expect_success GPG 'verify signatures with --raw' ' ( for tag in eighth-signed-alt do - git verify-tag --raw $tag 2>actual && + test_must_fail git verify-tag --raw $tag 2>actual && grep "GOODSIG" actual && ! grep "BADSIG" actual && grep "TRUST_UNDEFINED" actual && diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 6e2015ed9..5cb388cb6 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -89,8 +89,8 @@ test_expect_success GPG 'verify and show signatures' ' ) ' -test_expect_success GPG 'verify-commit exits success on untrusted signature' ' - git verify-commit eighth-signed-alt 2>actual && +test_expect_success GPG 'verify-commit exits unsuccessfully on untrusted signature' ' + test_must_fail git verify-commit eighth-signed-alt 2>actual && grep "Good signature from" actual && ! grep "BAD signature from" actual && grep "not certified" actual @@ -118,7 +118,7 @@ test_expect_success GPG 'verify signatures with --raw' ' ( for commit in eighth-signed-alt do - git verify-commit --raw $commit 2>actual && + test_must_fail git verify-commit --raw $commit 2>actual && grep "GOODSIG" actual && ! grep "BADSIG" actual && grep "TRUST_UNDEFINED" actual && -- 2.18.0 signature.asc Description: OpenPGP digital signature
