Expose gpgsig in pretty-print

Currently, users who do not have GPG installed have no way to discern signed from unsigned commits without examining raw commit data. I propose two new pretty-print placeholders to expose this information:

%GR: full ("R"aw) contents of gpgsig header %G+: Y/N if the commit has nonempty gpgsig header or not

The second is of course much more likely to be used, but having exposed the one, exposing the other too adds almost no complexity.

I'm open to suggestion on the names of these placeholders.

This commit is based on master but e5a329a279 ("run-command: report exec failure" 2018-12-11) is required for the tests to pass.

One note is that this change touches areas of the pretty-format documentation that are radically revamped in aw/pretty-trailers: see 42617752d4 ("doc: group pretty-format.txt placeholders descriptions" 2018-12-08). I have another version of this branch based on that branch as well, so you can use that in case conflicts with aw/pretty-trailers arise.

See:
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers

John Passaro (4):
pretty: expose raw commit signature t/t7510-signed-commit.sh: test new placeholders doc, tests: pretty behavior when gpg missing docs/pretty-formats: add explanation + copy edits

 Documentation/pretty-formats.txt |  21 ++++--
 pretty.c                         |  36 ++++++++-
 t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--
 3 files changed, 167 insertions(+), 15 deletions(-)

base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db

Quoted 32 lines · John Passaro

Currently, users who do not have GPG installed have no way to discern signed from unsigned commits without examining raw commit data. I propose two new pretty-print placeholders to expose this information:

%GR: full ("R"aw) contents of gpgsig header %G+: Y/N if the commit has nonempty gpgsig header or not

The second is of course much more likely to be used, but having exposed the one, exposing the other too adds almost no complexity.

I'm open to suggestion on the names of these placeholders.

This commit is based on master but e5a329a279 ("run-command: report exec failure" 2018-12-11) is required for the tests to pass.

One note is that this change touches areas of the pretty-format documentation that are radically revamped in aw/pretty-trailers: see 42617752d4 ("doc: group pretty-format.txt placeholders descriptions" 2018-12-08). I have another version of this branch based on that branch as well, so you can use that in case conflicts with aw/pretty-trailers arise.

See:
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers

John Passaro (4):
pretty: expose raw commit signature t/t7510-signed-commit.sh: test new placeholders doc, tests: pretty behavior when gpg missing docs/pretty-formats: add explanation + copy edits

Quoted 8 lines

 Documentation/pretty-formats.txt |  21 ++++--
 pretty.c                         |  36 ++++++++-
 t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--
 3 files changed, 167 insertions(+), 15 deletions(-)

base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db

Just a suggestion: since the raw signature is not very useful without the commit data to check it against, and the commit data is non-trivial to construct (requires mangling raw data anyway), maybe you could either add another placeholder to get the data for signature verification, or (alternatively or simultaneously) add a placeholder that prints both data and signature in the OpenPGP message format (i.e. something you can pass straight to 'gpg --verify').

Quoted 43 lines · Michał Górny

Quoted 32 lines · John Passaro

Currently, users who do not have GPG installed have no way to discern signed from unsigned commits without examining raw commit data. I propose two new pretty-print placeholders to expose this information:

%GR: full ("R"aw) contents of gpgsig header %G+: Y/N if the commit has nonempty gpgsig header or not

The second is of course much more likely to be used, but having exposed the one, exposing the other too adds almost no complexity.

I'm open to suggestion on the names of these placeholders.

This commit is based on master but e5a329a279 ("run-command: report exec failure" 2018-12-11) is required for the tests to pass.

One note is that this change touches areas of the pretty-format documentation that are radically revamped in aw/pretty-trailers: see 42617752d4 ("doc: group pretty-format.txt placeholders descriptions" 2018-12-08). I have another version of this branch based on that branch as well, so you can use that in case conflicts with aw/pretty-trailers arise.

See:
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers

John Passaro (4):
pretty: expose raw commit signature t/t7510-signed-commit.sh: test new placeholders doc, tests: pretty behavior when gpg missing docs/pretty-formats: add explanation + copy edits

Quoted 8 lines

 Documentation/pretty-formats.txt |  21 ++++--
 pretty.c                         |  36 ++++++++-
 t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--
 3 files changed, 167 insertions(+), 15 deletions(-)

base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db

Quoted 8 lines

Just a suggestion: since the raw signature is not very useful without the commit data to check it against, and the commit data is non-trivial to construct (requires mangling raw data anyway), maybe you could either add another placeholder to get the data for signature verification, or (alternatively or simultaneously) add a placeholder that prints both data and signature in the OpenPGP message format (i.e. something you can pass straight to 'gpg --verify').

That's a great idea!

Then I might rename the other new placeholders too:

%Gs: signed commit signature (blank when unsigned)
%Gp: signed commit payload (i.e. in practice minus the gpgsig header; also blank when unsigned as well)
%Gq: query/question whether is signed commit ("Y"/"N")

Thus establishing %G<lowercase> as the gpg-related placeholders that do not actually require gpg.

