threads / patch / 5331

patchAdded support for dropping privileges to git-daemon.

Subject: [PATCH] Added support for dropping privileges to git-daemon.

## tl;dr

13 messages between Aug 19, 2006 and Aug 23, 2006. Diffs are folded; open one to read it.

replies: 12people: 5as markdown or json

Tilman Sauerbeck· Aug 19, 2006, 12:27 UTC · lore
Signed-off-by: Tilman Sauerbeck <tilman@code-monkey.de>
---
 I'm not sure how useful this is. I'd like to start git-daemon as root,
 so it can store it's PID in /var/run, but I don't want it to keep root
 privileges. My git repos are world readable, so for serving them, root
 privileges aren't needed at all.
 What do you think?
 Documentation/git-daemon.txt |    8 +++++++-
 daemon.c                     |   39 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 2 deletions(-)
Show changes to 2 files +45 −2

Documentation/git-daemon.txt, daemon.c

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 0f7d274..a8d75d9 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -11,7 +11,8 @@ SYNOPSIS
 'git-daemon' [--verbose] [--syslog] [--inetd | --port=n] [--export-all]
              [--timeout=n] [--init-timeout=n] [--strict-paths]
              [--base-path=path] [--user-path | --user-path=path]
-	     [--reuseaddr] [--detach] [--pid-file=file] [directory...]
+	     [--reuseaddr] [--detach] [--pid-file=file]
+	     [--user=u] [--group=g] [directory...]
 
 DESCRIPTION
 -----------
@@ -93,6 +94,11 @@ OPTIONS
 --pid-file=file::
 	Save the process id in 'file'.
 
+--user=u::
+--group=g::
+	If both options are given, `git-daemon` will change it's uid and gid to
+	the ones of 'u' and 'g' before entering the server loop.
+
 <directory>::
 	A directory to add to the whitelist of allowed directories. Unless
 	--strict-paths is specified this will also include subdirectories
diff --git a/daemon.c b/daemon.c
index 012936f..78658c1 100644
--- a/daemon.c
+++ b/daemon.c
@@ -7,6 +7,8 @@ #include <netdb.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <syslog.h>
+#include <pwd.h>
+#include <grp.h>
 #include "pkt-line.h"
 #include "cache.h"
 #include "exec_cmd.h"
@@ -14,12 +16,15 @@ #include "exec_cmd.h"
 static int log_syslog;
 static int verbose;
 static int reuseaddr;
+static const char *user;
+static const char *group;
 
 static const char daemon_usage[] =
 "git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n"
 "           [--timeout=n] [--init-timeout=n] [--strict-paths]\n"
 "           [--base-path=path] [--user-path | --user-path=path]\n"
-"           [--reuseaddr] [--detach] [--pid-file=file] [directory...]";
+"           [--reuseaddr] [--detach] [--pid-file=file]\n"
+"           [--user=u] [--group=g] [directory...]";
 
 /* List of acceptable pathname prefixes */
 static char **ok_paths;
@@ -701,6 +706,24 @@ static void store_pid(const char *path)
 	fclose(f);
 }
 
+static void drop_privileges()
+{
+	struct passwd *p;
+	struct group *g;
+
+	p = getpwnam (user);
+	if (!p)
+		die("user not found - %s", user);
+
+	g = getgrnam (group);
+	if (!g)
+		die("group not found - %s", group);
+
+	if (initgroups (p->pw_name, g->gr_gid) || setgid (g->gr_gid) ||
+	    setuid (p->pw_uid))
+		die("cannot drop privileges");
+}
+
 static int serve(int port)
 {
 	int socknum, *socklist;
@@ -709,6 +732,9 @@ static int serve(int port)
 	if (socknum == 0)
 		die("unable to allocate any listen sockets on port %u", port);
 
+	if (user && group)
+		drop_privileges();
+
 	return service_loop(socknum, socklist);
 }
 
@@ -791,6 +817,14 @@ int main(int argc, char **argv)
 			log_syslog = 1;
 			continue;
 		}
+		if (!strncmp(arg, "--user=", 7)) {
+			user = arg + 7;
+			continue;
+		}
+		if (!strncmp(arg, "--group=", 8)) {
+			group = arg + 8;
+			continue;
+		}
 		if (!strcmp(arg, "--")) {
 			ok_paths = &argv[i+1];
 			break;
@@ -802,6 +836,9 @@ int main(int argc, char **argv)
 		usage(daemon_usage);
 	}
 
