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

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Apr 20, 2021, 23:07 UTC
Message-ID: <YH9enUedtHjE87ET@camp.crustytoothpaste.net>
In-Reply-To: <87tuo0q3ma.wl-lukeshu@lukeshu.com>

On 2021-04-20 at 17:15:25, Luke Shumaker wrote:

Show 19 quoted lines

> On Mon, 19 Apr 2021 19:41:55 -0600,
> brian m. carlson wrote:
> > 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).

If you're using a repository that's SHA-1, then the tag signature that's part of the message is a signature over the SHA-1 contents of the object, and the gpgsig-sha256 header is a signature over the SHA-256 contents of the object. If you're using a repository that's SHA-256, it's reversed: the signature at the end of the message covers the SHA-256 contents of the object and the gpgsig header covers the SHA-1 contents.

It isn't currently possible to create objects with both signatures in place, but that will be possible in the future.

Show 10 quoted lines

> >                                                   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.

Currently, repositories using SHA-256 currently don't interoperate with SHA-1 repositories. However, if you want to create a test repo, you can do so with "git init --object-format=sha256" in an empty directory.

If you want to run the testsuite in SHA-256 mode, set GIT_TEST_DEFAULT_HASH=sha256, and all the repositories created will use SHA-256.

That should be sufficient to get this series such that it will work with simple SHA-256 repos. If you have more questions about this work or how to get things working, I'm happy to answer them.

> 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.

Not implementing this means the CI will fail when the testsuite is run in SHA-256 mode, so your patch probably won't be accepted.

Show 17 quoted lines

> > 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?

Yes, dual-signed objects have to manually created right now; there's no tooling to create them because that code hasn't landed yet. It's in my tree and very broken. But you can create SHA-256 repositories as I mentioned above and test those, and the testsuite does run in that mode, so it should be easy enough to check at least single-signed commits for now, even if you don't implement dual-signed ones. I think it's fine if that comes later, and I can pick that up as part of a future series.

-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US