git-daemon doesn't work as expected in v2.45.1 and friends

git-daemon doesn't work as expected in v2.45.1 and friends

[Ondrej Pohorelsky]

Hi,

during testing the newest security releases in RHEL and in Fedora, we
have encountered broken git-daemon behavior. In a nutshell, git client
refuses to clone locally hosted repositories because of detected
dubious ownership.

I'll paste part of the Fedora report [0] here:

```
Upon clone, git-daemon logs the following:

    [1482] Connection from ::1:45090
    [1482] Extended attribute "host": localhost
    [1482] Extended attribute "protocol": version=2
    [1482] Request upload-pack for '/test.git'
    fatal: detected dubious ownership in repository at '/var/lib/git/test.git'
    To add an exception for this directory, call:
            git config --global --add safe.directory /var/lib/git/test.git


Reproducible: Always

Steps to Reproduce:
1. Create repository under /var/lib/git/test.git
2. Ensure git.socket systemd unit is started
3. Run git clone git://localhost/test.git
Actual Results:
git server refuses to read /var/lib/git/site.git because it detects dubious
ownership
```

Is there a way to make git-daemon hosted repositories safe to clone,
without specifying safe.directory in git config? AFAIK this is widely
used feature of Git not only by the end users, but also quite a lot of
tests rely on it.

[0]https://bugzilla.redhat.com/show_bug.cgi?id=2281530

Cheers,
Ondřej Pohořelský

Re: git-daemon doesn't work as expected in v2.45.1 and friends

[Konstantin Ryabitsev]

On Mon, May 20, 2024 at 10:21:23AM GMT, Ondrej Pohorelsky wrote:
> Is there a way to make git-daemon hosted repositories safe to clone,
> without specifying safe.directory in git config? AFAIK this is widely
> used feature of Git not only by the end users, but also quite a lot of
> tests rely on it.

I would say more -- this is very broken and needs to be rolled back. Running
git-daemon as a different user is the recommended setup for read-only
deployments.

-K

Re: git-daemon doesn't work as expected in v2.45.1 and friends

[Junio C Hamano]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> On Mon, May 20, 2024 at 10:21:23AM GMT, Ondrej Pohorelsky wrote:
>> Is there a way to make git-daemon hosted repositories safe to clone,
>> without specifying safe.directory in git config? AFAIK this is widely
>> used feature of Git not only by the end users, but also quite a lot of
>> tests rely on it.
>
> I would say more -- this is very broken and needs to be rolled back. Running
> git-daemon as a different user is the recommended setup for read-only
> deployments.

Is this from f4aa8c8b (fetch/clone: detect dubious ownership of
local repositories, 2024-04-10) where the commit sprinkled a
"protection" meant only for "local clone" to more generic code paths
that are also used by "git daemon" serving a remote clients?  It may
be that the commit was a bit too aggressive and had a blast radius
that is much larger than intended?

I think that 1204e1a8 (builtin/clone: refuse local clones of unsafe
repositories, 2024-04-15), which is a very pointed fix to ensure we
do not "hardlink copy" local repository owned by others for
security, was a good use of die_upon_dubious_ownership() call,
though.

Reverting f4aa8c8b may not be easy to do mechanically, as it
introduces the die_upon_dubious_ownership(), but 1204e1a8 uses an
identical copy of the same function introduced by 8c9c051b (setup.c:
introduce `die_upon_dubious_ownership()`, 2024-04-15), and reverting
f4aa8c8b mechanically out of the merged result in v2.45.1 would
likely to remove the function that is still in use, which would need
to be retained.

Re: git-daemon doesn't work as expected in v2.45.1 and friends

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Reverting f4aa8c8b may not be easy to do mechanically, as it
> introduces the die_upon_dubious_ownership(), but 1204e1a8 uses an
> identical copy of the same function introduced by 8c9c051b (setup.c:
> introduce `die_upon_dubious_ownership()`, 2024-04-15), and reverting
> f4aa8c8b mechanically out of the merged result in v2.45.1 would
> likely to remove the function that is still in use, which would need
> to be retained.

Well the result can be seen at

    https://lore.kernel.org/git/20240521195659.870714-1-gitster@pobox.com/

but I am inclined to say that its [12/12] (partial reversion of
f4aa8c8b) is highly questionable, after thinking about it a bit
more.  It is true that you can run git-daemon as 'nobody', let it
peek into repositories owned by real users, feeling safe that
'nobody' would not be able to harm these repositories at all.  

But unless this is a tightly controlled hosting environment where no
repository owned by "real users" have malicious hooks and config
files, a "real user" could attack "nobody", and the safe.directory
mechanism that is over-agressive in denying things may alleviate the
problem.  At places like k.org where the repository data including
its hooks and configuration files are trusted, setting up the
configuration safe.directory in the ~nobody/.gitconfig to cover the
real user repositories would probably be a simple enough workaround.