[PATCH v2 1/1] gpg-interface: limit search for primary key fingerprint

From: Hans Jerry Illikainen <hji@dyntopia.com>
Date: Nov 16, 2019, 21:58 UTC
Message-ID: <20191116215850.3919-2-hji@dyntopia.com>
In-Reply-To: <20191116215850.3919-1-hji@dyntopia.com>

As part of implementing signature verification for git clone, I decided to refactor/unify the code for commit and merge verification to make it reusable during clones.

This lead me to discover that git requires merge signatures to be trusted (as opposed to TRUST_UNKNOWN or TRUST_NEVER). This is unlike the behavior of verify-tag and verify-commit.

So, I figured that I'd make the minimum trust level configurable to make the behavior of merge/commit/tag consistent. And while doing so, I noticed that parse_gpg_output() in gpg-interface.c assumes that the VALIDSIG status line has a field with a fingerprint for the primary key; but that is only the case for OpenPGP signatures.

The consequence of that assumption is that the subsequent status line is interpreted as the primary fingerprint for X509 signatures. I'm not sure if the order is hardcoded in GnuPG, but in my testing the TRUST_ status line always came after VALIDSIG -- and that breaks the possibility of a configuration option to set a minimum trust level since the TRUST_ line is consumed by VALIDSIG.

Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com>
---
 gpg-interface.c | 20 +++++++++++++++-----
 t/t4202-log.sh  |  6 ++++++
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index d60115ca40..01c7ef42d4 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -148,21 +148,31 @@ static void parse_gpg_output(struct signature_check *sigc)
 				}
 				/* Do we have fingerprint? */
 				if (sigcheck_gpg_status[i].flags & GPG_STATUS_FINGERPRINT) {
+					const char *limit;
+
 					next = strchrnul(line, ' ');
 					free(sigc->fingerprint);
 					sigc->fingerprint = xmemdupz(line, next - line);
 
-					/* Skip interim fields */
+					/* Skip interim fields.  The search is
+					 * limited to the same line since only
+					 * OpenPGP signatures has a field with
+					 * the primary fingerprint. */
+					limit = strchrnul(line, '\n');
 					for (j = 9; j > 0; j--) {
-						if (!*next)
+						if (!*next || next >= limit)
 							break;
 						line = next + 1;
 						next = strchrnul(line, ' ');
 					}
 
-					next = strchrnul(line, '\n');
-					free(sigc->primary_key_fingerprint);
-					sigc->primary_key_fingerprint = xmemdupz(line, next - line);
+					if (j == 0) {
+						next = strchrnul(line, '\n');
+						free(sigc->primary_key_fingerprint);
+						sigc->primary_key_fingerprint =
+							xmemdupz(line,
+								 next - line);
+					}
 				}
 
 				break;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e803ba402e..5d893b3137 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1580,6 +1580,12 @@ test_expect_success GPGSM 'setup signed branch x509' '
 	git commit -S -m signed_commit
 '
 
+test_expect_success GPGSM 'log x509 fingerprint' '
+	echo "F8BF62E0693D0694816377099909C779FA23FD65 | " >expect &&
+	git log -n1 --format="%GF | %GP" signed-x509 >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success GPG 'log --graph --show-signature' '
 	git log --graph --show-signature -n1 signed >actual &&
 	grep "^| gpg: Signature made" actual &&

-- 
2.24.GIT