The word "whitelist" has cultural implications that make its use non-inclusive. A previous version of this series recommended the replacement of "allowlist", but that term is still new and not accepted by some common dictionaries. Instead, this version avoids the use of "whitelist" by rewording the sentences that use it. In many cases, this improves readability since the term is used suddenly without other context (and in some cases, is not necessary at all). There is one case where "whitelist" is replaced with "allow_list" but that is because we are operating on a string list parsed from the GIT_ALLOW_PROTOCOL environment variable. Updates in v3 ============= * Reorganized patches so changes to git-daemon and git-cvsserver are bunched together across code, documentation, and tests. Their documentation is more thoroughly modified to clarify the role of the directory arguments. * The changes surrounding SANITIZE=leak are delayed for Ævar's work that will update the documentation while changing the behavior of the test variables. * Several replacements are made to clarify meaning instead of doing a simple replacement. Thanks, Ævar for the suggestions. Thanks, -Stolee Derrick Stolee (5): daemon: clarify directory arguments git-cvsserver: clarify directory list git.txt: remove redundant language t: avoid "whitelist" transport.c: avoid "whitelist" Documentation/git-cvsserver.txt | 19 ++++++++++--------- Documentation/git-daemon.txt | 21 +++++++++++---------- Documentation/git.txt | 4 +--- daemon.c | 8 ++++---- git-cvsserver.perl | 2 +- t/lib-proto-disable.sh | 6 +++--- t/t5812-proto-disable-http.sh | 2 +- t/t5815-submodule-protos.sh | 4 ++-- t/t9400-git-cvsserver-server.sh | 2 +- t/test-lib-functions.sh | 3 +-- transport.c | 8 ++++---- 11 files changed, 39 insertions(+), 40 deletions(-) base-commit: e4a4b31577c7419497ac30cebe30d755b97752c5 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1274%2Fderrickstolee%2Fallow-deny-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1274/derrickstolee/allow-deny-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/1274 Range-diff vs v2: 1: 19632a2d245 ! 1: d5ca7bffac0 Documentation: remove use of whitelist @@ Metadata Author: Derrick Stolee <derrickstolee@xxxxxxxxxx> ## Commit message ## - Documentation: remove use of whitelist + daemon: clarify directory arguments - The word "whitelist" has cultural implications that are not inclusive. - Thankfully, it is not difficult to reword and avoid its use. + The undecorated arguments to the 'git-daemon' command provide a list of + directories. When at least one directory is specified, then 'git-daemon' + only serves requests that are within that directory list. The boolean + '--strict-paths' option makes the list more explicit in that + subdirectories are no longer included. - Remove uses of "whitelist" in the Documentation/ directory. In all - cases, we can rewrite the sentences to add clarity instead of relying on - the reader understanding this term. + The existing documentation and error messages around this directory list + refer to it and its behavior as a "whitelist". The word "whitelist" has + cultural implications that are not inclusive. Thankfully, it is not + difficult to reword and avoid its use. In the process, we can define the + purpose of this directory list directly. - The most substantial change is to git-daemon.txt which had several uses, - but we can refer to the input directories as a list of included - directories, making the descriptions slightly simpler. + In Documentation/git-daemon.txt, rewrite the OPTIONS section around the + '<directory>' option. Add additional clarity to the other options that + refer to these directories. - Helped-by: Jeff King <peff@xxxxxxxx> - Helped-by: Junio C Hamano <gitster@xxxxxxxxx> - Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx> + Some error messages can also be improved in daemon.c. The + '--strict-paths' option requires '<directory>' arguments, so refer to + that section of the documentation directly. A logerror() call points out + that a requested directory is not in the specified directory list. We + can use "list" here without any loss of information. - ## Documentation/git-cvsserver.txt ## -@@ Documentation/git-cvsserver.txt: circumstances, allowing easier restricted usage through git-shell. - - GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path. - --GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The -+GIT_CVSSERVER_ROOT specifies a single allowed directory. The - repository must still be configured to allow access through - git-cvsserver, as described above. - + Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx> ## Documentation/git-daemon.txt ## @@ Documentation/git-daemon.txt: that service if it is enabled. @@ Documentation/git-daemon.txt: OPTIONS "/foo/repo.git" or "/foo/repo/.git") and don't do user-relative paths. 'git daemon' will refuse to start when this option is enabled and no - whitelist is specified. -+ specific directories are specified. ++ directory arguments are provided. --base-path=<path>:: Remap all the path requests as relative to the given path. @@ Documentation/git-daemon.txt: standard output to be sent to the requestor as an <directory>:: - A directory to add to the whitelist of allowed directories. Unless -+ A directory to add to the list of allowed directories. Unless - --strict-paths is specified this will also include subdirectories - of each named directory. +- --strict-paths is specified this will also include subdirectories +- of each named directory. ++ The remaining arguments provide a list of directories. If any ++ directories are specified, then the `git-daemon` process will ++ serve a requested directory only if it is contained in one of ++ these directories. If `--strict-paths` is specified, then the ++ requested directory must match one of these directories exactly. + SERVICES + -------- @@ Documentation/git-daemon.txt: git 9418/tcp # Git Version Control System 'git daemon' as inetd server:: @@ Documentation/git-daemon.txt: git 9418/tcp # Git Version Control System - repository under the whitelisted set of directories, /pub/foo - and /pub/bar, place an entry like the following into - /etc/inetd all on one line: -+ repository in the directory list of `/pub/foo` and `/pub/bar`, -+ place an entry like the following into `/etc/inetd` all on one line: ++ repository within `/pub/foo` or `/pub/bar`, place an entry like ++ the following into `/etc/inetd` all on one line: + ------------------------------------------------ git stream tcp nowait nobody /usr/bin/git - ## Documentation/git.txt ## -@@ Documentation/git.txt: for full details. - `protocol.allow` is set to `never`, and each of the listed - protocols has `protocol.<name>.allow` set to `always` - (overriding any existing configuration). In other words, any -- protocol not mentioned will be disallowed (i.e., this is a -- whitelist, not a blacklist). See the description of -+ protocol not mentioned will be disallowed. See the description of - `protocol.allow` in linkgit:git-config[1] for more details. + ## daemon.c ## +@@ daemon.c: static const char *path_ok(const char *directory, struct hostinfo *hi) + /* The validation is done on the paths after enter_repo + * appends optional {.git,.git/.git} and friends, but + * it does not use getcwd(). So if your /pub is +- * a symlink to /mnt/pub, you can whitelist /pub and ++ * a symlink to /mnt/pub, you can include /pub and + * do not have to say /mnt/pub. + * Do not say /pub/. + */ +@@ daemon.c: static const char *path_ok(const char *directory, struct hostinfo *hi) + return path; + } + +- logerror("'%s': not in whitelist", path); ++ logerror("'%s': not in directory list", path); + return NULL; /* Fallthrough. Deny by default */ + } + +@@ daemon.c: static int run_service(const char *dir, struct daemon_service *service, + * a "git-daemon-export-ok" flag that says that the other side + * is ok with us doing this. + * +- * path_ok() uses enter_repo() and does whitelist checking. ++ * path_ok() uses enter_repo() and checks for included directories. + * We only need to make sure the repository is exported. + */ + +@@ daemon.c: int cmd_main(int argc, const char **argv) + cred = prepare_credentials(user_name, group_name); + + if (strict_paths && (!ok_paths || !*ok_paths)) +- die("option --strict-paths requires a whitelist"); ++ die("option --strict-paths requires '<directory>' arguments"); - `GIT_PROTOCOL_FROM_USER`:: + if (base_path && !is_directory(base_path)) + die("base-path '%s' does not exist or is not a directory", -: ----------- > 2: db32444b879 git-cvsserver: clarify directory list -: ----------- > 3: c1c83c4284b git.txt: remove redundant language 2: 3c3c8c20bcb ! 4: e08342f07d2 t/*: avoid "whitelist" @@ Metadata Author: Derrick Stolee <derrickstolee@xxxxxxxxxx> ## Commit message ## - t/*: avoid "whitelist" + t: avoid "whitelist" The word "whitelist" has cultural implications that are not inclusive. Thankfully, it is not difficult to reword and avoid its use. @@ Commit message Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx> - ## t/README ## -@@ t/README: GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole - test suite. Accept any boolean values that are accepted by git-config. - - GIT_TEST_PASSING_SANITIZE_LEAK=<boolean> when compiled with --SANITIZE=leak will run only those tests that have whitelisted --themselves as passing with no memory leaks. Tests can be whitelisted --by setting "TEST_PASSES_SANITIZE_LEAK=true" before sourcing --"test-lib.sh" itself at the top of the test script. This test mode is --used by the "linux-leaks" CI target. -+SANITIZE=leak will run only those tests that have marked themselves as -+passing with no memory leaks by setting "TEST_PASSES_SANITIZE_LEAK=true" -+before sourcing "test-lib.sh" itself at the top of the test script. This -+test mode is used by the "linux-leaks" CI target. - - GIT_TEST_PROTOCOL_VERSION=<n>, when set, makes 'protocol.version' - default to n. - ## t/lib-proto-disable.sh ## @@ # Test routines for checking protocol disabling. @@ t/t5812-proto-disable-http.sh: test_expect_success 'create git-accessible repo' test_proto "smart http" http "$HTTPD_URL/smart/repo.git" -test_expect_success 'curl redirects respect whitelist' ' -+test_expect_success 'curl redirects respect allowed protocols' ' ++test_expect_success 'http(s) transport respects GIT_ALLOW_PROTOCOL' ' test_must_fail env GIT_ALLOW_PROTOCOL=http:https \ GIT_SMART_HTTP=0 \ git clone "$HTTPD_URL/ftp-redir/repo.git" 2>stderr && @@ t/t5815-submodule-protos.sh #!/bin/sh -test_description='test protocol whitelisting with submodules' -+test_description='test protocol restrictions with submodules' ++test_description='test protocol filtering with submodules' . ./test-lib.sh . "$TEST_DIRECTORY"/lib-proto-disable.sh @@ t/t5815-submodule-protos.sh: test_expect_success 'update of ext not allowed' ' ' -test_expect_success 'user can override whitelist' ' -+test_expect_success 'user can override with environment variable' ' ++test_expect_success 'user can filter protocols with GIT_ALLOW_PROTOCOL' ' GIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module ' - ## t/t9400-git-cvsserver-server.sh ## -@@ t/t9400-git-cvsserver-server.sh: test_expect_success 'req_Root (export-all)' \ - 'cat request-anonymous | git-cvsserver --export-all pserver "$WORKDIR" >log 2>&1 && - sed -ne \$p log | grep "^I LOVE YOU\$"' - --test_expect_success 'req_Root failure (export-all w/o whitelist)' \ -+test_expect_success 'req_Root failure (export-all w/o directory)' \ - '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)' - - test_expect_success 'req_Root (everything together)' \ - ## t/test-lib-functions.sh ## @@ t/test-lib-functions.sh: test_set_prereq () { # test_unset_prereq() !*) ;; - # (Temporary?) whitelist of things we can't easily -+ # (Temporary?) list of things we can't easily - # pretend not to support +- # pretend not to support ++ # List of things we can't easily pretend to not support SYMLINKS) ;; - - ## t/test-lib.sh ## -@@ t/test-lib.sh: then - test_done - fi - --# skip non-whitelisted tests when compiled with SANITIZE=leak -+# skip unmarked tests when compiled with SANITIZE=leak - if test -n "$SANITIZE_LEAK" - then - if test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false + # Inspecting whether GIT_TEST_FAIL_PREREQS is on 3: 0d862cbbebe ! 5: a4f31877b15 *: avoid "whitelist" @@ Metadata Author: Derrick Stolee <derrickstolee@xxxxxxxxxx> ## Commit message ## - *: avoid "whitelist" + transport.c: avoid "whitelist" The word "whitelist" has cultural implications that are not inclusive. Thankfully, it is not difficult to reword and avoid its use. - A previous change already modified the documentation for git-cvsserver - and git-daemon to refer to a directory list. It is simple to update the - comments and error messages here to refer to that directory list. - - In the case of transport.c, the GIT_ALLOW_PROTOCOL environment variable - was referred to as a "whitelist", but the word "allow" is already part - of the variable. Replace "whitelist" with "allow_list" in these cases to - demonstrate that we are processing a list of allowed protocols. - - After this change, the only remaining uses of "whitelist" and its - companion "blacklist" are in release notes for older versions of Git and - in the sha1dc project, which is maintained independently. + The GIT_ALLOW_PROTOCOL environment variable was referred to as a + "whitelist", but the word "allow" is already part of the variable. + Replace "whitelist" with "allow_list" in these cases to demonstrate that + we are processing a list of allowed protocols. Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx> - ## daemon.c ## -@@ daemon.c: static const char *path_ok(const char *directory, struct hostinfo *hi) - /* The validation is done on the paths after enter_repo - * appends optional {.git,.git/.git} and friends, but - * it does not use getcwd(). So if your /pub is -- * a symlink to /mnt/pub, you can whitelist /pub and -+ * a symlink to /mnt/pub, you can include /pub and - * do not have to say /mnt/pub. - * Do not say /pub/. - */ -@@ daemon.c: static const char *path_ok(const char *directory, struct hostinfo *hi) - return path; - } - -- logerror("'%s': not in whitelist", path); -+ logerror("'%s': not in directory list", path); - return NULL; /* Fallthrough. Deny by default */ - } - -@@ daemon.c: static int run_service(const char *dir, struct daemon_service *service, - * a "git-daemon-export-ok" flag that says that the other side - * is ok with us doing this. - * -- * path_ok() uses enter_repo() and does whitelist checking. -+ * path_ok() uses enter_repo() and checks for included directories. - * We only need to make sure the repository is exported. - */ - -@@ daemon.c: int cmd_main(int argc, const char **argv) - cred = prepare_credentials(user_name, group_name); - - if (strict_paths && (!ok_paths || !*ok_paths)) -- die("option --strict-paths requires a whitelist"); -+ die("option --strict-paths requires a directory list"); - - if (base_path && !is_directory(base_path)) - die("base-path '%s' does not exist or is not a directory", - - ## git-cvsserver.perl ## -@@ git-cvsserver.perl: $state->{allowed_roots} = [ @ARGV ]; - - # don't export the whole system unless the users requests it - if ($state->{'export-all'} && !@{$state->{allowed_roots}}) { -- die "--export-all can only be used together with an explicit whitelist\n"; -+ die "--export-all can only be used together with an explicit directory list\n"; - } - - # Environment handling for running under git-shell - ## transport.c ## @@ transport.c: static int external_specification_len(const char *url) return strchr(url, ':') - url; -- gitgitgadget
