diff mbox series

[1/3] gpg-interface/gpgsm: fix for v2.3

Message ID 20220224100628.612789-1-fs@gigacodes.de (mailing list archive)
State New, archived
Headers show
Series [1/3] gpg-interface/gpgsm: fix for v2.3 | expand

Commit Message

Fabian Stelzer Feb. 24, 2022, 10:06 a.m. UTC
gpgsm v2.3 changed some details about its output:
 - instead of displaying `fingerprint:` for keys it will print `sha1
   fpr:` and `sha2 fpr:`
 - some wording of errors has changed
 - signing will omit an extra debug output line before the [GNUPG]: tag

This change adjusts the gpgsm test prerequisite to work with v2.3 as
well by accepting `sha1 fpr:` as well as `fingerprint:`. To make this
parsing more robust switch to gpg's `--with-colons` output format.
Also allow both variants of errors for unknown certs.
Checking if signing was successful will now accept '[GNUPG]:
SIG_CREATED' on any beginning of a line. Not just explictly the second
one anymore.

Helped-By: Junio C Hamano <gitster@pobox.com>
Helped-By: Todd Zullinger <tmz@pobox.com>
---
 gpg-interface.c | 9 ++++++++-
 t/lib-gpg.sh    | 8 +++-----
 t/t4202-log.sh  | 2 +-
 3 files changed, 12 insertions(+), 7 deletions(-)

Comments

Todd Zullinger Feb. 28, 2022, 5:57 p.m. UTC | #1
Hi,

Fabian Stelzer wrote:
> gpgsm v2.3 changed some details about its output:
>  - instead of displaying `fingerprint:` for keys it will print `sha1
>    fpr:` and `sha2 fpr:`
>  - some wording of errors has changed
>  - signing will omit an extra debug output line before the [GNUPG]: tag
> 
> This change adjusts the gpgsm test prerequisite to work with v2.3 as
> well by accepting `sha1 fpr:` as well as `fingerprint:`. To make this
> parsing more robust switch to gpg's `--with-colons` output format.
> Also allow both variants of errors for unknown certs.

I ran this series through the fedora buildsystem on releases
with gnupg 2.2 and 2.3.  All the tests pass, as expected.

I think we may be able to simplify the wording above and the
patch below regarding the fingerprint/shaN fpr output
change, I'll add a comment below the changed hunk.

> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> index 3e7ee1386a..e997ce10ea 100644
> --- a/t/lib-gpg.sh
> +++ b/t/lib-gpg.sh
> @@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '
>  		--passphrase-fd 0 --pinentry-mode loopback \
>  		--import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
>  
> -	gpgsm --homedir "${GNUPGHOME}" -K |
> -	grep fingerprint: |
> -	cut -d" " -f4 |
> -	tr -d "\\n" >"${GNUPGHOME}/trustlist.txt" &&
> +	gpgsm --homedir "${GNUPGHOME}" -K --with-colons |
> +	awk -F ":" "/^(fpr|fingerprint):/ {printf \"%s S relax\\n\", \$10}" \
> +		>"${GNUPGHOME}/trustlist.txt" &&

Using --with-colons to parse the output, we shouldn't be
affected by the changed output.  The pattern for awk can be
simplified to '^fpr:' as older and newer versions of gnupg
have used that string in the --with-colons output for many,
many years.

Perhaps that allows the commit message to say less about the
specific's the gnugp-2.3 output change and just mention that
it changed and using --with-colons is the preferred way to
parse the output (where we must parse output at all).

    Switch to gpg's `--with-colons` output format to make
    parsing more robust.  This avoids issues where the
    human-readable output from gpg commands changes.

or something?

Thanks,
diff mbox series

Patch

diff --git a/gpg-interface.c b/gpg-interface.c
index 17b1e44baa..94abb3090b 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -934,6 +934,7 @@  static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
 	struct child_process gpg = CHILD_PROCESS_INIT;
 	int ret;
 	size_t bottom;
+	const char *cp;
 	struct strbuf gpg_status = STRBUF_INIT;
 
 	strvec_pushl(&gpg.args,
@@ -953,7 +954,13 @@  static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
 			   signature, 1024, &gpg_status, 0);
 	sigchain_pop(SIGPIPE);
 
-	ret |= !strstr(gpg_status.buf, "\n[GNUPG:] SIG_CREATED ");
+	for (cp = gpg_status.buf;
+	     cp && (cp = strstr(cp, "[GNUPG:] SIG_CREATED "));
+	     cp++) {
+		if (cp == gpg_status.buf || cp[-1] == '\n')
+			break; /* found */
+	}
+	ret |= !cp;
 	strbuf_release(&gpg_status);
 	if (ret)
 		return error(_("gpg failed to sign the data"));
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 3e7ee1386a..e997ce10ea 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -72,12 +72,10 @@  test_lazy_prereq GPGSM '
 		--passphrase-fd 0 --pinentry-mode loopback \
 		--import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
 
-	gpgsm --homedir "${GNUPGHOME}" -K |
-	grep fingerprint: |
-	cut -d" " -f4 |
-	tr -d "\\n" >"${GNUPGHOME}/trustlist.txt" &&
+	gpgsm --homedir "${GNUPGHOME}" -K --with-colons |
+	awk -F ":" "/^(fpr|fingerprint):/ {printf \"%s S relax\\n\", \$10}" \
+		>"${GNUPGHOME}/trustlist.txt" &&
 
-	echo " S relax" >>"${GNUPGHOME}/trustlist.txt" &&
 	echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
 	       -u committer@example.com -o /dev/null --sign -
 '
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 544f0aa82e..493e376e73 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -2013,7 +2013,7 @@  test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss
 	git merge --no-ff -m msg signed_tag_x509_nokey &&
 	GNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&
 	grep "^|\\\  merged tag" actual &&
-	grep "^| | gpgsm: certificate not found" actual
+	grep -Ei "^| | gpgsm:( failed to find the)? certificate:? not found" actual
 '
 
 test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 bad signature' '