From: Phillip Wood <phillip.wood123@gmail.com>
To: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>, git@vger.kernel.org
Cc: gitster@pobox.com, bagasdotme@gmail.com,
"SZEDER Gábor" <szeder.dev@gmail.com>
Subject: Re: [PATCH v3 1/3] t: document regression git safe.directory when using sudo
Date: Tue, 3 May 2022 15:03:59 +0100 [thread overview]
Message-ID: <9b92b380-1da1-b76d-1eb4-469aba289694@gmail.com> (raw)
In-Reply-To: <20220503065442.95699-2-carenas@gmail.com>
Hi Carlo
On 03/05/2022 07:54, Carlo Marcelo Arenas Belón wrote:
> 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, and special care should be taken when invoking
> commands through sudo, since the environment is otherwise independent
> from what the test framework expects. Indeed `git status` was used
> as a proxy because it doesn't even require commits in the repository
> to work.
>
> 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 and doesn't mess with git execution paths, but
> otherwise additional work will be required to ensure additional
> commands behave as expected and that will be addressed in a later patch.
>
> Most of those characteristics make this test mostly suitable only for
> CI, but it could be executed locally if special care is taken to provide
> for some 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 by doing:
>
> $ IKNOWWHATIAMDOING=YES ./t0034-root-safe-directory.sh
>
> Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> t/t0034-root-safe-directory.sh | 49 ++++++++++++++++++++++++++++++++++
> 1 file changed, 49 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 00000000000..6dac7a05cfd
> --- /dev/null
> +++ b/t/t0034-root-safe-directory.sh
> @@ -0,0 +1,49 @@
> +#!/bin/sh
> +
> +test_description='verify safe.directory checks while running as root'
> +
> +. ./test-lib.sh
> +
> +if [ "$IKNOWWHATIAMDOING" != "YES" ]
> +then
> + skip_all="You must set env var IKNOWWHATIAMDOING=YES in order to run this test"
> + test_done
> +fi
> +
> +# this prerequisite should be added to all the tests, it not only prevents
> +# the test from failing but also warms up any authentication cache sudo
> +# might need to avoid asking for a password
If this is required for all the tests then it would be simpler just to
skip all the tests if it is not satisfied as you do above.
Running "sudo env" shows that it sets $HOME to /root which means that
these tests will pick up /root/.gitconfig if it exists. Normally when
running the tests we set $HOME to $TEST_DIRECTORY so they are run in a
predictable environment. At least anything pointed to by core.hooksPath
or core.fsmontior in that file is expecting to be run as root. I think
it is worth spelling this out explicitly in the commit message
(currently it is a bit vague about what the implications of not having
better integration with the test framework are) and the top of the test
file. Note that t1509 sources test-lib.sh as the root user so does not
have this issue.
> +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
Using git -C <directory> would eliminate a lot of the sub shells in this
file
Best Wishes
Phillip
> + )
> +'
> +
> +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, if used more than once, the next
> +# test should do a full setup again.
> +test_expect_success SUDO 'cleanup' '
> + sudo rm -rf root
> +'
> +
> +test_done
next prev parent reply other threads:[~2022-05-03 14:04 UTC|newest]
Thread overview: 170+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-26 18:31 [RFC PATCH] git-compat-util: avoid failing dir ownership checks if running priviledged Carlo Marcelo Arenas Belón
2022-04-26 19:48 ` Derrick Stolee
2022-04-26 19:56 ` Junio C Hamano
2022-04-26 20:10 ` rsbecker
2022-04-26 20:45 ` Carlo Arenas
2022-04-26 21:10 ` Junio C Hamano
2022-04-26 20:12 ` Carlo Arenas
2022-04-26 20:26 ` Carlo Arenas
2022-04-29 16:16 ` Derrick Stolee
2022-04-27 0:05 ` [PATCH] git-compat-util: avoid failing dir ownership checks if running privileged Carlo Marcelo Arenas Belón
2022-04-27 9:33 ` Phillip Wood
2022-04-27 12:30 ` Phillip Wood
2022-04-27 14:15 ` rsbecker
2022-04-27 15:58 ` Carlo Arenas
2022-04-27 16:14 ` Phillip Wood
2022-04-27 18:54 ` Junio C Hamano
2022-04-27 20:59 ` Carlo Arenas
2022-04-27 21:09 ` rsbecker
2022-04-27 21:25 ` Junio C Hamano
2022-04-28 17:56 ` Phillip Wood
2022-04-27 15:38 ` Carlo Arenas
2022-04-27 15:50 ` rsbecker
2022-04-27 16:19 ` Junio C Hamano
2022-04-27 16:45 ` Carlo Arenas
2022-04-27 17:22 ` Phillip Wood
2022-04-27 17:49 ` rsbecker
2022-04-27 17:54 ` Carlo Arenas
2022-04-27 18:05 ` rsbecker
2022-04-27 18:11 ` Carlo Arenas
2022-04-27 18:16 ` rsbecker
2022-04-27 16:31 ` Phillip Wood
2022-04-27 16:54 ` Carlo Arenas
2022-04-27 17:28 ` Phillip Wood
2022-04-27 17:49 ` Carlo Arenas
2022-04-27 22:26 ` [RFC PATCH v2] " Carlo Marcelo Arenas Belón
2022-04-27 22:33 ` Junio C Hamano
2022-04-28 3:35 ` [PATCH 0/2] fix `sudo make install` regression in maint Carlo Marcelo Arenas Belón
2022-04-28 3:35 ` [PATCH 1/2] Documentation: explain how safe.directory works when running under sudo Carlo Marcelo Arenas Belón
2022-04-28 5:17 ` Junio C Hamano
2022-04-28 5:58 ` Carlo Arenas
2022-04-28 6:41 ` Junio C Hamano
2022-04-28 3:35 ` [PATCH 2/2] t: add tests for safe.directory when running with sudo Carlo Marcelo Arenas Belón
2022-04-28 5:34 ` Junio C Hamano
2022-04-28 4:57 ` [PATCH 0/2] fix `sudo make install` regression in maint Junio C Hamano
2022-04-28 10:58 ` [PATCH v2 0/3] " Carlo Marcelo Arenas Belón
2022-04-28 10:58 ` [PATCH v2 1/3] git-compat-util: avoid failing dir ownership checks if running privileged Carlo Marcelo Arenas Belón
2022-04-28 18:02 ` Phillip Wood
2022-04-28 18:57 ` Carlo Arenas
2022-04-28 10:58 ` [PATCH v2 2/3] Documentation: explain how safe.directory works when running under sudo Carlo Marcelo Arenas Belón
2022-04-30 6:17 ` Bagas Sanjaya
2022-04-30 6:39 ` Junio C Hamano
2022-04-30 14:15 ` Carlo Marcelo Arenas Belón
2022-04-28 10:58 ` [PATCH v2 3/3] t: add tests for safe.directory when running with sudo Carlo Marcelo Arenas Belón
2022-04-28 16:55 ` Junio C Hamano
2022-04-28 18:08 ` Phillip Wood
2022-04-28 18:12 ` Junio C Hamano
2022-05-06 17:50 ` Carlo Arenas
2022-05-06 21:43 ` Junio C Hamano
2022-05-06 22:57 ` Carlo Arenas
2022-05-06 23:55 ` Junio C Hamano
2022-05-07 11:57 ` Carlo Marcelo Arenas Belón
2022-04-28 19:53 ` rsbecker
2022-04-28 20:22 ` Carlo Arenas
2022-04-28 20:43 ` rsbecker
2022-04-28 20:51 ` Junio C Hamano
2022-04-28 20:56 ` Carlo Arenas
2022-04-28 21:55 ` rsbecker
2022-04-28 22:21 ` Junio C Hamano
2022-04-28 22:45 ` rsbecker
2022-04-28 20:46 ` Junio C Hamano
2022-04-28 20:32 ` Junio C Hamano
2022-04-28 20:40 ` rsbecker
2022-04-28 20:48 ` Carlo Arenas
2022-04-28 21:02 ` Carlo Arenas
2022-04-28 21:07 ` Junio C Hamano
2022-04-29 1:24 ` Carlo Marcelo Arenas Belón
2022-04-29 18:50 ` Junio C Hamano
2022-04-29 20:05 ` Carlo Marcelo Arenas Belón
2022-05-02 18:39 ` [RFC PATCH v3 0/3] fix `sudo make install` regression in maint Carlo Marcelo Arenas Belón
2022-05-02 18:39 ` [RFC PATCH v3 1/3] t: document regression git safe.directory when using sudo Carlo Marcelo Arenas Belón
2022-05-02 21:35 ` Junio C Hamano
2022-05-02 23:07 ` Carlo Arenas
2022-05-02 18:39 ` [RFC PATCH v3 2/3] git-compat-util: avoid failing dir ownership checks if running privileged Carlo Marcelo Arenas Belón
2022-05-02 18:39 ` [RFC PATCH v3 3/3] t0034: enhance framework to allow testing more commands under sudo Carlo Marcelo Arenas Belón
2022-05-02 22:10 ` Junio C Hamano
2022-05-03 0:00 ` Carlo Arenas
2022-05-03 6:54 ` [PATCH v3 0/3] fix `sudo make install` regression in maint Carlo Marcelo Arenas Belón
2022-05-03 6:54 ` [PATCH v3 1/3] t: document regression git safe.directory when using sudo Carlo Marcelo Arenas Belón
2022-05-03 14:03 ` Phillip Wood [this message]
2022-05-03 15:56 ` Carlo Marcelo Arenas Belón
2022-05-04 11:15 ` Phillip Wood
2022-05-04 13:02 ` Carlo Arenas
2022-05-04 14:11 ` Phillip Wood
2022-05-05 13:44 ` Johannes Schindelin
2022-05-05 14:34 ` Phillip Wood
2022-05-05 15:50 ` Junio C Hamano
2022-05-05 18:33 ` Junio C Hamano
2022-05-05 19:39 ` Junio C Hamano
2022-05-06 21:03 ` Carlo Arenas
2022-05-09 8:21 ` Phillip Wood
2022-05-09 14:51 ` Carlo Arenas
2022-05-09 15:18 ` Phillip Wood
2022-05-09 16:01 ` Junio C Hamano
2022-05-09 16:21 ` Carlo Arenas
2022-05-06 17:39 ` Carlo Arenas
2022-05-03 6:54 ` [PATCH v3 2/3] git-compat-util: avoid failing dir ownership checks if running privileged Carlo Marcelo Arenas Belón
2022-05-05 14:01 ` Johannes Schindelin
2022-05-05 14:32 ` Phillip Wood
2022-05-06 19:15 ` Carlo Arenas
2022-05-06 20:00 ` Junio C Hamano
2022-05-06 20:22 ` Carlo Arenas
2022-05-06 20:59 ` Junio C Hamano
2022-05-06 21:40 ` Carlo Arenas
2022-05-06 21:07 ` rsbecker
2022-05-05 16:09 ` Junio C Hamano
2022-05-06 20:02 ` Carlo Arenas
2022-05-03 6:54 ` [PATCH v3 3/3] t0034: enhance framework to allow testing more commands under sudo Carlo Marcelo Arenas Belón
2022-05-03 14:12 ` Phillip Wood
2022-05-03 15:27 ` Junio C Hamano
2022-05-06 16:54 ` Carlo Arenas
2022-05-07 16:35 ` [RFC PATCH v4 0/3] fix `sudo make install` regression in maint Carlo Marcelo Arenas Belón
2022-05-07 16:35 ` [RFC PATCH v4 1/3] t: regression git needs safe.directory when using sudo Carlo Marcelo Arenas Belón
2022-05-07 16:35 ` [RFC PATCH v4 2/3] git-compat-util: avoid failing dir ownership checks if running privileged Carlo Marcelo Arenas Belón
2022-05-07 17:34 ` Junio C Hamano
2022-05-07 18:56 ` Carlo Marcelo Arenas Belón
2022-05-09 16:54 ` Junio C Hamano
2022-05-09 17:36 ` rsbecker
2022-05-09 18:48 ` Carlo Arenas
2022-05-09 19:16 ` rsbecker
2022-05-09 19:41 ` Junio C Hamano
2022-05-07 16:35 ` [RFC PATCH v4 3/3] t0034: add negative tests and allow git init to mostly work under sudo Carlo Marcelo Arenas Belón
2022-05-10 14:17 ` [RFC PATCH v4 0/3] fix `sudo make install` regression in maint Phillip Wood
2022-05-10 15:47 ` Carlo Arenas
2022-05-10 17:46 ` [PATCH " Carlo Marcelo Arenas Belón
2022-05-10 17:46 ` [PATCH v4 1/3] t: regression git needs safe.directory when using sudo Carlo Marcelo Arenas Belón
2022-05-10 22:10 ` Junio C Hamano
2022-05-10 23:11 ` Carlo Arenas
2022-05-10 23:44 ` Junio C Hamano
2022-05-11 0:56 ` Carlo Arenas
2022-05-11 1:11 ` Junio C Hamano
2022-05-10 17:46 ` [PATCH v4 2/3] git-compat-util: avoid failing dir ownership checks if running privileged Carlo Marcelo Arenas Belón
2022-05-10 22:57 ` Junio C Hamano
2022-05-11 7:34 ` Carlo Arenas
2022-05-11 14:58 ` Junio C Hamano
2022-05-10 17:46 ` [PATCH v4 3/3] t0034: add negative tests and allow git init to mostly work under sudo Carlo Marcelo Arenas Belón
2022-05-10 23:11 ` Junio C Hamano
2022-05-10 23:25 ` Junio C Hamano
2022-05-11 14:04 ` Carlo Arenas
2022-05-11 15:29 ` Junio C Hamano
2022-05-13 1:00 ` [PATCH v5 0/4] fix `sudo make install` regression in maint Carlo Marcelo Arenas Belón
2022-05-13 1:00 ` [PATCH v5 1/4] t: regression git needs safe.directory when using sudo Carlo Marcelo Arenas Belón
2022-06-03 12:12 ` SZEDER Gábor
2022-05-13 1:00 ` [PATCH v5 2/4] git-compat-util: avoid failing dir ownership checks if running privileged Carlo Marcelo Arenas Belón
2022-06-03 11:05 ` SZEDER Gábor
2022-06-03 16:54 ` Junio C Hamano
2022-06-03 17:34 ` SZEDER Gábor
2022-05-13 1:00 ` [PATCH v5 3/4] t0034: add negative tests and allow git init to mostly work under sudo Carlo Marcelo Arenas Belón
2022-05-13 1:20 ` Junio C Hamano
2022-05-14 14:36 ` Carlo Arenas
2022-05-15 16:54 ` Junio C Hamano
2022-05-15 19:21 ` Carlo Arenas
2022-05-16 5:27 ` Junio C Hamano
2022-05-16 13:07 ` Carlo Marcelo Arenas Belón
2022-05-16 16:25 ` Junio C Hamano
2022-05-13 1:00 ` [PATCH v5 4/4] git-compat-util: allow root to access both SUDO_UID and root owned Carlo Marcelo Arenas Belón
2022-06-15 14:02 ` Johannes Schindelin
2022-06-17 14:26 ` Carlo Arenas
2022-06-17 16:00 ` Junio C Hamano
2022-06-17 20:23 ` [PATCH v6] " Carlo Marcelo Arenas Belón
2022-06-17 21:02 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9b92b380-1da1-b76d-1eb4-469aba289694@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=bagasdotme@gmail.com \
--cc=carenas@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=phillip.wood@dunelm.org.uk \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).