Re: [PATCH] transport: add core.allowProtocol config option

From: Jeff King <peff@peff.net>
Date: Nov 3, 2016, 14:38 UTC
Message-ID: <20161103143806.hce4msk3dhxtgpre@sigill.intra.peff.net>
In-Reply-To: <20161103002225.GA13369@google.com>

On Wed, Nov 02, 2016 at 05:22:25PM -0700, Jonathan Nieder wrote:

Show 5 quoted lines

> Another difficulty with setting GIT_ALLOW_PROTOCOL globally is that it
> requires copy/pasting the default value from upstream and then adding
> the values I want.  There's no straightforward way to get the current
> value and add to it, in case I want to benefit from future upstream
> fixes to the default list.

I agree that this is a big drawback of the current scheme, and it would be nice to be able to say "also allow".

Show 11 quoted lines

> That is, would it be possible to use something like
> 
> 	[protocol "sso"]
> 		allow = always
> 
> instead of
> 
> 	[core]
> 		allowProtocol = file:git:http:https:....:sso
> 
> ?

One complication is that the whitelist has multiple states:

  1. if it's not used at all, anything goes

  2. if it exists and has zero or more entries, only those entries are
     allowed

And then submodules are an exception to (1), because it's not anything goes. It's "this default safe whitelist".

So when does protocol.sso.allow kick in? We wouldn't want it to trigger case (2) for things like fetch (disabling other non-allowed protocols). Nor do I think we'd only want it for the submodule case, as I would assume that "protocol.sso.allow = false" should disable it.

So I think this probably needs to be a separate parallel system where each protocol can be white- or black-listed in a context-specific way. Like:

  protocol.X.allow = always | user | never

Where "user" specifies that the protocol is OK coming directly from the user, but not from other sources. And we default known-common-and-good ones like protocol.http.allow to "always", unknown ones (like "foo" which runs "remote-foo") to "user"), and possibly known-scary ones like "ext") to "never".

Then we need some way of telling git "you are in a context where the URL parameter is not coming from the user". Probably via the environment in GIT_PROTOCOL_FROM_USER or similar. Which git-submodule would sent when recursing clones, along with things like "go get".

In other words, stop asking git-submodule or "go get" to specify policy, and let them specify context that can be used to implement policy that the user specifies (and have git provide a sane default policy).

I think this would all take a backseat to GIT_ALLOW_PROTOCOL, for backwards compatibility, and then GIT_ALLOW_PROTOCOL could slowly die off over time.

Show 8 quoted lines

> That reminds me: external tools also set GIT_ALLOW_PROTOCOL when the
> user hasn't set it explicitly, like git-submodule.sh does.  E.g.
> repo <https://gerrit.googlesource.com/git-repo/+/466b8c4e/git_command.py#171>,
> mercurial <https://www.mercurial-scm.org/repo/hg/file/b032a7b676c6/mercurial/subrepo.py#l1404>.
> Other external tools consume GIT_ALLOW_PROTOCOL, like 'go get'
> <https://go.googlesource.com/go/+/55620a0e/src/cmd/go/vcs.go#64>.
> Can we make it more convenient for them to support this configuration
> too?

I think under my proposal above this ugliness just goes away, as all they have to do is say "trust my URLs less; they come from an automated source" without specifying policy themselves.

But we may still want...

Show 10 quoted lines

> An example approach would be a GIT_ALLOW_PROTOCOL var returned by
> "git var".
> 
> That way git-submodule.sh could do
> 
> 	: ${GIT_ALLOW_PROTOCOL=$(git var GIT_ALLOW_PROTOCOL)}
> 
> and it would just work.  Other tools could do the same, with a
> fallback to the current default until new enough git is in widespread
> use.

...some automated way to say "is this protocol supported"? I think it is not just "give me ALLOW_PROTOCOL" anymore, though, but "apply your rules to this protocol, and tell me if it is supported".

I don't think things like "go get" would need it, but you would if you had a porcelain built around git that was accessing a URL _not_ via git-fetch, but wanted to apply git's rules. That could come as a step 2 later, though.

-Peff