Re: [PATCH] transport: add core.allowProtocol config option
From: Stefan Beller <hidden>
Date: 2016-11-02 22:41:08
On Wed, Nov 2, 2016 at 3:20 PM, Brandon Williams [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Add configuration option 'core.allowProtocol' to allow users to create a whitelist of allowed protocols for fetch/push/clone in their gitconfig. For git-submodule.sh, fallback to default whitelist only if the user hasn't explicitly set `GIT_ALLOW_PROTOCOL` or doesn't have a whitelist in their gitconfig. Signed-off-by: Brandon Williams <redacted> --- Documentation/config.txt | 9 +++++++++ git-submodule.sh | 3 ++- transport.c | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-)diff --git a/Documentation/config.txt b/Documentation/config.txt index 27069ac..7f83e40 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt@@ -455,6 +455,15 @@ core.sshCommand:: the `GIT_SSH_COMMAND` environment variable and is overridden when the environment variable is set. +core.allowProtocol:: + Provide a colon-separated list of protocols which are allowed to be + used with fetch/push/clone.
ok.
This is useful to restrict recursive + submodule initialization from an untrusted repository.
ok. Though as a user submodules may not spring to mind immediately here. I think this is generally useful, too. e.g. an admin could put this in the system wide config to prevent certain protocols from being used.
Any protocol not + mentioned will be disallowed
For the regular fetch/clone/pull case. For the submodule case we still fall back to the hardcoded list of known good things?
(i.e., this is a whitelist, not a + blacklist).
That is very explicit, I'd drop it. However this inspires bike shedding on the name: What about core.protocolWhitelist instead?
If the variable is not set at all, all protocols are + enabled. If the `GIT_ALLOW_PROTOCOL` enviornment variable is set, it is + used as the protocol whitelist instead of this config option.
So the env var is of higher priority than this config.
quoted hunk ↗ jump to hunk
+ core.ignoreStat:: If true, Git will avoid using lstat() calls to detect if files have changed by setting the "assume-unchanged" bit for those tracked filesdiff --git a/git-submodule.sh b/git-submodule.sh index a024a13..ad94c75 100755 --- a/git-submodule.sh +++ b/git-submodule.sh@@ -27,7 +27,8 @@ cd_to_toplevel # # If the user has already specified a set of allowed protocols, # we assume they know what they're doing and use that instead. -: ${GIT_ALLOW_PROTOCOL=file:git:http:https:ssh} +config_whitelist=$(git config core.allowProtocol)
So first we lookup the configured protocols.
+: ${GIT_ALLOW_PROTOCOL=${config_whitelist:-file:git:http:https:ssh}}Then if they are not configured use the current hard coded white list.
quoted hunk ↗ jump to hunk
export GIT_ALLOW_PROTOCOL command=diff --git a/transport.c b/transport.c index d57e8de..b1098cd 100644 --- a/transport.c +++ b/transport.c@@ -652,7 +652,7 @@ static const struct string_list *protocol_whitelist(void) if (enabled < 0) { const char *v = getenv("GIT_ALLOW_PROTOCOL"); - if (v) { + if (v || !git_config_get_value("core.allowProtocol", &v)) {
This implementation matches what the config promised, I would think. Do we have any tests for this that could be extended? (Otherwise we'd maybe want to add a test for both the regular case as well as a forbidden submodule?)
string_list_split(&allowed, v, ':', -1);
string_list_sort(&allowed);
enabled = 1;
--
2.8.0.rc3.226.g39d4020