+	if (!user ^ !group)
+		die("either set both user and group or none of them");
+
 	if (log_syslog) {
 		openlog("git-daemon", 0, LOG_DAEMON);
 		set_die_routine(daemon_die);
-- 
1.4.2
Marco Costalba· Aug 19, 2006, 13:23 UTC · re: Tilman Sauerbeck · lore

Re: [PATCH] Added support for dropping privileges to git-daemon.

Show 5 quoted lines
>
> +       if (!user ^ !group)
> +               die("either set both user and group or none of them");
> +
>
Just a  question. Why simply not
       if (user ^ group)
Tilman Sauerbeck· Aug 19, 2006, 13:29 UTC · re: Marco Costalba · lore

Re: [PATCH] Added support for dropping privileges to git-daemon.

Marco Costalba [2006-08-19 15:23]:
Show 9 quoted lines
> >
> >+       if (!user ^ !group)
> >+               die("either set both user and group or none of them");
> >+
> >
> 
> Just a  question. Why simply not
> 
>       if (user ^ group)
Because gcc doesn't like that:
 error: invalid operands to binary ^

Regards, Tilman

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
Marco Costalba· Aug 19, 2006, 15:19 UTC · re: Tilman Sauerbeck · lore

Re: [PATCH] Added support for dropping privileges to git-daemon.

On 8/19/06, Tilman Sauerbeck <tilman@code-monkey.de> wrote:
Show 14 quoted lines
> Marco Costalba [2006-08-19 15:23]:
> > >
> > >+       if (!user ^ !group)
> > >+               die("either set both user and group or none of them");
> > >+
> > >
> >
> > Just a  question. Why simply not
> >
> >       if (user ^ group)
>
> Because gcc doesn't like that:
>  error: invalid operands to binary ^
>
Interesting!

Indeed gcc it's right, operator ^ it's a bitwise operator, not a logical one. So operands must be booleans (or numerical types in case of C).

The trick is that operator '!' performs an implicit conversion to integral on the 'user' and 'group' pointers.

BTW the following (very ugly) works.
         if ((int)user ^ (int)group)
Mitchell Blank Jr· Aug 19, 2006, 17:15 UTC · re: Marco Costalba · lore

Re: [PATCH] Added support for dropping privileges to git-daemon.

Marco Costalba wrote:
Show 6 quoted lines
> >> >+       if (!user ^ !group)
> >> >+               die("either set both user and group or none of them");
> 
> BTW the following (very ugly) works.
> 
>         if ((int)user ^ (int)group)

No it doesn't. Besides being a dangerous cast (no guarantee that a pointer will fit in an "int") your code is basically just a fancy way of saying

	if (user != group)

Which is definitely NOT what is intended. "user" and "group" are pointers -- unless they're both NULL we expect them to have different values. The original code is equivalent to:

	if ((user == NULL) != (group == NULL))
which is what is actually intended.
-Mitch
Johannes Schindelin· Aug 19, 2006, 13:32 UTC · re: Tilman Sauerbeck · lore

Re: [PATCH] Added support for dropping privileges to git-daemon.

Hi,
On Sat, 19 Aug 2006, Tilman Sauerbeck wrote:
>  What do you think?

I think it is a good addition. Note that most people will probably use inetd instead, though.

> +	     [--user=u] [--group=g] [directory...]
Since you enforce both --user and --group, this should read
	 [--user=u --group=g]

Ciao, Dscho

Mitchell Blank Jr· Aug 19, 2006, 17:25 UTC · re: Tilman Sauerbeck · lore

Re: [PATCH] Added support for dropping privileges to git-daemon.

Tilman Sauerbeck wrote:
> +	if (user && group)
> +		drop_privileges();
It seems "if (user)" would be sufficient here.
> +	if (!user ^ !group)
> +		die("either set both user and group or none of them");

For simplicities sake I'd actually suggest allowing group==NULL. So change this test to be

	if (group && !user)
		die("--group supplied without --user");
Then in drop_privileges() do something like:
	struct passwd *p;
	gid_t gid;
	p = getpwnam(user);
	if (!p)
		die("user not found - %s", user);
	if (group == NULL)
		gid = p->pw_gid;
	else {
		struct group *g = getgrnam(group);
		if (!g)
			die("group not found - %s", group);
		gid = g->gr_gid;
	}

Since usually you want to use the same gid that is normally associated with that pid, this just makes things a little easier on the user

-Mitch
Tilman Sauerbeck· Aug 19, 2006, 12:27 UTC · re: Mitchell Blank Jr · lore
Signed-off-by: Tilman Sauerbeck <tilman@code-monkey.de>
---
 My idea was to keep the code simple :)
 Anyway, this patch has the code you proposed.
 Documentation/git-daemon.txt |   11 +++++++++-
 daemon.c                     |   45 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 54 insertions(+), 2 deletions(-)
