From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 0/2] Revert defense-in-depth patches breaking Git LFS
Date: Tue, 14 May 2024 21:07:28 +0200 (CEST) [thread overview]
Message-ID: <0f7597aa-6697-9a70-0405-3dcbb9649d68@gmx.de> (raw)
In-Reply-To: <20240514181641.150112-1-sandals@crustytoothpaste.net>
brian,
On Tue, 14 May 2024, brian m. carlson wrote:
> 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.
> 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
next prev parent reply other threads:[~2024-05-14 19:07 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-14 18:16 [PATCH 0/2] Revert defense-in-depth patches breaking Git LFS brian m. carlson
2024-05-14 18:16 ` [PATCH 1/2] Revert "clone: prevent hooks from running during a clone" brian m. carlson
2024-05-14 18:16 ` [PATCH 2/2] Revert "core.hooksPath: add some protection while cloning" brian m. carlson
2024-05-14 19:07 ` Johannes Schindelin [this message]
2024-05-14 19:41 ` [PATCH 0/2] Revert defense-in-depth patches breaking Git LFS brian m. carlson
2024-05-22 9:49 ` Joey Hess
2024-05-27 19:35 ` Johannes Schindelin
2024-05-28 2:13 ` Joey Hess
[not found] ` <ZlZSZ1-0F2DEp9yV@tapette.crustytoothpaste.net>
2024-05-28 23:46 ` Junio C Hamano
2024-05-29 8:54 ` Jeff King
2024-05-29 12:17 ` Johannes Schindelin
2024-05-29 16:17 ` Junio C Hamano
2024-05-30 8:17 ` Jeff King
2024-05-24 17:37 ` Joey Hess
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=0f7597aa-6697-9a70-0405-3dcbb9649d68@gmx.de \
--to=johannes.schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sandals@crustytoothpaste.net \
/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.
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).