git-daemon --inetd

Some people have asked why we haven't enabled git-daemon on kernel.org.
The reason is that git-daemon --inetd is broken; there is a patch in
the current repository which claims to fix it, but there isn't a release version yet.

On that note, though, it would be very nice if there was a way to run git-daemon in a chroot, or otherwise restrict it to a specific subhierarchy of the filesystem.

-hpa

Hmm.. That should work fine. You can already just run it that way by just wrapping it in "chroot", but if you don't want that for some reason, how about a patch like this?

It compiles. That's just about all I can say about it.

Linus

---

@@ -1,11 +1,19 @@
+/* setresuid/setresgid. GNU? Who are you kidding, glibc? */
+#define _GNU_SOURCE
+
 #include "cache.h"
 #include "pkt-line.h"
 #include <signal.h>
 #include <sys/wait.h>
 #include <sys/socket.h>
 #include <sys/time.h>
+#include <sys/types.h>
 #include <netdb.h>
 #include <netinet/in.h>
+#include <unistd.h>
+
+#include <pwd.h>
+#include <grp.h>
 
 static const char daemon_usage[] = "git-daemon [--inetd | --port=n]";
 

@@ -326,10 +334,50 @@ static int serve(int port)
 	}
 }
 
+/*
+ * This will drop our effective uid/gid to the real one,
+ * even if no "--user=" or "--group=" arguments were used.
+ *
+ * So it's not a no-op even without that.
+ */
+static void set_user_group(const char *user, const char *group)
+{
+	uid_t uid = getuid();
+	gid_t gid = getgid();
+	char *end;
+
+	if (user) {
+		struct passwd *pw = getpwnam(user);
+		if (pw) {
+			uid = pw->pw_uid;
+			gid = pw->pw_gid;
+		} else {
+			uid = strtol(user, &end, 10);
+			if (*end || end == user)
+				die("invalid user name '%s'", user);
+		}
+	}
+	if (group) {
+		struct group *grp = getgrnam(group);
+		if (grp) {
+			gid = grp->gr_gid;
+		} else {
+			gid = strtol(group, &end, 10);
+			if (*end || end == group)
+				die("invalid group name '%s'", group);
+		}
+	}
+	if (!uid || !gid)
+		die("I refuse to run as root");
+	if (setresgid(gid, gid, gid) || setresuid(uid, uid, uid))
+		die("I can't run as %d:%d", uid, gid);
+}
+
 int main(int argc, char **argv)
 {
 	int port = DEFAULT_GIT_PORT;
 	int inetd_mode = 0;
+	const char *user = NULL, *group = NULL;
 	int i;
 
 	for (i = 1; i < argc; i++) {

@@ -350,9 +398,32 @@ int main(int argc, char **argv)
 			continue;
 		}
 
+		if (!strncmp(arg, "--chroot=", 9)) {
+			if (chroot(arg+9) < 0)
+				die("unable to chroot to '%s': %s", arg+9, strerror(errno));
+			if (chdir("/") < 0)
+				die("unable to chdir to new root");
+
+			user = user ? user : "nobody";
+			group = group ? group : "nobody";
+			continue;
+		}
+
+		if (!strncmp(arg, "--user=", 7)) {
+			user = arg+7;
+			continue;
+		}
+
+		if (!strncmp(arg, "--group=", 8)) {
+			group = arg + 8;
+			continue;
+		}
+
 		usage(daemon_usage);
 	}
 
