Subject: Re: [PATCH] gpg-interface: fix for gpgsm v2.3

From: Todd Zullinger <[email-masked]>
Date: Wed, 9 Feb 2022 11:20:00 -0500

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).

--- 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 <[email-masked]>
---

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.  :)
    
    ¹ <[email-masked]>

 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 [email-masked] -o /dev/null --sign -

--- 8< ---

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 <[email-masked]>
---
 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.

Thanks,

-- 
Todd