git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "brian m. carlson" <sandals@crustytoothpaste.net>
To: Jeff King <peff@peff.net>
Cc: "Wu, Zhichen" <zhwu@amazon.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: Question About Git V2 Protocol & SHA256
Date: Wed, 30 Sep 2020 02:19:25 +0000	[thread overview]
Message-ID: <20200930021925.GI1392312@camp.crustytoothpaste.net> (raw)
In-Reply-To: <20200930004630.GA623061@coredump.intra.peff.net>

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

On 2020-09-30 at 00:46:30, Jeff King wrote:
> I agree that we _could_ just dump sha256 within a v1 protocol and wait
> for the client to choke. But that seems like an awfully lousy user
> experience. By contrast, assuming that the client wants sha1 means that
> either:
> 
>   - we can reject it with a sensible ERR message that tells the user
>     what is going on
> 
>   - we can serve them by talking in terms of sha1 if we're willing to
>     do the extra conversion work server-side (and/or have a cache of
>     sha1-format objects)

If you have a suitably new Git, it will fail with an appropriate
message on both sides.  The client will complain that you are trying to
use a remote that uses the wrong hash algorithm and the server will
complain that you have failed to pass a suitable object-format
extension.  That will look like this:

  fatal: mismatched object format: server sha256; client sha1
  fatal: expected flush after ref listing

That's true if you're using protocol v2 at all, or a suitably new Git
with protocol v1, although the messages may differ slightly.  However,
if you're using an older Git with v1, then you'll get this:

  fatal: protocol error: unexpected '304c98b4860fa40b3e04f3e2e24db1a13ab959922a63209685813908c4fabc83 HEAD'
  fatal: the remote end hung up unexpectedly

That's because, as you pointed out, we don't get to the point where the
client fails to send the value.

> The only thing we lose is that a recent client who understands sha256
> wouldn't be able to contact us and do a sha256-over-v1 transaction. But
> why would they want to do so?

Because v2 isn't implemented for pushes yet.  The testsuite does not
even come close to passing unless you have a fully functional remote
system.  That's why I implemented protocol support, even though it
wasn't originally planned.

That was mentioned in the cover letter of the series that introduced
protocol support.

I should also point out that v2 SSH support doesn't work at all with
OpenSSH unless the server is specifically configured to allow the
GIT_PROTOCOL environment variable.  Since there are many use cases for
people to push Git data over SSH to servers they do not control, there
is a valid and viable use case for v1 support.  I personally know people
with shell accounts who could not use v2 over SSH but could viably use
v1.  Heck, that's even true for our shell host at work.

Absent a significant outreach campaign to all major operating system
vendors to get this environment variable added to all supported versions
of their OpenSSH packages, I don't see how restricting SHA-256 to
protocol v2 is acceptable.  Even then, many systems would still fail to
work correctly due to local config file modifications.

In retrospect, we probably should have sent "GIT_PROTOCOL=version=2" as
part of the shell command and let implementers deal with the fact that
their SSH servers would need to adequately parse and interpret shell
commands to function properly.  But it's probably too late to do that
now without bumping the protocol to v3.

> This is all hypothetical at this point, though, right? I tried finding
> details in the hash transition document, but protocol changes are listed
> as out of scope there. It does say that sha256 servers may just reject
> sha1 clients; but even so I'd prefer if we could do it with a nice
> message (i.e., my bullet one above).

It is not hypothetical.  Git in master has a fully functional SHA-256
implementation that fully interoperates with other SHA-256 repositories.
As I mentioned above and elsewhere, that is a prerequisite for a fully
functional testsuite.  I would not have shipped user-visible SHA-256
support with a testsuite as broken as it would be without protocol
support.

> My suggestion does also require that we have a v2 receive-pack protocol,
> which does not yet exist (but following the blueprint for fetch, I don't
> expect it to be a big deal).

If someone would like to implement protocol v2 for push in time for
2.29, then that could be a viable approach if we can address the serious
problem of the SSH situation in that time frame as well.

I don't plan to implement v2 push support because that is a substantial
amount of work and I have even less time to work on Git than normal due
to taking classes multiple nights a week.  As I know you are aware,
working on Git is not my day job.

I will admit to being a bit annoyed that we're discussing changes like
this at this point.  I've tried to be open about the design and
implementation, answering questions and providing complete branches and
reasonably thorough cover letters that people can peruse to know what's
going on even if they aren't interested in the full series themselves.
LWN has even summarized some of the later work.  It's fine if folks
don't want to participate in that process, but it's hard to incorporate
feedback that comes in well after the fact.  I certainly hope the nature
and content of my SHA-256 work isn't a surprise to regulars on the list,
because I've clearly failed to communicate effectively if that's the
case.

I am at this point not planning any other major changes to SHA-256
support before the interop work starts to land.  I am of course happy to
address any bug reports that come up, and if others would like to send
in patches related to SHA-256, I will review them and provide feedback
as my time allows.
-- 
brian m. carlson: Houston, Texas, US

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

  reply	other threads:[~2020-09-30  2:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-29  1:17 Question About Git V2 Protocol & SHA256 Wu, Zhichen
2020-09-29 22:13 ` Jeff King
2020-09-29 22:41   ` Wu, Zhichen
2020-09-29 22:43   ` brian m. carlson
2020-09-29 23:07     ` Wu, Zhichen
2020-09-30  0:46     ` Jeff King
2020-09-30  2:19       ` brian m. carlson [this message]
2020-09-30 12:20         ` Jeff King
2020-10-01 23:52           ` brian m. carlson

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=20200930021925.GI1392312@camp.crustytoothpaste.net \
    --to=sandals@crustytoothpaste.net \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=zhwu@amazon.com \
    /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).