[ovs-dev] [PATCH 2/3] lib: Add --user for daemon
Ansis Atteka
ansisatteka at gmail.com
Tue Sep 8 00:39:09 UTC 2015
On 3 September 2015 at 16:33, Andy Zhou <azhou at nicira.com> wrote:
> Allow daemon running as root to accept --user option, that accepts
> "user:group" string as input. Performs sanity check on the input,
> and store the converted uid and gid.
>
> daemon_become_new_user() needs to be called to make the actual
> switch.
> Signed-off-by: Andy Zhou <azhou at nicira.com>
> ---
> lib/daemon-unix.c | 71
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/daemon.h | 27 +++++++++++++++------
> 2 files changed, 91 insertions(+), 7 deletions(-)
>
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index d52ac2d..44eb800 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -20,6 +20,7 @@
> #include <errno.h>
> #include <fcntl.h>
> #include <grp.h>
> +#include <pwd.h>
> #include <signal.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -693,6 +694,76 @@ should_service_stop(void)
> return false;
> }
>
> +void daemon_set_new_user(const char *user_spec)
> +{
> + char *pos = strchr(user_spec, ':');
> +
> + uid = getuid();
> + gid = getgid();
> +
> + if (gid || uid) {
> + VLOG_FATAL("%s: only root can use --user option", pidfile);
> + }
> +
> + user_spec += strspn(user_spec, " \t\r\n");
> + int len = pos ? pos - user_spec : strlen(user_spec);
> + struct passwd pwd, *res;
> + char buf[4096];
> +
> + if (len) {
> + user = xzalloc(len + 1);
> + strncpy(user, user_spec, len);
> +
> + if (getpwnam_r(user, &pwd, buf, 4096, &res)) {
>
instead of using 4096 I would use "sizeof buf" here and in other places.
However, from where did you get this "4096" number in the first place? I
see that in the GETPWNAM man page the documented way to get the expected
buffer size is: sysconf(_SC_GETPW_R_SIZE_MAX); Perhaps there is a reason
you are not using that approach?
> + VLOG_FATAL("%s: Invalid --user option %s (no such user %s)",
> + pidfile, user_spec, user);
> + }
> + } else {
> + /* User is not specified, use current user. */
> + if (getpwuid_r(uid, &pwd, buf, 4096, &res)) {
> + VLOG_FATAL("%s: Invalid --user option %s (failed to lookup "
> + "current user with uid %d)", pidfile, user_spec,
> uid);
> + }
> + user = strdup(pwd.pw_name);
> + }
> +
> + uid = pwd.pw_uid;
> + gid = pwd.pw_gid;
> +
> + if (pos) {
> + char *grpstr = pos + 1;
> + grpstr += strspn(grpstr, " \t\r\n");
> +
> + if (*grpstr) {
> + struct group grp, *res;
> +
> + if(getgrnam_r(grpstr, &grp, buf, 4096, &res)) {
> + VLOG_FATAL("%s: Invalid --user option %s (unknown group
> %s)",
> + pidfile, user_spec, grpstr);
> + }
> +
> + if(gid != grp.gr_gid) {
> + char **mem;
> +
> + for(mem = grp.gr_mem; *mem; ++mem) {
> + if (!strcmp(*mem, user)) {
> + break;
> + }
> + }
> +
> + if (!*mem) {
> + VLOG_FATAL("%s: Invalid --user option %s (user %s is "
> + "not in group %s)", pidfile, user_spec,
> + user, grpstr);
> + }
> + gid = grp.gr_gid;
> + }
> + }
> + }
> +
> + switch_to_new_user = true;
> +}
> +
> void
> daemon_become_new_user(void)
> {
> diff --git a/lib/daemon.h b/lib/daemon.h
> index fb97cde..4b25f46 100644
> --- a/lib/daemon.h
> +++ b/lib/daemon.h
> @@ -42,14 +42,16 @@
> OPT_NO_CHDIR, \
> OPT_OVERWRITE_PIDFILE, \
> OPT_PIDFILE, \
> - OPT_MONITOR
> + OPT_MONITOR, \
> + OPT_USER_GROUP
>
> -#define DAEMON_LONG_OPTIONS \
> - {"detach", no_argument, NULL, OPT_DETACH}, \
> - {"no-chdir", no_argument, NULL, OPT_NO_CHDIR}, \
> - {"pidfile", optional_argument, NULL, OPT_PIDFILE}, \
> +#define DAEMON_LONG_OPTIONS \
> + {"detach", no_argument, NULL, OPT_DETACH}, \
> + {"no-chdir", no_argument, NULL, OPT_NO_CHDIR}, \
> + {"pidfile", optional_argument, NULL, OPT_PIDFILE}, \
> {"overwrite-pidfile", no_argument, NULL, OPT_OVERWRITE_PIDFILE}, \
> - {"monitor", no_argument, NULL, OPT_MONITOR}
> + {"monitor", no_argument, NULL, OPT_MONITOR}, \
> + {"user", required_argument, NULL, OPT_USER_GROUP}
>
> #define DAEMON_OPTION_HANDLERS \
> case OPT_DETACH: \
> @@ -70,6 +72,10 @@
> \
> case OPT_MONITOR: \
> daemon_set_monitor(); \
> + break; \
> + \
> + case OPT_USER_GROUP: \
> + daemon_set_new_user(optarg); \
> break;
>
> void set_detach(void);
> @@ -77,6 +83,7 @@ void daemon_set_monitor(void);
> void set_no_chdir(void);
> void ignore_existing_pidfile(void);
> void daemon_become_new_user(void);
> +void daemon_set_new_user(const char *);
> pid_t read_pidfile(const char *name);
> #else
> #define DAEMON_OPTION_ENUMS \
> @@ -85,7 +92,7 @@ pid_t read_pidfile(const char *name);
> OPT_PIDFILE, \
> OPT_PIPE_HANDLE, \
> OPT_SERVICE, \
> - OPT_SERVICE_MONITOR
> + OPT_SERVICE_MONITOR \
>
> #define DAEMON_LONG_OPTIONS
> \
> {"detach", no_argument, NULL, OPT_DETACH},
> \
> @@ -120,6 +127,12 @@ void control_handler(DWORD request);
> void set_pipe_handle(const char *pipe_handle);
>
> static inline void
> +daemon_set_new_user(const char *)
> +{
> + /* Not implemented. */
> +}
> +
> +static inline void
> daemon_become_new_user(void)
> {
> /* Not implemented. */
> --
>
> I was able to downgrade ovsdb-server to a non-root user (aatteka in this
case):
aatteka at aatteka-MacBookPro:~/Git/ovs$ ps -Af | grep ovsdb
aatteka 20116 985 0 17:14 ? 00:00:00 ovsdb-server: monitoring
pid 20117 (healthy)
aatteka 20117 20116 0 17:14 ? 00:00:00 ovsdb-server /tmp/conf.db
-vconsole:emer -vsyslog:err -vfile:info
--remote=punix:/var/run/openvswitch/db.sock
--private-key=db:Open_vSwitch,SSL,private_key
--certificate=db:Open_vSwitch,SSL,certificate
--bootstrap-ca-cert=db:Open_vSwitch,SSL,ca_cert --no-chdir
--log-file=/var/log/openvswitch/ovsdb-server.log
--pidfile=/var/run/openvswitch/ovsdb-server.pid --detach --monitor
--user=aatteka
1. However, before being able to do that I had to: sudo chown
aatteka:aatteka /var/run/openvswitch. I think you need to get right
permissions for this directory
2. Should the monitor process still be run as root? There is a precedent
that dnsmasq does that. There are some pros and cons of implementing it
that way if the child process on restart would need to acquire certain
resources.
More information about the dev
mailing list