Re: [PATCH 0/2] Revert defense-in-depth patches breaking Git LFS

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 14, 2024, 19:07 UTC
Message-ID: <0f7597aa-6697-9a70-0405-3dcbb9649d68@gmx.de>
In-Reply-To: <20240514181641.150112-1-sandals@crustytoothpaste.net>

brian,

On Tue, 14 May 2024, brian m. carlson wrote:

Show 28 quoted lines

> The recent defense-in-depth patches to restrict hooks while cloning
> broke Git LFS because it installs necessary hooks when it is invoked by
> Git's smudge filter.  This means that currently, anyone with Git LFS
> installed who attempts to clone a repository with at least one LFS file
> will see a message like the following (fictitious example):
>
> ----
> $ git clone https://github.com/octocat/xyzzy.git
> Cloning into 'pull-bug'...
> remote: Enumerating objects: 1275, done.
> remote: Counting objects: 100% (343/343), done.
> remote: Compressing objects: 100% (136/136), done.
> remote: Total 1275 (delta 221), reused 327 (delta 206), pack-reused 932
> Receiving objects: 100% (1275/1275), 290.78 KiB | 2.88 MiB/s, done.
> Resolving deltas: 100% (226/226), done.
> Filtering content: 100% (504/504), 1.86 KiB | 0 bytes/s, done.
> fatal: active `post-checkout` hook found during `git clone`:
>         /home/octocat/xyzzy/.git/hooks/post-checkout
> For security reasons, this is disallowed by default.
> If this is intentional and the hook should actually be run, please
> run the command again with `GIT_CLONE_PROTECTION_ACTIVE=false`
> warning: Clone succeeded, but checkout failed.
> You can inspect what was checked out with 'git status'
> and retry with 'git restore --source=HEAD :/'
> ----
>
> This causes most CI systems to be broken in such a case, as well as a
> confusing message for the user.

When using `actions/checkout` in GitHub workflows, nothing is broken because `actions/checkout` uses a fetch + checkout (to allow for things like sparse checkout), which obviously lacks the clone protections because it is not a clone.

Show 5 quoted lines

> It's not really possible to avoid the need to install the hooks at this
> location because the post-checkout hook must be ready during the
> checkout that's part of the clone in order to properly adjust
> permissions on files.  Thus, we'll need to revert the changes to
> restrict hooks while cloning, which this series does.

Dropping protections is in general a bad idea. While previously, hackers wishing to exploit weaknesses in Git might have been unaware of the particular attack vector we want to prevent with these defense-in-depth measurements, we now must assume that they are fully aware. Reverting those protections can be seen as a very public invitation to search for ways to exploit the now re-introduced avenues to craft Remote Code Execution attacks.

I have pointed out several times that there are alternatives while discussing this under embargo, even sent them to the git-security list before the embargo was lifted, and have not received any reply. One proposal was to introduce a way to cross-check the SHA-256 of hooks that _were_ written during a clone operation against a list of known-good ones. Another alternative was to special-case Git LFS by matching the hooks' contents against a regular expression that matches Git LFS' current hooks'.

Both alternatives demonstrate that we are far from _needing_ to revert the changes that were designed to prevent future vulnerabilities from immediately becoming critical Remote Code Executions. It might be an easier way to address the Git LFS breakage, but "easy" does not equal "right".

I did not yet get around to sending these patches to the Git mailing list solely because I am still busy with a lot of follow-up work of the embargoed release. It was an unwelcome surprise to see this here patch series in my inbox and still no reply to the patches I had sent to the git-security list for comments.

I am still busy wrapping up follow-up work and won't be able to participate in this here mail thread meaningfully for the next hours. I do want to invite you to think about alternative ways to address the Git LFS issues, alternatives that do not re-open weaknesses we had hoped to address for good.

I do want to extend the invitation to work with me on that, for example by reviewing those patches I sent to the git-security mailing list (or even to send them to the Git mailing list for public review on my behalf, that would be helpful).

Ciao, Johannes