Re: [PATCH] gpg-interface: fix for gpgsm v2.3
- From
Fabian Stelzer <fs@gigacodes.de>
- Date
- Feb 21, 2022, 09:22 UTC
- Message-ID
- <20220221092234.6kg66c3tuo2pya2a@fs>
- In-Reply-To
- <YgPpsJ1UCEI0a4b6@pobox.com>
On 09.02.2022 11:20, Todd Zullinger wrote:
Show 24 quoted lines
>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?
Show 68 quoted lines
>--- 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 <tmz@pobox.com>
>---
>
>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.
Show 56 quoted lines
>
>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 <tmz@pobox.com>
>---
> 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
Show 5 quoted lines
> >Thanks, > >-- >Todd