git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Jonathan Tan <jonathantanmy@google.com>,
	git@vger.kernel.org, bwilliamseng@gmail.com
Subject: Re: [PATCH 0/3] protocol v2 and hidden refs
Date: Sat, 15 Dec 2018 20:53:35 +0100	[thread overview]
Message-ID: <87d0q21s8w.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20181214083507.GB11777@sigill.intra.peff.net>


On Fri, Dec 14 2018, Jeff King wrote:

> On Thu, Dec 13, 2018 at 11:53:05AM -0800, Jonathan Tan wrote:
>
>> >     I don't know if there's a good solution. I tried running the whole
>> >     test suite with v2 as the default. It does find this bug, but it has
>> >     a bunch of other problems (notably fetch-pack won't run as v2, but
>> >     some other tests I think also depend on v0's reachability rules,
>> >     which v2 is documented not to enforce).
>>
>> I think Aevar's patches (sent after you wrote this) is a good start, and
>> I have started looking at it too.
>
> Yeah, I'm excited to see it working with fetch-pack, as the current
> behavior is to complain if you've tried to enable v2 config:
>
>   $ git config protocol.version 2
>   $ git fetch-pack git://github.com/git/git
>   fatal: support for protocol v2 not implemented yet
>
> I haven't actually run into it in the real world, but somebody might if
> they have scripted around fetch-pack and are experimenting with v2. A
> much friendlier behavior would be falling back to v1, but actually
> supporting v2 is better still. :)
>
>> >   - The "serve" command is funky, because it has no concept of whether
>> >     the "ls-refs" is for fetching or pushing. Is git-serve even a thing
>> >     that we want to support going forward?  I know part of the original
>> >     v2 conception was that one would be able to just connect to
>> >     "git-serve" and do a number of operations. But in practice the v2
>> >     probing requires saying "I'd like to git-upload-pack, and v2 if you
>> >     please". So no client ever calls git-serve.
>> >
>> >     Is this something we plan to eventually move to? Or can it be
>> >     considered a funny vestige of the development? In the latter case, I
>> >     think we should consider removing it.
>>
>> Personally, I lean towards removing it, but there are arguments on both
>> sides. In particular, removing "serve" means that both developers and
>> users of Git need not be concerned with a 3rd endpoint, but preserving
>> "serve" (and planning to migrate away from "upload-pack" and
>> "receive-pack") means that we will only have one endpoint, eliminating
>> confusion about which endpoint to use when making certain requests (when
>> we add requests other than "fetch" and "push").
>
> Yeah, at first glance I like the simplicity of a unified model. But the
> separate fetch/push endpoints have been useful in the past. Separate
> uploadpack/receive.hiderefs that I dealt with here are one form. Another
> is that many people do HTTP access control using the endpoints. For
> example, if I have a repo which is public-read and private-write, the
> config we recommend in git-http-backend(1) is to lock down the
> receive-pack access using webserver config.
>
> If all the webserver sees is "somebody wants to connect to git-serve",
> it doesn't know if it should be challenging them for authentication or
> not. It would have to start peeking into the git-serve conversation to
> see what the client actually wants to do. That's _possible_ to do, but
> it gets pretty awkward with existing webserver tools (whereas matching
> the URI endpoint is pretty easy).
>
> Ditto for locked down ssh sessions like git-shell (or custom solutions
> like gitolite). Right now we can "git-upload-pack is OK on this repo,
> git-receive-pack is not". But blindly running "git serve" would be
> dangerous. In this case I think we have a few more options, because the
> user has always already authenticated. So we can just tell "git serve"
> via the environment whether the user is authorized for push. It's harder
> with HTTP because most setups avoid even challenging for auth unless
> it's necessary.
>
> So I'm a bit worried that the unified endpoint model is going to be a
> dead end, at which point carrying around git-serve just makes things
> more complicated.

