On Fri, Nov 4, 2016 at 1:55 PM, Brandon Williams <[email protected]> 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 <[email protected]>
> ---
> 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
