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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 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@xxxxxxxxxxx> 1465981137 +0000
>  committer C O Mitter <committer@xxxxxxxxxxx> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux