Re: what are the chances of a 'pre-upload' hook?

On Sat, Nov 26, 2011 at 11:51:29PM -0800, Junio C Hamano wrote:

I agree. If we can avoid adding any feature which is even potentially dangerous, it's good to explore alternatives. At the same time, the subject of remote fetch hooks has come up before, and I don't think it's going away. It would be nice to reach more resolution this time.

I also have mild concerns about the current receive-pack hooks. The thought so far has been "well, you're less likely push into a bad person's repository". Which is probably true. But it seems like a flaky thing to base security on, especially in multi-user environments with shared repositories.

Right, that was the "socket" solution I mentioned earlier. The problem is that it's a pain to set up. If you really want to have a daemon running and handling git tasks, then you might as well just have the whole upload-pack/receive-pack side accessible over a socket (e.g., just run git-daemon if you don't care about authentication, and git-over-https if you do).

If you really care about ssh authentication and keeping track of individual unix users, then one possibility is to run git-daemon listening on a unix socket, use SO_PEERCRED to get the user on the other end, and then process everything as a single git user.

For example, imagine Bob and Alice are in the "research" group, and use a shared central repo. Bob wants to push:

  1. Bob runs "git push" on his workstation. Git on the workstation
     connects to "ssh bob@server git-receive-pack $repo".

  2. git-receive-pack on the server runs as Bob. It notices that
     $repo/daemon-socket exists, and runs in "shim" mode, connecting to
     the socket and proxying data over the socket.

  3. git-daemon, running as the "research" user, listens on the other
     end of the socket (and potentially sockets for other repos). It
     sees the connection and serves it. The daemon already knows which
     repo to serve because it knows which socket Bob connected to. And
     it knows it's OK for Bob to access it, because he was able to get
     to the socket via filesystem permissions.

     The daemon can use getsockopt(SO_PEERCRED) to find out that Bob is
     the user, and probably set an environment variable. It can freely
     run any hooks, because the hooks are owned by the "research" user,
     and that's the user the daemon runs as. Hooks can trust the
     environment from the daemon, and can use Bob's identity for further
     authorization checks if they want to. IOW, we are roughly at the
     same place you would get by connecting over git-over-https.

This naturally extends to "Bob fetches" or "Bob runs his push from the server". As an added bonus over the usual shared-central-repo model, malicious users can't just ignore hooks (in the usual shared-repo model, everybody can write directly to the object db and refs, so hooks are somewhat advisory).

I suspect people who really care about this use case are already running git-over-https, possibly with something like gerrit doing some policy enforcement. I also suspect there are a lot of corporate environments where they just have a big shared repo, set core.sharedRepository, and don't worry too much about users hurting each other (in most dev environments I've seen, people are perfectly willing to fetch and run make without looking at the diff, so it's sort of a moot point).

People in the latter half would be one of the potential audiences for the "screw it, just run hooks from the remote repo" flag.

That's not necessarily safe. Think about a site like kernel.org. The administrator is the one running the daemon, on behalf of all of the users. But my impression is that pre-August, developers had shell access to their own repos and could write their own hook files. So if git-daemon runs hooks, then any repo owner could run arbitrary code as the git-daemon, including killing the running daemon and running their own trojan.

So you really need some way for the admin to say "it's OK, hooks are locked down and it's safe for them to run". Which again, I was thinking of as an environment variable.

However, I think for the daemon case, the other alternative I mentioned earlier is a bit more elegant: an environment variable which doesn't say "respect the hooks in the repo", but rather "run this _particular_ program as the pre-upload-pack hook". And then the admin doesn't have to care what's in the repos. The daemon runs some particular program every time a fetch happens.

It's tempting to say "can't they just replace git-upload-pack with a shim that does what they want and then runs upload-pack?"[1]. But I think the answer is no, for the same reason we have hooks at all: they might want to operate under a lock, or use some data produced by upload-pack, or make a decision that blocks the fetch.

[1] Actually, I think Sitaram's case may not be a pre-upload-pack hook,
    but rather the simple "run before upload-pack" case. Because a
    pre-upload-pack would probably feed the set of requested objects to
    the hook. But he will want to fetch from the upstream before even
    advertising any refs to the client.

Right. This is a failure of the training course and/or the admin who wrote the default profile, and is exactly the thing that would be mentioned in the red flags in the docs. I agree there is a risk of bone-headed admins, though.

This, btw, is more or less the case I am concerned about _now_ with receive-pack. With current git, if any of those general repositories is a shared central repo, then project A is similarly vulnerable to anyone who can write the hook.

-Peff