| Message ID | 575676c760d9a2ce4a59d50e93aa0f45d54620ab.1653685761.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | config: introduce discovery.bare and protected config | expand |
"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes: > safe.directory:: > - These config entries specify Git-tracked directories that are > - considered safe even if they are owned by someone other than the > - current user. By default, Git will refuse to even parse a Git > - config of a repository owned by someone else, let alone run its > - hooks, and this config setting allows users to specify exceptions, > - e.g. for intentionally shared repositories (see the `--shared` > - option in linkgit:git-init[1]). > + '(Protected config only) ' These config entries specify What's the SP in "only) '" doing? > diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt > index aa2f41f5e70..a669983abd6 100644 > --- a/Documentation/glossary-content.txt > +++ b/Documentation/glossary-content.txt > @@ -483,6 +483,24 @@ exclude;; > head ref. If the remote <<def_head,head>> is not an > ancestor to the local head, the push fails. > > +[[def_protected_config]]protected configuration:: > + Protected configuration is configuration that Git considers more > + trustworthy because it is unlikely to be tampered with by an > + attacker. For security reasons, some configuration variables are > + only respected when they are defined in protected configuration. > ++ > +Protected configuration includes: > ++ > +- system-level config, e.g. `/etc/git/config` > +- global config, e.g. `$XDG_CONFIG_HOME/git/config` and > + `$HOME/.gitconfig` > +Protected configuration excludes: > ++ > +- repository config, e.g. `$GIT_DIR/config` and > + `$GIT_DIR/config.worktree` > +- the command line option `-c` and its equivalent environment variables The description is a bit unclear what "protected configuration" refers. If it is the scopes (as in "git config --show-scope") Git can trust more, in other words, a statement like this safe.directory is honored only when it comes from a protected configuration. is what you want to make easier to write by introducing a new phrase, perhaps use the word "scope" for more consistency? E.g. Only safe.directory that is defined in a trusted scope is honored. I dunno. It would make sense to give a rationale behind the seemingly arbitrary choice of what is and what is not "protected". Not necessarily in the glossary, but in the proposed log message of the commit that makes the decision. The rationale must help readers to be able to answer the following questions. - The system level is "protected" because? Is it because we do not even try to protect ourselves from those who can write anywhere in /etc/ or other system directories? - The per-user config is "protected" because? Is it because our primary interest in "protection" is to protect individual users from landmines laid in the filesystem by other users, and those who can already write into $HOME are not we try to guard against? - The per-repo config is not "protected" (i.e. "trusted"), because? If we are not honoring a configuration in the repository, why are we working in that repository in the first place? - The per invocation config is not "protected" (i.e. "trusted"), because? If we cannot trusting our own command line, what prevents an attacker from mucking with our command line to say "sudo whatever" using the same attack vector? Thanks.
On 5/27/2022 7:29 PM, Junio C Hamano wrote: > "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes: >> +[[def_protected_config]]protected configuration:: >> + Protected configuration is configuration that Git considers more >> + trustworthy because it is unlikely to be tampered with by an >> + attacker. For security reasons, some configuration variables are >> + only respected when they are defined in protected configuration. >> ++ >> +Protected configuration includes: >> ++ >> +- system-level config, e.g. `/etc/git/config` >> +- global config, e.g. `$XDG_CONFIG_HOME/git/config` and >> + `$HOME/.gitconfig` >> +Protected configuration excludes: >> ++ >> +- repository config, e.g. `$GIT_DIR/config` and >> + `$GIT_DIR/config.worktree` >> +- the command line option `-c` and its equivalent environment variables > > The description is a bit unclear what "protected configuration" > refers. > > If it is the scopes (as in "git config --show-scope") Git can trust > more, in other words, a statement like this > > safe.directory is honored only when it comes from a protected > configuration. > > is what you want to make easier to write by introducing a new > phrase, perhaps use the word "scope" for more consistency? E.g. > > Only safe.directory that is defined in a trusted scope is > honored. > > I dunno. > > It would make sense to give a rationale behind the seemingly > arbitrary choice of what is and what is not "protected". Not > necessarily in the glossary, but in the proposed log message of the > commit that makes the decision. The rationale must help readers to > be able to answer the following questions. > > - The system level is "protected" because? Is it because we do not > even try to protect ourselves from those who can write anywhere > in /etc/ or other system directories? > > - The per-user config is "protected" because? Is it because our > primary interest in "protection" is to protect individual users > from landmines laid in the filesystem by other users, and those > who can already write into $HOME are not we try to guard against? I think the answers to these two questions is "yes", so they can be turned into an affirmative sentence: We do not event try to protect ourselves from those who can write anywhere... > - The per-repo config is not "protected" (i.e. "trusted"), because? > If we are not honoring a configuration in the repository, why are > we working in that repository in the first place? This requires an example: Some workflows use repositories stored in shared directories, which are writable by multiple unprivileged users. > - The per invocation config is not "protected" (i.e. "trusted"), > because? If we cannot trusting our own command line, what > prevents an attacker from mucking with our command line to say > "sudo whatever" using the same attack vector? With this argument, I agree that -c config can be considered protected. At the very least, it is visible to the user when they are running a command. This would unify our expectations with uploadPack.packObjectsHook, too. Thanks, -Stolee
Derrick Stolee <derrickstolee@github.com> writes: >> It would make sense to give a rationale behind the seemingly >> arbitrary choice of what is and what is not "protected". Not >> necessarily in the glossary, but in the proposed log message of the >> commit that makes the decision. The rationale must help readers to >> be able to answer the following questions. >> >> - The system level is "protected" because? Is it because we do not >> even try to protect ourselves from those who can write anywhere >> in /etc/ or other system directories? >> >> - The per-user config is "protected" because? Is it because our >> primary interest in "protection" is to protect individual users >> from landmines laid in the filesystem by other users, and those >> who can already write into $HOME are not we try to guard against? > > I think the answers to these two questions is "yes", so they can > be turned into an affirmative sentence: > > We do not event try to protect ourselves from those who can > write anywhere... s/event/even/. > >> - The per-repo config is not "protected" (i.e. "trusted"), because? >> If we are not honoring a configuration in the repository, why are >> we working in that repository in the first place? > > This requires an example: > > Some workflows use repositories stored in shared directories, > which are writable by multiple unprivileged users. Hmph, "... and we do not trust these colleagues"? It might be true, but sounds a bit weak rationale, at least to me. A natural reaction coming form a devil's advocate naïve me would be "well, then I would not be directly interacting with such a repository; I'd work in a clone of it of my own, and pull and push as needed". Isn't the reason more like "users may go spelunking random places in the filesystem, with PS1 settings and the like that causes some "git" command invoked automatically in their current directory, and we want to protect these users from getting harmed by a random repository with hostile contents in their configuration and hooks without even realizing they have wandered into such a repository"? >> - The per invocation config is not "protected" (i.e. "trusted"), >> because? If we cannot trusting our own command line, what >> prevents an attacker from mucking with our command line to say >> "sudo whatever" using the same attack vector? > > With this argument, I agree that -c config can be considered > protected. At the very least, it is visible to the user when they > are running a command. This would unify our expectations with > uploadPack.packObjectsHook, too. Yup, that matches my understanding. In any case, I'd prefer to see not just the definition but the reasoning behind the decision that made some "protected" while leaving others not-"protected" clearly documented to help users. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > Derrick Stolee <derrickstolee@github.com> writes: >>> - The per-repo config is not "protected" (i.e. "trusted"), because? >>> If we are not honoring a configuration in the repository, why are >>> we working in that repository in the first place? >> >> This requires an example: >> >> Some workflows use repositories stored in shared directories, >> which are writable by multiple unprivileged users. > > Isn't the reason more like "users may go spelunking random places in > the filesystem, with PS1 settings and the like that causes some > "git" command invoked automatically in their current directory, and > we want to protect these users from getting harmed by a random > repository with hostile contents in their configuration and hooks > without even realizing they have wandered into such a repository"? Hm, this is my understanding as well, i.e. `safe.directory` is meant to protect you from shared repositories that you didn't expect, but it lets you trust the shared repositories that you need (and there is no protection once you decide to trust the repo).
Junio C Hamano <gitster@pobox.com> writes: > "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> safe.directory:: >> - These config entries specify Git-tracked directories that are >> - considered safe even if they are owned by someone other than the >> - current user. By default, Git will refuse to even parse a Git >> - config of a repository owned by someone else, let alone run its >> - hooks, and this config setting allows users to specify exceptions, >> - e.g. for intentionally shared repositories (see the `--shared` >> - option in linkgit:git-init[1]). >> + '(Protected config only) ' These config entries specify > > What's the SP in "only) '" doing? Silly typo. Thanks for the catch :) >> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt >> index aa2f41f5e70..a669983abd6 100644 >> --- a/Documentation/glossary-content.txt >> +++ b/Documentation/glossary-content.txt >> @@ -483,6 +483,24 @@ exclude;; >> head ref. If the remote <<def_head,head>> is not an >> ancestor to the local head, the push fails. >> >> +[[def_protected_config]]protected configuration:: >> + Protected configuration is configuration that Git considers more >> + trustworthy because it is unlikely to be tampered with by an >> + attacker. For security reasons, some configuration variables are >> + only respected when they are defined in protected configuration. >> ++ >> +Protected configuration includes: >> ++ >> +- system-level config, e.g. `/etc/git/config` >> +- global config, e.g. `$XDG_CONFIG_HOME/git/config` and >> + `$HOME/.gitconfig` >> +Protected configuration excludes: >> ++ >> +- repository config, e.g. `$GIT_DIR/config` and >> + `$GIT_DIR/config.worktree` >> +- the command line option `-c` and its equivalent environment variables > > The description is a bit unclear what "protected configuration" > refers. > > If it is the scopes (as in "git config --show-scope") Git can trust > more, in other words, a statement like this > > safe.directory is honored only when it comes from a protected > configuration. > > is what you want to make easier to write by introducing a new > phrase, perhaps use the word "scope" for more consistency? E.g. > > Only safe.directory that is defined in a trusted scope is > honored. Good point. I think using scope would be a lot clearer, and maybe I will consider s/protected configuration/protected scope. I'm hesitant to call the scope "trusted", because I don't want to insinuate that repository config is "untrusted" since we _do_ trust it in most cases. I don't think Documentation/git-config.txt has adequately defined what a 'scope' is though, even though scopes have been with us since 9acc591111 (config: add a notion of "scope", 2016-05-18). The best I could find is "--show-scope", introduced in 145d59f482 (config: add '--show-scope' to print the scope of a config value, 2020-02-10), which mentions scopes but doesn't link the idea back to the specific files or CLI options ("--system", "--global", etc). So I'll see if I can improve the docs around scopes since that will help the language in this patch.
diff --git a/Documentation/config.txt b/Documentation/config.txt index e284b042f22..07832de1a6c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -369,6 +369,12 @@ inventing new variables for use in your own tool, make sure their names do not conflict with those that are used by Git itself and other popular tools, and describe them in your documentation. +Variables marked with '(Protected config only)' are only respected when +they are specified in protected configuration. This includes global and +system-level config, and excludes repository config, the command line +option `-c`, and environment variables. For more details, see the +'protected configuration' entry in linkgit:gitglossary[7]. + include::config/advice.txt[] include::config/core.txt[] diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt index ae0e2e3bdb4..c1caec460e8 100644 --- a/Documentation/config/safe.txt +++ b/Documentation/config/safe.txt @@ -1,21 +1,18 @@ safe.directory:: - These config entries specify Git-tracked directories that are - considered safe even if they are owned by someone other than the - current user. By default, Git will refuse to even parse a Git - config of a repository owned by someone else, let alone run its - hooks, and this config setting allows users to specify exceptions, - e.g. for intentionally shared repositories (see the `--shared` - option in linkgit:git-init[1]). + '(Protected config only) ' These config entries specify + Git-tracked directories that are considered safe even if they + are owned by someone other than the current user. By default, + Git will refuse to even parse a Git config of a repository owned + by someone else, let alone run its hooks, and this config + setting allows users to specify exceptions, e.g. for + intentionally shared repositories (see the `--shared` option in + linkgit:git-init[1]). + This is a multi-valued setting, i.e. you can add more than one directory via `git config --add`. To reset the list of safe directories (e.g. to override any such directories specified in the system config), add a `safe.directory` entry with an empty value. + -This config setting is only respected when specified in a system or global -config, not when it is specified in a repository config, via the command -line option `-c safe.directory=<path>`, or in environment variables. -+ The value of this setting is interpolated, i.e. `~/<path>` expands to a path relative to the home directory and `%(prefix)/<path>` expands to a path relative to Git's (runtime) prefix. diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index aa2f41f5e70..a669983abd6 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -483,6 +483,24 @@ exclude;; head ref. If the remote <<def_head,head>> is not an ancestor to the local head, the push fails. +[[def_protected_config]]protected configuration:: + Protected configuration is configuration that Git considers more + trustworthy because it is unlikely to be tampered with by an + attacker. For security reasons, some configuration variables are + only respected when they are defined in protected configuration. ++ +Protected configuration includes: ++ +- system-level config, e.g. `/etc/git/config` +- global config, e.g. `$XDG_CONFIG_HOME/git/config` and + `$HOME/.gitconfig` ++ +Protected configuration excludes: ++ +- repository config, e.g. `$GIT_DIR/config` and + `$GIT_DIR/config.worktree` +- the command line option `-c` and its equivalent environment variables + [[def_reachable]]reachable:: All of the ancestors of a given <<def_commit,commit>> are said to be "reachable" from that commit. More