subject:"\[PATCH v2\] gpg\-interface\: use more status letters"

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

2016-10-10 Thread Junio C Hamano

Michael J Gruber  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 ;-)

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

2016-10-10 Thread Michael J Gruber

Junio C Hamano venit, vidit, dixit 06.10.2016 23:43:
> Junio C Hamano  writes:
> 
>> Michael J Gruber  writes:
>>
>>> Also, I'm open to using another letter for EXPKEYSIG but couldn't decide
>>> between 'Y', 'Z', 'K'. 'K' could be confused with REVKEYSIG, I'm afraid.
>>> 'Y' is next to 'X' and contained in 'KEY', it would be my first choice.
>>
>> Sounds good enough to me.  Thanks.
> 
> I really do not want to leave too many topics listed in the "What's
> cooking" report to be in undecided / waiting state.  How about
> squashing this in, with a fully updated log message to replace?

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?

Michael

> -- >8 --
> From: Michael J Gruber 
> Date: Wed, 28 Sep 2016 16:24:13 +0200
> Subject: [PATCH] SQUASH: gpg-interface: use more status letters
> 
> 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 
> Signed-off-by: Michael J Gruber 
> Signed-off-by: Junio C Hamano 
> ---
>  Documentation/pretty-formats.txt | 3 ++-
>  gpg-interface.c  | 2 +-
>  pretty.c | 1 +
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/pretty-formats.txt 
> b/Documentation/pretty-formats.txt
> index c28ff2b919..179c9389aa 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -146,7 +146,8 @@ endif::git-rev-list[]
>  - '%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 expired signature, or good signature made by an expired key,
> +  "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
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 6999e7b469..e44cc27da1 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -35,7 +35,7 @@ static struct {
>   { 'U', "\n[GNUPG:] TRUST_UNDEFINED" },
>   { 'E', "\n[GNUPG:] ERRSIG "},
>   { 'X', "\n[GNUPG:] EXPSIG "},
> - { 'X', "\n[GNUPG:] EXPKEYSIG "},
> + { 'Y', "\n[GNUPG:] EXPKEYSIG "},
>   { 'R', "\n[GNUPG:] REVKEYSIG "},
>  };
>  
> diff --git a/pretty.c b/pretty.c
> index 39a36cd825..f98b271069 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1236,6 +1236,7 @@ static size_t format_commit_one(struct strbuf *sb, /* 
> in UTF-8 */
>   case 'U':
>   case 'N':
>   case 'X':
> + case 'Y':
>   case 'R':
>   strbuf_addch(sb, c->signature_check.result);
>   }
>

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

2016-10-06 Thread Junio C Hamano

Junio C Hamano  writes:

> Michael J Gruber  writes:
>
>> Also, I'm open to using another letter for EXPKEYSIG but couldn't decide
>> between 'Y', 'Z', 'K'. 'K' could be confused with REVKEYSIG, I'm afraid.
>> 'Y' is next to 'X' and contained in 'KEY', it would be my first choice.
>
> Sounds good enough to me.  Thanks.

I really do not want to leave too many topics listed in the "What's
cooking" report to be in undecided / waiting state.  How about
squashing this in, with a fully updated log message to replace?

-- >8 --
From: Michael J Gruber 
Date: Wed, 28 Sep 2016 16:24:13 +0200
Subject: [PATCH] SQUASH: gpg-interface: use more status letters

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 
Signed-off-by: Michael J Gruber 
Signed-off-by: Junio C Hamano 
---
 Documentation/pretty-formats.txt | 3 ++-
 gpg-interface.c  | 2 +-
 pretty.c | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index c28ff2b919..179c9389aa 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -146,7 +146,8 @@ endif::git-rev-list[]
 - '%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 expired signature, or good signature made by an expired key,
+  "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
diff --git a/gpg-interface.c b/gpg-interface.c
index 6999e7b469..e44cc27da1 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -35,7 +35,7 @@ static struct {
{ 'U', "\n[GNUPG:] TRUST_UNDEFINED" },
{ 'E', "\n[GNUPG:] ERRSIG "},
{ 'X', "\n[GNUPG:] EXPSIG "},
-   { 'X', "\n[GNUPG:] EXPKEYSIG "},
+   { 'Y', "\n[GNUPG:] EXPKEYSIG "},
{ 'R', "\n[GNUPG:] REVKEYSIG "},
 };

