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

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ý

On Mon, May 20, 2024 at 10:21:23AM GMT, Ondrej Pohorelsky wrote:

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

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

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.

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

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.