Re: [PATCH v2] gpg-interface: use more status letters

From: Junio C Hamano <hidden>
Date: 2016-10-10 17:58:38

Michael J Gruber [off-list ref] writes:

Sorry, this got "lost in vacation". Before that, I was looking for an
easy way to test expired signatures, but gpg1 and gpg2 behave somewhat
differently in that respect (2 does not allow to create already expired
signatures).

Is there anything I should or could do now?

I dunno.  It's your itch.

You can say "I'll need more time to figure out the way to test what
I am not testing here, so do not merge it to 'next' yet".

You can also say "This is good enough for now, so go ahead and merge
it to 'next'; more detailed tests can be done as follow-up patches
if needed".

You can also say "Thinking about it again, there is no strong reason
why we need to differentiate EXPSIG and EXPKEYSIG, so don't do this
SQUASH and use my original one as-is".

I'd be happy with any of the above and there may be other ones I'd
be happy with that I haven't thought of ;-)

[PATCH v3] gpg-interface: use more status letters

From: Michael J Gruber <hidden>
Date: 2016-10-12 13:04:56

According to gpg2's doc/DETAILS:

    For each signature only one of the codes GOODSIG, BADSIG,
    EXPSIG, EXPKEYSIG, REVKEYSIG or ERRSIG will be emitted.

gpg1 ("classic") behaves the same (although doc/DETAILS differs).

Currently, we parse gpg's status output for GOODSIG, BADSIG and
trust information and translate that into status codes G, B, U, N
for the %G?  format specifier.

git-verify-* returns success in the GOODSIG case only. This is
somewhat in disagreement with gpg, which considers the first 5 of
the 6 above as VALIDSIG, but we err on the very safe side.

Introduce additional status codes E, X, Y, R for ERRSIG, EXPSIG,
EXPKEYSIG, and REVKEYSIG so that a user of %G? gets more information
about the absence of a 'G' on first glance.

Requested-by: Alex [off-list ref]
Signed-off-by: Michael J Gruber <redacted>
---
v3 incorporates Junios' changes to the commit message, as well as his
suggestion how to use an undefined gpghome the way test-lib does.
Also, all Y-related changes (including the if in pretty.c).

Testing X, Y, and R from our test scripts is somewhat problematic
(some gpg versions do not allow back-dating, and we cannot ship pre-made
signatures easily) but I have tested all of them locally.

 Documentation/pretty-formats.txt | 10 ++++++++--
 gpg-interface.c                  | 13 ++++++++++---
 pretty.c                         |  4 ++++
 t/t7510-signed-commit.sh         | 13 ++++++++++++-
 4 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index a942d57f73..179c9389aa 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt

@@ -143,8 +143,14 @@ ifndef::git-rev-list[]
 - '%N': commit notes
 endif::git-rev-list[]
 - '%GG': raw verification message from GPG for a signed commit
-- '%G?': show "G" for a good (valid) signature, "B" for a bad signature,
-  "U" for a good signature with unknown validity and "N" for no signature
+- '%G?': show "G" for a good (valid) signature,
+  "B" for a bad signature,
+  "U" for a good signature with unknown validity,
+  "X" for a good signature that has expired,
+  "Y" for a good signature made by an expired key,
+  "R" for a good signature made by a revoked key,
+  "E" if the signature cannot be checked (e.g. missing key)
+  and "N" for no signature
 - '%GS': show the name of the signer for a signed commit
 - '%GK': show the key used to sign a signed commit
 - '%gD': reflog selector, e.g., `refs/stash@{1}` or

diff --git a/gpg-interface.c b/gpg-interface.c
index 8672edaf48..e44cc27da1 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c

@@ -33,6 +33,10 @@ static struct {
 	{ 'B', "\n[GNUPG:] BADSIG " },
 	{ 'U', "\n[GNUPG:] TRUST_NEVER" },
 	{ 'U', "\n[GNUPG:] TRUST_UNDEFINED" },
+	{ 'E', "\n[GNUPG:] ERRSIG "},
+	{ 'X', "\n[GNUPG:] EXPSIG "},
+	{ 'Y', "\n[GNUPG:] EXPKEYSIG "},
+	{ 'R', "\n[GNUPG:] REVKEYSIG "},
 };
 
 void parse_gpg_output(struct signature_check *sigc)

@@ -54,9 +58,12 @@ void parse_gpg_output(struct signature_check *sigc)
 		/* The trust messages are not followed by key/signer information */
 		if (sigc->result != 'U') {
 			sigc->key = xmemdupz(found, 16);
-			found += 17;
-			next = strchrnul(found, '\n');
-			sigc->signer = xmemdupz(found, next - found);
+			/* The ERRSIG message is not followed by signer information */
+			if (sigc-> result != 'E') {
+				found += 17;
+				next = strchrnul(found, '\n');
+				sigc->signer = xmemdupz(found, next - found);
+			}
 		}
 	}
 }

diff --git a/pretty.c b/pretty.c
index 25efbcac92..d89ca30911 100644
--- a/pretty.c
+++ b/pretty.c

@@ -1232,8 +1232,12 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 			switch (c->signature_check.result) {
 			case 'G':
 			case 'B':
+			case 'E':
 			case 'U':
 			case 'N':
+			case 'X':
+			case 'Y':
+			case 'R':
 				strbuf_addch(sb, c->signature_check.result);
 			}
 			break;

diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 6e839f5489..762135adea 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh

@@ -2,6 +2,7 @@
 
 test_description='signed commit tests'
 . ./test-lib.sh
+GNUPGHOME_NOT_USED=$GNUPGHOME
 . "$TEST_DIRECTORY/lib-gpg.sh"
 
 test_expect_success GPG 'create signed commits' '

@@ -190,7 +191,7 @@ test_expect_success GPG 'show bad signature with custom format' '
 	test_cmp expect actual
 '
 
-test_expect_success GPG 'show unknown signature with custom format' '
+test_expect_success GPG 'show untrusted signature with custom format' '
 	cat >expect <<-\EOF &&
 	U
 	61092E85B7227189

@@ -200,6 +201,16 @@ test_expect_success GPG 'show unknown signature with custom format' '
 	test_cmp expect actual
 '
 
+test_expect_success GPG 'show unknown signature with custom format' '
+	cat >expect <<-\EOF &&
+	E
+	61092E85B7227189
+
+	EOF
+	GNUPGHOME="$GNUPGHOME_NOT_USED" git log -1 --format="%G?%n%GK%n%GS" eighth-signed-alt >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success GPG 'show lack of signature with custom format' '
 	cat >expect <<-\EOF &&
 	N

-- 
2.10.1.532.gfe29b57

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help