Show changes to 2 files +54 −2

Documentation/git-daemon.txt, daemon.c

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 0f7d274..3c1bc85 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -11,7 +11,8 @@ SYNOPSIS
 'git-daemon' [--verbose] [--syslog] [--inetd | --port=n] [--export-all]
              [--timeout=n] [--init-timeout=n] [--strict-paths]
              [--base-path=path] [--user-path | --user-path=path]
-	     [--reuseaddr] [--detach] [--pid-file=file] [directory...]
+	     [--reuseaddr] [--detach] [--pid-file=file]
+	     [--user=u [--group=g]] [directory...]
 
 DESCRIPTION
 -----------
@@ -93,6 +94,14 @@ OPTIONS
 --pid-file=file::
 	Save the process id in 'file'.
 
+--user=u::
+--group=g::
+	These two options may be used to make `git-daemon` change its uid and
+	gid	before entering the server loop.
+	The uid that's used is the one of 'u'. If `group` is specified,
+	the gid is set to the one of 'g', otherwise, the default gid
+	of 'u' is used.
+
 <directory>::
 	A directory to add to the whitelist of allowed directories. Unless
 	--strict-paths is specified this will also include subdirectories
diff --git a/daemon.c b/daemon.c
index 012936f..70be10f 100644
--- a/daemon.c
+++ b/daemon.c
@@ -7,6 +7,8 @@ #include <netdb.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <syslog.h>
+#include <pwd.h>
+#include <grp.h>
 #include "pkt-line.h"
 #include "cache.h"
 #include "exec_cmd.h"
@@ -14,12 +16,15 @@ #include "exec_cmd.h"
 static int log_syslog;
 static int verbose;
 static int reuseaddr;
+static const char *user;
+static const char *group;
 
 static const char daemon_usage[] =
 "git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n"
 "           [--timeout=n] [--init-timeout=n] [--strict-paths]\n"
 "           [--base-path=path] [--user-path | --user-path=path]\n"
-"           [--reuseaddr] [--detach] [--pid-file=file] [directory...]";
+"           [--reuseaddr] [--detach] [--pid-file=file]\n"
+"           [--user=u] [--group=g] [directory...]";
 
 /* List of acceptable pathname prefixes */
 static char **ok_paths;
@@ -701,6 +706,30 @@ static void store_pid(const char *path)
 	fclose(f);
 }
 
+static void drop_privileges()
+{
+	struct passwd *p;
+	struct group *g;
+	gid_t gid;
+
+	p = getpwnam (user);
+	if (!p)
+		die("user not found - %s", user);
+
+	if (!group)
+		gid = p->pw_gid;
+	else {
+		g = getgrnam (group);
+		if (!g)
+			die("group not found - %s", group);
+
+		gid = g->gr_gid;
+	}
+
+	if (initgroups (p->pw_name, gid) || setgid (gid) || setuid (p->pw_uid))
+		die("cannot drop privileges");
+}
+
 static int serve(int port)
 {
 	int socknum, *socklist;
@@ -709,6 +738,9 @@ static int serve(int port)
 	if (socknum == 0)
 		die("unable to allocate any listen sockets on port %u", port);
 
+	if (user)
+		drop_privileges();
+
 	return service_loop(socknum, socklist);
 }
 
@@ -791,6 +823,14 @@ int main(int argc, char **argv)
 			log_syslog = 1;
 			continue;
 		}
+		if (!strncmp(arg, "--user=", 7)) {
+			user = arg + 7;
+			continue;
+		}
+		if (!strncmp(arg, "--group=", 8)) {
+			group = arg + 8;
+			continue;
+		}
 		if (!strcmp(arg, "--")) {
 			ok_paths = &argv[i+1];
 			break;
@@ -802,6 +842,9 @@ int main(int argc, char **argv)
 		usage(daemon_usage);
 	}
 
