On Mon, 19 Apr 2021 19:41:55 -0600,
brian m. carlson wrote:
>
> [1 <text/plain; utf-8 (quoted-printable)>]
> On 2021-04-19 at 22:54:41, Luke Shumaker wrote:
> > From: Luke Shumaker <lukeshu@xxxxxxxxxxx>
> >
> > 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,
I don't believe that's true? With SHA-1-signed tags, the signature
gets included in the fast-import stream as part of the tag message
(the `data` line in the BNF). Since SHA-256-signed tags have their
signature as a header (rather than just appending it to the message),
we'd have to add a 'gpgsig' sub-command to the 'tag' top-level-command
(like I've done to the 'commit' top-level-command).
> 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.
The main reason I didn't implement SHA-256 support (well, besides that
the repo I'm working on turned out to not have any SHA-256-signed
commits in it) is that I had questions about SHA-256 that I didn't
know/couldn't find the answers to.
However, looking again, I see a few of the answers in
t7510-signed-commit.sh, so I'll have a go at it. If I get stuck, I'll
go ahead and implement the below "gpgsig sha1" suggestion, and leave
the sha256 implementation to someone else.
> 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.
That's one of the big reasons I didn't implement both--I wasn't sure
how to test sha256 (within the test harness, `git commit -S` gives a
sha1 signature).
I see that t7510-signed-commit.sh 'verify-commit verifies multiply
signed commits' tests sha256 by hard-coding a raw commit object in the
test itself, and feeding that to `git hash-object`. I'd prefer to
figure out how to get `git commit` itself to generate a sha256
signature rather than a sha1 signature, so that I can _know_ that I'm
getting the ordering of headers the same as `git commit`. But I don't
think that needs to be a blocker; if the test doesn't do the same
ordering as `git commit`, I guess that can just be a bugfix later?
> > 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.
I like that idea. I'll implement it.
FWIW, I thought about instead adding a fast-import command to insert
arbitrary headers in to the commit object, rather than having to add a
new command for every new header we want to be able to round-trip.
But it's like, if we're exposing that much of the low-levels of a
commit object, why are we keeping up the facade fast-import stream at
all, instead of streaming raw Git objects around?
--
Happy hacking,
~ Luke Shumaker