This is from wetware memory of something discussed at a previous Git
Merge, so I may have (inadvertently) made it up, but wasn't part of the
idea of "git-serve" to have an extensible next-gen protocol where we
could add new actions & verbs unrelated to sending or receiving packs?

E.g. client<->server optimistic cooperation like offloading a
long-running "git-grep", "git log -G" etc. to a more powerful workhorse
server, which would use "git-serve" as a routing layer.

Of course that's not in itself an argument for having a general "serve"
command, actually the opposite for the reasons you mention with locking
down things. E.g. maybe I want to support server-side git-grep on my
server, but not git-log, and if it's one command it becomes a hassle to
do that via SSH config or HTTPD config for the reasons you mention.

The upside would be that once a host understands "git serve" I'm more
likely to be able to get past whatever middle layer there is between my
client and the "git" binary on the other side. E.g. if I have a newly
compiled "git" client/server binary, but something like GitLab's
"gitaly" sitting between the two of us.

  reply	other threads:[~2018-12-15 19:53 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-11 10:42 [PATCH 0/3] protocol v2 and hidden refs Jeff King
2018-12-11 10:43 ` [PATCH 1/3] serve: pass "config context" through to individual commands Jeff King
2018-12-14  2:09   ` Junio C Hamano
2018-12-14  8:20     ` Jeff King
2018-12-15  0:31       ` Junio C Hamano
2018-12-16 10:25         ` Jeff King
2018-12-16 11:12           ` Junio C Hamano
2018-12-18 12:47             ` Jeff King
2018-12-14  8:36   ` Jonathan Nieder
2018-12-14  8:55     ` Jeff King
2018-12-14  9:28       ` Jonathan Nieder
2018-12-14  9:55         ` Jeff King
2018-12-11 10:43 ` [PATCH 2/3] parse_hide_refs_config: handle NULL section Jeff King
2018-12-14  2:11   ` Junio C Hamano
2018-12-11 10:44 ` [PATCH 3/3] upload-pack: support hidden refs with protocol v2 Jeff King
2018-12-11 11:45 ` [PATCH 0/3] protocol v2 and hidden refs Ævar Arnfjörð Bjarmason
2018-12-11 13:55   ` Jeff King
2018-12-11 21:21     ` [PATCH 0/3] Add a GIT_TEST_PROTOCOL_VERSION=X test mode Ævar Arnfjörð Bjarmason
2018-12-11 21:24       ` Ævar Arnfjörð Bjarmason
2018-12-11 21:21     ` [PATCH 1/3] tests: add a special setup where for protocol.version Ævar Arnfjörð Bjarmason
2018-12-12  0:27       ` [PATCH 0/3] Some fixes and improvements Jonathan Tan
2018-12-12  0:27         ` [PATCH 1/3] squash this into your patch Jonathan Tan
2018-12-12  0:27         ` [PATCH 2/3] builtin/fetch-pack: support protocol version 2 Jonathan Tan
2018-12-12  0:27         ` [PATCH 3/3] also squash this into your patch Jonathan Tan
2018-12-13  2:49         ` [PATCH 0/3] Some fixes and improvements Junio C Hamano
2018-12-13 15:58           ` [PATCH v2 0/8] protocol v2 fixes Ævar Arnfjörð Bjarmason
2018-12-17 22:40             ` [PATCH v3 0/4] " Ævar Arnfjörð Bjarmason
2018-12-18 12:48               ` Jeff King
2018-12-17 22:40             ` [PATCH v3 1/4] serve: pass "config context" through to individual commands Ævar Arnfjörð Bjarmason
2018-12-17 22:40             ` [PATCH v3 2/4] parse_hide_refs_config: handle NULL section Ævar Arnfjörð Bjarmason
2018-12-17 22:40             ` [PATCH v3 3/4] upload-pack: support hidden refs with protocol v2 Ævar Arnfjörð Bjarmason
2018-12-17 22:40             ` [PATCH v3 4/4] fetch-pack: support protocol version 2 Ævar Arnfjörð Bjarmason
2019-01-08 19:45               ` Junio C Hamano
2019-01-08 20:38                 ` Jonathan Tan
2019-01-08 21:14                   ` Jeff King
2018-12-13 15:58           ` [PATCH v2 1/8] serve: pass "config context" through to individual commands Ævar Arnfjörð Bjarmason
2018-12-13 15:58           ` [PATCH v2 2/8] parse_hide_refs_config: handle NULL section Ævar Arnfjörð Bjarmason
2018-12-13 15:58           ` [PATCH v2 3/8] upload-pack: support hidden refs with protocol v2 Ævar Arnfjörð Bjarmason
2018-12-13 15:58           ` [PATCH v2 4/8] tests: add a check for unportable env --unset Ævar Arnfjörð Bjarmason
2018-12-13 15:58           ` [PATCH v2 5/8] tests: add a special setup where for protocol.version Ævar Arnfjörð Bjarmason
2018-12-13 19:48             ` Jonathan Tan
2018-12-13 15:58           ` [PATCH v2 6/8] tests: mark & fix tests broken under GIT_TEST_PROTOCOL_VERSION=1 Ævar Arnfjörð Bjarmason
2018-12-13 15:58           ` [PATCH v2 7/8] builtin/fetch-pack: support protocol version 2 Ævar Arnfjörð Bjarmason
2018-12-14 10:17             ` Jeff King
2018-12-13 15:58           ` [PATCH v2 8/8] tests: mark tests broken under GIT_TEST_PROTOCOL_VERSION=2 Ævar Arnfjörð Bjarmason
2018-12-13 16:08             ` Ævar Arnfjörð Bjarmason
2018-12-14  2:18               ` Junio C Hamano
2018-12-14 10:12               ` Jeff King
2018-12-14 10:55                 ` Ævar Arnfjörð Bjarmason
2018-12-14 11:08                   ` Ævar Arnfjörð Bjarmason
2018-12-17 19:59                     ` Jeff King
2018-12-17 19:57                   ` Jeff King
2018-12-17 22:16                     ` [PATCH] upload-pack: turn on uploadpack.allowAnySHA1InWant=true Ævar Arnfjörð Bjarmason
2018-12-17 22:34                       ` David Turner
2018-12-17 22:57                         ` Ævar Arnfjörð Bjarmason
2018-12-17 23:07                           ` David Turner
2018-12-17 23:14                     ` [PATCH v2 8/8] tests: mark tests broken under GIT_TEST_PROTOCOL_VERSION=2 Jonathan Nieder
2018-12-17 23:36                       ` Ævar Arnfjörð Bjarmason
2018-12-18  0:02                         ` Jonathan Nieder
2018-12-18  9:28                           ` Ævar Arnfjörð Bjarmason
2018-12-18 12:41                             ` Jeff King
2018-12-18 12:36                       ` Jeff King
2018-12-18 13:10                         ` Ævar Arnfjörð Bjarmason
2018-12-26 22:14                           ` Junio C Hamano
2018-12-27 11:26                             ` Ævar Arnfjörð Bjarmason
2018-12-27 17:10                               ` Jonathan Nieder
2018-12-11 21:21     ` [PATCH 2/3] tests: mark tests broken under GIT_TEST_PROTOCOL_VERSION=1 Ævar Arnfjörð Bjarmason
2018-12-11 21:21     ` [PATCH 3/3] tests: mark tests broken under GIT_TEST_PROTOCOL_VERSION=2 Ævar Arnfjörð Bjarmason
2018-12-13 19:53 ` [PATCH 0/3] protocol v2 and hidden refs Jonathan Tan
2018-12-14  8:35   ` Jeff King
2018-12-15 19:53     ` Ævar Arnfjörð Bjarmason [this message]
2018-12-16 10:40       ` Jeff King
2018-12-16 11:47         ` Ævar Arnfjörð Bjarmason

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d0q21s8w.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=bwilliamseng@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=peff@peff.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).