git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Brandon WilliamsNext: Brandon Williams
Message 33 of 117 in “transport: add core.allowProtocol config option”
  1. transport: add core.allowProtocol config optionBrandon Williams, Nov 2, 2016
  2. Stefan BellerNov 2, 2016
  3. Brandon WilliamsNov 2, 2016
  4. Jeff KingNov 2, 2016
  5. Jeff KingNov 2, 2016
  6. Brandon WilliamsNov 2, 2016
  7. Brandon WilliamsNov 2, 2016
  8. Brandon WilliamsNov 2, 2016
  9. Jeff KingNov 3, 2016
  10. Jonathan NiederNov 3, 2016
  11. Blake BurkhartNov 3, 2016
  12. Junio C HamanoNov 3, 2016
  13. Jeff KingNov 3, 2016
  14. Brandon WilliamsNov 3, 2016
  15. Stefan BellerNov 3, 2016
  16. Brandon WilliamsNov 3, 2016
  17. Jeff KingNov 3, 2016
  18. Brandon WilliamsNov 3, 2016
  19. Jeff KingNov 3, 2016
  20. Jeff KingNov 3, 2016
  21. Brandon WilliamsNov 3, 2016
  22. Jeff KingNov 3, 2016
  23. Jeff KingNov 3, 2016
  24. Brandon WilliamsNov 3, 2016
  25. Jeff KingNov 3, 2016
  26. Brandon WilliamsNov 3, 2016
  27. transport: add core.allowProtocol config optionBrandon Williams, Nov 3, 2016
  28. transport: add protocol policy config optionBrandon Williams, Nov 4, 2016
  29. Brandon WilliamsNov 4, 2016
  30. Stefan BellerNov 4, 2016
  31. Jeff KingNov 4, 2016
  32. Brandon WilliamsNov 5, 2016
  33. Stefan BellerNov 4, 2016
  34. Brandon WilliamsNov 7, 2016
  35. Jeff KingNov 4, 2016
  36. Brandon WilliamsNov 7, 2016
  37. 1/2 lib-proto-disable: variable name fixBrandon Williams, Nov 7, 2016
  38. 2/2 transport: add protocol policy config optionBrandon Williams, Nov 7, 2016
  39. Jeff KingNov 7, 2016
  40. Brandon WilliamsNov 7, 2016
  41. Jeff KingNov 7, 2016
  42. Brandon WilliamsNov 7, 2016
  43. Jeff KingNov 7, 2016
  44. Jacob KellerNov 8, 2016
  45. Junio C HamanoNov 8, 2016
  46. 1/2 lib-proto-disable: variable name fixBrandon Williams, Nov 7, 2016
  47. 2/2 transport: add protocol policy config optionBrandon Williams, Nov 7, 2016
  48. Jeff KingNov 8, 2016
  49. Brandon WilliamsNov 8, 2016
  50. 0/4 transport protocol policy configurationBrandon Williams, Dec 1, 2016
  51. 4/4 transport: check if protocol can be used on a redirectBrandon Williams, Dec 1, 2016
  52. 4/4 transport: check if protocol can be used on a redirectBrandon Williams, Dec 1, 2016
  53. Brandon WilliamsDec 1, 2016
  54. Jeff KingDec 1, 2016
  55. Junio C HamanoDec 1, 2016
  56. Jeff KingDec 1, 2016
  57. Brandon WilliamsDec 1, 2016
  58. 2/4 transport: add protocol policy config optionBrandon Williams, Dec 1, 2016
  59. 1/4 lib-proto-disable: variable name fixBrandon Williams, Dec 1, 2016
  60. 3/4 http: always warn if libcurl version is too oldBrandon Williams, Dec 1, 2016
  61. 0/4 transport protocol policy configurationBrandon Williams, Dec 1, 2016
  62. 3/4 http: always warn if libcurl version is too oldBrandon Williams, Dec 1, 2016
  63. 1/4 lib-proto-disable: variable name fixBrandon Williams, Dec 1, 2016
  64. 2/4 transport: add protocol policy config optionBrandon Williams, Dec 1, 2016
  65. 4/4 transport: add from_user parameter to is_transport_allowedBrandon Williams, Dec 1, 2016
  66. Jeff KingDec 1, 2016
  67. Junio C HamanoDec 1, 2016
  68. Brandon WilliamsDec 1, 2016
  69. Brandon WilliamsDec 1, 2016
  70. Jeff KingDec 2, 2016
  71. Brandon WilliamsDec 2, 2016
  72. Junio C HamanoDec 1, 2016
  73. Brandon WilliamsDec 1, 2016
  74. Junio C HamanoDec 5, 2016
  75. Brandon WilliamsDec 5, 2016
  76. Junio C HamanoDec 5, 2016
  77. Brandon WilliamsDec 5, 2016
  78. Jeff KingDec 6, 2016
  79. Junio C HamanoDec 6, 2016
  80. Jeff KingDec 6, 2016
  81. Brandon WilliamsDec 6, 2016
  82. 0/5 transport protocol policy configurationBrandon Williams, Dec 2, 2016
  83. 1/5 lib-proto-disable: variable name fixBrandon Williams, Dec 2, 2016
  84. 3/5 http: always warn if libcurl version is too oldBrandon Williams, Dec 2, 2016
  85. 5/5 transport: add from_user parameter to is_transport_allowedBrandon Williams, Dec 2, 2016
  86. 2/5 transport: add protocol policy config optionBrandon Williams, Dec 2, 2016
  87. 4/5 http: create function to get curl allowed protocolsBrandon Williams, Dec 2, 2016
  88. 0/5 transport protocol policy configurationBrandon Williams, Dec 14, 2016
  89. 1/5 lib-proto-disable: variable name fixBrandon Williams, Dec 14, 2016
  90. 2/5 transport: add protocol policy config optionBrandon Williams, Dec 14, 2016
  91. 3/5 http: always warn if libcurl version is too oldBrandon Williams, Dec 14, 2016
  92. Jeff KingDec 14, 2016
  93. Brandon WilliamsDec 14, 2016
  94. 4/5 http: create function to get curl allowed protocolsBrandon Williams, Dec 14, 2016
  95. Jeff KingDec 14, 2016
  96. Brandon WilliamsDec 14, 2016
  97. 5/5 transport: add from_user parameter to is_transport_allowedBrandon Williams, Dec 14, 2016
  98. Jeff KingDec 14, 2016
  99. Brandon WilliamsDec 14, 2016
  100. Jeff KingDec 14, 2016
  101. Jeff KingDec 14, 2016
  102. Brandon WilliamsDec 14, 2016
  103. Brandon WilliamsDec 14, 2016
  104. Jeff KingDec 14, 2016
  105. Brandon WilliamsDec 14, 2016
  106. 0/6 transport protocol policy configurationBrandon Williams, Dec 14, 2016
  107. 2/6 http: always warn if libcurl version is too oldBrandon Williams, Dec 14, 2016
  108. Jeff KingDec 15, 2016
  109. Junio C HamanoDec 15, 2016
  110. 3/6 transport: add protocol policy config optionBrandon Williams, Dec 14, 2016
  111. 5/6 transport: add from_user parameter to is_transport_allowedBrandon Williams, Dec 14, 2016
  112. 6/6 http: respect protocol.*.allow=user for http-alternatesBrandon Williams, Dec 14, 2016
  113. 4/6 http: create function to get curl allowed protocolsBrandon Williams, Dec 14, 2016
  114. 1/6 lib-proto-disable: variable name fixBrandon Williams, Dec 14, 2016
  115. Junio C HamanoDec 14, 2016
  116. Jeff KingDec 15, 2016
  117. Jeff KingDec 14, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.