[PATCH v3 2/2] gpg-interface: limit search for primary key fingerprint

From: Hans Jerry Illikainen <hji@dyntopia.com>
Date: Nov 21, 2019, 23:43 UTC
Message-ID: <20191121234336.26300-3-hji@dyntopia.com>
In-Reply-To: <20191121234336.26300-1-hji@dyntopia.com>

The VALIDSIG status line from GnuPG with --status-fd is documented to have 9 required and 1 optional fields [1]. The final, and optional, field is used to specify the fingerprint of the primary key that made the signature in case it was made by a subkey. However, this field is only available for OpenPGP signatures; not for CMS/X.509.

The current code assumes that the VALIDSIG status line always has 10 fields. Furthermore, the current code assumes that each field is separated by a space (0x20) character.

If the VALIDSIG status line does not have the optional 10th field, the current code will continue reading onto the next status line -- because only 0x20 is considered a field separator, not 0xa. And this is the case for non-OpenPGP signatures [1].

The consequence is that a subsequent status line may be considered as the "primary key" for signatures that does not have an actual primary key.

The solution introduced by this commit is to add 0xa as a bound for the search for a primary key. The search for the 10th VALIDSIG field is aborted as soon as it sees a newline character. This keeps the parser from interpreting subsequent lines as the primary key.

[1] https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=doc/DETAILS#l483

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

diff --git a/gpg-interface.c b/gpg-interface.c
index b4c4443287..4269937b83 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -119,7 +119,8 @@ static void replace_cstring(const char **field, const char *line,
 static void parse_gpg_output(struct signature_check *sigc)
 {
 	const char *buf = sigc->gpg_status;
-	const char *line, *next;
+	const char *line, *next, *limit;
+	const char **field;
 	int i, j;
 	int seen_exclusive_status = 0;
 
@@ -160,18 +161,27 @@ static void parse_gpg_output(struct signature_check *sigc)
 					next = strchrnul(line, ' ');
 					replace_cstring(&sigc->fingerprint, line, next);
 
-					/* 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 || limit <= next)
 							break;
 						line = next + 1;
 						next = strchrnul(line, ' ');
 					}
 
-					next = strchrnul(line, '\n');
-					free(sigc->primary_key_fingerprint);
-					replace_cstring(&sigc->primary_key_fingerprint,
-							line, next);
+					field = &sigc->primary_key_fingerprint;
+					if (!j) {
+						next = strchrnul(line, '\n');
+						replace_cstring(field, line, next);
+					} else {
+						replace_cstring(field, NULL, NULL);
+					}
 				}
 
 				break;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e803ba402e..17ec2401ec 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1570,6 +1570,14 @@ test_expect_success GPG 'setup signed branch' '
 	git commit -S -m signed_commit
 '
 
+test_expect_success GPG 'setup signed branch with subkey' '
+        test_when_finished "git reset --hard && git checkout master" &&
+        git checkout -b signed-subkey master &&
+        echo foo >foo &&
+        git add foo &&
+        git commit -SB7227189 -m signed_commit
+'
+
 test_expect_success GPGSM 'setup signed branch x509' '
 	test_when_finished "git reset --hard && git checkout master" &&
 	git checkout -b signed-x509 master &&
@@ -1580,6 +1588,18 @@ 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 GPGSM 'log OpenPGP fingerprint' '
+        echo "D4BE22311AD3131E5EDA29A461092E85B7227189" > expect &&
+        git log -n1 --format="%GP" signed-subkey >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.0.157.gba9f894af8