And add a test that %Gp%n%Gs or the like passes gpg --verify.

I'll put in a v2 later today or tomorrow. Thank you for the feedback!

Quoted 67 lines · John Passaro

Quoted 43 lines · Michał Górny

Quoted 32 lines · John Passaro

Currently, users who do not have GPG installed have no way to discern signed from unsigned commits without examining raw commit data. I propose two new pretty-print placeholders to expose this information:

%GR: full ("R"aw) contents of gpgsig header %G+: Y/N if the commit has nonempty gpgsig header or not

The second is of course much more likely to be used, but having exposed the one, exposing the other too adds almost no complexity.

I'm open to suggestion on the names of these placeholders.

This commit is based on master but e5a329a279 ("run-command: report exec failure" 2018-12-11) is required for the tests to pass.

One note is that this change touches areas of the pretty-format documentation that are radically revamped in aw/pretty-trailers: see 42617752d4 ("doc: group pretty-format.txt placeholders descriptions" 2018-12-08). I have another version of this branch based on that branch as well, so you can use that in case conflicts with aw/pretty-trailers arise.

See:
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers

John Passaro (4):
pretty: expose raw commit signature t/t7510-signed-commit.sh: test new placeholders doc, tests: pretty behavior when gpg missing docs/pretty-formats: add explanation + copy edits

Quoted 8 lines

 Documentation/pretty-formats.txt |  21 ++++--
 pretty.c                         |  36 ++++++++-
 t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--
 3 files changed, 167 insertions(+), 15 deletions(-)

base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db

Quoted 8 lines

Just a suggestion: since the raw signature is not very useful without the commit data to check it against, and the commit data is non-trivial to construct (requires mangling raw data anyway), maybe you could either add another placeholder to get the data for signature verification, or (alternatively or simultaneously) add a placeholder that prints both data and signature in the OpenPGP message format (i.e. something you can pass straight to 'gpg --verify').

That's a great idea!

Then I might rename the other new placeholders too:

%Gs: signed commit signature (blank when unsigned)
%Gp: signed commit payload (i.e. in practice minus the gpgsig header; also blank when unsigned as well)
%Gq: query/question whether is signed commit ("Y"/"N")

Thus establishing %G<lowercase> as the gpg-related placeholders that do not actually require gpg.

And add a test that %Gp%n%Gs or the like passes gpg --verify.

Quoted 2 lines

I'll put in a v2 later today or tomorrow. Thank you for the feedback!

Technically speaking, '%Gp%n%Gs' sounds a bit odd, given that the payload needs to be preceded by the PGP message header but instead of footer it has the signature's header. Also note that some lines in the payload may need to be escaped.

--
Best regards,
Michał Górny

Quoted 69 lines · Michał Górny

Quoted 67 lines · John Passaro

Quoted 43 lines · Michał Górny

Quoted 32 lines · John Passaro

Currently, users who do not have GPG installed have no way to discern signed from unsigned commits without examining raw commit data. I propose two new pretty-print placeholders to expose this information:

%GR: full ("R"aw) contents of gpgsig header %G+: Y/N if the commit has nonempty gpgsig header or not

The second is of course much more likely to be used, but having exposed the one, exposing the other too adds almost no complexity.

I'm open to suggestion on the names of these placeholders.

This commit is based on master but e5a329a279 ("run-command: report exec failure" 2018-12-11) is required for the tests to pass.

One note is that this change touches areas of the pretty-format documentation that are radically revamped in aw/pretty-trailers: see 42617752d4 ("doc: group pretty-format.txt placeholders descriptions" 2018-12-08). I have another version of this branch based on that branch as well, so you can use that in case conflicts with aw/pretty-trailers arise.

See:
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers

John Passaro (4):
pretty: expose raw commit signature t/t7510-signed-commit.sh: test new placeholders doc, tests: pretty behavior when gpg missing docs/pretty-formats: add explanation + copy edits

Quoted 8 lines

 Documentation/pretty-formats.txt |  21 ++++--
 pretty.c                         |  36 ++++++++-
 t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--
 3 files changed, 167 insertions(+), 15 deletions(-)

base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db

Quoted 8 lines

Just a suggestion: since the raw signature is not very useful without the commit data to check it against, and the commit data is non-trivial to construct (requires mangling raw data anyway), maybe you could either add another placeholder to get the data for signature verification, or (alternatively or simultaneously) add a placeholder that prints both data and signature in the OpenPGP message format (i.e. something you can pass straight to 'gpg --verify').

That's a great idea!

Then I might rename the other new placeholders too:

%Gs: signed commit signature (blank when unsigned)
%Gp: signed commit payload (i.e. in practice minus the gpgsig header; also blank when unsigned as well)
%Gq: query/question whether is signed commit ("Y"/"N")

Thus establishing %G<lowercase> as the gpg-related placeholders that do not actually require gpg.

And add a test that %Gp%n%Gs or the like passes gpg --verify.

Quoted 7 lines

Quoted 2 lines

