NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.59k stars 13.74k forks source link

sourcehut.git: permission issues for `fcgiwrap` after upgrading to 24.05 #317865

Open nessdoor opened 3 months ago

nessdoor commented 3 months ago

Describe the bug

Git introduced "safe directories" as an additional security mechanism that prevents users from operating on other users' repositories, even if filesystem permissions allow so, in order to avoid accidental execution of untrusted hooks. This protection mechanism can be deactivated for selected repositories by specifying their path under the safe.directories setting.

The default Sourcehut installation ships with a CGI responder in charge of satisfying HTTP requests operating on the hosted repositories, including cloning. This service runs under the unprivileged user gitsrht-fcgiwrap.

Since gitsrht-fcgiwrap.service runs under a user that is different from the one owning the targeted repo (the core gitsrht user), the new Git security mechanism prevents any HTTP-based interaction with any repository.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Perform a clone of a self-hosted Sourcehut repository via the HTTP API (e.g. by issuing git clone https://<your-domain>/~<username>/<repo>")

Expected behavior

The operation completes successfully.

Actual behavior

On the git clone invocation:

Cloning into '<repo>'...
fatal: unable to access 'https://<your-domain>/~<username>/<repo>/': The requested URL returned error: 500

On the gitsrht-fcgiwrap.service logs:

 6月 07 02:02:08 fcgiwrap[229922]: fatal: detected dubious ownership in repository at '/var/lib/sourcehut/gitsrht/repos/~<username>/<repo>'
 6月 07 02:02:08 fcgiwrap[229922]: To add an exception for this directory, call:
 6月 07 02:02:08 fcgiwrap[229922]:         git config --global --add safe.directory '/var/lib/sourcehut/gitsrht/repos/~<username>/<repo>'

Workaround

We can disable the security mechanism globally by setting programs.git.config.safe.directory to the "*" string, voiding any added protection. This could be insecure, depending on the context.

Notify maintainers

@christoph-heiss @tomberek How do you think we should tackle this? Unfortunately the safe.directory setting does not seem to expand wildcards, which could have allowed us to whitelist the entire sourcehut.settings."git.sr.ht".repos directory. I think we have three options:

  1. we disable the protection mechanism whenever Sourcehut is running on the machine;
  2. we run gitsrht-fcgiwrap.service under the gitsrht user;
  3. we modify the environment of gitsrht-fcgiwrap.service so that it either sees an ad-hoc /etc/gitconfig (/etc is bind-mounted inside its chroot, so it would be a simple redirection), or we try to pass the relevant config option via environment variables.

Personally, I think the third option would be the most secure. Also, am I the only one experiencing this behavior?

Metadata

 - system: `"aarch64-linux"`
 - host os: `Linux 6.6.32, NixOS, 24.05 (Uakari), 24.05.20240530.0f1a94c`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.2`
 - nixpkgs: `/nix/store/v0sia9lh4vl46dsx8dcw1q5vpj1gzndm-source`

Add a :+1: reaction to issues you find important.

res0Nanz commented 3 months ago

The breaking change seems to be git/git@f4aa8c8bb11dae6e769cd930565173808cbb69c8. I just ran into the same issue trying to configure git-http-backend on an nginx server. I ended up starting git-fcgiwrap.service with permission git:git, which is the same as the second option you provide.

christoph-heiss commented 3 months ago

3. we modify the environment of gitsrht-fcgiwrap.service so that it either sees an ad-hoc /etc/gitconfig (/etc is bind-mounted inside its chroot, so it would be a simple redirection), or we try to pass the relevant config option via environment variables.

It sounds relatively simple. We should be able to do that by adding a additional BindReadOnlyPaths entry, creating a mapping for /etc/gitconfig to a custom one generated by us, right?

If it works as I imagine, I'll try to reproduce it and whip up a patch for this.

nessdoor commented 3 months ago

I think we might even be able to achieve the same result through environment variables alone (see the "Protected configuration" paragraph of the git-config manual), but I'm not sure how fcgiwrap manages to pass them around to its scripts.

And sorry if I didn't provide a PR immediately, @christoph-heiss, and I know I still have to review your other modification, but I have been caught into some other business that has kept me away from my Nixpkgs duties for the past couple of weeks. Sorry...