From patchwork Tue May 10 17:46:14 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Carlo_Marcelo_Arenas_Bel=C3=B3n?= X-Patchwork-Id: 12845345 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 7F8EDC433EF for ; Tue, 10 May 2022 17:46:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343614AbiEJRug (ORCPT ); Tue, 10 May 2022 13:50:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37880 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1348387AbiEJRua (ORCPT ); Tue, 10 May 2022 13:50:30 -0400 Received: from mail-qk1-x732.google.com (mail-qk1-x732.google.com [IPv6:2607:f8b0:4864:20::732]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9252B56FB1 for ; Tue, 10 May 2022 10:46:32 -0700 (PDT) Received: by mail-qk1-x732.google.com with SMTP id a76so13734799qkg.12 for ; Tue, 10 May 2022 10:46:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=couz2yvVijOlpxItzLIoHD0R8RVjhamGn1kW5vMTtj8=; b=ffaa/La3A/sfK4zCf5mCqYUGuXY2tKXRHi38qR1MxbEPIpO6Bjsg1Mo4tyk7YV/0KW L6fPnubYZQUPwNZPLI5e0ldqvN7F8tL/GwLBi36jEAyDL9S7hJ5ts9oVK9fhwNWOTPxC iZX5vdY1rVCtuI43rCM/JjnZ1CNQRFRwdig37nAjPfjMTTZBkCNatJ75SvWK4FvjC3Uh wIbXM9H2/0YeGBcqvdbL9FDAWSVI3cqj3HVvtSMjS+bjkyiu4xtFj4oUTpP411P8roF4 vNYrJWbC+Q0oFog3JztOoLkUMEPR8s77EhZYHxPzoK4vlhUAeFGZ5DFqlZMoZm7rwRaD 9ndw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=couz2yvVijOlpxItzLIoHD0R8RVjhamGn1kW5vMTtj8=; b=JdsCpKwk7oBap2K85vpoJV57GjIr9UKNPKdbETxpQLnAbkzzYrvyfy3a0G3LTJw03x cU8BP/oF4LPs1Xk+BwPteAhLY3W+KgDMjQC+jE/KZN8tboOb7KO6zltolEBdJwZHn+wa Si86ZD9dGYkrYdZtnfkidhxldTA6TwlTa+g6p+JQ65U8GiYulUTfGTjIBGG18HwVYSOG zMm0UJ1VHv2KugUM1Ojm15B251BiywgoWsSAE7vXniW4dzGnLlqY22s2Mjpw1Ar+yB+N CBwaNaI5CP2bbzwTS0yz3dpEp/e2XZ2xad7Aqq3w8fRa2KrzQEIUkgX2hW6AjSCT9dhp WgBQ== X-Gm-Message-State: AOAM532LvqkG30ju3/6uD5vr0MH2zfcOSPAo1G0m1Wlfv8IXL4sH+ioK SIQviuZmllGzcbkaQnlNNqJza9PUaiM= X-Google-Smtp-Source: ABdhPJyQhcrFIhCl2jH2RnR79FgWGic53YEVocHAdp4QehK3+owvxTSseuEW9aXLQZn29o8Dmtu/dg== X-Received: by 2002:a37:a98c:0:b0:6a0:d9ff:4959 with SMTP id s134-20020a37a98c000000b006a0d9ff4959mr73082qke.336.1652204791510; Tue, 10 May 2022 10:46:31 -0700 (PDT) Received: from localhost.localdomain (104-1-92-200.lightspeed.sntcca.sbcglobal.net. [104.1.92.200]) by smtp.gmail.com with ESMTPSA id h191-20020a379ec8000000b0069fc347ef5dsm8719280qke.74.2022.05.10.10.46.30 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 May 2022 10:46:31 -0700 (PDT) From: =?utf-8?q?Carlo_Marcelo_Arenas_Bel=C3=B3n?= To: git@vger.kernel.org Cc: gitster@pobox.com, bagasdotme@gmail.com, phillip.wood123@gmail.com, Johannes.Schindelin@gmx.de, =?utf-8?q?Carlo_Marcelo_Arenas_Bel=C3=B3n?= , =?utf-8?q?SZEDER_G=C3=A1bor?= Subject: [PATCH v4 1/3] t: regression git needs safe.directory when using sudo Date: Tue, 10 May 2022 10:46:14 -0700 Message-Id: <20220510174616.18629-2-carenas@gmail.com> X-Mailer: git-send-email 2.36.1.371.g0fb0ef0c8d In-Reply-To: <20220510174616.18629-1-carenas@gmail.com> References: <20220507163508.78459-1-carenas@gmail.com> <20220510174616.18629-1-carenas@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Originally reported after release of v2.35.2 (and other maint branches) for CVE-2022-24765 and blocking otherwise harmless commands that were done using sudo in a repository that was owned by the user. Add a new test script with very basic support to allow running git commands through sudo, so a reproduction could be implemented and that uses only `git status` as a proxy of the issue reported. Note that because of the way sudo interacts with the system, a much more complete integration with the test framework will require a lot more work and that was therefore intentionally punted for now. The current implementation requires the execution of a special cleanup function which should always be kept as the last "test" or otherwise the standard cleanup functions will fail because they can't remove the root owned directories that are used. This also means that if failures are found while running, the specifics of the failure might not be kept for further debugging and if the test was interrupted, it will be necessary to clean the working directory manually before restarting by running: $ sudo rm -rf trash\ directory.t0034-root-safe-directory/ The test file also uses at least one initial "setup" test that creates a parallel execution directory, while ignoring the repository created by the test framework. Special care should be taken when invoking commands through sudo, since the environment is otherwise independent from what the test framework setup and might have changed the values for HOME, SHELL and dropped several relevant environment variables for your test. Indeed `git status` was used as a proxy because it doesn't even require commits in the repository to work and usually doesn't require much from the environment to run, but a future patch will add calls to `git init` and that will fail to honor the default branch name, unless that setting is NOT provided through an environment variable (which means even a CI run could fail that test if enabled incorrectly). A new SUDO prerequisite is provided that does some sanity checking to make sure the sudo command that will be used allows for passwordless execution as root without restrictions and doesn't mess with git's execution path. This matches what is provided by the macOS agents that are used as part of GitHub actions and probably nowhere else. Most of those characteristics make this test mostly only suitable for CI, but it might be executed locally if special care is taken to provide for all of them in the local configuration and maybe making use of the sudo credential cache by first invoking sudo, entering your password if needed, and then invoking the test with: $ GIT_TEST_ALLOW_SUDO=YES ./t0034-root-safe-directory.sh If it fails to run, then it means your local setup wouldn't work for the test because of the configuration sudo has or other system settings, and things that might help are to comment out sudo's secure_path config, and make sure that the account you are using has no restrictions on the commands it can run through sudo, just like is provided for the user in the CI. For example (assuming a username of marta for you) something probably similar to the following entry in your /etc/sudoers (or equivalent) file: marta ALL=(ALL:ALL) NOPASSWD: ALL Reported-by: SZEDER Gábor Helped-by: Phillip Wood Signed-off-by: Carlo Marcelo Arenas Belón --- t/t0034-root-safe-directory.sh | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100755 t/t0034-root-safe-directory.sh diff --git a/t/t0034-root-safe-directory.sh b/t/t0034-root-safe-directory.sh new file mode 100755 index 0000000000..2e4492a66d --- /dev/null +++ b/t/t0034-root-safe-directory.sh @@ -0,0 +1,45 @@ +#!/bin/sh + +test_description='verify safe.directory checks while running as root' + +. ./test-lib.sh + +if [ "$GIT_TEST_ALLOW_SUDO" != "YES" ] +then + skip_all="You must set env var GIT_TEST_ALLOW_SUDO=YES in order to run this test" + test_done +fi + +test_lazy_prereq SUDO ' + sudo -n id -u >u && + id -u root >r && + test_cmp u r && + command -v git >u && + sudo command -v git >r && + test_cmp u r +' + +test_expect_success SUDO 'setup' ' + sudo rm -rf root && + mkdir -p root/r && + sudo chown root root && + ( + cd root/r && + git init + ) +' + +test_expect_failure SUDO 'sudo git status as original owner' ' + ( + cd root/r && + git status && + sudo git status + ) +' + +# this MUST be always the last test +test_expect_success SUDO 'cleanup' ' + sudo rm -rf root +' + +test_done From patchwork Tue May 10 17:46:15 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Carlo_Marcelo_Arenas_Bel=C3=B3n?= X-Patchwork-Id: 12845346 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 845D6C433FE for ; Tue, 10 May 2022 17:46:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1348405AbiEJRuk (ORCPT ); Tue, 10 May 2022 13:50:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38008 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1348386AbiEJRud (ORCPT ); Tue, 10 May 2022 13:50:33 -0400 Received: from mail-qk1-x732.google.com (mail-qk1-x732.google.com [IPv6:2607:f8b0:4864:20::732]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6712E58E61 for ; Tue, 10 May 2022 10:46:34 -0700 (PDT) Received: by mail-qk1-x732.google.com with SMTP id 185so4284819qke.7 for ; Tue, 10 May 2022 10:46:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=OQ+Zw/Rsfd09yVX5FsQ4w2Y+Uv0WtuP/cJZg0jXBKp8=; b=d4C8LiXcU1OVwgNWmTmyyeNhsDlfQe8A3zkjVL1o8lUVzJotlT2DutHGRXcsV/r1tn f3hgp4jw3AIaH8yxaE2RsKT3v8lndCM7/UMrklHkgHG5liS0MohBoDtY0jOni35pL4PX IKGip64o35lDPTaLkBGCyyDr4NTPiV8RtTdoDChxJobX99j1SX6bEAdg724e5YR62kHA x5LZBiGP+WqTdm6i1LrnbB2ROOmYFZtxSMFI+AixmbhPR/y+0eZd9ThHOWtRjotl8E2I 1AKFImGWd/3Si8+FVGnd96c58EYNyPqoNCX2r3xiIglITlKH5cbSMkrw176bQ2BJmobN idZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=OQ+Zw/Rsfd09yVX5FsQ4w2Y+Uv0WtuP/cJZg0jXBKp8=; b=HKxayboJmCnX39xEy6k/ilDQF/SCK798XwZCzPdU1sOOzzPYCqmw4NFKLFD+mxbXWx 228eLLYxRRFEwYsSmQrG+e0l29DSBQCWjapHXmlLgsvyrFoQBmCyKF5QKSnoSo+7wL/T 00AKm+JnGYPARB0h0fn+CAIzQQsmwsKuQ4n0bKOMMvuL41ye+eawAWjK3IQCgMphLM1m 2uQOq026OQizc4+h+5NKIkx4XSVh0bwcRh9W8/+a6NhzGPksH+EuBZkjfBHR4cp9Ukhb pmBncci70b0l+YIS0auy5CZx6XbrFZV6+1WsJ832SfamnYrUdraFKV5Et1IZKfc0qhIg +YYA== X-Gm-Message-State: AOAM533LtupUdh9M+om4Td8RuTo7aP+JF/VRt/hOPfhGuJGAtvbZzmvj fPaJYhThvaTgMZg+PtVioEOfI6bOyGg= X-Google-Smtp-Source: ABdhPJx5oUe6JgP1zrn6JeVfjyBSzCsmfu+d4P23fOViIAv0OKw/EWHTYuvg5NM4TCuxhFQy/aQWiw== X-Received: by 2002:a05:620a:2402:b0:69f:ff32:bd37 with SMTP id d2-20020a05620a240200b0069fff32bd37mr16166337qkn.657.1652204793231; Tue, 10 May 2022 10:46:33 -0700 (PDT) Received: from localhost.localdomain (104-1-92-200.lightspeed.sntcca.sbcglobal.net. [104.1.92.200]) by smtp.gmail.com with ESMTPSA id h191-20020a379ec8000000b0069fc347ef5dsm8719280qke.74.2022.05.10.10.46.31 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 May 2022 10:46:32 -0700 (PDT) From: =?utf-8?q?Carlo_Marcelo_Arenas_Bel=C3=B3n?= To: git@vger.kernel.org Cc: gitster@pobox.com, bagasdotme@gmail.com, phillip.wood123@gmail.com, Johannes.Schindelin@gmx.de, =?utf-8?q?Carlo_Marcelo_Arenas_Bel=C3=B3n?= , Guy Maurel , =?utf-8?q?SZEDER_G=C3=A1bor?= , Randall Becker Subject: [PATCH v4 2/3] git-compat-util: avoid failing dir ownership checks if running privileged Date: Tue, 10 May 2022 10:46:15 -0700 Message-Id: <20220510174616.18629-3-carenas@gmail.com> X-Mailer: git-send-email 2.36.1.371.g0fb0ef0c8d In-Reply-To: <20220510174616.18629-1-carenas@gmail.com> References: <20220507163508.78459-1-carenas@gmail.com> <20220510174616.18629-1-carenas@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org bdc77d1d685 (Add a function to determine whether a path is owned by the current user, 2022-03-02) checks for the effective uid of the running process using geteuid() but didn't account for cases where that user was root (because git was invoked through sudo or a compatible tool) and the original uid that repository trusted for its config was no longer known, therefore failing the following otherwise safe call: guy@renard ~/Software/uncrustify $ sudo git describe --always --dirty [sudo] password for guy: fatal: unsafe repository ('/home/guy/Software/uncrustify' is owned by someone else) Attempt to detect those cases by using the environment variables that those tools create to keep track of the original user id, and do the ownership check using that instead. This assumes the environment the user is running on after going privileged can't be tampered with, and also adds code to restrict that the new behavior only applies if running as root, therefore keeping the most common case, which runs unprivileged, from changing, but because of that, it will miss cases where sudo (or an equivalent) was used to change to another unprivileged user or where the equivalent tool used to raise privileges didn't track the original id in a sudo compatible way. Because of compatibility with sudo, the code assumes that uid_t is an unsigned integer type (which is not required by the standard) but is used that way in their codebase to generate SUDO_UID. In systems where uid_t is signed, sudo might be also patched to NOT be unsigned and that might be able to trigger an edge case and a bug (as described in the code), but it is considered unlikely to be triggered, and even when it does the code would just safely fail, so there is no attempt either to detect it or prevent it in the code, which might need to be changed in the future. Reported-by: Guy Maurel Helped-by: SZEDER Gábor Helped-by: Randall Becker Helped-by: Phillip Wood Suggested-by: Johannes Schindelin Signed-off-by: Carlo Marcelo Arenas Belón --- Documentation/config/safe.txt | 10 +++++++ git-compat-util.h | 54 +++++++++++++++++++++++++++++++++- t/t0034-root-safe-directory.sh | 2 +- 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt index 6d764fe0cc..dba9d5b2d1 100644 --- a/Documentation/config/safe.txt +++ b/Documentation/config/safe.txt @@ -26,3 +26,13 @@ directory was listed in the `safe.directory` list. If `safe.directory=*` is set in system config and you want to re-enable this protection, then initialize your list with an empty value before listing the repositories that you deem safe. ++ +When git tries to check for ownership of git repositories, it will +obviously do so with the uid of the user that is running git itself, +but if git is running as root, in a platform that provides sudo and is +not Windows, it will check first if it might have been started through +it, and if that is the case, will use the uid of the user that invoked +sudo instead. +If that is not what you would prefer and want git to only trust +repositories that are owned by root instead, then you should remove +the `SUDO_UID` variable from root's environment. diff --git a/git-compat-util.h b/git-compat-util.h index 63ba89dd31..754cd90d43 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -393,12 +393,64 @@ static inline int git_offset_1st_component(const char *path) #endif #ifndef is_path_owned_by_current_user + +#ifdef __TANDEM +#define ROOT_UID 65535 +#else +#define ROOT_UID 0 +#endif + +/* + * This helper function overrides a ROOT_UID with the one provided by + * an environment variable, do not use unless the original user is + * root or could be used as means to escalate to root privileges. + * + * PORTABILITY WARNING: + * This code assumes uid_t is unsigned because that is what sudo does. + * If your uid_t type is signed and all your ids are positive then it + * should all work fine, but need to make sure sudo never generates a + * value higher than what can be represented by your uid_t type or a + * negative value or it will trigger wraparound bugs. + * + * If that happens the uid used might be incorrect and then trigger + * an access error from the filesystem itself. + * + * In the unlikely scenario this happened to you, and that is how you + * got to this message, we would like to know about it by letting us + * now with an email to git@vger.kernel.org indicating which platform, + * you are running on and which version of sudo you used to see if we + * can provide you a patch that would prevent this issue in the future. + */ +static inline void extract_id_from_env(const char *env, uid_t *id) +{ + const char *real_uid = getenv(env); + + /* discard anything empty to avoid a more complex check below */ + if (real_uid && *real_uid) { + char *endptr = NULL; + unsigned long env_id; + + errno = 0; + /* silent overflow errors could trigger a bug below */ + env_id = strtoul(real_uid, &endptr, 10); + if (!*endptr && !errno) + *id = env_id; + } +} + static inline int is_path_owned_by_current_uid(const char *path) { struct stat st; + uid_t euid; + if (lstat(path, &st)) return 0; - return st.st_uid == geteuid(); + + euid = geteuid(); + if (euid == ROOT_UID) + extract_id_from_env("SUDO_UID", &euid); + + return st.st_uid == euid; } #define is_path_owned_by_current_user is_path_owned_by_current_uid diff --git a/t/t0034-root-safe-directory.sh b/t/t0034-root-safe-directory.sh index 2e4492a66d..ecd9dca6b3 100755 --- a/t/t0034-root-safe-directory.sh +++ b/t/t0034-root-safe-directory.sh @@ -29,7 +29,7 @@ test_expect_success SUDO 'setup' ' ) ' -test_expect_failure SUDO 'sudo git status as original owner' ' +test_expect_success SUDO 'sudo git status as original owner' ' ( cd root/r && git status && From patchwork Tue May 10 17:46:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Carlo_Marcelo_Arenas_Bel=C3=B3n?= X-Patchwork-Id: 12845347 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 841D3C433EF for ; Tue, 10 May 2022 17:46:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1348407AbiEJRum (ORCPT ); Tue, 10 May 2022 13:50:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38126 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1348388AbiEJRue (ORCPT ); Tue, 10 May 2022 13:50:34 -0400 Received: from mail-qv1-xf36.google.com (mail-qv1-xf36.google.com [IPv6:2607:f8b0:4864:20::f36]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 295B659978 for ; Tue, 10 May 2022 10:46:35 -0700 (PDT) Received: by mail-qv1-xf36.google.com with SMTP id c8so13121690qvh.10 for ; Tue, 10 May 2022 10:46:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=cn83vrYcwSmTs+Yn4rucxqISZ4LQ1+HVmZDCs1zjaq8=; b=owHyOO3uj+esoWkWViyV7Xbsib9NtszRxCXvReV7/gVY3u1RdPnk73PlL8fJAtabXO jilS7npUKn5q7w+HFMO8Gn6ofkCaItN0G0X18O+r3jH/zOOn0W8Q9B+mg7G/C2HfkdtK aE5I8E5aaAXw9ZQPjvXY69bP0o7DNKDgonkSWIsPKnvh0PHKA/nhMzHKqxoFyBJyLRR3 Bm5TVZ6Qe2mABRM8loKoQKNfpgk4r2K9VlTBM63ozHGLyTPMzdli3bZrqpNZHvEYGq2X /2OsELEPdow3UfTNrsSeAwz4Rmf0D/UH76P3LVzvC6ltT7yjR6gdqx5O4LdxpqRuVxox bwzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=cn83vrYcwSmTs+Yn4rucxqISZ4LQ1+HVmZDCs1zjaq8=; b=kK14em9ctFAAXL6QsYIso27rmShXCefMSiTjodOgVOvbh4EPUTRnBETeDnnUi7ilZ+ 6Yi/OFr8q+gq/nznWvJthHBVnr72EeFj97TYPub+qeTwZ5NYvH30Dxqmz+GyTiV5R7hW /84B2h+3XgNo4E5CnupKzxsvf+FI4/sfs7HxC+OE/83AOrONje0bvTShDRkIezfmV8JW Lzw/iBIiePVdYJDAGpyvN7Tbdc17kgUOKPVI7GYdVyINsgDH1nGQBpb+UYdPjl4ea8us s6nUr6i7G+eTX3lSyFli24Ub+VU6knNGFdKLZSjmZHijLN+tjzLCUY/Z3tmmzDy+czIx GF1Q== X-Gm-Message-State: AOAM532Dgoqam9Zr2e3P61lKCHnQxCYQ/6Sd3bCGSkB1/Bo2UVyjiSz7 YoFB90dJSJhKmWfBZCzZN88q6nD2VR8= X-Google-Smtp-Source: ABdhPJy5kMFRT7dwYpKT1AQt4KvNtNwrIGGsgVA9eNpeZvEH0ISiCNP95UUKGf8fVYmTVhPU+qWSgw== X-Received: by 2002:a05:6214:5284:b0:444:10c8:ee59 with SMTP id kj4-20020a056214528400b0044410c8ee59mr19144221qvb.68.1652204794666; Tue, 10 May 2022 10:46:34 -0700 (PDT) Received: from localhost.localdomain (104-1-92-200.lightspeed.sntcca.sbcglobal.net. [104.1.92.200]) by smtp.gmail.com with ESMTPSA id h191-20020a379ec8000000b0069fc347ef5dsm8719280qke.74.2022.05.10.10.46.33 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 May 2022 10:46:34 -0700 (PDT) From: =?utf-8?q?Carlo_Marcelo_Arenas_Bel=C3=B3n?= To: git@vger.kernel.org Cc: gitster@pobox.com, bagasdotme@gmail.com, phillip.wood123@gmail.com, Johannes.Schindelin@gmx.de, =?utf-8?q?Carlo_Marcelo_Arenas_Bel=C3=B3n?= Subject: [PATCH v4 3/3] t0034: add negative tests and allow git init to mostly work under sudo Date: Tue, 10 May 2022 10:46:16 -0700 Message-Id: <20220510174616.18629-4-carenas@gmail.com> X-Mailer: git-send-email 2.36.1.371.g0fb0ef0c8d In-Reply-To: <20220510174616.18629-1-carenas@gmail.com> References: <20220507163508.78459-1-carenas@gmail.com> <20220510174616.18629-1-carenas@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add a support library that provides one function that can be used to run a "scriplet" of commands through sudo and that helps invoking sudo in the slightly awkward way that is required to ensure it doesn't block the call (if shell was allowed as tested in the prerequisite) and it doesn't run the command through a different shell than the one we intended. Add additional negative tests as suggested by Junio and that use a new workspace that is owned by root. Note that the specific test that documents that after the previous changes, it is no longer possible for root (if obtained through sudo) to NOT add an exception or NOT need a "workaround" to be able to run git commands in a repository owned by thyself, is marked as a regression and is expected to be fixed with a future change, which hasn't been provided yet and that is not part of this series. As described in the comments from the test itself, at least one of the documented workarounds could fail if sudo doesn't allow root to call sudo or if root is not in sudoers, but that is very unlikely, and more importantly actually not what is provided by the currently supported sudo configuration this test already relies on and therefore adding a way to validate it works in the prerequisite has been punted for now. Helped-by: Junio C Hamano Helped-by: Phillip Wood Signed-off-by: Carlo Marcelo Arenas Belón --- t/lib-sudo.sh | 12 +++++++ t/t0034-root-safe-directory.sh | 58 ++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 t/lib-sudo.sh diff --git a/t/lib-sudo.sh b/t/lib-sudo.sh new file mode 100644 index 0000000000..d8a88fb9db --- /dev/null +++ b/t/lib-sudo.sh @@ -0,0 +1,12 @@ +# Helpers for running git commands under sudo. + +# Runs a scriplet passed through stdin under sudo. +run_with_sudo () { + local ret + local RUN="$TEST_DIRECTORY/$$.sh" + write_script "$RUN" "$TEST_SHELL_PATH" + sudo "$TEST_SHELL_PATH" -c "\"$RUN\"" + ret=$? + rm -f "$RUN" + return $ret +} diff --git a/t/t0034-root-safe-directory.sh b/t/t0034-root-safe-directory.sh index ecd9dca6b3..54f687d787 100755 --- a/t/t0034-root-safe-directory.sh +++ b/t/t0034-root-safe-directory.sh @@ -3,6 +3,7 @@ test_description='verify safe.directory checks while running as root' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-sudo.sh if [ "$GIT_TEST_ALLOW_SUDO" != "YES" ] then @@ -10,6 +11,12 @@ then test_done fi +if ! test_have_prereq NOT_ROOT +then + skip_all="These tests do not support running as root" + test_done +fi + test_lazy_prereq SUDO ' sudo -n id -u >u && id -u root >r && @@ -19,6 +26,12 @@ test_lazy_prereq SUDO ' test_cmp u r ' +if ! test_have_prereq SUDO +then + skip_all="Your sudo/system configuration is either too strict or unsupported" + test_done +fi + test_expect_success SUDO 'setup' ' sudo rm -rf root && mkdir -p root/r && @@ -37,6 +50,51 @@ test_expect_success SUDO 'sudo git status as original owner' ' ) ' +test_expect_success SUDO 'setup root owned repository' ' + sudo mkdir -p root/p && + sudo git init root/p +' + +test_expect_success 'cannot access if owned by root' ' + ( + cd root/p && + test_must_fail git status + ) +' + +test_expect_failure SUDO 'can access with sudo if root' ' + ( + cd root/p && + sudo git status + ) +' + +test_expect_success SUDO 'can access with sudo using a workaround' ' + # run sudo twice; would fail if root is not in sudoers + ( + cd root/p && + sudo sudo git status + ) && + # provide explicit GIT_DIR + ( + cd root/p && + run_with_sudo <<-END + GIT_DIR=.git && + GIT_WORK_TREE=. && + export GIT_DIR GIT_WORK_TREE && + git status + END + ) && + # discard SUDO_UID + ( + cd root/p && + run_with_sudo <<-END + unset SUDO_UID && + git status + END + ) +' + # this MUST be always the last test test_expect_success SUDO 'cleanup' ' sudo rm -rf root