diff --git a/pretty.c b/pretty.c
index 39a36cd825..f98b271069 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1236,6 +1236,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
case 'U':
case 'N':
case 'X':
+   case 'Y':
case 'R':
strbuf_addch(sb, c->signature_check.result);
}
-- 
2.10.1-520-ge127bfd383

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

2016-09-30 Thread Michael J Gruber

Ramsay Jones venit, vidit, dixit 28.09.2016 23:09:
> 
> 
> On 28/09/16 20:59, Junio C Hamano wrote:
>> Michael J Gruber  writes:
>  
>>> +  "X" for a good expired signature, or good signature made by an expired 
>>> key,
>>
>> As an attempt to clarify that we cover both EXPSIG and EXPKEYSIG
>> cases, I think this is good enough.  I may have phrased the former
>> slightly differently, though: "a good signature that has expired".
>>
>> I have no strong opinion if we want to stress that we cover both
>> cases, though, which is I think what Ramsay's comment was about.
> 
> Kinda! ;-)
> 
> I'm not sure that it is a good idea to mash both EXPSIG and EXPKEYSIG
> into one status letter, but I was also fishing for some information
> about EXPSIG. I was only vaguely aware that a signature could expire
> _independently_ of the key used to do the signing. Also, according to
> https://www.gnupg.org/documentation/manuals/gnupg/Automated-signature-checking.html
> for the EXPSIG case 'Note, that this case is currently not implemented.'

A key can have an expiration date.

A signature can have an expiration date.

The "goodness" of a signature is independent of the expiraton dates.

Signature expiration is implemented, I tested that (gpg 1 aka "classic").

> Hmm, I guess these are so closely related that a single status letter
> is OK, but I think I would prefer your phrasing; namely:
> 
>  "X" for a good signature that has expired, or a good signature made with an 
> expired key,
> 

I'm open to whatever phrasing you deem clearer.

Also, I'm open to using another letter for EXPKEYSIG but couldn't decide
between 'Y', 'Z', 'K'. 'K' could be confused with REVKEYSIG, I'm afraid.
'Y' is next to 'X' and contained in 'KEY', it would be my first choice.

Cheers,
Michael

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

2016-09-30 Thread Michael J Gruber

Junio C Hamano venit, vidit, dixit 28.09.2016 21:59:
> Michael J Gruber  writes:
> 
>> - Use GNUPGHOME="$HOME/gnupg-home-not-used" just like in other tests (lib).
> 
> If you are not using /dev/null, I expected you to do
> 
>   . ./test-lib.sh
>   GNUPGHOME_saved=$GNPGHOME
> . "$TEST_DIRECTORY/lib-gpg.sh"
> 
> and then use
> 
>   GNUPGHOME="$GNUPGHOME_saved" git log -1 ...
> 
> in the test.
> 
> Otherwise, you are not futureproofing your use and only adding to
> maintenance burden.  The gnupg-home-not-used hack may turn out to be
> a problematic and test-lib.sh may update to point to somewhere else,
> which will leave your copy still pointing at the old problematic
> place).

Well, I understood you told me to do what test-lib.sh does.

You obviously wanted me to piggy-bak on test-lib.sh's behavior instead.

I don't know what's more likely to break - the latter relies on
test-lib.sh's setting GNUPGHOME to a non existing gpg home, which is
something funny to do if you don't even plan to use gpg.

>> - Do not parse for signer UID in the ERRSIG case (and test that we do not).
> 
> Good.
> 
>> - Retreat "rather" addition from the doc: good/valid are terms that we use
>>   differently from gpg anyways.
> 
> OK.
> 
>> +  "X" for a good expired signature, or good signature made by an expired 
>> key,
> 
> As an attempt to clarify that we cover both EXPSIG and EXPKEYSIG
> cases, I think this is good enough.  I may have phrased the former
> slightly differently, though: "a good signature that has expired".
> 
> I have no strong opinion if we want to stress that we cover both
> cases, though, which is I think what Ramsay's comment was about.

I'll comment in the reply to his 2nd e-mail

Michael

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

2016-09-28 Thread Ramsay Jones

On 28/09/16 20:59, Junio C Hamano wrote:
> Michael J Gruber  writes:

>> +  "X" for a good expired signature, or good signature made by an expired 
>> key,
> 
> As an attempt to clarify that we cover both EXPSIG and EXPKEYSIG
> cases, I think this is good enough.  I may have phrased the former
> slightly differently, though: "a good signature that has expired".
> 
> I have no strong opinion if we want to stress that we cover both
> cases, though, which is I think what Ramsay's comment was about.

