On 3 September 2015 at 16:33, Andy Zhou <[email protected]> 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 <[email protected]>
> ---
>  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@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.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to