git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Todd ZullingerNext: Todd Zullinger
Message 11 of 24 in “gpg-interface: fix for gpgsm v2.3”
  1. gpg-interface: fix for gpgsm v2.3Fabian Stelzer, Feb 3, 2022
  2. Junio C HamanoFeb 3, 2022
  3. Todd ZullingerFeb 3, 2022
  4. Junio C HamanoFeb 3, 2022
  5. Todd ZullingerFeb 3, 2022
  6. Junio C HamanoFeb 3, 2022
  7. Fabian StelzerFeb 7, 2022
  8. Todd ZullingerFeb 7, 2022
  9. Fabian StelzerFeb 9, 2022
  10. Todd ZullingerFeb 9, 2022
  11. Fabian StelzerFeb 21, 2022
  12. Todd ZullingerFeb 23, 2022
  13. 2/3 t/lib-gpg: reload gpg components after updating trustlistFabian Stelzer, Feb 24, 2022
  14. 3/3 t/lib-gpg: kill all gpg components, not just gpg-agentFabian Stelzer, Feb 24, 2022
  15. 1/3 gpg-interface/gpgsm: fix for v2.3Fabian Stelzer, Feb 24, 2022
  16. Todd ZullingerFeb 28, 2022
  17. 1/3 gpg-interface/gpgsm: fix for v2.3Fabian Stelzer, Mar 2, 2022
  18. Junio C HamanoMar 2, 2022
  19. Fabian StelzerMar 3, 2022
  20. 1/3 gpg-interface/gpgsm: fix for v2.3Fabian Stelzer, Mar 4, 2022
  21. 3/3 t/lib-gpg: kill all gpg components, not just gpg-agentFabian Stelzer, Mar 4, 2022
  22. 2/3 t/lib-gpg: reload gpg components after updating trustlistFabian Stelzer, Mar 4, 2022
  23. 2/3 t/lib-gpg: reload gpg components after updating trustlistFabian Stelzer, Mar 2, 2022
  24. 3/3 t/lib-gpg: kill all gpg components, not just gpg-agentFabian Stelzer, Mar 2, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.