{"thread":{"id":"49638","subject":"[PATCH 1/3] gpg-interface.c: use flags to determine key/signer info presence","startedAt":"2018-10-22T16:38:31Z","lastAt":"2018-10-24T03:10:44Z","messageCount":5,"participants":["Michał Górny","brian m. carlson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"361169","messageId":"20181022163821.23523-1-mgorny@gentoo.org","threadId":"49638","inReplyTo":null,"subject":"[PATCH 1/3] gpg-interface.c: use flags to determine key/signer info presence","fromName":"Michał Górny","fromEmail":"mgorny@gentoo.org","sentAt":"2018-10-22T16:38:19Z","receivedAt":"2018-10-22T16:38:31Z","isPatch":true,"sender":{"key":"mgorny@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/110765?v=4"},"body":"Replace the logic used to determine whether key and signer information\nis present to use explicit flags in sigcheck_gpg_status[] array.  This\nis more future-proof, since it makes it possible to add additional\nstatuses without having to explicitly update the conditions.\n\nSigned-off-by: Michał Górny <mgorny@gentoo.org>\n---\n gpg-interface.c | 27 +++++++++++++++++----------\n 1 file changed, 17 insertions(+), 10 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex d72a43b77..c7cd24ec0 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -77,20 +77,27 @@ void signature_check_clear(struct signature_check *sigc)\n \n /* An exclusive status -- only one of them can appear in output */\n #define GPG_STATUS_EXCLUSIVE\t(1<<0)\n+/* The status includes key identifier */\n+#define GPG_STATUS_KEYID\t(1<<1)\n+/* The status includes user identifier */\n+#define GPG_STATUS_UID\t\t(1<<2)\n+\n+/* Short-hand for standard exclusive *SIG status with keyid & UID */\n+#define GPG_STATUS_STDSIG\t(GPG_STATUS_EXCLUSIVE|GPG_STATUS_KEYID|GPG_STATUS_UID)\n \n static struct {\n \tchar result;\n \tconst char *check;\n \tunsigned int flags;\n } sigcheck_gpg_status[] = {\n-\t{ 'G', \"GOODSIG \", GPG_STATUS_EXCLUSIVE },\n-\t{ 'B', \"BADSIG \", GPG_STATUS_EXCLUSIVE },\n+\t{ 'G', \"GOODSIG \", GPG_STATUS_STDSIG },\n+\t{ 'B', \"BADSIG \", GPG_STATUS_STDSIG },\n \t{ 'U', \"TRUST_NEVER\", 0 },\n \t{ 'U', \"TRUST_UNDEFINED\", 0 },\n-\t{ 'E', \"ERRSIG \", GPG_STATUS_EXCLUSIVE },\n-\t{ 'X', \"EXPSIG \", GPG_STATUS_EXCLUSIVE },\n-\t{ 'Y', \"EXPKEYSIG \", GPG_STATUS_EXCLUSIVE },\n-\t{ 'R', \"REVKEYSIG \", GPG_STATUS_EXCLUSIVE },\n+\t{ 'E', \"ERRSIG \", GPG_STATUS_EXCLUSIVE|GPG_STATUS_KEYID },\n+\t{ 'X', \"EXPSIG \", GPG_STATUS_STDSIG },\n+\t{ 'Y', \"EXPKEYSIG \", GPG_STATUS_STDSIG },\n+\t{ 'R', \"REVKEYSIG \", GPG_STATUS_STDSIG },\n };\n \n static void parse_gpg_output(struct signature_check *sigc)\n@@ -117,13 +124,13 @@ static void parse_gpg_output(struct signature_check *sigc)\n \t\t\t\t}\n \n \t\t\t\tsigc->result = sigcheck_gpg_status[i].result;\n-\t\t\t\t/* The trust messages are not followed by key/signer information */\n-\t\t\t\tif (sigc->result != 'U') {\n+\t\t\t\t/* Do we have key information? */\n+\t\t\t\tif (sigcheck_gpg_status[i].flags & GPG_STATUS_KEYID) {\n \t\t\t\t\tnext = strchrnul(line, ' ');\n \t\t\t\t\tfree(sigc->key);\n \t\t\t\t\tsigc->key = xmemdupz(line, next - line);\n-\t\t\t\t\t/* The ERRSIG message is not followed by signer information */\n-\t\t\t\t\tif (*next && sigc->result != 'E') {\n+\t\t\t\t\t/* Do we have signer information? */\n+\t\t\t\t\tif (*next && (sigcheck_gpg_status[i].flags & GPG_STATUS_UID)) {\n \t\t\t\t\t\tline = next + 1;\n \t\t\t\t\t\tnext = strchrnul(line, '\\n');\n \t\t\t\t\t\tfree(sigc->signer);\n-- \n2.19.1\n\n"},{"id":"361170","messageId":"20181022163821.23523-2-mgorny@gentoo.org","threadId":"49638","inReplyTo":"20181022163821.23523-1-mgorny@gentoo.org","subject":"[PATCH 2/3] gpg-interface.c: Support getting key fingerprint via %GF format","fromName":"Michał Górny","fromEmail":"mgorny@gentoo.org","sentAt":"2018-10-22T16:38:20Z","receivedAt":"2018-10-22T16:38:33Z","isPatch":true,"sender":{"key":"mgorny@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/110765?v=4"},"body":"Support processing VALIDSIG status that provides additional information\nfor valid signatures.  Use this information to propagate signing key\nfingerprint and expose it via %GF pretty format.  This format can be\nused to build safer key verification systems that verify the key via\ncomplete fingerprint rather than short/long identifier provided by %GK.\n\nSigned-off-by: Michał Górny <mgorny@gentoo.org>\n---\n Documentation/pretty-formats.txt |  1 +\n gpg-interface.c                  | 14 +++++++++++++-\n gpg-interface.h                  |  1 +\n pretty.c                         |  4 ++++\n t/t7510-signed-commit.sh         | 18 ++++++++++++------\n 5 files changed, 31 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 6109ef09a..8ab7d6dd1 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -153,6 +153,7 @@ endif::git-rev-list[]\n   and \"N\" for no signature\n - '%GS': show the name of the signer for a signed commit\n - '%GK': show the key used to sign a signed commit\n+- '%GF': show the fingerprint of the key used to sign a signed commit\n - '%gD': reflog selector, e.g., `refs/stash@{1}` or\n   `refs/stash@{2 minutes ago`}; the format follows the rules described\n   for the `-g` option. The portion before the `@` is the refname as\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex c7cd24ec0..a406484e4 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -73,6 +73,7 @@ void signature_check_clear(struct signature_check *sigc)\n \tFREE_AND_NULL(sigc->gpg_status);\n \tFREE_AND_NULL(sigc->signer);\n \tFREE_AND_NULL(sigc->key);\n+\tFREE_AND_NULL(sigc->fingerprint);\n }\n \n /* An exclusive status -- only one of them can appear in output */\n@@ -81,6 +82,8 @@ void signature_check_clear(struct signature_check *sigc)\n #define GPG_STATUS_KEYID\t(1<<1)\n /* The status includes user identifier */\n #define GPG_STATUS_UID\t\t(1<<2)\n+/* The status includes key fingerprints */\n+#define GPG_STATUS_FINGERPRINT\t(1<<3)\n \n /* Short-hand for standard exclusive *SIG status with keyid & UID */\n #define GPG_STATUS_STDSIG\t(GPG_STATUS_EXCLUSIVE|GPG_STATUS_KEYID|GPG_STATUS_UID)\n@@ -98,6 +101,7 @@ static struct {\n \t{ 'X', \"EXPSIG \", GPG_STATUS_STDSIG },\n \t{ 'Y', \"EXPKEYSIG \", GPG_STATUS_STDSIG },\n \t{ 'R', \"REVKEYSIG \", GPG_STATUS_STDSIG },\n+\t{ 0, \"VALIDSIG \", GPG_STATUS_FINGERPRINT },\n };\n \n static void parse_gpg_output(struct signature_check *sigc)\n@@ -123,7 +127,8 @@ static void parse_gpg_output(struct signature_check *sigc)\n \t\t\t\t\t\tgoto found_duplicate_status;\n \t\t\t\t}\n \n-\t\t\t\tsigc->result = sigcheck_gpg_status[i].result;\n+\t\t\t\tif (sigcheck_gpg_status[i].result)\n+\t\t\t\t\tsigc->result = sigcheck_gpg_status[i].result;\n \t\t\t\t/* Do we have key information? */\n \t\t\t\tif (sigcheck_gpg_status[i].flags & GPG_STATUS_KEYID) {\n \t\t\t\t\tnext = strchrnul(line, ' ');\n@@ -137,6 +142,12 @@ static void parse_gpg_output(struct signature_check *sigc)\n \t\t\t\t\t\tsigc->signer = xmemdupz(line, next - line);\n \t\t\t\t\t}\n \t\t\t\t}\n+\t\t\t\t/* Do we have fingerprint? */\n+\t\t\t\tif (sigcheck_gpg_status[i].flags & GPG_STATUS_FINGERPRINT) {\n+\t\t\t\t\tnext = strchrnul(line, ' ');\n+\t\t\t\t\tfree(sigc->fingerprint);\n+\t\t\t\t\tsigc->fingerprint = xmemdupz(line, next - line);\n+\t\t\t\t}\n \n \t\t\t\tbreak;\n \t\t\t}\n@@ -154,6 +165,7 @@ static void parse_gpg_output(struct signature_check *sigc)\n \t */\n \tsigc->result = 'E';\n \t/* Clear partial data to avoid confusion */\n+\tFREE_AND_NULL(sigc->fingerprint);\n \tFREE_AND_NULL(sigc->signer);\n \tFREE_AND_NULL(sigc->key);\n }\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex acf50c461..8ce614fc9 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -23,6 +23,7 @@ struct signature_check {\n \tchar result;\n \tchar *signer;\n \tchar *key;\n+\tchar *fingerprint;\n };\n \n void signature_check_clear(struct signature_check *sigc);\ndiff --git a/pretty.c b/pretty.c\nindex 8ca29e928..4567b5321 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1256,6 +1256,10 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\tif (c->signature_check.key)\n \t\t\t\tstrbuf_addstr(sb, c->signature_check.key);\n \t\t\tbreak;\n+\t\tcase 'F':\n+\t\t\tif (c->signature_check.fingerprint)\n+\t\t\t\tstrbuf_addstr(sb, c->signature_check.fingerprint);\n+\t\t\tbreak;\n \t\tdefault:\n \t\t\treturn 0;\n \t\t}\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 180f0be91..19ccae286 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -175,8 +175,9 @@ test_expect_success GPG 'show good signature with custom format' '\n \tG\n \t13B6F51ECDDE430D\n \tC O Mitter <committer@example.com>\n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n \tEOF\n-\tgit log -1 --format=\"%G?%n%GK%n%GS\" sixth-signed >actual &&\n+\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF\" sixth-signed >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -185,8 +186,9 @@ test_expect_success GPG 'show bad signature with custom format' '\n \tB\n \t13B6F51ECDDE430D\n \tC O Mitter <committer@example.com>\n+\n \tEOF\n-\tgit log -1 --format=\"%G?%n%GK%n%GS\" $(cat forged1.commit) >actual &&\n+\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF\" $(cat forged1.commit) >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -195,8 +197,9 @@ test_expect_success GPG 'show untrusted signature with custom format' '\n \tU\n \t61092E85B7227189\n \tEris Discordia <discord@example.net>\n+\tD4BE22311AD3131E5EDA29A461092E85B7227189\n \tEOF\n-\tgit log -1 --format=\"%G?%n%GK%n%GS\" eighth-signed-alt >actual &&\n+\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF\" eighth-signed-alt >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -205,8 +208,9 @@ test_expect_success GPG 'show unknown signature with custom format' '\n \tE\n \t61092E85B7227189\n \n+\n \tEOF\n-\tGNUPGHOME=\"$GNUPGHOME_NOT_USED\" git log -1 --format=\"%G?%n%GK%n%GS\" eighth-signed-alt >actual &&\n+\tGNUPGHOME=\"$GNUPGHOME_NOT_USED\" git log -1 --format=\"%G?%n%GK%n%GS%n%GF\" eighth-signed-alt >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -215,8 +219,9 @@ test_expect_success GPG 'show lack of signature with custom format' '\n \tN\n \n \n+\n \tEOF\n-\tgit log -1 --format=\"%G?%n%GK%n%GS\" seventh-unsigned >actual &&\n+\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF\" seventh-unsigned >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -255,8 +260,9 @@ test_expect_success GPG 'show double signature with custom format' '\n \tE\n \n \n+\n \tEOF\n-\tgit log -1 --format=\"%G?%n%GK%n%GS\" $(cat double-commit.commit) >actual &&\n+\tgit log -1 --format=\"%G?%n%GK%n%GS%n%GF\" $(cat double-commit.commit) >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.19.1\n\n"},{"id":"361171","messageId":"20181022163821.23523-3-mgorny@gentoo.org","threadId":"49638","inReplyTo":"20181022163821.23523-1-mgorny@gentoo.org","subject":"[PATCH 3/3] gpg-interface.c: Obtain primary key fingerprint as well","fromName":"Michał Górny","fromEmail":"mgorny@gentoo.org","sentAt":"2018-10-22T16:38:21Z","receivedAt":"2018-10-22T16:38:34Z","isPatch":true,"sender":{"key":"mgorny@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/110765?v=4"},"body":"Obtain the primary key fingerprint off VALIDSIG status message,\nand expose it via %GP format.\n\nSigned-off-by: Michał Górny <mgorny@gentoo.org>\n---\n Documentation/pretty-formats.txt |  2 ++\n gpg-interface.c                  | 16 +++++++++++++++-\n gpg-interface.h                  |  1 +\n pretty.c                         |  4 ++++\n 4 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 8ab7d6dd1..417b638cd 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -154,6 +154,8 @@ endif::git-rev-list[]\n - '%GS': show the name of the signer for a signed commit\n - '%GK': show the key used to sign a signed commit\n - '%GF': show the fingerprint of the key used to sign a signed commit\n+- '%GP': show the fingerprint of the primary key whose subkey was used\n+  to sign a signed commit\n - '%gD': reflog selector, e.g., `refs/stash@{1}` or\n   `refs/stash@{2 minutes ago`}; the format follows the rules described\n   for the `-g` option. The portion before the `@` is the refname as\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex a406484e4..8ed274533 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -74,6 +74,7 @@ void signature_check_clear(struct signature_check *sigc)\n \tFREE_AND_NULL(sigc->signer);\n \tFREE_AND_NULL(sigc->key);\n \tFREE_AND_NULL(sigc->fingerprint);\n+\tFREE_AND_NULL(sigc->primary_key_fingerprint);\n }\n \n /* An exclusive status -- only one of them can appear in output */\n@@ -108,7 +109,7 @@ static void parse_gpg_output(struct signature_check *sigc)\n {\n \tconst char *buf = sigc->gpg_status;\n \tconst char *line, *next;\n-\tint i;\n+\tint i, j;\n \tint seen_exclusive_status = 0;\n \n \t/* Iterate over all lines */\n@@ -147,6 +148,18 @@ static void parse_gpg_output(struct signature_check *sigc)\n \t\t\t\t\tnext = strchrnul(line, ' ');\n \t\t\t\t\tfree(sigc->fingerprint);\n \t\t\t\t\tsigc->fingerprint = xmemdupz(line, next - line);\n+\n+\t\t\t\t\t/* Skip interim fields */\n+\t\t\t\t\tfor (j = 9; j > 0; j--) {\n+\t\t\t\t\t\tif (!*next)\n+\t\t\t\t\t\t\tbreak;\n+\t\t\t\t\t\tline = next + 1;\n+\t\t\t\t\t\tnext = strchrnul(line, ' ');\n+\t\t\t\t\t}\n+\n+\t\t\t\t\tnext = strchrnul(line, '\\n');\n+\t\t\t\t\tfree(sigc->primary_key_fingerprint);\n+\t\t\t\t\tsigc->primary_key_fingerprint = xmemdupz(line, next - line);\n \t\t\t\t}\n \n \t\t\t\tbreak;\n@@ -165,6 +178,7 @@ static void parse_gpg_output(struct signature_check *sigc)\n \t */\n \tsigc->result = 'E';\n \t/* Clear partial data to avoid confusion */\n+\tFREE_AND_NULL(sigc->primary_key_fingerprint);\n \tFREE_AND_NULL(sigc->fingerprint);\n \tFREE_AND_NULL(sigc->signer);\n \tFREE_AND_NULL(sigc->key);\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex 8ce614fc9..3e624ec28 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -24,6 +24,7 @@ struct signature_check {\n \tchar *signer;\n \tchar *key;\n \tchar *fingerprint;\n+\tchar *primary_key_fingerprint;\n };\n \n void signature_check_clear(struct signature_check *sigc);\ndiff --git a/pretty.c b/pretty.c\nindex 4567b5321..b83a3ecd2 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1260,6 +1260,10 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\tif (c->signature_check.fingerprint)\n \t\t\t\tstrbuf_addstr(sb, c->signature_check.fingerprint);\n \t\t\tbreak;\n+\t\tcase 'P':\n+\t\t\tif (c->signature_check.primary_key_fingerprint)\n+\t\t\t\tstrbuf_addstr(sb, c->signature_check.primary_key_fingerprint);\n+\t\t\tbreak;\n \t\tdefault:\n \t\t\treturn 0;\n \t\t}\n-- \n2.19.1\n\n"},{"id":"361337","messageId":"20181023225605.GB6119@genre.crustytoothpaste.net","threadId":"49638","inReplyTo":"20181022163821.23523-1-mgorny@gentoo.org","subject":"Re: [PATCH 1/3] gpg-interface.c: use flags to determine key/signer info presence","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-23T22:56:05Z","receivedAt":"2018-10-23T22:56:13Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Oct 22, 2018 at 06:38:19PM +0200, Michał Górny wrote:\n> Replace the logic used to determine whether key and signer information\n> is present to use explicit flags in sigcheck_gpg_status[] array.  This\n> is more future-proof, since it makes it possible to add additional\n> statuses without having to explicitly update the conditions.\n\nThis series looks good to me.  I was going to ask after patch 2 whether\nyou were printing the subkey or primary key fingerprint, and then you\nanswered my question in patch 3.  Thanks for including both.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"361351","messageId":"xmqqwoq8ox8w.fsf@gitster-ct.c.googlers.com","threadId":"49638","inReplyTo":"20181023225605.GB6119@genre.crustytoothpaste.net","subject":"Re: [PATCH 1/3] gpg-interface.c: use flags to determine key/signer info presence","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-24T03:10:39Z","receivedAt":"2018-10-24T03:10:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> On Mon, Oct 22, 2018 at 06:38:19PM +0200, Michał Górny wrote:\n>> Replace the logic used to determine whether key and signer information\n>> is present to use explicit flags in sigcheck_gpg_status[] array.  This\n>> is more future-proof, since it makes it possible to add additional\n>> statuses without having to explicitly update the conditions.\n>\n> This series looks good to me.  I was going to ask after patch 2 whether\n> you were printing the subkey or primary key fingerprint, and then you\n> answered my question in patch 3.  Thanks for including both.\n\nYeah, this looks good to me too.  Thanks, both.\n"}]}