Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 2752 bytes --]

On 2021-04-19 at 22:54:41, Luke Shumaker wrote:
> From: Luke Shumaker <lukeshu@datawire.io>
> 
> fast-export has an existing --signed-tags= flag that controls how to
> handle tag signatures.  However, there is no equivalent for commit
> signatures; it just silently strips the signature out of the commit
> (analogously to --signed-tags=strip).
> 
> While signatures are generally problematic for fast-export/fast-import
> (because hashes are likely to change), if they're going to support tag
> signatures, there's no reason to not also support commit signatures.
> 
> So, implement signed-commits.
> 
> On the fast-export side, try to be as much like signed-tags as possible,
> in both implementation and in user-interface; with the exception that
> the default should be `--signed-commits=strip` (compared to the default
> `--signed-tags=abort`), in order to continue defaulting to the
> historical behavior.  Only bother implementing "gpgsig", not
> "gpgsig-sha256"; the existing signed-tag support doesn't implement
> "gpgsig-sha256" either.

I would appreciate it if we did in fact implement it.  I would like to
use this functionality to round-trip objects between SHA-1 and SHA-256,
and it would be nice if both worked.

The situation with tags is different: the signature using the current
algorithm is always trailing, and the signature for the other algorithm
is in the header.  That wasn't how we intended it to be, but that's how
it ended up being.

As a result, tag output can support SHA-256 data, but with your
proposal, SHA-256 commits wouldn't work at all.  Considering SHA-1 is
wildly insecure and therefore signing SHA-1 commits adds very little
security, whereas SHA-256 is presently considered strong, I'd argue that
only supporting SHA-1 isn't the right move here.

Provided we do that and the test suite passes under both algorithms, I'm
strongly in favor of this feature.  In fact, I had been thinking about
implementing this feature myself just the other day, so I'm delighted
you decided to do it.

> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
> index 458af0a2d6..3d0c5dbf7d 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -437,6 +437,7 @@ change to the project.
>  	original-oid?
>  	('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
>  	'committer' (SP <name>)? SP LT <email> GT SP <when> LF
> +	('gpgsig' LF data)?

Could we emit this as "gpgsig sha1 data" and "gpgsig sha256 data"?  That
would allow us to consider the future case where the hash algorithm
changes again without requiring a change of format.
-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]