From: Fabian Stelzer Date: Mon, 21 Feb 2022 09:22:34 GMT Subject: Re: [PATCH] gpg-interface: fix for gpgsm v2.3 Message-ID: <20220221092234.6kg66c3tuo2pya2a@fs> In-Reply-To: On 09.02.2022 11:20, Todd Zullinger wrote: >Fabian Stelzer wrote: >> On 07.02.2022 11:38, Todd Zullinger wrote: >>> How did it fail for you? It passes all the tests when I've >>> run it against Fedora and RHEL-based hosts. If it's flaky >>> on other systems, that would put a damper on doing it this >>> way. Though it _should_ work. >> >> Sorry for the delays, I'm a bit busy with other things at the moment. > >No apologies needed. This is something I worked on back in >November and had yet to send to the list, so I'm the last >person to rush another. :) > >> I did get an interactive popup asking if I would like to >> trust the key when I ran the t4202 test. This never >> happened with the old variant. > >Interesting. I do have a patch in my gnupg-2.3 series to >reload the gpg agent after changing the trustlist, as the >changes were not picked up prior to that. In my case, I was >running the tests in an environment where gpg could not >prompt me. (It also seems like we should try harder to have >the test suite reject such prompts). > Yes, gpg-agent in general can be problematic for the tests. I'm not familiar enough with gpg but I don't know if we can get by without it? >--- 8< --- >Subject: [PATCH] t/lib-gpg: reload gpg components after updating trustlist > >With gpgsm from gnupg-2.3, the changes to the trustlist.txt do not >appear to be picked up without refreshing the gpg-agent. Use the 'all' >keyword to reload all of the gpg components. The scdaemon is started as >a child of gpg-agent, for example. > >We used to have a --kill at this spot, but I removed it in 2e285e7803 >(t/lib-gpg: drop redundant killing of gpg-agent, 2019-02-07). It seems >like it might be necessary (again) for 2.3. > >Signed-off-by: Todd Zullinger >--- > >Notes: > An alternative to doing this dance with the trustlist.txt and having to > kill and/or reload the gpg-agent to pick up the change right after the > import in each test might be to make this part of the steps used when > adding/updating/removing certificates in t/lib-gpg. > > If not as a one-time affair when a cert is added/update/removed, then > perhaps as a step taken by/in t/lib-gpg.sh only once. It could populate > a gpghome to be copied into the trash dir for each test which used > gpg/gpgsm. I haven't measured the effect of the extra reload precisely, > but I'm sure it's not free. > > (For what it's worth, it didn't add any noticeable amount of time to the > full builds/test runs I made while working on this, so it's a seemingly > small cost, at least.) > > Also, hello Henning, > > Way back, in February 2019¹, when I submitted 2e285e7803 to remove the > "redundant" killing of the gpg-agent, you said: > > > Killing the agent once should be enough, i remember manually killing > > it many times as i was looking for a way to generate certs and trust > > (configure gpgsm for the test). That is probably why i copied it over > > in the first place. > > As I wrote this patch to partially restore the gpg-agent killing (now > just a reload), I thought this might have been the sort of issue that > you hit while testing. > > It could be unrelated, but it sounds quite similar to what I found with > gnupg-2.3 when trying to get it to pick up the trustlist.txt changes. I > thought you might at least enjoy seeing it come back around. :) > > ¹ <20190208093324.7b17f270@md1za8fc.ad001.siemens.net> > > t/lib-gpg.sh | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh >index 6bc083ca77..38e2c0f4fb 100644 >--- a/t/lib-gpg.sh >+++ b/t/lib-gpg.sh >@@ -75,6 +75,7 @@ test_lazy_prereq GPGSM ' > gpgsm --homedir "${GNUPGHOME}" -K --with-colons | > awk -F ":" "/^fpr:/ {printf \"%s S relax\\n\", \$10}" \ > >"${GNUPGHOME}/trustlist.txt" && >+ (gpgconf --reload all || : ) && > > echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \ > -u committer@example.com -o /dev/null --sign - > >--- 8< --- This patch fixes it for me. > >I have another patch which changes the earlier gpgconf call >which kills gpg-agent to kill all gpg daemons, as there are >some others which could potentially interfere with the >tests: > >--- 8< --- >Subject: [PATCH] t/lib-gpg: kill all gpg components, not just gpg-agent > >The gpg-agent is one of several processes that newer releases of GnuPG >start automatically. Issue a kill to each of them to ensure they do not >affect separate tests. (Yes, the separate GNUPGHOME should do that >already. If we find that is case, we could drop the --kill entirely.) > >In terms of compatibility, the 'all' keyword was added to the --kill & >--reload options in GnuPG 2.1.18. Debian and RHEL are often used as >indicators of how a change might affect older systems we often try to >support. > > - Debian Strech (old old stable), which has limited security support > until June 2022, has GnuPG 2.1.18 (or 2.2.x in backports). > > - CentOS/RHEL 7, which is supported until June 2024, has GnuPG > 2.0.22, which lacks the --kill option, so the change won't have > any impact. > >Signed-off-by: Todd Zullinger >--- > t/lib-gpg.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh >index d675698a2d..2bb309a8c1 100644 >--- a/t/lib-gpg.sh >+++ b/t/lib-gpg.sh >@@ -40,7 +40,7 @@ test_lazy_prereq GPG ' > # > lib-gpg/ownertrust > mkdir "$GNUPGHOME" && > chmod 0700 "$GNUPGHOME" && >- (gpgconf --kill gpg-agent || : ) && >+ (gpgconf --kill all || : ) && > gpg --homedir "${GNUPGHOME}" --import \ > "$TEST_DIRECTORY"/lib-gpg/keyring.gpg && > gpg --homedir "${GNUPGHOME}" --import-ownertrust \ >--- 8< --- > >I have the series in the gpg-misc-fixes branch: > > https://github.com/tmzullinger/git/commits/gpg-misc-fixes > >(That series does differ in that it has `string_list_split()` >instead of the simpler `strbuf_addch(&gpg_status, '\n');` to >fix the gpgsm parsing.) > >Iff you think it would be useful or helpful, I can post that >series. I have prepared the patch with the simple strstr() matching I can post in a bit. I would add your two gpg test lib patches to it if thats ok? Thanks > >Thanks, > >-- >Todd