I'll put in a v2 later today or tomorrow. Thank you for the feedback!

Technically speaking, '%Gp%n%Gs' sounds a bit odd, given that the payload needs to be preceded by the PGP message header but instead of footer it has the signature's header. Also note that some lines in the payload may need to be escaped.

It's indeed failing with
"-----BEGIN PGP SIGNED MESSAGE-----%n%Gp%n%Gs"

This appears to be a misunderstanding on my part of how cleartext GPG messages work, as this message seems to fail verification even when I construct it manually:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

tree 3820419b50e9827493beedf6a1423e7d9c7edf3b parent 356f37356edf1a45c8494870e1c051e2fbe529fa author A U Thor <[email protected]> 1112912413 -0700 committer C O Mitter <[email protected]> 1112912533 -0700

sixth
-----BEGIN PGP SIGNATURE-----

iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCXBQzKRYcY29tbWl0dGVy QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN2QYAnA2A/VpD4qMI9rJlIYnyyO32rTXz AJ0drWlJsASMcf6AEX6nSQPxWq81Fg==
=fr2F
-----END PGP SIGNATURE-----

I got this by running tho following inside the trash directory for t7510, which as far as I can tell is roughly equivalent to
The gpg --verify fails.
{
echo "-----BEGIN PGP SIGNED MESSAGE-----"
echo Hash: SHA256
echo
git cat-file commit sixth-signed | perl -ne '/^(?:gpgsig)? / || print' git cat-file commit sixth-signed | perl -ne 's/^(?:gpgsig)? // && print'
} | tee commit-as-signed-message | gpg --verify

All seems to work fine when I treat %Gs as a detached signature. How should the combined message be constructed properly? (Goes to usefulness of printing signature payload, and indeed of raw crypto data in general.)

Quoted 3 lines

--
Best regards,
Michał Górny

Then I might rename the other new placeholders too:

%Gs: signed commit signature (blank when unsigned)
%Gp: signed commit payload (i.e. in practice minus the gpgsig header; also blank when unsigned as well)

All seems to work fine when I treat %Gs as a detached signature.

Quoted 5 lines · John Passaro

Then I might rename the other new placeholders too:

%Gs: signed commit signature (blank when unsigned)
%Gp: signed commit payload (i.e. in practice minus the gpgsig header; also blank when unsigned as well)

One complication: the pretty-printing code sees the commit data in the i18n.logOutputEncoding charset (utf8 by default). But the signature will be over the raw commit data. That's also utf8 by default, but there may be an encoding header indicating that it's something else. In that case, you couldn't actually verify the signature from the "%Gs%Gp" pair.

I don't think that's insurmountable in the code. You'll have to jump through a few hoops to make sure you have the _original_ payload, but we obviously do have that data. However, it does feel a little weird to include content from a different encoding in the middle of the log output stream which claims to be i18n.logOutputEncoding.

Quoted 1 line · John Passaro

All seems to work fine when I treat %Gs as a detached signature.

In light of this, my best guess as to why the cleartext PGP message didn't verify properly is that the commit data normally doesn't end with \n, but as far as I can tell there's no way to express that in the cleartext format. I don't see a way around this.

However, as long
as the following works, I think we have proof-of-concept that this enhancement allows you to play with signature data however you please without leaving it to git under the hood:

gpg --verify <(git show -s --format=format:%Gs commit) <(git show -s --format=format:%Gp commit)

Quoted 15 lines · John Passaro

Quoted 6 lines

Then I might rename the other new placeholders too:

%Gs: signed commit signature (blank when unsigned)
%Gp: signed commit payload (i.e. in practice minus the gpgsig header; also blank when unsigned as well)

One complication: the pretty-printing code sees the commit data in the i18n.logOutputEncoding charset (utf8 by default). But the signature will be over the raw commit data. That's also utf8 by default, but there may be an encoding header indicating that it's something else. In that case, you couldn't actually verify the signature from the "%Gs%Gp" pair.

Quoted 6 lines

I don't think that's insurmountable in the code. You'll have to jump through a few hoops to make sure you have the _original_ payload, but we obviously do have that data. However, it does feel a little weird to include content from a different encoding in the middle of the log output stream which claims to be i18n.logOutputEncoding.

Thanks for the feedback! This is an interesting conflict. If the user requests %Gp, the payload for the signature, they almost certainly do want it in the original encoding; if i18n.logOutputEncoding is something incompatible, whether explicitly or by default, that seems like an error. Not much we can do to reconcile the two requests (commit encoding vs output encoding) so seems reasonable to treat it as fatal.

Updated patch coming as soon as I work out Peff's aforementioned "few hoops" to get properly encoded data -- and also how to test success and failure!

Just a suggestion: since the raw signature is not very useful without the commit data to check it against, and the commit data is non-trivial to construct (requires mangling raw data anyway), maybe you could either add another placeholder to get the data for signature verification, or (alternatively or simultaneously) add a placeholder that prints both data and signature in the OpenPGP message format (i.e. something you can pass straight to 'gpg --verify').