On Fri, Nov 4, 2016 at 1:55 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote: > Previously the `GIT_ALLOW_PROTOCOL` environment variable was used to > specify a whitelist of protocols to be used in clone/fetch/pull > commands. This patch introduces new configuration options for more > fine-grained control for allowing/disallowing protocols. This also has > the added benefit of allowing easier construction of a protocol > whitelist on systems where setting an environment variable is > non-trivial. > > Now users can specify a policy to be used for each type of protocol via > the 'protocol.<name>.allow' config option. A default policy for all > unknown protocols can be set with the 'protocol.allow' config option. > If no user configured default is made git, by default, will allow > known-safe protocols (http, https, git, ssh, file), disallow > known-dangerous protocols (ext), and have a default poliy of `user` for > all other protocols. > > The supported policies are `always`, `never`, and `user`. The `user` > policy can be used to configure a protocol to be usable when explicitly > used by a user, while disallowing it for commands which run > clone/fetch/pull commands without direct user intervention (e.g. > recursive initialization of submodules). Commands which can potentially > clone/fetch/pull from untrusted repositories without user intervention > can export `GIT_PROTOCOL_FROM_USER` with a value of '0' to prevent > protocols configured to the `user` policy from being used. > > Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx> > --- > Documentation/config.txt | 25 ++++++++ > Documentation/git.txt | 19 +++--- > git-submodule.sh | 12 ++-- > t/lib-proto-disable.sh | 132 +++++++++++++++++++++++++++++++++++---- > t/t5509-fetch-push-namespaces.sh | 1 + > t/t5802-connect-helper.sh | 1 + > transport.c | 73 +++++++++++++++++++++- > 7 files changed, 235 insertions(+), 28 deletions(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 27069ac..5d845c4 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -2308,6 +2308,31 @@ pretty.<name>:: > Note that an alias with the same name as a built-in format > will be silently ignored. > > +protocol.allow:: > + If set, provide a user defined default policy for all protocols which > + don't explicitly have a policy (protocol.<name>.allow). By default, Use hyphens (`protocol.<name>.allow`) to highlight the config option. By default, if unset, ... have a default policy ... sounds strange. How about just dropping the first 4 words here: Known-safe protocols (http, https, git, ssh, file) have a default policy of `always`, known-dangerous protocols (ext) have a default policy of `never`, and all other protocols have a default policy of `user`. Supported policies: What happens if protocol.allow is set to true? > + if unset, known-safe protocols (http, https, git, ssh, file) have a > + default policy of `always`, known-dangerous protocols (ext) have a > + default policy of `never`, and all other protocols have a default policy > + of `user`. Supported policies: > ++ > +-- > + > +* `always` - protocol is always able to be used. > + > +* `never` - protocol is never able to be used. > + > +* `user` - protocol is only able to be used when `GIT_PROTOCOL_FROM_USER` is > + either unset or has a value of 1. This policy should be used when you want a > + protocol to be usable by the user but don't want it used by commands which > + execute clone/fetch/pull commands without user input, e.g. recursive > + submodule initialization. > + > +-- > + > +protocol.<name>.allow:: > + Set a policy to be used by protocol <name> with clone/fetch/pull commands. How does this interact with protocol.allow? When protocol.allow is set, this overrides the specific protocol. If protocol is not set, it overrides the specific protocol as well(?) > + > pull.ff:: > By default, Git does not create an extra merge commit when merging > a commit that is a descendant of the current commit. Instead, the > diff --git a/Documentation/git.txt b/Documentation/git.txt > index ab7215e..ab25580 100644 > --- a/Documentation/git.txt > +++ b/Documentation/git.txt > @@ -1150,13 +1150,13 @@ of clones and fetches. > cloning a repository to make a backup). > > `GIT_ALLOW_PROTOCOL`:: > - If set, provide a colon-separated list of protocols which are > - allowed to be used with fetch/push/clone. This is useful to > - restrict recursive submodule initialization from an untrusted > - repository. Any protocol not mentioned will be disallowed (i.e., > - this is a whitelist, not a blacklist). If the variable is not > - set at all, all protocols are enabled. The protocol names > - currently used by git are: > + The new way to configure allowed protocols is done through the config This is not the right place to mention what is newer. ;) However it is useful to know about the config interface, which is * (supposedly) easier to use * more fine grained * taking less priority than this env var. > + test_expect_success "clone $desc (disabled)" ' > + rm -rf tmp.git && > + test_must_fail git clone --bare "$url" tmp.git > + ' I could not spot a test for GIT_ALLOW_PROTOCOL overriding any protocol*allow policy. Is that also worth testing? (for backwards compatibility of tools that make use of GIT_ALLOW_PROTOCOL but the user already setup a policy. > > void transport_check_allowed(const char *type) > -- Thanks! Stefan