From: Fabian Stelzer Date: Tue, 11 Jan 2022 17:26:21 GMT Subject: Re: [PATCH] t/gpg: simplify test for unknown key Message-ID: <20220111172621.qwupg4oyosvpeguu@fs> In-Reply-To: <220111.86r19exl4z.gmgdl@evledraar.gmail.com> On 11.01.2022 17:56, Ævar Arnfjörð Bjarmason wrote: > >On Fri, Jan 07 2022, Fabian Stelzer wrote: > >> To test for a key that is completely unknown to the keyring we need one >> to sign the commit with. This was done by generating a new key and not >> add it into the keyring. To avoid the key generation overhead and >> problems where GPG did hang in CI during it, switch GNUPGHOME to an >> empty directory instead, therefore making all used keys unknown for this >> single `verify-commit` call. >> >> Reported-by: Ævar Arnfjörð Bjarmason >> Signed-off-by: Fabian Stelzer >> --- >> This was reported by Ævar in <211222.86ilvhpbl0.gmgdl@evledraar.gmail.com>. >> Just using an empty keyring / gpg homedir should achieve the same effect and >> keeps the stress of generating a gpg key out of the CI. > >Thanks, it would be great to have this in and before v2.35.0. I've run >into several boxes (on the GCC farm) that hang without this patch. > >> t/t7510-signed-commit.sh | 22 ++-------------------- >> 1 file changed, 2 insertions(+), 20 deletions(-) >> >> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh >> index 9882b69ae2..2d38580847 100755 >> --- a/t/t7510-signed-commit.sh >> +++ b/t/t7510-signed-commit.sh >> @@ -71,25 +71,7 @@ test_expect_success GPG 'create signed commits' ' >> git tag eleventh-signed $(cat oid) && >> echo 12 | git commit-tree --gpg-sign=B7227189 HEAD^{tree} >oid && >> test_line_count = 1 oid && >> - git tag twelfth-signed-alt $(cat oid) && >> - >> - cat >keydetails <<-\EOF && >> - Key-Type: RSA >> - Key-Length: 2048 >> - Subkey-Type: RSA >> - Subkey-Length: 2048 >> - Name-Real: Unknown User >> - Name-Email: unknown@git.com >> - Expire-Date: 0 >> - %no-ask-passphrase >> - %no-protection >> - EOF >> - gpg --batch --gen-key keydetails && >> - echo 13 >file && git commit -a -S"unknown@git.com" -m thirteenth && >> - git tag thirteenth-signed && >> - DELETE_FINGERPRINT=$(gpg -K --with-colons --fingerprint --batch unknown@git.com | grep "^fpr" | head -n 1 | awk -F ":" "{print \$10;}") && >> - gpg --batch --yes --delete-secret-keys $DELETE_FINGERPRINT && >> - gpg --batch --yes --delete-keys unknown@git.com >> + git tag twelfth-signed-alt $(cat oid) >> ' >> >> test_expect_success GPG 'verify and show signatures' ' >> @@ -129,7 +111,7 @@ test_expect_success GPG 'verify and show signatures' ' >> ' >> >> test_expect_success GPG 'verify-commit exits failure on unknown signature' ' >> - test_must_fail git verify-commit thirteenth-signed 2>actual && >> + GNUPGHOME=./empty_home test_must_fail git verify-commit initial 2>actual && > >Before I noticed this thread (I looked at >https://lore.kernel.org/git/20211230111038.jtoqytdhkilv2732@fs/ first, >and the In-Reply-To chain wasn't connected) Yeah, sorry about that. I forgot to add the in-reply-to :/ >I was about to submit >exactly this patch for you but with: > > - test_must_fail git verify-commit thirteenth-signed 2>actual && > + test_must_fail env GNUPGHOME="$GNUPGHOME_NOT_USED" git verify-commit initial 2>actual && > >Both of those are probably a good thing to do here. I.e.: > > 1. Didn't we have portability issues with "ENV_VAR=VALUE shell_function ..." ? I'm not good with portability stuff and trust your judgment on this. > 2. You're pointing to a nonexisting ./empty_home, but shouldn't we use > $GNUPGHOME_NOT_USED? The existing "show unknown signature with custom format" > test in the same file does that. I was not aware of $GNUPGHOME_NOT_USED but it is used in a similar fashion. However it is set to the old value of $GNUPGHOME before we change it in lib-gpg.sh which seems wrong to me. Wouldn't it then just pick up the gpg homedir of whatever the test environment has? Using the variable is good, but i would set it to a known empty directory or?