Thread (99 messages) flat view 99 messages, 7 authors, 2016-12-15

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

From: Brandon Williams <hidden>
Date: 2016-11-03 17:25:23

On 11/03, Jeff King wrote:
On Wed, Nov 02, 2016 at 05:22:25PM -0700, Jonathan Nieder wrote:
quoted
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".
quoted
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
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?  Or do we simply generate a
whitelist of allowed protocols which includes all protocols with allow
set to 'always' and if it is set to 'never' then it just isn't included
in the whitelist?

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.
quoted
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 agree, if we do add different states to a protocol then we couldn't
simply ask for a whitelist/blacklist of protocols anymore since its more
of a graylist :) (if such a thing exits). 

-- 
Brandon Williams
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help