+	set_user_group(user, group);
+
 	if (inetd_mode) {
 		fclose(stderr); //FIXME: workaround
 		return execute();

Wrapping it in chroot() would mean having enough things in the chroot environment to support starting up programs, which is ugly. I'll test the patch when I get a chance.

-hpa

I'd appreciate a forward of the patch with your acked by if you find it acceptable for kernel.org use.

I'd apply Linus' patch myself and push it out in the proposed updates branch, if you prefer.

Fair enough. This still requires "git-upload-pack" and any libraries that requires, of course. You might want to do a statically linked version or something.

Hmm. A quick check seems to say that git-upload-pack only does libz and libc. And the SHA1 stuff, of course, but that you can always get static by using the git built-in mozilla versions.

Linus

Could git-upload-pack be integrated into the git-daemon binary, so that the exec isn't needed?

-hpa

In the longer term, I'd like to extend git-daemon-export-ok to mean:

- the presense of empty file means any supported operation not just git-upload-pack is allowed for backward compatibility.

- otherwise list of allowed git-* server side program.

and be able to extend daemon.c:execute() function to check and execute server side programs other than git-upload-pack. So the answer from me is a mild no.

Hmm.. Looking a bit closer, that's not really very helpful.

git-upload-pack ends up fork/exec'ing git-rev-list too, to generate the list of commits. And then it ends up finishing off with "git-pack-objects".

Yes, we could make it all be one huge thing, and try to have lots of functional interfaces. But that would be really quite ugly and big changes (they'd have to be asynchronous and give incremental results etc etc).

Maybe a post-1.0 effort. For now, I think you need to have a minimal execution environment inside the chroot. Not a lot: git-upload-pack, git-rev-list and git-pack-objects.

Now, it turns out that git-rev-list really is the most complex of them all, largely due to --merge-order. If you define NO_OPENSSL, you'll get a much simpler execution environment.

Btw, I think --merge-order was cool, but its weaker cousin --topo-order is what is actually _used_. Maybe we should deprecate --merge-order? Right now the only real user is git-archimport, and I think that one too really only wants topo-order too. For example, right now I think git-archimport won't actually work if you build without openssl.

Jon? Martin?

Linus

I've used merge-order because it made sense... in that it gave me the same history and order that walking the arch history would. Most of the time, anyway, and decided that I'd rather trust GIT over Arch on merging strategies...

If topo-order is sane, then I'll just change that. Let me run a couple of tests with it first ;)

WRT SSL... archimport doesn't care a bit about SSL, and if it does, it's a bug. It calls tla to perform all the "fetch stuff from the Arch repo operations", because I was too lazy to implement native support for reading the (trivial) repo format over all the transports. And with Arch, access to the remote repos is _fast_ (unlike cvs).

cheers,

martin

Sorry for the long delay in replying - I need to fix my mail filter so that I do see GIT mail that is actually directed to me!

Is the concern with --merge-order the complexity of the logic (and hence size of object), the intrusiveness into rev-list.c or the fact that it uses the OPEN_SSL?

I believe I can rework the merge-order algorithm to avoid the need for bignum support for OPEN_SSL, though I am not going to get a chance to do that until December at the earliest.

Much as I hate to see my baby being orphaned, how about we deprecate --merge-order for now and arrange things so that users have to explicitly enable it if they want it. If enough people complain about that, then I can produce a bignum-free version in the December timeframe?

Regards,

jon.

Some of all. But mostly just the basic fact being that right now, nobody can really use --merge-order anyway, because if somebody compiles without OPEN_SSL, it just won't be there. So it's _practically_ useless.

Linus

Later exchanges between you and HPA appeared to me that we would need a chroot environment which has "enough stuff" and that this patch may not help him very much. Am I mistaken?

If not, then...

Well, building a chroot environment which supports execing is a bit of a pain, but it's fully doable. mount --bind especially makes that quite feasible. It's just more work.

Yes, resolve the username first.

-hpa

No, I think that's correct.

Right you are. Much better.

Well, the symbolic names are much nicer and more readable. So it would be better to do the uid/gid translation early, and change the "chroot" thing to be done after all that.

It gets a bit messy.. Easy enough to just save a "const char *new_root", but then you have to split up the "set_user_group()" to be two functions, around the actual chroot(), since the chroot needs to be done while we're still root.

I think we can drop the patch for now. It doesn't buy much.

Linus

Actually, initgroups() and setgroups(), and setgid() for that matter, can be done before the chroot(). The only thing that needs to remain until the end is setuid().

At one time I played around in tftp-hpa with trying to get Linux to keep only CAP_SYS_CHROOT around, but I think I gave up on it. The way Linux capabilities play with the rest of the permission system isn't very useful :(

-hpa