From: Luke Shumaker <lukeshu@lukeshu.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
"Luke Shumaker" <lukeshu@lukeshu.com>,
git@vger.kernel.org, "Luke Shumaker" <lukeshu@datawire.io>,
"Junio C Hamano" <gitster@pobox.com>,
"Elijah Newren" <newren@gmail.com>, "Jeff King" <peff@peff.net>,
"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits
Date: Tue, 20 Apr 2021 11:15:25 -0600 [thread overview]
Message-ID: <87tuo0q3ma.wl-lukeshu@lukeshu.com> (raw)
In-Reply-To: <YH4xY/oSwYIUmJyL@camp.crustytoothpaste.net>
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@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,
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
next prev parent reply other threads:[~2021-04-20 17:15 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-19 22:54 [PATCH 0/3] fast-export, fast-import: implement signed-commits Luke Shumaker
2021-04-19 22:54 ` [PATCH 1/3] git-fast-import.txt: add missing LF in the BNF Luke Shumaker
2021-04-19 22:54 ` [PATCH 2/3] fast-export: rename --signed-tags='warn' to 'warn-verbatim' Luke Shumaker
2021-04-20 0:27 ` Taylor Blau
2021-04-20 15:45 ` Luke Shumaker
2021-04-19 22:54 ` [PATCH 3/3] fast-export, fast-import: implement signed-commits Luke Shumaker
2021-04-20 1:41 ` brian m. carlson
2021-04-20 17:15 ` Luke Shumaker [this message]
2021-04-20 23:07 ` brian m. carlson
2021-04-21 22:03 ` Luke Shumaker
2021-04-20 1:45 ` Taylor Blau
2021-04-20 16:23 ` Luke Shumaker
2021-04-20 15:51 ` Luke Shumaker
2021-04-21 18:12 ` [PATCH 0/3] " Elijah Newren
2021-04-21 19:28 ` Luke Shumaker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tuo0q3ma.wl-lukeshu@lukeshu.com \
--to=lukeshu@lukeshu.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=lukeshu@datawire.io \
--cc=newren@gmail.com \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).