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

[Jeff King <peff@peff.net>]

On Thu, Nov 03, 2016 at 10:39:35AM -0700, Stefan Beller wrote:

> >>   protocol.X.allow = always | user | never
> >
> > It sounds like there is interest for this sort of behavior, it would
> > definitely require a larger change than what I initially proposed.  One
> > problem I see though is that with this we have support for both a
> > blacklist and a whitelist.  Which wins?
> 
> For the submodule operations we'll use a whitelist, because we want to
> provide security and for the other case we can offer a blacklist as a bandaid.
> 
> My opinion on blacklists is roughly aligned with e.g. :
> https://blog.codinghorror.com/blacklists-dont-work/
> http://blog.deepinstinct.com/2016/02/04/when-blacklists-dont-really-work/
> 
> So IMHO we could drop the "never" and substitute it with a "warn" or
> "ask-user", such that this configuration becomes a white list for both cases:
> 
>      protocol.X.allow = always | user | warn

I don't think blacklists work in the general case, because they grow out
of date and fail-open. But you want to have _some_ blacklisting
mechanism, in order override a decision of the whitelist.

For instance, the default submodule whitelist would probably include
https and ssh. But if I'm cloning potentially malicious repos and I
don't ever want them to trigger ssh (because I don't want them to use my
ssh keys, whereas I have explicitly set up my credentials such that http
is safe to use), I would want to be able to do:

  git config protocol.ssh.allow never

(or "git -c", or whatever).

True, a whitelist is safer. If we add a new "foo" protocol that also
looks at your ssh keys, you're screwed. And that's why I designed it as
a pure-whitelist in the first place. But it comes at the price of
convenience, because you have to manually add each new innocent protocol
to the whitelist.

> So you're suggesting that setting it to "never" doesn't have any effect
> except for cluttering the config file?
> I don't think we should do that; each setting should have an impact.
> So maybe the "never" would be there to disallow protocols of the hardcoded
> white list (e.g. http)

Exactly.

> > I don't know if I'm sold on a 'user' state just yet, perhaps that's just
> > because I view a whitelist or blacklist as well black and white and
> > having this user state adds in a gray area.
> 
> Well the "user" state is to differentiate between the
> * "I consciously typed `git clone ...` (and e.g. I know what happens as
>   I know the server admin and they are trustworthy.)
> * a repository contains a possible hostile .gitmodules file such
>   that I am not aware of the network connection.

Right. I had assumed that we would tell the difference between those
automatically (by seeing if we got the URL on the command line), but
things like "go get" show that the context is often before git is even
called.

-Peff