Re: [PATCH 1/1] signature-format.txt: add space to fix gpgsig continuation line

From: Jeff King <peff@peff.net>
Date: Oct 11, 2021, 16:41 UTC
Message-ID: <YWRpPw4eTwTmgVvC@coredump.intra.peff.net>
In-Reply-To: <20211009163338.2175170-1-rlb@defaultvalue.org>

On Sat, Oct 09, 2021 at 11:33:38AM -0500, Rob Browning wrote:

> Add a space to the blank line after the version header in the example
> gpgsig commit header.

Thanks, this is a good catch. The space is important for the header continuation.

Show 13 quoted lines

> diff --git a/Documentation/technical/signature-format.txt b/Documentation/technical/signature-format.txt
> index 2c9406a56a..6acc0b1247 100644
> --- a/Documentation/technical/signature-format.txt
> +++ b/Documentation/technical/signature-format.txt
> @@ -78,7 +78,7 @@ author A U Thor <author@example.com> 1465981137 +0000
>  committer C O Mitter <committer@example.com> 1465981137 +0000
>  gpgsig -----BEGIN PGP SIGNATURE-----
>   Version: GnuPG v1
> -
> + 
>   iQEcBAABAgAGBQJXYRjRAAoJEGEJLoW3InGJ3IwIAIY4SA6GxY3BjL60YyvsJPh/
>   HRCJwH+w7wt3Yc/9/bW2F+gF72kdHOOs2jfv+OZhq0q4OAN6fvVSczISY/82LpS7
>   DVdMQj2/YcHDT4xrDNBnXnviDO9G7am/9OE77kEbXrp7QPxvhjkicHNwy2rEflAA

The patch is quite subtle to read, of course. :) But more importantly, it is subtle for somebody reading the documentation to notice. Perhaps it's worth calling it out explicitly? E.g., squashing in something like:

diff --git a/Documentation/technical/signature-format.txt b/Documentation/technical/signature-format.txt
index 6acc0b1247..f418431050 100644
--- a/Documentation/technical/signature-format.txt
+++ b/Documentation/technical/signature-format.txt
@@ -68,7 +68,9 @@ signed tag message body
 - created by: `git commit -S`
 - payload: commit object
 - embedding: header entry `gpgsig`
-  (content is preceded by a space)
+  (content is preceded by a space; note that this includes the
+   "empty" line between the GnuPG header and signature, which
+   consists of a single space).
 - example: commit with subject `signed commit`
 
 ----

>  While investigating this (while parsing commit objects) I also
>  noticed a commit in a repo that had a blank continuation line (" \n")
>  after the "END PGP SIGNATURE" line.
> 
>  Presuming that's valid, I suppose we could consider detailing any
>  additional specifications, i.e. what kind of trailing content a
>  parser should expect/ignore, etc.

It is valid. It looks like GitHub's signed-merges may do this. From our
perspective, we are not really defining what is valid content or not. We
take everything in the gpgsig header (including multiple lines connected
by space-continuation), and feed it to gpg. So if gpg is happy with it,
we are happy with it.

Thinking with a security hat on, it's possible this could lead to
confusion (say, if I include multiple signatures but some tools check
one and some the other). But I'd be hesitant to start adding
restrictions without even a plausible attack scenario.

-Peff