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