gpg-interface: limit search for primary key fingerprint

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.

If the VALIDSIG status line does not have the optional 10th field, the
current code will continue reading onto the next status line.  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.

Limit the search of these 9 or 10 fields to the single line to avoid
this problem.  If the 10th field is missing, report that there is no
primary key fingerprint.

[Reference]

[1] GnuPG Details, General status codes
https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=doc/DETAILS;h=6ce340e8c04794add995e84308bb3091450bd28f;hb=HEAD#l483

The documentation says:

    VALIDSIG <args>

    The args are:

    - <fingerprint_in_hex>
    - <sig_creation_date>
    - <sig-timestamp>
    - <expire-timestamp>
    - <sig-version>
    - <reserved>
    - <pubkey-algo>
    - <hash-algo>
    - <sig-class>
    - [ <primary-key-fpr> ]

    This status indicates that the signature is cryptographically
    valid. [...] PRIMARY-KEY-FPR is the fingerprint of the primary key
    or identical to the first argument.

    The primary-key-fpr parameter is used for OpenPGP and not available
    for CMS signatures.  [...]

Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/gpg-interface.c b/gpg-interface.c
index 37162c9a43c7e3b5d9135da55520021798a0d160..131e7d529e9bdb480c88a01a33a3dd8339d219cd 100644 (file)
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -156,21 +156,33 @@ 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;
+                                       char **field;
+
                                        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 819c24d10eaa3cb4a58b72ae8a8e96151b5cabe9..da8cb06f9be8368925d462f183d12184463d1d86 100755 (executable)
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1555,6 +1555,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 &&
@@ -1565,6 +1573,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 &&