From: Joey Hess <id@joeyh•name>
To: Junio C Hamano <gitster@pobox•com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx•de>, git@vger•kernel.org
Subject: Re: [PATCH 00/12] Fix various overly aggressive protections in 2.45.1 and friends
Date: Thu, 23 May 2024 12:31:37 -0400 [thread overview]
Message-ID: <Zk9vafYPijqyWpXv@kitenet.net> (raw)
In-Reply-To: <xmqqv835xekc.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 3276 bytes --]
Junio C Hamano wrote:
> - The extra check seems to have meant to target the symbolic links
> that point at objects, refs, config, and anything _we_ care
> about, as opposed to random garbage (from _our_ point of view)
> files third-parties throw into .git/ directory. Would it have
> made a better trade-off if we tried to make the check more
> precise, only complaining about the things we care about (in
> other words, what _we_ use)
I wondered about that possibility too. But it's not at all clear to
me how a symlink to .git/objects/foo risks any more security problem
to git than one to .git/annex/whatever, or indeed to /home/linus/.bashrc.
Git clearly has to get the security right of handling working tree files
that are symlinks.
The security hole that triggered this defense in depth, CVE-2024-32021,
involved an attacker with write access to .git/objects/ making a symlink
in there while another repo was cloning it. So it involved symlinks
inside a remote .git/objects/, which is very different than symlinks
into .git/objects/.
While it's understandable that dealing with such a symlink related
security hole may make one want to throw out the baby with the
bathwater, this fsck check is more like you've kept the bathwater and
only thrown out the baby. ;-)
> - In any case, if it is merely warnings, not errors, these checks
> can be configured out. Wouldn't that be an escape-hatch enough?
The issue with that is, as we've experienced with Gitlab, git hosts that
choose to set receive.fsckObjects will prevent pushes of git-annex
repositories, and there's probably no way for a user to configure it
out. So every major git host that does it has to be approached to
configure it out, and some fraction probably won't. Which will be a
major impact and ongoing concern for git-annex users[1], all for
something that certainly adds no security to a bare repository on such a
host.
> I am not sure which one is more practical between ripping
> everything out and demoting these new fsck error types with
> FSCK_WARN to FSCK_IGNORE.
It could indeed be beneficial to have some kind of symlink check that is
at FSCK_IGNORE by default. If someone is receiving a repository from an
untrusted source, and doesn't want to deal with the security risks of
symlinks in the working tree, they could configure it to be an error.
Such a symlink check would probably need to catch more symlinks than
only the ones into .git/ though. Having this available to git users
seems like it could prevent a much larger class of security holes.
As for the symlink length check, I do think it makes sense for fsck to
notice symlinks that are too long to make sense for any OS and so picking
some appropriate value, rather than the local PATH_MAX, could keep that one.
--
see shy jo
[1] I'm particularly concerned about the class of large institutional
users who are managing more data with git-annex than the total size of
all of Github[2]. They have a good reason to be risk averse,
and it could be a major disruption to cross-organizational workflows
and need updates to DOIs etc for them to switch hosting providers.
[2] https://hachyderm.io/@joeyh/112486445240754919
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2024-05-23 16:31 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-21 19:56 [PATCH 00/12] Fix various overly aggressive protections in 2.45.1 and friends Junio C Hamano
2024-05-21 19:56 ` [PATCH 01/12] send-email: drop FakeTerm hack Junio C Hamano
2024-05-22 8:19 ` Dragan Simic
2024-05-21 19:56 ` [PATCH 02/12] send-email: avoid creating more than one Term::ReadLine object Junio C Hamano
2024-05-22 8:15 ` Dragan Simic
2024-05-21 19:56 ` [PATCH 03/12] ci: drop mention of BREW_INSTALL_PACKAGES variable Junio C Hamano
2024-05-21 19:56 ` [PATCH 04/12] ci: avoid bare "gcc" for osx-gcc job Junio C Hamano
2024-05-21 19:56 ` [PATCH 05/12] ci: stop installing "gcc-13" for osx-gcc Junio C Hamano
2024-05-21 19:56 ` [PATCH 06/12] hook: plug a new memory leak Junio C Hamano
2024-05-21 19:56 ` [PATCH 07/12] init: use the correct path of the templates directory again Junio C Hamano
2024-05-21 19:56 ` [PATCH 08/12] Revert "core.hooksPath: add some protection while cloning" Junio C Hamano
2024-05-21 19:56 ` [PATCH 09/12] tests: verify that `clone -c core.hooksPath=/dev/null` works again Junio C Hamano
2024-05-21 22:57 ` Brooke Kuhlmann
2024-05-21 19:56 ` [PATCH 10/12] clone: drop the protections where hooks aren't run Junio C Hamano
2024-05-21 19:56 ` [PATCH 11/12] Revert "Add a helper function to compare file contents" Junio C Hamano
2024-05-21 19:56 ` [PATCH 12/12] Revert "fetch/clone: detect dubious ownership of local repositories" Junio C Hamano
2024-05-21 20:43 ` Junio C Hamano
2024-05-22 7:27 ` Johannes Schindelin
2024-05-22 17:20 ` Junio C Hamano
2024-05-21 20:45 ` [rPATCH 13/12] Merge branch 'jc/fix-aggressive-protection-2.39' Junio C Hamano
2024-05-23 10:36 ` Reviewing merge commits, was " Johannes Schindelin
2024-05-23 14:41 ` Junio C Hamano
2024-05-21 20:45 ` [rPATCH 14/12] Merge branch 'jc/fix-aggressive-protection-2.40' Junio C Hamano
2024-05-21 21:33 ` Junio C Hamano
2024-05-21 21:14 ` [PATCH 00/12] Fix various overly aggressive protections in 2.45.1 and friends Johannes Schindelin
2024-05-21 21:46 ` Junio C Hamano
2024-05-21 22:13 ` Junio C Hamano
2024-05-22 10:01 ` Joey Hess
2024-05-23 5:49 ` Junio C Hamano
2024-05-23 16:31 ` Joey Hess [this message]
2024-05-27 19:51 ` Johannes Schindelin
2024-05-28 2:25 ` Joey Hess
2024-05-28 15:02 ` Phillip Wood
2024-05-28 16:13 ` Junio C Hamano
2024-05-28 17:47 ` Junio C Hamano
2024-05-23 23:32 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zk9vafYPijqyWpXv@kitenet.net \
--to=id@joeyh$(echo .)name \
--cc=Johannes.Schindelin@gmx$(echo .)de \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox