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:
signature.asc
Description: PGP signature
