Tilman Sauerbeck <tilman@xxxxxxxxxxxxxx> writes:
> + [--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?
> 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.
> +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.
> @@ -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.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html