Kinda! ;-)

I'm not sure that it is a good idea to mash both EXPSIG and EXPKEYSIG
into one status letter, but I was also fishing for some information
about EXPSIG. I was only vaguely aware that a signature could expire
_independently_ of the key used to do the signing. Also, according to
https://www.gnupg.org/documentation/manuals/gnupg/Automated-signature-checking.html
for the EXPSIG case 'Note, that this case is currently not implemented.'

Hmm, I guess these are so closely related that a single status letter
is OK, but I think I would prefer your phrasing; namely:

 "X" for a good signature that has expired, or a good signature made with an 
expired key,

[Although that is still a bit cumbersome.]

ATB,
Ramsay Jones

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

2016-09-28 Thread Junio C Hamano

Michael J Gruber  writes:

> - Use GNUPGHOME="$HOME/gnupg-home-not-used" just like in other tests (lib).

If you are not using /dev/null, I expected you to do

. ./test-lib.sh
GNUPGHOME_saved=$GNPGHOME
. "$TEST_DIRECTORY/lib-gpg.sh"

and then use

GNUPGHOME="$GNUPGHOME_saved" git log -1 ...

in the test.

Otherwise, you are not futureproofing your use and only adding to
maintenance burden.  The gnupg-home-not-used hack may turn out to be
a problematic and test-lib.sh may update to point to somewhere else,
which will leave your copy still pointing at the old problematic
place).

> - Do not parse for signer UID in the ERRSIG case (and test that we do not).

Good.

> - Retreat "rather" addition from the doc: good/valid are terms that we use
>   differently from gpg anyways.

OK.

> +  "X" for a good expired signature, or good signature made by an expired key,

As an attempt to clarify that we cover both EXPSIG and EXPKEYSIG
cases, I think this is good enough.  I may have phrased the former
slightly differently, though: "a good signature that has expired".

I have no strong opinion if we want to stress that we cover both
cases, though, which is I think what Ramsay's comment was about.

Thanks.

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

2016-09-28 Thread Ramsay Jones



On 28/09/16 15:24, Michael J Gruber wrote:
> 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, R for ERRSIG, EXP*SIG, REVKEYSIG
> so that a user of %G? gets more information about the absence of a 'G'
> on first glance.
> 
> Requested-by: Alex 
> Signed-off-by: Michael J Gruber 
> ---
> Changes in v2:
> 
> - Use GNUPGHOME="$HOME/gnupg-home-not-used" just like in other tests (lib).
> - Do not parse for signer UID in the ERRSIG case (and test that we do not).
> - Retreat "rather" addition from the doc: good/valid are terms that we use
>   differently from gpg anyways.
> 
>  Documentation/pretty-formats.txt |  9 +++--
>  gpg-interface.c  | 13 ++---
>  pretty.c |  3 +++
>  t/t7510-signed-commit.sh | 12 +++-
>  4 files changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/pretty-formats.txt 
> b/Documentation/pretty-formats.txt
> index a942d57..c28ff2b 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -143,8 +143,13 @@ 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 expired signature, or good signature made by an expired key,

Hmm, this looks odd. Would the following:

"X" for a good signature made with an expired key,

mean something different?

ATB,
Ramsay Jones

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

2016-09-28 Thread Michael J Gruber

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, R for ERRSIG, EXP*SIG, REVKEYSIG
so that a user of %G? gets more information about the absence of a 'G'
on first glance.

Requested-by: Alex 
Signed-off-by: Michael J Gruber 
---
Changes in v2:

- Use GNUPGHOME="$HOME/gnupg-home-not-used" just like in other tests (lib).
- Do not parse for signer UID in the ERRSIG case (and test that we do not).
- Retreat "rather" addition from the doc: good/valid are terms that we use
  differently from gpg anyways.

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

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index a942d57..c28ff2b 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -143,8 +143,13 @@ 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 expired signature, or 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 8672eda..6999e7b 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 "},
+   { 'X', "\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 493edb0..39a36cd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1232,8 +1232,11 @@ 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 'R':
strbuf_addch(sb, c->signature_check.result);
}
break;
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 6e839f5..9f487f9 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -190,7 +190,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 +200,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 &&

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

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

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

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

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

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

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

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

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

9 matches

Site Navigation

Mail list logo

Footer information