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: Sun, 16 Dec 2018 12:47:59 +0100 [thread overview]
Message-ID: <87bm5l1ymo.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20181216104017.GB13704@sigill.intra.peff.net>
On Sun, Dec 16 2018, Jeff King wrote:
> On Sat, Dec 15, 2018 at 08:53:35PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> > 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?
>
> Yes, I think that's a goal (and we already have upload-archive, which is
> a similar thing).
>
>> 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.
>
> Right, exactly. It pushes more of the information down into Git's own
> protocol. Of course we _can_ build mechanisms at that level for
> configuring which verbs are allowed. But if some context is available at
> the higher protocol level, then we can use the mechanisms at that higher
> level.
>
> I think of it as a tradeoff. By including the endpoint in the transport
> protocol (e.g., in ssh the command name, in HTTP the URL), we get to use
> the mechanisms in those transports to make policy decisions on the
> server. But it also means we _have_ to implement those policies twice,
> once per transport.
>
> IMHO having to deal with both transports is not that big a loss,
> considering that there are only two, and really not likely to be more.
> git:// is already unauthenticated, and IMHO is mostly a dead-end for
> future protocol work since it provides no advantage over HTTP, and the
> future is mostly HTTP, with ssh for people who really prefer its
> authentication mode.
>
>> 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.
>
> But I think that's what makes it dangerous, too. :)
>
> Gitaly (and we have our own equivalent at GitHub) is responsible for
> making those policy decisions about who can run what. Opening a pipe
> between the client and the backend that can issue arbitrary verbs is
> exactly what they _don't_ want to do.
>
> So they have to intercept the conversation at least at the verb level.
> It _is_ nice if conversation for each verb is standardized (so once a
> verb is issued, they can just step out of the way and proxy bytes[1]),
> and v2 helps with that.
>
> That's not too hard for a Git-aware endpoint to implement. But when
> that verb interception can be done at the HTTP/ssh level, then it's easy
> for tools that _aren't_ Git-aware to do handle it (again, like the
> Apache config we recommend in git-http-backend(1)).
>
> -Peff
>
> [1] Actually, we do much more intimate interception than that at GitHub
> already. The upload-pack conversation is mostly vanilla, but for
> receive-pack we handle replication at that layer. So your pack is
> streamed to 3-6 backend receive-packs simultaneously, and that
> endpoint layer handles quorum for updating refs, etc.
Yeah I think overall this makes sense. I was just thinking we'd have
stuff like this needing to be maintained in all middleware:
https://gitlab.com/gitlab-org/gitlab-shell/blob/v8.4.4/lib/gitlab_shell.rb#L15-28
Which, if and when we have a lot of verbs would be a pain, but of course
server operators might want to explicitly whitelist them.
Also for things like "git-grep" I can see e.g. "no POSIX regex" (due to
well known DoS issues0 being a configuration we ourselves would want to
carry, at that point server operators would need to maintain two
whitelists anyway, one in their custom code & another in /etc/gitconfig.
But I think that trade-off is worth it as you note because when you want
to filter these it's handy to be able to do it in a dumb ssh/web server.
Another thing to consider is not having a proliferation of things in
git-<TAB> completion again. AFAIK these things can't have spaces in them
for /etc/passwd & inetd tab completion. So perhaps call them all
git-serve-*, or not put them in our bin/ install path as a special case?
prev parent reply other threads:[~2018-12-16 11:48 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
2018-12-16 10:40 ` Jeff King
2018-12-16 11:47 ` Ævar Arnfjörð Bjarmason [this message]
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=87bm5l1ymo.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.