From a7d2f7e53e982258913020f1bf245f8dd58e2a1f Mon Sep 17 00:00:00 2001 From: Todd Zullinger Date: Thu, 25 Nov 2021 05:52:09 -0500 Subject: [PATCH] fix gpgsm issues with gnupg-2.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The output of gpgsm changed slightly in gnupg-2.3, causing the git tests for x509 signatures to be skipped. Update the tests to use the machine-parseable --with-colons output. It also appears that we need to reload the gpg-agent in order to pick up the changes the test library makes to the trustlist.txt file. It might be better to store that file with the other gpg files in the test suite rather than generating it. While we're at it, reload all the gpg components rather than just gpg-agent. Adjust the earlier gpgconf kill to use the 'all' keyword as well. Next up, gpgsm removed a debug line from it's output which exposes a problem in git's gpg-interface code. The git code presumes that the '[GNUPG:] SIG_CREATED' line will follow a newline. That is no longer true. The debug line was removed from GnuPG in a6d2f3133 (sm: Replace some debug message by log_error or log_info, 2020-04-21). Finally, a minor bug in gpgsm causes the error message returned when a certificate is not found to differ from previous versions¹. Extend the grep pattern in the test suite to catch both error messages. ¹ https://lists.gnupg.org/pipermail/gnupg-devel/2021-November/034991.html --- ...ith-colons-when-parsing-gpgsm-output.patch | 47 ++++++++++++++++++ ...-gpg-components-after-updating-trust.patch | 31 ++++++++++++ ...ll-gpg-components-not-just-gpg-agent.patch | 40 ++++++++++++++++ ...02-match-gpgsm-output-from-GnuPG-2.3.patch | 33 +++++++++++++ ...tch-SIG_CREATED-if-it-s-the-first-li.patch | 48 +++++++++++++++++++ git.spec | 8 ++++ 6 files changed, 207 insertions(+) create mode 100644 0001-t-lib-gpg-use-with-colons-when-parsing-gpgsm-output.patch create mode 100644 0002-t-lib-gpg-reload-gpg-components-after-updating-trust.patch create mode 100644 0003-t-lib-gpg-kill-all-gpg-components-not-just-gpg-agent.patch create mode 100644 0004-t4202-match-gpgsm-output-from-GnuPG-2.3.patch create mode 100644 0005-gpg-interface-match-SIG_CREATED-if-it-s-the-first-li.patch diff --git a/0001-t-lib-gpg-use-with-colons-when-parsing-gpgsm-output.patch b/0001-t-lib-gpg-use-with-colons-when-parsing-gpgsm-output.patch new file mode 100644 index 0000000..5c003a5 --- /dev/null +++ b/0001-t-lib-gpg-use-with-colons-when-parsing-gpgsm-output.patch @@ -0,0 +1,47 @@ +From e155951262e6dea419db8b9010342b08b487f96a Mon Sep 17 00:00:00 2001 +From: Todd Zullinger +Date: Thu, 25 Nov 2021 05:05:08 -0500 +Subject: [PATCH] t/lib-gpg: use --with-colons when parsing gpgsm output +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The output of `gpgsm -K` changed in gnupg-2.3¹, breaking the parsing +used by the GPGSM prereq. + +Use the `--with-colons` options for stable, machine-parseable output. +This allows the grep/cut/tr pipeline (and the subsequent echo which +appends ' S relax') to be replaced with a single call to awk to create +the ${GNUPGHOME}/trustlist.txt file. + +¹ https://dev.gnupg.org/rGe7d70923901e is the change in 2.3, while + https://dev.gnupg.org/rG9c57de75cf36 is the similar change in 2.2. + + The latter says: Here in 2.2 we keep the string "fingerprint:" and no + not change it to "sha1 fpr" as we did in master (2.3). (sic) + +Signed-off-by: Todd Zullinger +--- + t/lib-gpg.sh | 8 +++----- + 1 file changed, 3 insertions(+), 5 deletions(-) + +diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh +index a3f285f515..cbbf74e725 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:/ {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/0002-t-lib-gpg-reload-gpg-components-after-updating-trust.patch b/0002-t-lib-gpg-reload-gpg-components-after-updating-trust.patch new file mode 100644 index 0000000..2c27b74 --- /dev/null +++ b/0002-t-lib-gpg-reload-gpg-components-after-updating-trust.patch @@ -0,0 +1,31 @@ +From 93299b9b221da01d4055528f7c760d04ee83b82b Mon Sep 17 00:00:00 2001 +From: Todd Zullinger +Date: Thu, 25 Nov 2021 08:07:32 -0500 +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 +--- + t/lib-gpg.sh | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh +index cbbf74e725..d675698a2d 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 - diff --git a/0003-t-lib-gpg-kill-all-gpg-components-not-just-gpg-agent.patch b/0003-t-lib-gpg-kill-all-gpg-components-not-just-gpg-agent.patch new file mode 100644 index 0000000..2905564 --- /dev/null +++ b/0003-t-lib-gpg-kill-all-gpg-components-not-just-gpg-agent.patch @@ -0,0 +1,40 @@ +From da340dd76714474126f73f6b53087da0ffd4e8d8 Mon Sep 17 00:00:00 2001 +From: Todd Zullinger +Date: Fri, 26 Nov 2021 21:11:54 -0500 +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 \ diff --git a/0004-t4202-match-gpgsm-output-from-GnuPG-2.3.patch b/0004-t4202-match-gpgsm-output-from-GnuPG-2.3.patch new file mode 100644 index 0000000..005ace7 --- /dev/null +++ b/0004-t4202-match-gpgsm-output-from-GnuPG-2.3.patch @@ -0,0 +1,33 @@ +From d1efcac68414b80cc0fd7b7e3b4781f313d98697 Mon Sep 17 00:00:00 2001 +From: Todd Zullinger +Date: Sat, 27 Nov 2021 05:31:13 -0500 +Subject: [PATCH] t4202: match gpgsm output from GnuPG 2.3 +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +In GnuPG 2.3, the output from gpgsm when a certificate is not found +differs from that of earlier versions. This appears to be a bug¹, but +there are several releases in use now which have this output. Extend +the grep pattern to catch it rather than failing the test. + +¹ https://lists.gnupg.org/pipermail/gnupg-devel/2021-November/034991.html + +Signed-off-by: Todd Zullinger +--- + t/t4202-log.sh | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/t/t4202-log.sh b/t/t4202-log.sh +index 7884e3d46b..c69f9ac469 100755 +--- a/t/t4202-log.sh ++++ b/t/t4202-log.sh +@@ -1851,7 +1851,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' ' diff --git a/0005-gpg-interface-match-SIG_CREATED-if-it-s-the-first-li.patch b/0005-gpg-interface-match-SIG_CREATED-if-it-s-the-first-li.patch new file mode 100644 index 0000000..458af9d --- /dev/null +++ b/0005-gpg-interface-match-SIG_CREATED-if-it-s-the-first-li.patch @@ -0,0 +1,48 @@ +From edb5eafc9945b2d400c2d777a9750cee06ab500f Mon Sep 17 00:00:00 2001 +From: Todd Zullinger +Date: Sat, 27 Nov 2021 02:55:47 -0500 +Subject: [PATCH] gpg-interface: match SIG_CREATED if it's the first line + +In `sign_buffer_gpg`, "\n[GNUPG:] SIG_CREATED " in the gpg status output +is used to signal a successful signature. This fails if "SIG_CREATED" +is the first line in the gpg output, as is the case with `gpgsm` in +GnuPG 2.3. + +In earlier versions of GnuPG, there was a debug line in the `gpgsm` +output which allowed the check in `sign_buffer_gpg` to work. This debug +line was removed from GnuPG in a6d2f3133 (sm: Replace some debug message +by log_error or log_info, 2020-04-21). + +The result is the `gpgsm --status-fd` output for a signing operation +starts with "[GNUPG:] SIG_CREATED" and we mistakenly report "gpg failed +to sign the data" to the user. The `gpg` command has other `[GNUPG:]` +output for signing operations, so it is not affected by this issue. +It's best not to rely on something as subtle and out of our control as +the order if the gnupg status messages. + +This likely went unnoticed because the GPGSM test prereq was failing for +a different reason with GnuPG 2.3. No tests failed, they were simply +skipped due to the missing GPGSM prereq. + +Signed-off-by: Todd Zullinger +--- + gpg-interface.c | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/gpg-interface.c b/gpg-interface.c +index 3e7255a2a9..d179dfb3ab 100644 +--- a/gpg-interface.c ++++ b/gpg-interface.c +@@ -859,6 +859,12 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature, + + bottom = signature->len; + ++ /* ++ * Ensure gpg_status begins with a newline or we'll fail to match if ++ * the SIG_CREATED line is at the start of the gpg output. ++ */ ++ strbuf_addch(&gpg_status, '\n'); ++ + /* + * When the username signingkey is bad, program could be terminated + * because gpg exits without reading and then write gets SIGPIPE. diff --git a/git.spec b/git.spec index 45d25c1..6b12273 100644 --- a/git.spec +++ b/git.spec @@ -112,6 +112,13 @@ Source99: print-failed-test-output # https://bugzilla.redhat.com/490602 Patch0: git-cvsimport-Ignore-cvsps-2.2b1-Branches-output.patch +# Fix a few tests and issues with gnupg-2.3 +Patch1: 0001-t-lib-gpg-use-with-colons-when-parsing-gpgsm-output.patch +Patch2: 0002-t-lib-gpg-reload-gpg-components-after-updating-trust.patch +Patch3: 0003-t-lib-gpg-kill-all-gpg-components-not-just-gpg-agent.patch +Patch4: 0004-t4202-match-gpgsm-output-from-GnuPG-2.3.patch +Patch5: 0005-gpg-interface-match-SIG_CREATED-if-it-s-the-first-li.patch + %if %{with docs} # pod2man is needed to build Git.3pm BuildRequires: %{_bindir}/pod2man @@ -1003,6 +1010,7 @@ rmdir --ignore-fail-on-non-empty "$testdir" %changelog * Thu Nov 25 2021 Todd Zullinger - 2.34.1-1 - update to 2.34.1 +- fix gpgsm issues with gnupg-2.3 * Mon Nov 15 2021 Todd Zullinger - 2.34.0-1 - update to 2.34.0