Re: [PATCH v3] transport: add protocol policy config option
- From
Stefan Beller <sbeller@google.com>
- Date
- Nov 4, 2016, 22:38 UTC
- Message-ID
- <CAGZ79ka18s3-nzkCog3+g70NCoAawoqO1t-qghKSZJxjK6jkmw@mail.gmail.com>
- In-Reply-To
- <1478292933-7873-1-git-send-email-bmwill@google.com>
On Fri, Nov 4, 2016 at 1:55 PM, Brandon Williams <bmwill@google.com> wrote:
Show 47 quoted lines
> 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@google.com> > --- > 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?
Show 21 quoted lines
> + 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(?)Show 20 quoted lines
> + > 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