From patchwork Fri May 27 21:09:17 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12863832 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 65B86C433EF for ; Fri, 27 May 2022 21:09:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1352223AbiE0VJb (ORCPT ); Fri, 27 May 2022 17:09:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58838 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242658AbiE0VJ2 (ORCPT ); Fri, 27 May 2022 17:09:28 -0400 Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 71C13127196 for ; Fri, 27 May 2022 14:09:26 -0700 (PDT) Received: by mail-wr1-x42d.google.com with SMTP id u3so7411634wrg.3 for ; Fri, 27 May 2022 14:09:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=dnD1P4ADjoBiyKOEyyalqXCKrH6TVI6Q+vaQo6UVDdU=; b=FSOuNhNQQvDRDjhqkptIkLaL8UghfF4ERVRpsLgElEFRsuw2UmOWRuOMuNi7Pajlso T7xzw1IGMEXyLEs+qP/zvlgRuSpZ/BstueHvXHSPnNWqexePRMwuo+SOGImMhgTM2Ida 0lbd8r/XRwMU5DYUO3ejloaYFM5cG5iYr5EBI8gc1/JiTxP7BhZDba/MRw36IWKEM0LV MmK3gLUYonNE2ROza9alwvgcL7WmxjILs5b4DKFdc1wYTDcaQf2GgWmk3Wmc3PjZDbia /bKJsZjKiCfZUQt1+uaDGn+9im3/aQluNaE7LHaTWQRQV5LlsH2u2lVUbSygZqbGy5J0 eCmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=dnD1P4ADjoBiyKOEyyalqXCKrH6TVI6Q+vaQo6UVDdU=; b=f33HnTJ9LOQtFMZBGJjfNkCntt7+BvosPzXofolPaPbmkNqB3IZqHLo41BYamJA3r1 JC50Vb3bO/fEhEtIBvRhMNMy/N1YVaQBu94DaFR4jEM9yFuMmKw5Zlk0o22ydyCCxNHJ kSGABFQkjgbWUGlcQvd1Gqo7BWTutAgE3GQEop9cufPhG9Ste1whaZglzRogQtzJTTYY kjEU0aU2AJ2D8f7xsAIl2P26xTaw8X79ytHStIwDlCoikb53Rt3cIVO3fjx9RWfq7qSq x4gzlYR4XTWc4KtcWWDMM7oLi0P+FJdPv/ZuNRxUJPS/q0akQfY7WeK3U3PRBO3oXxlz PzPw== X-Gm-Message-State: AOAM530MDa3Oc4ML0teRSgIra2YR6FOLTPtDX0ss7moU9O0jyb2wWOvk PVcuKvWjkJCqIWgzK+jjizuUQz1k+9w= X-Google-Smtp-Source: ABdhPJyWFBy0qEJw9I/bH4AvWWT0LN4Uk/I267QC66wEhYPVyGV4RSXZ94Xb6PFWGMb+TDGQIeKBSw== X-Received: by 2002:a5d:4ccc:0:b0:210:179c:e503 with SMTP id c12-20020a5d4ccc000000b00210179ce503mr4414689wrt.151.1653685764651; Fri, 27 May 2022 14:09:24 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id d15-20020a05600c34cf00b003949dbc3790sm3098462wmq.18.2022.05.27.14.09.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 May 2022 14:09:24 -0700 (PDT) Message-Id: <575676c760d9a2ce4a59d50e93aa0f45d54620ab.1653685761.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 27 May 2022 21:09:17 +0000 Subject: [PATCH v3 1/5] Documentation: define protected configuration Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , "brian m. carlson" , Derrick Stolee , Junio C Hamano , Emily Shaffer , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo For security reasons, some config variables are only trusted when they are specified in so-called 'protected configuration' [1]. A future commit will introduce another such config variable, so this is a good time to standardize the documentation and implementation of 'protected configuration'. Define 'protected configuration' as global and system-level config, and mark `safe.directory` 'Protected config only'. In a future commit, protected configuration will also include "-c". The following variables are intentionally not marked 'Protected config only': - `uploadpack.packObjectsHook` has the same security concerns as `safe.directory`, but due to a different implementation, it also respects the "-c" option. When protected configuration includes "-c", `upload.packObjectsHook` will be marked 'Protected config only'. - `trace2.*` happens to read the same config as `safe.directory` because they share an implementation. However, this is not for security reasons; it is because we want to start tracing so early that repository-level config and "-c" are not available [2]. This requirement is unique to `trace2.*`, so it does not makes sense for protected configuration to be subject to the same constraints. [1] For example, https://lore.kernel.org/git/6af83767-576b-75c4-c778-0284344a8fe7@github.com/ [2] https://lore.kernel.org/git/a0c89d0d-669e-bf56-25d2-cbb09b012e70@jeffhostetler.com/ Signed-off-by: Glen Choo --- Documentation/config.txt | 6 ++++++ Documentation/config/safe.txt | 19 ++++++++----------- Documentation/glossary-content.txt | 18 ++++++++++++++++++ 3 files changed, 32 insertions(+), 11 deletions(-) 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=`, or in environment variables. -+ The value of this setting is interpolated, i.e. `~/` expands to a path relative to the home directory and `%(prefix)/` 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 <> 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 <> are said to be "reachable" from that commit. More From patchwork Fri May 27 21:09:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12863833 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 90EC4C433FE for ; Fri, 27 May 2022 21:09:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1354697AbiE0VJc (ORCPT ); Fri, 27 May 2022 17:09:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58854 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1351476AbiE0VJ3 (ORCPT ); Fri, 27 May 2022 17:09:29 -0400 Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EBF5D1269B9 for ; Fri, 27 May 2022 14:09:27 -0700 (PDT) Received: by mail-wm1-x32b.google.com with SMTP id n124-20020a1c2782000000b003972dfca96cso3391282wmn.4 for ; Fri, 27 May 2022 14:09:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=vyWCBPY9wgVNipCFxb7f47vQ1iJ3QXZEZYyXxib4ai8=; b=hO6d6rLzQI/xgIya94xlviO0LS4P4d25e00M3/jxVUI5y4VQ3djHuwZIcrnlD4jV2G PV2e9SaPfUQf0h9R7r0hdbzFflAgmeOhRfkljnaGaiZN0m3FtoFBfQ6gjmOT3/QuUkjY pkWn2/mbuMLQOMbafggoFkWksEMTNl2j1S7YwJsn4YMqhEZoShTiEA5AU+1iBS3yjGr3 IeAnMgubkDYsB89Up6UR+Dwd4NkwUDdvPF8I2pnhlKsL9oSnoHHxaqsAwik3RGCG3jmf B/bUxuruWrAkAkAW0EWeYhhcSIWhbdGRU011LiWA/aDRJA7ORDrGl+9GQOWFKbCjij6b Khtg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=vyWCBPY9wgVNipCFxb7f47vQ1iJ3QXZEZYyXxib4ai8=; b=3A0YnjuNm8D7vT2oRJfYlEHYybxAwZesgVsK3aBGZA5kjx50uoWZvYPYTPJCCW9Mnf YgMt7t2G5jBdNkUbxK+Kx6nfaV2yqy9U1Gm7jtp5mhIsJk8xjalODCJ+KwWfoeP8tFL0 LTnlUgcuNuiswPjeE8FkFddRJ0QZxf5jHD1iHILvmtBPrTqWb4VfdIsxS49t7C1fZ++/ tk/3lnN8t7TEM91WKOnsdjSt4bQwmGj0QLLBYJwuW7B8Z+LyYDuow2WSCDNL/Rfa6YDF /8GboHGUltI6RbY/lDLZz8QMTXYXssI68hMeVnVDecBhNn3wza0BzP30ZOWyDPrbvdSj b/Tw== X-Gm-Message-State: AOAM530j5Ti+c4ZpMjsTxvIAPrUp9ErvtIlgoaMpipR7CVUmibaWI5QK P06Y+yuNLzy2hSkP58VaF04cMHDWQwo= X-Google-Smtp-Source: ABdhPJw31Tdk02cyznOstNz16MRCki0gDXud+14sOm4QwJRCtwrpDSK3QRSc58QBciH02mzQ8Fh57g== X-Received: by 2002:a05:600c:4fd6:b0:394:55ae:32c7 with SMTP id o22-20020a05600c4fd600b0039455ae32c7mr8619383wmq.73.1653685766136; Fri, 27 May 2022 14:09:26 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id q4-20020adfea04000000b0020d0c48d135sm2622868wrm.15.2022.05.27.14.09.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 May 2022 14:09:25 -0700 (PDT) Message-Id: <7499a2809615d42eaf3649e1c33f38d099d27c1a.1653685761.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 27 May 2022 21:09:18 +0000 Subject: [PATCH v3 2/5] config: read protected config with `git_protected_config()` Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , "brian m. carlson" , Derrick Stolee , Junio C Hamano , Emily Shaffer , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo Protected config is read using `read_very_early_config()`, which has several downsides: - Every call to `read_very_early_config()` parses global and system-level config files anew, but this can be optimized by just parsing them once [1]. - Protected variables should respect "-c" because we can reasonably assume that it comes from the user. But, `read_very_early_config()` can't use "-c" because it is called so early that it does not have access to command line arguments. Introduce `git_protected_config()`, which reads protected config and caches the values in `the_repository.protected_config`. Then, refactor `safe.directory` to use `git_protected_config()`. This implementation can still be improved, however: - `git_protected_config()` iterates through every variable in `the_repository.protected_config`, which may still be too expensive to be called in every "git" invocation. There exist constant time lookup functions for non-protected config (repo_config_get_*()), but for simplicity, this commit does not implement similar functions for protected config. - Protected config is stored in `the_repository` so that we don't need to statically allocate it. But this might be confusing since protected config ignores repository config by definition. [1] While `git_protected_config()` should save on file I/O, I wasn't able to measure a meaningful difference between that and `read_very_early_config()` on my machine (which has an SSD). Signed-off-by: Glen Choo --- config.c | 35 +++++++++++++++++++++++++++++++++++ config.h | 8 ++++++++ repository.c | 5 +++++ repository.h | 8 ++++++++ setup.c | 2 +- 5 files changed, 57 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index fa471dbdb89..c30bb7c5d09 100644 --- a/config.c +++ b/config.c @@ -2614,6 +2614,41 @@ int repo_config_get_pathname(struct repository *repo, return ret; } +/* Read protected config into the_repository->protected_config. */ +static void read_protected_config(void) +{ + char *xdg_config = NULL, *user_config = NULL, *system_config = NULL; + + CALLOC_ARRAY(the_repository->protected_config, 1); + git_configset_init(the_repository->protected_config); + + system_config = git_system_config(); + git_global_config(&user_config, &xdg_config); + + git_configset_add_file(the_repository->protected_config, system_config); + git_configset_add_file(the_repository->protected_config, xdg_config); + git_configset_add_file(the_repository->protected_config, user_config); + + free(system_config); + free(xdg_config); + free(user_config); +} + +/* Ensure that the_repository->protected_config has been initialized. */ +static void git_protected_config_check_init(void) +{ + if (the_repository->protected_config && + the_repository->protected_config->hash_initialized) + return; + read_protected_config(); +} + +void git_protected_config(config_fn_t fn, void *data) +{ + git_protected_config_check_init(); + configset_iter(the_repository->protected_config, fn, data); +} + /* Functions used historically to read configuration from 'the_repository' */ void git_config(config_fn_t fn, void *data) { diff --git a/config.h b/config.h index 7654f61c634..411965f52b5 100644 --- a/config.h +++ b/config.h @@ -505,6 +505,14 @@ int repo_config_get_maybe_bool(struct repository *repo, int repo_config_get_pathname(struct repository *repo, const char *key, const char **dest); +/* + * Functions for reading protected config. By definition, protected + * config ignores repository config, so it is unnecessary to read + * protected config from any `struct repository` other than + * the_repository. + */ +void git_protected_config(config_fn_t fn, void *data); + /** * Querying For Specific Variables * ------------------------------- diff --git a/repository.c b/repository.c index 5d166b692c8..ec319a5e09a 100644 --- a/repository.c +++ b/repository.c @@ -295,6 +295,11 @@ void repo_clear(struct repository *repo) FREE_AND_NULL(repo->remote_state); } + if (repo->protected_config) { + git_configset_clear(repo->protected_config); + FREE_AND_NULL(repo->protected_config); + } + repo_clear_path_cache(&repo->cached_paths); } diff --git a/repository.h b/repository.h index 6cc661e5a43..24251aac553 100644 --- a/repository.h +++ b/repository.h @@ -126,6 +126,14 @@ struct repository { struct repo_settings settings; + /* + * Config that comes from trusted sources, namely + * - system config files (e.g. /etc/gitconfig) + * - global config files (e.g. $HOME/.gitconfig, + * $XDG_CONFIG_HOME/git) + */ + struct config_set *protected_config; + /* Subsystems */ /* * Repository's config which contains key-value pairs from the usual diff --git a/setup.c b/setup.c index f818dd858c6..847d47f9195 100644 --- a/setup.c +++ b/setup.c @@ -1128,7 +1128,7 @@ static int ensure_valid_ownership(const char *path) is_path_owned_by_current_user(path)) return 1; - read_very_early_config(safe_directory_cb, &data); + git_protected_config(safe_directory_cb, &data); return data.is_safe; } From patchwork Fri May 27 21:09:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12863834 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7C655C433EF for ; Fri, 27 May 2022 21:09:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347937AbiE0VJo (ORCPT ); Fri, 27 May 2022 17:09:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58904 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242658AbiE0VJb (ORCPT ); Fri, 27 May 2022 17:09:31 -0400 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 580A2132A14 for ; Fri, 27 May 2022 14:09:29 -0700 (PDT) Received: by mail-wr1-x434.google.com with SMTP id k5so1132789wrd.5 for ; Fri, 27 May 2022 14:09:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=hutjqO2Dgq5cUHiwKBTEfuYuwznC5/mL1yUVQ8ypFSs=; b=KD33z9roJ1FhhV4D/hcgc3hMkZhl2CvF9H1fvkXu/350ZjOeIghe2Z5y0JPV9Txa7Q 5P3p1hqFRNSTLGkOu9WTXIFTBvjmnTguYtSe0yt58FbnhVzNM7HMZb6bJ8DJ91mxSfmI JKZhIbWQ7+BX4bNMDBucD5pmyaF37nNCRYudowWyua187fhf0/ZJbK4ETzcr7Gva4nR4 1fDBtaHLhyRgHlxHsFHMwy8Ft+pbwkaT8Tb9I9eLog7MDetN83DObLxt2ySu76pxwbWg 0zyf8Gr+YdWTBBVn+4IwlX8w6hY1F/VQc9fY7S659oL1Vaz9rW+AMOY+DTN7Z1ncJhSn zt7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=hutjqO2Dgq5cUHiwKBTEfuYuwznC5/mL1yUVQ8ypFSs=; b=nLQ39lX6DSNFkgsP9QCJ4Ouw5/IsY9cMGT5qqA5WQbsE1lqhb/Cuvn19FuWnRaDHa1 9k5TSELwRKuH/UKImnQxqjwPVj4sB3p9FSgfQahhMd672UrXzWy/Mo2r+Bcr2htca8Wb E2iXQv21B9WUagI8terIpi/2UQ61LmOXXtVca4Dco76QoV+G2+/0kDLgysXjZ1ClkG6R 6FfrPZfHZhICLldZzSAPGlxL4jR0emk4KkP0sBvbDPSioaq6CELFjA+C+072kpMgMtIb 9xD1IFEdpdHD3dsH6kKJDY0swbj0X2SU7VNgdx+Klwl6Dw6twn25xZiGbbuZBn/NQdUw B5hg== X-Gm-Message-State: AOAM531ixHaEeNip1oNWDiwoT6/01x7QYI+RPPBKJprTXlKY0gazj0/R b3O1ydkKT94bK8YBzLV6NpoqROrjJl0= X-Google-Smtp-Source: ABdhPJzI90ekOsKS8sXogeRJPwEa1E1M4G8+pRLZM4C5RLrXL+DELMNuh7Ev4wWei9VVG6yoQPOd7w== X-Received: by 2002:a5d:6da3:0:b0:20e:67a2:6779 with SMTP id u3-20020a5d6da3000000b0020e67a26779mr35964019wrs.418.1653685767503; Fri, 27 May 2022 14:09:27 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id 128-20020a1c1986000000b003972c672859sm3126459wmz.21.2022.05.27.14.09.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 May 2022 14:09:26 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Fri, 27 May 2022 21:09:19 +0000 Subject: [PATCH v3 3/5] setup.c: create `discovery.bare` Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , "brian m. carlson" , Derrick Stolee , Junio C Hamano , Emily Shaffer , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo There is a known social engineering attack that takes advantage of the fact that a working tree can include an entire bare repository, including a config file. A user could run a Git command inside the bare repository thinking that the config file of the 'outer' repository would be used, but in reality, the bare repository's config file (which is attacker-controlled) is used, which may result in arbitrary code execution. See [1] for a fuller description and deeper discussion. A simple mitigation is to forbid bare repositories unless specified via `--git-dir` or `GIT_DIR`. In environments that don't use bare repositories, this would be minimally disruptive. Create a config variable, `discovery.bare`, that tells Git whether or not to die() when it discovers a bare repository. This only affects repository discovery, thus it has no effect if discovery was not done (e.g. `--git-dir` was passed). This config is an enum of: - "always": always allow bare repositories (this is the default) - "never": never allow bare repositories If we want to protect users from such attacks by default, neither value will suffice - "always" provides no protection, but "never" is impractical for bare repository users. A more usable default would be to allow only non-embedded bare repositories ([2] contains one such proposal), but detecting if a repository is embedded is potentially non-trivial, so this work is not implemented in this series. [1]: https://lore.kernel.org/git/kl6lsfqpygsj.fsf@chooglen-macbookpro.roam.corp.google.com [2]: https://lore.kernel.org/git/5b969c5e-e802-c447-ad25-6acc0b784582@github.com Signed-off-by: Glen Choo --- Documentation/config.txt | 2 + Documentation/config/discovery.txt | 19 +++++++++ setup.c | 66 +++++++++++++++++++++++++++++- t/t0035-discovery-bare.sh | 64 +++++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 Documentation/config/discovery.txt create mode 100755 t/t0035-discovery-bare.sh diff --git a/Documentation/config.txt b/Documentation/config.txt index 07832de1a6c..34133288d75 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -415,6 +415,8 @@ include::config/diff.txt[] include::config/difftool.txt[] +include::config/discovery.txt[] + include::config/extensions.txt[] include::config/fastimport.txt[] diff --git a/Documentation/config/discovery.txt b/Documentation/config/discovery.txt new file mode 100644 index 00000000000..fbe93597e7c --- /dev/null +++ b/Documentation/config/discovery.txt @@ -0,0 +1,19 @@ +discovery.bare:: + '(Protected config only)' Specifies whether Git will work with a + bare repository that it found during repository discovery. This + has no effect if the repository is specified directly via the + --git-dir command-line option or the GIT_DIR environment + variable (see linkgit:git[1]). ++ +The currently supported values are: ++ +* `always`: Git always works with bare repositories +* `never`: Git never works with bare repositories ++ +This defaults to `always`, but this default may change in the future. ++ +If you do not use bare repositories in your workflow, then it may be +beneficial to set `discovery.bare` to `never` in your global config. +This will protect you from attacks that involve cloning a repository +that contains a bare repository and running a Git command within that +directory. diff --git a/setup.c b/setup.c index 847d47f9195..6686743ab7d 100644 --- a/setup.c +++ b/setup.c @@ -10,6 +10,13 @@ static int inside_git_dir = -1; static int inside_work_tree = -1; static int work_tree_config_is_bogus; +enum discovery_bare_config { + DISCOVERY_BARE_UNKNOWN = -1, + DISCOVERY_BARE_NEVER = 0, + DISCOVERY_BARE_ALWAYS, +}; +static enum discovery_bare_config discovery_bare_config = + DISCOVERY_BARE_UNKNOWN; static struct startup_info the_startup_info; struct startup_info *startup_info = &the_startup_info; @@ -1133,6 +1140,52 @@ static int ensure_valid_ownership(const char *path) return data.is_safe; } +static int discovery_bare_cb(const char *key, const char *value, void *d) +{ + if (strcmp(key, "discovery.bare")) + return 0; + + if (!strcmp(value, "never")) { + discovery_bare_config = DISCOVERY_BARE_NEVER; + return 0; + } + if (!strcmp(value, "always")) { + discovery_bare_config = DISCOVERY_BARE_ALWAYS; + return 0; + } + return -1; +} + +static int check_bare_repo_allowed(void) +{ + if (discovery_bare_config == DISCOVERY_BARE_UNKNOWN) { + discovery_bare_config = DISCOVERY_BARE_ALWAYS; + git_protected_config(discovery_bare_cb, NULL); + } + switch (discovery_bare_config) { + case DISCOVERY_BARE_NEVER: + return 0; + case DISCOVERY_BARE_ALWAYS: + return 1; + case DISCOVERY_BARE_UNKNOWN: + BUG("invalid discovery_bare_config %d", discovery_bare_config); + } + return 0; +} + +static const char *discovery_bare_config_to_string(void) +{ + switch (discovery_bare_config) { + case DISCOVERY_BARE_NEVER: + return "never"; + case DISCOVERY_BARE_ALWAYS: + return "always"; + case DISCOVERY_BARE_UNKNOWN: + BUG("invalid discovery_bare_config %d", discovery_bare_config); + } + return NULL; +} + enum discovery_result { GIT_DIR_NONE = 0, GIT_DIR_EXPLICIT, @@ -1142,7 +1195,8 @@ enum discovery_result { GIT_DIR_HIT_CEILING = -1, GIT_DIR_HIT_MOUNT_POINT = -2, GIT_DIR_INVALID_GITFILE = -3, - GIT_DIR_INVALID_OWNERSHIP = -4 + GIT_DIR_INVALID_OWNERSHIP = -4, + GIT_DIR_DISALLOWED_BARE = -5, }; /* @@ -1239,6 +1293,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, } if (is_git_directory(dir->buf)) { + if (!check_bare_repo_allowed()) + return GIT_DIR_DISALLOWED_BARE; if (!ensure_valid_ownership(dir->buf)) return GIT_DIR_INVALID_OWNERSHIP; strbuf_addstr(gitdir, "."); @@ -1385,6 +1441,14 @@ const char *setup_git_directory_gently(int *nongit_ok) } *nongit_ok = 1; break; + case GIT_DIR_DISALLOWED_BARE: + if (!nongit_ok) { + die(_("cannot use bare repository '%s' (discovery.bare is '%s')"), + dir.buf, + discovery_bare_config_to_string()); + } + *nongit_ok = 1; + break; case GIT_DIR_NONE: /* * As a safeguard against setup_git_directory_gently_1 returning diff --git a/t/t0035-discovery-bare.sh b/t/t0035-discovery-bare.sh new file mode 100755 index 00000000000..94c2f76d774 --- /dev/null +++ b/t/t0035-discovery-bare.sh @@ -0,0 +1,64 @@ +#!/bin/sh + +test_description='verify discovery.bare checks' + +. ./test-lib.sh + +pwd="$(pwd)" + +expect_rejected () { + test_must_fail git rev-parse --git-dir 2>err && + grep "discovery.bare" err +} + +test_expect_success 'setup bare repo in worktree' ' + git init outer-repo && + git init --bare outer-repo/bare-repo +' + +test_expect_success 'discovery.bare unset' ' + ( + cd outer-repo/bare-repo && + git rev-parse --git-dir + ) +' + +test_expect_success 'discovery.bare=always' ' + git config --global discovery.bare always && + ( + cd outer-repo/bare-repo && + git rev-parse --git-dir + ) +' + +test_expect_success 'discovery.bare=never' ' + git config --global discovery.bare never && + ( + cd outer-repo/bare-repo && + expect_rejected + ) +' + +test_expect_success 'discovery.bare in the repository' ' + ( + cd outer-repo/bare-repo && + # Temporarily set discovery.bare=always, otherwise git + # config fails with "fatal: not in a git directory" + # (like safe.directory) + git config --global discovery.bare always && + git config discovery.bare always && + git config --global discovery.bare never && + expect_rejected + ) +' + +test_expect_success 'discovery.bare on the command line' ' + git config --global discovery.bare never && + ( + cd outer-repo/bare-repo && + test_must_fail git -c discovery.bare=always rev-parse --git-dir 2>err && + grep "discovery.bare" err + ) +' + +test_done From patchwork Fri May 27 21:09:20 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12863836 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 29F53C433EF for ; Fri, 27 May 2022 21:09:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1354701AbiE0VJq (ORCPT ); Fri, 27 May 2022 17:09:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58938 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1354696AbiE0VJc (ORCPT ); Fri, 27 May 2022 17:09:32 -0400 Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 656AA1339FA for ; Fri, 27 May 2022 14:09:30 -0700 (PDT) Received: by mail-wm1-x32f.google.com with SMTP id f23-20020a7bcc17000000b003972dda143eso5258194wmh.3 for ; Fri, 27 May 2022 14:09:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=Hw1LDv0GzA4OB1oJQkdVGfyx3ceT1MPERg3FQv07nqo=; b=DFlcyng7l4bb5yaGxg1K/9wZZAf0WCN/VF+r2eYZqI+YQ46fegtHcBXXPPnwh0OhL5 Jv+myr2jW0oPkcgNnAV9+yNnO/U6dbANm2WzP/w1QK+EQ7GuyA6/KAz/WBbLZdajIhCI mdnYN8exyJ1FODLE8+pzMXMMpTu6m9Kzx65hYlQBjOUGMUh0ixT7x+25AA32nUZcmC2L KvCfrFNHzmcuy32ty44mbClk0W3oDSCg/E+383zlGS2miwp0rBpl2FUHFPK5NgGNfgWN OI3sJNDSnzM1Rz/OLhX9qPViQ6Gkv0XnvOleLtMYhLeblCRc15CO914m/6Y1HFA3JW8x Tn1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=Hw1LDv0GzA4OB1oJQkdVGfyx3ceT1MPERg3FQv07nqo=; b=SL9imYB2OE9Vt9fyNpi4fyo5fYRiSE15hDhBkrnymV2lKDYCeE8OvaZi77QDCS6XZA z51ubQ0xrcY4+hc2kbsuqF4f3sWOdWOoQeLN7JGJuUaed3hHCV6cWWMIa62ZiVx8wWtk HK19+jTnk2T2CS9gFU0stsKiPQFE8yeCYDkA2Cgi6mWpcKUGut2aa7TPlDd0HUm5LqmK 8AC5nhqOimcnB9dUcB61XSGR7KJrJI+NH2oG2OoLfwNVfiD7GNjnQnU7U4B05v0RB0b5 BoQo93U9FuawSajzo0CTInrxl8hYIkqqb27zFWAzUsXFKnedHVBiyDVSNMbrzkzU0gGl yBRA== X-Gm-Message-State: AOAM531yqJQetiwpaVnbKzSo/56T0kLZzlqmjqV0Az3IpYkew5kC+Qvn P5N1FIxyMaVYdvbhVFQIuVdNnqzkO0s= X-Google-Smtp-Source: ABdhPJxiC2yX9neNIMSWDpRl0rBaDfajLdE0BBNZt4KOEw30FyblpWbn5D05iOY6jtcmbDtGvcCy/g== X-Received: by 2002:a05:600c:34d1:b0:397:4c0d:598a with SMTP id d17-20020a05600c34d100b003974c0d598amr8563699wmq.36.1653685768631; Fri, 27 May 2022 14:09:28 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id q62-20020a1c4341000000b003942a244f38sm2879618wma.17.2022.05.27.14.09.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 May 2022 14:09:28 -0700 (PDT) Message-Id: <66a0a208176094415d7b0ae1f686bdd1638b8a8a.1653685761.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 27 May 2022 21:09:20 +0000 Subject: [PATCH v3 4/5] config: include "-c" in protected config Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , "brian m. carlson" , Derrick Stolee , Junio C Hamano , Emily Shaffer , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo Protected config should include the command line (aka "-c") because we can be quite certain that this config is specified by the user. Introduce a function, `git_configset_add_parameters()`, that adds "-c" config to a config_set, and use it to add "-c" to protected config. Signed-off-by: Glen Choo --- Documentation/config.txt | 6 +++--- Documentation/glossary-content.txt | 2 +- config.c | 6 ++++++ config.h | 9 +++++++++ t/t0033-safe-directory.sh | 24 ++++++++++-------------- t/t0035-discovery-bare.sh | 3 +-- 6 files changed, 30 insertions(+), 20 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 34133288d75..f40a3e297ce 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -370,9 +370,9 @@ 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 +they are specified in protected configuration. This includes global, +system-level config, the command line option `-c`, and environment +variables, and excludes repository config. For more details, see the 'protected configuration' entry in linkgit:gitglossary[7]. include::config/advice.txt[] diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index a669983abd6..4190c410a00 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -494,12 +494,12 @@ Protected configuration includes: - system-level config, e.g. `/etc/git/config` - global config, e.g. `$XDG_CONFIG_HOME/git/config` and `$HOME/.gitconfig` +- the command line option `-c` and its equivalent environment variables + 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 <> are said to be diff --git a/config.c b/config.c index c30bb7c5d09..22192ca1d63 100644 --- a/config.c +++ b/config.c @@ -2373,6 +2373,11 @@ int git_configset_add_file(struct config_set *cs, const char *filename) return git_config_from_file(config_set_callback, filename, cs); } +int git_configset_add_parameters(struct config_set *cs) +{ + return git_config_from_parameters(config_set_callback, cs); +} + int git_configset_get_value(struct config_set *cs, const char *key, const char **value) { const struct string_list *values = NULL; @@ -2628,6 +2633,7 @@ static void read_protected_config(void) git_configset_add_file(the_repository->protected_config, system_config); git_configset_add_file(the_repository->protected_config, xdg_config); git_configset_add_file(the_repository->protected_config, user_config); + git_configset_add_parameters(the_repository->protected_config); free(system_config); free(xdg_config); diff --git a/config.h b/config.h index 411965f52b5..e3ff1fcf683 100644 --- a/config.h +++ b/config.h @@ -446,6 +446,15 @@ void git_configset_init(struct config_set *cs); */ int git_configset_add_file(struct config_set *cs, const char *filename); +/** + * Parses command line options and environment variables, and adds the + * variable-value pairs to the `config_set`. Returns 0 on success, or -1 + * if there is an error in parsing. The caller decides whether to free + * the incomplete configset or continue using it when the function + * returns -1. + */ +int git_configset_add_parameters(struct config_set *cs); + /** * Finds and returns the value list, sorted in order of increasing priority * for the configuration variable `key` and config set `cs`. When the diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh index 238b25f91a3..5a1cd0d0947 100755 --- a/t/t0033-safe-directory.sh +++ b/t/t0033-safe-directory.sh @@ -16,24 +16,20 @@ test_expect_success 'safe.directory is not set' ' expect_rejected_dir ' -test_expect_success 'ignoring safe.directory on the command line' ' - test_must_fail git -c safe.directory="$(pwd)" status 2>err && - grep "unsafe repository" err +test_expect_success 'safe.directory on the command line' ' + git -c safe.directory="$(pwd)" status ' -test_expect_success 'ignoring safe.directory in the environment' ' - test_must_fail env GIT_CONFIG_COUNT=1 \ - GIT_CONFIG_KEY_0="safe.directory" \ - GIT_CONFIG_VALUE_0="$(pwd)" \ - git status 2>err && - grep "unsafe repository" err +test_expect_success 'safe.directory in the environment' ' + env GIT_CONFIG_COUNT=1 \ + GIT_CONFIG_KEY_0="safe.directory" \ + GIT_CONFIG_VALUE_0="$(pwd)" \ + git status ' -test_expect_success 'ignoring safe.directory in GIT_CONFIG_PARAMETERS' ' - test_must_fail env \ - GIT_CONFIG_PARAMETERS="${SQ}safe.directory${SQ}=${SQ}$(pwd)${SQ}" \ - git status 2>err && - grep "unsafe repository" err +test_expect_success 'safe.directory in GIT_CONFIG_PARAMETERS' ' + env GIT_CONFIG_PARAMETERS="${SQ}safe.directory${SQ}=${SQ}$(pwd)${SQ}" \ + git status ' test_expect_success 'ignoring safe.directory in repo config' ' diff --git a/t/t0035-discovery-bare.sh b/t/t0035-discovery-bare.sh index 94c2f76d774..0d5983df307 100755 --- a/t/t0035-discovery-bare.sh +++ b/t/t0035-discovery-bare.sh @@ -56,8 +56,7 @@ test_expect_success 'discovery.bare on the command line' ' git config --global discovery.bare never && ( cd outer-repo/bare-repo && - test_must_fail git -c discovery.bare=always rev-parse --git-dir 2>err && - grep "discovery.bare" err + git -c discovery.bare=always rev-parse --git-dir ) ' From patchwork Fri May 27 21:09:21 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12863835 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 888CFC433F5 for ; Fri, 27 May 2022 21:09:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1354698AbiE0VJp (ORCPT ); Fri, 27 May 2022 17:09:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59114 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1351476AbiE0VJn (ORCPT ); Fri, 27 May 2022 17:09:43 -0400 Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 68054134E0B for ; Fri, 27 May 2022 14:09:31 -0700 (PDT) Received: by mail-wr1-x42d.google.com with SMTP id j25so7408162wrb.6 for ; Fri, 27 May 2022 14:09:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=N1Eo3S49SuLE5/nvsGEp1jeWzQchxpAz/jsT0VzhlG0=; b=JO1Zd7UT/+IlBuLzNvxOORrV0T1an8T56YfRjUM7NsJ07gZDbO37QEhWIytT69hhIX 7hgAmVxeULNe7gIv2mKuMTYP7xVep4wwz1lpig+8uOdpooE+7l33Mt+4zG96c8vLgl5y wPG93jT6D7Vnxcknuyfrc/mP3l8fHhkT8gbHTbva8i2fP3h8dyrUkKcUmuWygSUlUV3c XXppM3z2fcymfcIbhrTJ/RBRtt+w+FSsDBw/dJocG/rUxj8GWuWLPY3ieh8VtxwT2/vk 2sfrNuAVJCpeYKP1nMMH2UMebGjJU/efxYMvelziKaSsAF+LqXOB4EOXkL7GBC/n7Foo UDJA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=N1Eo3S49SuLE5/nvsGEp1jeWzQchxpAz/jsT0VzhlG0=; b=g8ZryJCIFQhWlub8lSudxhb7OJfgQQdgjKMo+YP8RcTNzNS5qtiNxXwV/ukKPYKBSm DH8rg4CiJGw4JP/jJmvCg6fMmYTi9CSZrJ5vp9zbzHi6JbVymAkaavPm6MCoxp0ZjFtu 7wVt/dFDFurJNjdEN2aL7kgeRQcmuNEuvvSW+PvMczVPOfJoBXLODZpnS7WvIol3UVc7 CI4+YV1P9bf9UAk81ifND1D+1KiMkZhZ++FwERpCic+7ixw++0ZW7iSoHzhS+kDCJUKq iAhhPd3I7XztOXJZ6qaC/N6/OfaY63w3eWprKgL7WrYolV+U8DUz2iUY5YwZbF30rZ23 /J7A== X-Gm-Message-State: AOAM531GqecQ983DdIf/zyDMlxmt2g8xENHS6UTpHVGx2UW6ykpzD1fy ZU4cUF1jc4AnM/YjTjHEZqvazdbUZSE= X-Google-Smtp-Source: ABdhPJxk8iQJC0edjotDUMyyYUfxOppRTyJcR17EVTzPROrOiDQhX7yU0tE6J1UVDG1hHU/as3rtKQ== X-Received: by 2002:adf:cd87:0:b0:20f:e5fd:d265 with SMTP id q7-20020adfcd87000000b0020fe5fdd265mr20668450wrj.196.1653685769670; Fri, 27 May 2022 14:09:29 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id g11-20020a5d64eb000000b0020fcc655e4asm2785697wri.5.2022.05.27.14.09.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 May 2022 14:09:29 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Fri, 27 May 2022 21:09:21 +0000 Subject: [PATCH v3 5/5] upload-pack: make uploadpack.packObjectsHook protected Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , "brian m. carlson" , Derrick Stolee , Junio C Hamano , Emily Shaffer , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo Now that protected config includes "-c", "uploadpack.packObjectsHook" behaves identically to a 'Protected config only' variable. Refactor it to use git_protected_config() and mark it 'Protected config only'. Signed-off-by: Glen Choo --- Documentation/config/uploadpack.txt | 22 +++++++++------------- upload-pack.c | 17 +++++++++++------ 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt index 32fad5bbe81..57e5e021323 100644 --- a/Documentation/config/uploadpack.txt +++ b/Documentation/config/uploadpack.txt @@ -39,19 +39,15 @@ uploadpack.keepAlive:: disables keepalive packets entirely. The default is 5 seconds. uploadpack.packObjectsHook:: - If this option is set, when `upload-pack` would run - `git pack-objects` to create a packfile for a client, it will - run this shell command instead. The `pack-objects` command and - arguments it _would_ have run (including the `git pack-objects` - at the beginning) are appended to the shell command. The stdin - and stdout of the hook are treated as if `pack-objects` itself - was run. I.e., `upload-pack` will feed input intended for - `pack-objects` to the hook, and expects a completed packfile on - stdout. -+ -Note that this configuration variable is ignored if it is seen in the -repository-level config (this is a safety measure against fetching from -untrusted repositories). + '(Protected config only)' If this option is set, when + `upload-pack` would run `git pack-objects` to create a packfile + for a client, it will run this shell command instead. The + `pack-objects` command and arguments it _would_ have run + (including the `git pack-objects` at the beginning) are appended + to the shell command. The stdin and stdout of the hook are + treated as if `pack-objects` itself was run. I.e., `upload-pack` + will feed input intended for `pack-objects` to the hook, and + expects a completed packfile on stdout. uploadpack.allowFilter:: If this option is set, `upload-pack` will support partial diff --git a/upload-pack.c b/upload-pack.c index 3a851b36066..2a39391369d 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -1321,18 +1321,21 @@ static int upload_pack_config(const char *var, const char *value, void *cb_data) data->advertise_sid = git_config_bool(var, value); } - if (current_config_scope() != CONFIG_SCOPE_LOCAL && - current_config_scope() != CONFIG_SCOPE_WORKTREE) { - if (!strcmp("uploadpack.packobjectshook", var)) - return git_config_string(&data->pack_objects_hook, var, value); - } - if (parse_object_filter_config(var, value, data) < 0) return -1; return parse_hide_refs_config(var, value, "uploadpack"); } +static int upload_pack_protected_config(const char *var, const char *value, void *cb_data) +{ + struct upload_pack_data *data = cb_data; + + if (!strcmp("uploadpack.packobjectshook", var)) + return git_config_string(&data->pack_objects_hook, var, value); + return 0; +} + void upload_pack(const int advertise_refs, const int stateless_rpc, const int timeout) { @@ -1342,6 +1345,7 @@ void upload_pack(const int advertise_refs, const int stateless_rpc, upload_pack_data_init(&data); git_config(upload_pack_config, &data); + git_protected_config(upload_pack_protected_config, &data); data.stateless_rpc = stateless_rpc; data.timeout = timeout; @@ -1697,6 +1701,7 @@ int upload_pack_v2(struct repository *r, struct packet_reader *request) data.use_sideband = LARGE_PACKET_MAX; git_config(upload_pack_config, &data); + git_protected_config(upload_pack_protected_config, &data); while (state != FETCH_DONE) { switch (state) {