From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2F7A2C433ED for ; Tue, 20 Apr 2021 17:15:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DB7EB61076 for ; Tue, 20 Apr 2021 17:15:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232916AbhDTRP7 (ORCPT ); Tue, 20 Apr 2021 13:15:59 -0400 Received: from mav.lukeshu.com ([104.207.138.63]:58156 "EHLO mav.lukeshu.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232473AbhDTRP6 (ORCPT ); Tue, 20 Apr 2021 13:15:58 -0400 Received: from lukeshu-dw-thinkpad (unknown [8.42.69.49]) by mav.lukeshu.com (Postfix) with ESMTPSA id EB48180590; Tue, 20 Apr 2021 13:15:25 -0400 (EDT) Date: Tue, 20 Apr 2021 11:15:25 -0600 Message-ID: <87tuo0q3ma.wl-lukeshu@lukeshu.com> From: Luke Shumaker To: "brian m. carlson" , Luke Shumaker , git@vger.kernel.org, Luke Shumaker , Junio C Hamano , Elijah Newren , Jeff King , Johannes Schindelin , =?UTF-8?B?Tmd1eeG7hW4g?= =?ISO-8859-1?Q?Th?= =?ISO-8859-1?Q?=E1i_?= =?UTF-8?B?Tmfhu41j?= Duy Subject: Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits In-Reply-To: References: <20210419225441.3139048-1-lukeshu@lukeshu.com> <20210419225441.3139048-4-lukeshu@lukeshu.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Mon, 19 Apr 2021 19:41:55 -0600, brian m. carlson wrote: > > [1 ] > On 2021-04-19 at 22:54:41, Luke Shumaker wrote: > > From: Luke Shumaker > > > > 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 )? SP LT GT SP LF)? > > 'committer' (SP )? SP LT GT SP 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