+	if (group && !user)
+		die("--group supplied without --user");
+
 	if (log_syslog) {
 		openlog("git-daemon", 0, LOG_DAEMON);
 		set_die_routine(daemon_die);
-- 
1.4.2
Junio C Hamano· Aug 22, 2006, 06:38 UTC · re: Tilman Sauerbeck · lore

Re: [PATCH] Added support for dropping privileges to git-daemon.

Tilman Sauerbeck <tilman@code-monkey.de> writes:
Show 20 quoted lines
> +	     [--reuseaddr] [--detach] [--pid-file=file]
> +	     [--user=u [--group=g]] [directory...]
>  
>  DESCRIPTION
>  -----------
> @@ -93,6 +94,14 @@ OPTIONS
>  --pid-file=file::
>  	Save the process id in 'file'.
>  
> +--user=u::
> +--group=g::
> +	These two options may be used to make `git-daemon` change its uid and
> +	gid	before entering the server loop.
> +	The uid that's used is the one of 'u'. If `group` is specified,
> +	the gid is set to the one of 'g', otherwise, the default gid
> +	of 'u' is used.
> +
>  <directory>::
>  	A directory to add to the whitelist of allowed directories. Unless
>  	--strict-paths is specified this will also include subdirectories

I'd prefer to spell <u> and <g> out, and clarify the description that the parameter parsing code would do getpwent() for the user, and the user does not have to supply numeric user and group ids, and what happens if numeric ids are given.

How well does this interact with --inetd (inetd_mode)? Do we want to have some notes in the documentation on that topic?

Show 19 quoted lines
> diff --git a/daemon.c b/daemon.c
> index 012936f..70be10f 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -7,6 +7,8 @@ #include <netdb.h>
>  #include <netinet/in.h>
>  #include <arpa/inet.h>
>  #include <syslog.h>
> +#include <pwd.h>
> +#include <grp.h>
>  #include "pkt-line.h"
>  #include "cache.h"
>  #include "exec_cmd.h"
> @@ -14,12 +16,15 @@ #include "exec_cmd.h"
>  static int log_syslog;
>  static int verbose;
>  static int reuseaddr;
> +static const char *user;
> +static const char *group;
Do these have to be file-level global?

Do we want to do getpwnam/getgrnam lookup in drop_privileges()? "Gaaaah, no such username/groupname!" would be needed anyway, and I have a feeling that it might be better done once immediately after argument parsing. Then you can pass whatever is needed for initgroups/setgid/setuid down as parameters to serve() and drop_privileges().

Do we want to use initgroups(), which is _BSD_SOURCE? I guess it is perhaps Ok.

Show 7 quoted lines
> +static void drop_privileges()
> +{
> +	struct passwd *p;
> +	struct group *g;
> +	gid_t gid;
> +
> +	p = getpwnam (user);
No space between function name and open parenthesis, please.
Show 9 quoted lines
> @@ -709,6 +738,9 @@ static int serve(int port)
>  	if (socknum == 0)
>  		die("unable to allocate any listen sockets on port %u", port);
>  
> +	if (user)
> +		drop_privileges();
> +
>  	return service_loop(socknum, socklist);
>  }

Makes one wonder why it checks only user and not group, and then makes one realize that the argument parsing code enforces that group is never supplied without user, which leaves funny taste in the mouth, but that is Ok, I guess.

Tilman Sauerbeck· Aug 22, 2006, 17:37 UTC · re: Junio C Hamano · lore
Signed-off-by: Tilman Sauerbeck <tilman@code-monkey.de>
---
 This one got the changes you proposed.
 Not sure whether the documentation is good enough, I'm not that good at
 writing tech docs in english :(
 Documentation/git-daemon.txt |   18 ++++++++++++++++
 daemon.c                     |   46 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 59 insertions(+), 5 deletions(-)
Show changes to 2 files +59 −5

Documentation/git-daemon.txt, daemon.c

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 0f7d274..b591895 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -11,7 +11,8 @@ SYNOPSIS
 'git-daemon' [--verbose] [--syslog] [--inetd | --port=n] [--export-all]
              [--timeout=n] [--init-timeout=n] [--strict-paths]
              [--base-path=path] [--user-path | --user-path=path]
-	     [--reuseaddr] [--detach] [--pid-file=file] [directory...]
+	     [--reuseaddr] [--detach] [--pid-file=file]
+	     [--user=user [--group=group]] [directory...]
 
 DESCRIPTION
 -----------
@@ -93,6 +94,21 @@ OPTIONS
 --pid-file=file::
 	Save the process id in 'file'.
 
+--user=user::
+--group=group::
+	These two options may be used to make `git-daemon` change its uid and
+	gid	before entering the server loop.
+	The uid that's used is the one of 'user'. If `group` is specified,
+	the gid is set to the one of 'group', otherwise, the default gid
+	of 'user' is used.
+
+	Both `group` and `user` need to be passed as the name of the resp of
+	the group, ie you'll get unexpected results if you pass an uid/a gid.
+
+	Note that you probably don't want to use these options if you run
+	git-daemon in inetd mode, since inetd can do the privilege dropping
+	for you.
+
 <directory>::
 	A directory to add to the whitelist of allowed directories. Unless
 	--strict-paths is specified this will also include subdirectories
diff --git a/daemon.c b/daemon.c
index 012936f..4e94210 100644
--- a/daemon.c
+++ b/daemon.c
@@ -7,6 +7,8 @@ #include <netdb.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <syslog.h>
+#include <pwd.h>
+#include <grp.h>
 #include "pkt-line.h"
 #include "cache.h"
 #include "exec_cmd.h"
@@ -19,7 +21,8 @@ static const char daemon_usage[] =
 "git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n"
 "           [--timeout=n] [--init-timeout=n] [--strict-paths]\n"
 "           [--base-path=path] [--user-path | --user-path=path]\n"
-"           [--reuseaddr] [--detach] [--pid-file=file] [directory...]";
+"           [--reuseaddr] [--detach] [--pid-file=file]\n"
+"           [--user=user [[--group=group]] [directory...]";
 
 /* List of acceptable pathname prefixes */
 static char **ok_paths;
@@ -701,7 +704,7 @@ static void store_pid(const char *path)
 	fclose(f);
 }
 
-static int serve(int port)
+static int serve(int port, struct passwd *pass, gid_t gid)
 {
 	int socknum, *socklist;
 
@@ -709,6 +712,11 @@ static int serve(int port)
 	if (socknum == 0)
 		die("unable to allocate any listen sockets on port %u", port);
 
+	if (pass && gid &&
+	    (initgroups(pass->pw_name, gid) || setgid (gid) ||
+	     setuid(pass->pw_uid)))
+		die("cannot drop privileges");
+
 	return service_loop(socknum, socklist);
 }
 
@@ -716,8 +724,11 @@ int main(int argc, char **argv)
 {
 	int port = DEFAULT_GIT_PORT;
 	int inetd_mode = 0;
-	const char *pid_file = NULL;
+	const char *pid_file = NULL, *user_name = NULL, *group_name = NULL;
 	int detach = 0;
+	struct passwd *pass = NULL;
+	struct group *group;
+	gid_t gid = 0;
 	int i;
 
 	/* Without this we cannot rely on waitpid() to tell
@@ -791,6 +802,14 @@ int main(int argc, char **argv)
 			log_syslog = 1;
 			continue;
 		}
+		if (!strncmp(arg, "--user=", 7)) {
+			user_name = arg + 7;
+			continue;
+		}
+		if (!strncmp(arg, "--group=", 8)) {
+			group_name = arg + 8;
+			continue;
+		}
 		if (!strcmp(arg, "--")) {
 			ok_paths = &argv[i+1];
 			break;
@@ -802,6 +821,25 @@ int main(int argc, char **argv)
 		usage(daemon_usage);
 	}
 
+	if (group_name && !user_name)
+		die("--group supplied without --user");
+
+	if (user_name) {
+		pass = getpwnam(user_name);
+		if (!pass)
+			die("user not found - %s", user_name);
+
+		if (!group_name)
+			gid = pass->pw_gid;
+		else {
+			group = getgrnam(group_name);
+			if (!group)
+				die("group not found - %s", group_name);
+
+			gid = group->gr_gid;
+		}
+	}
+
 	if (log_syslog) {
 		openlog("git-daemon", 0, LOG_DAEMON);
 		set_die_routine(daemon_die);
@@ -831,5 +869,5 @@ int main(int argc, char **argv)
 	if (pid_file)
 		store_pid(pid_file);
 
-	return serve(port);
+	return serve(port, pass, gid);
 }
-- 
1.4.2
Junio C Hamano· Aug 22, 2006, 23:40 UTC · re: Tilman Sauerbeck · lore

Re: [PATCH] Added support for dropping privileges to git-daemon.

Tilman Sauerbeck <tilman@code-monkey.de> writes:
Show 6 quoted lines
> @@ -93,6 +94,21 @@ OPTIONS
>  --pid-file=file::
>  	Save the process id in 'file'.
>  
> +--user=user::
> +--group=group::
Probably
	--user=user, --group=group::
Also check for asciidoc formatting please; it's rather picky.
Show 5 quoted lines
> +	These two options may be used to make `git-daemon` change its uid and
> +	gid	before entering the server loop.
> +	The uid that's used is the one of 'user'. If `group` is specified,
> +	the gid is set to the one of 'group', otherwise, the default gid
> +	of 'user' is used.
Funny whitespaces all over the place...

What is the pw_gid stored in struct passwd for the user? getgroups(2) gives supplementary group IDs, so perhaps it is called primary group ID?

> +	Both `group` and `user` need to be passed as the name of the resp of
> +	the group, ie you'll get unexpected results if you pass an uid/a gid.

Gaah, but that is probably OK. I'd explicitly say they are always interpreted as name and never numeric. An alternative would be to be nice and when getpwnam() and/or getgrnam() returns NULL try to interpret them as numeric, which might help a small dedicated server installation that does not have any /etc/passwd or /etc/group file ;-) but I do not think that would be worth the confusion.

> +	Note that you probably don't want to use these options if you run
> +	git-daemon in inetd mode, since inetd can do the privilege dropping
> +	for you.

Gaah again. These options do not have any effect (other than sanity checking) on the inetd_mode codepath, so instead of saying this in the documentation I would suggest specifying these options an error under --inetd.

Something like this on top of your patch perhaps.
Show changes to 2 files +13 −12

Documentation/git-daemon.txt, daemon.c

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 8f850fa..17619a3 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -94,20 +94,16 @@ OPTIONS
 --pid-file=file::
 	Save the process id in 'file'.
 
---user=user::
---group=group::
-	These two options may be used to make `git-daemon` change its uid and
-	gid	before entering the server loop.
-	The uid that's used is the one of 'user'. If `group` is specified,
-	the gid is set to the one of 'group', otherwise, the default gid
-	of 'user' is used.
-
-	Both `group` and `user` need to be passed as the name of the resp of
-	the group, ie you'll get unexpected results if you pass an uid/a gid.
-
-	Note that you probably don't want to use these options if you run
-	git-daemon in inetd mode, since inetd can do the privilege dropping
-	for you.
+--user=user, --group=group::
+	Change daemon's uid and gid before entering the service loop.
+	When only `--user` is given without `--group`, the
+	primary group ID for the user is used.  The values of
+	the option are given to `getpwnam(3)` and `getgrnam(3)`
+	and numeric IDs are not supported.
++
+Giving these options is an error when used with `--inetd`; use
+the facility of inet daemon to achieve the same before spawning
+`git-daemon` if needed.
 
 <directory>::
 	A directory to add to the whitelist of allowed directories. Unless
diff --git a/daemon.c b/daemon.c
index 4e94210..dd3915a 100644
--- a/daemon.c
+++ b/daemon.c
@@ -821,6 +821,9 @@ int main(int argc, char **argv)
 		usage(daemon_usage);
 	}
 
+	if (inetd_mode && (group_name || user_name))
+		die("--user and --group are incompatible with --inetd");
+
 	if (group_name && !user_name)
 		die("--group supplied without --user");
 
Tilman Sauerbeck· Aug 23, 2006, 16:45 UTC · re: Junio C Hamano · lore

Re: [PATCH] Added support for dropping privileges to git-daemon.

Junio C Hamano [2006-08-22 16:40]:
Show 9 quoted lines
> Tilman Sauerbeck <tilman@code-monkey.de> writes:
> 
> > +	These two options may be used to make `git-daemon` change its uid and
> > +	gid	before entering the server loop.
> > +	The uid that's used is the one of 'user'. If `group` is specified,
> > +	the gid is set to the one of 'group', otherwise, the default gid
> > +	of 'user' is used.
> 
> Funny whitespaces all over the place...

There's exactly _one_ stray \t in the middle of a line. Not quite what I'd call "all over the place' ;) I should probably tell vim to hilight space errors in txt files, too.

> Gaah again.  These options do not have any effect (other than
> sanity checking) on the inetd_mode codepath, so instead of
> saying this in the documentation I would suggest specifying
> these options an error under --inetd.
Good point.

Regards, Tilman

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

← back to recent threads