| Message ID | 1441323223-11889-1-git-send-email-azhou@nicira.com |
|---|---|
| State | Changes Requested |
| Headers | show |
There are trailing white space characters:
Applying: lib: add function to switch daemon user at run time
/home/aatteka/Git/ovs/.git/rebase-apply/patch:109: trailing whitespace.
{
warning: 1 line adds whitespace errors.
Applying: lib: Add --user for daemon
Applying: ovsdb-server: support --user option
On 3 September 2015 at 16:33, Andy Zhou <azhou@nicira.com> wrote:
> Added function to drop daemon's root privileges at run time by
> allowing it to run as a different user. Daemon can still start
> running as root. Each daemon's implementation can invoke this
> function when it is ready to drop the root privilege.
>
> Future patch will make use of this function.
>
> Signed-off-by: Andy Zhou <azhou@nicira.com>
> ---
> lib/daemon-unix.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/daemon.h | 9 ++++++++-
> 2 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index eb95521..d52ac2d 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -19,6 +19,7 @@
> #include "daemon-private.h"
> #include <errno.h>
> #include <fcntl.h>
> +#include <grp.h>
> #include <signal.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -64,6 +65,13 @@ static int daemonize_fd = -1;
> * it dies due to an error signal? */
> static bool monitor;
>
> +/* --user: Only root can have this option. Switch to new uid:gid after
> initial
>
s/Only root can have/Only root can use
s/initial/initially
> + * running as root. */
> +static bool switch_to_new_user = false;
> +static uid_t uid;
> +static gid_t gid;
> +static char *user = NULL;
+
> static void check_already_running(void);
> static int lock_pidfile(FILE *, int command);
> static pid_t fork_and_clean_up(void);
> @@ -684,3 +692,43 @@ should_service_stop(void)
> {
> return false;
> }
> +
> +void
> +daemon_become_new_user(void)
>
+{
> + if (switch_to_new_user) {
> + /* "Setuid Demystified" by Hao Chen, etc outlines some caveats of
> + * around unix system call setuid() and friends. This
> implementation
> + * mostly follow the advice given by the paper. The paper is
> + * published in 2002, so things could have changed.
> + */
> +
> + /* Change both real and effective uid and gid will permanently
> + * drop the process' privilege. "Setuid Demystified" suggested
> + * that calling getuid() after each setuid() call to verify they
> + * are actually set, because checking return code alone is not
> + * sufficient.
> + *
> + * Linux also has per process file system uid, i.e. fsuid. Without
> + * explicit setting it, it follows the process' effective uid.
>
s/explicit/explicitly
> + * This implementation does not explicitly set fsuid for better
> + * portability. (Although setresuid() is not available on
> Solaris,
> + * according to the paper above.) */
> +
> + if (setregid(gid, gid) == -1 || getgid() != gid || getegid() !=
> gid) {
> + VLOG_FATAL("%s: fail to switch group to gid as %d, aborting",
>
s/to gid/to gid
> + pidfile, gid);
>
Should you print path to pidfile or the pid itself? Isn't --pidfile an
optional argument?
> + }
> +
> + if (user && initgroups(user, gid) == -1) {
> + VLOG_FATAL("%s: fail to add supplementary group gid %d,
> aborting",
> + pidfile, gid);
> + }
> +
> + /* Change both real and effective uid and make sure they are
> set. */
> + if (setreuid(uid, uid) == -1 || getuid() != uid || geteuid() !=
> uid) {
> + VLOG_FATAL("%s: fail to switch to user %s, aborting",
> + pidfile, user);
> + }
> + }
> +}
> diff --git a/lib/daemon.h b/lib/daemon.h
> index 959341d..fb97cde 100644
> --- a/lib/daemon.h
> +++ b/lib/daemon.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -76,6 +76,7 @@ void set_detach(void);
> void daemon_set_monitor(void);
> void set_no_chdir(void);
> void ignore_existing_pidfile(void);
> +void daemon_become_new_user(void);
> pid_t read_pidfile(const char *name);
> #else
> #define DAEMON_OPTION_ENUMS \
> @@ -117,6 +118,12 @@ pid_t read_pidfile(const char *name);
>
> void control_handler(DWORD request);
> void set_pipe_handle(const char *pipe_handle);
> +
> +static inline void
> +daemon_become_new_user(void)
> +{
> + /* Not implemented. */
> +}
> #endif /* _WIN32 */
>
> bool get_detach(void);
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
On Thu, Sep 03, 2015 at 04:33:41PM -0700, Andy Zhou wrote: > Added function to drop daemon's root privileges at run time by > allowing it to run as a different user. Daemon can still start > running as root. Each daemon's implementation can invoke this > function when it is ready to drop the root privilege. > > Future patch will make use of this function. > > Signed-off-by: Andy Zhou <azhou@nicira.com> I suggest that behavior on Windows should be to exit with an error message instead of silently continuing. I'm going to go back to this after I see how it's used in later patches.
diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c index eb95521..d52ac2d 100644 --- a/lib/daemon-unix.c +++ b/lib/daemon-unix.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ #include "daemon-private.h" #include <errno.h> #include <fcntl.h> +#include <grp.h> #include <signal.h> #include <stdlib.h> #include <string.h> @@ -64,6 +65,13 @@ static int daemonize_fd = -1; * it dies due to an error signal? */ static bool monitor; +/* --user: Only root can have this option. Switch to new uid:gid after initial + * running as root. */ +static bool switch_to_new_user = false; +static uid_t uid; +static gid_t gid; +static char *user = NULL; + static void check_already_running(void); static int lock_pidfile(FILE *, int command); static pid_t fork_and_clean_up(void); @@ -684,3 +692,43 @@ should_service_stop(void) { return false; } + +void +daemon_become_new_user(void) +{ + if (switch_to_new_user) { + /* "Setuid Demystified" by Hao Chen, etc outlines some caveats of + * around unix system call setuid() and friends. This implementation + * mostly follow the advice given by the paper. The paper is + * published in 2002, so things could have changed. + */ + + /* Change both real and effective uid and gid will permanently + * drop the process' privilege. "Setuid Demystified" suggested + * that calling getuid() after each setuid() call to verify they + * are actually set, because checking return code alone is not + * sufficient. + * + * Linux also has per process file system uid, i.e. fsuid. Without + * explicit setting it, it follows the process' effective uid. + * This implementation does not explicitly set fsuid for better + * portability. (Although setresuid() is not available on Solaris, + * according to the paper above.) */ + + if (setregid(gid, gid) == -1 || getgid() != gid || getegid() != gid) { + VLOG_FATAL("%s: fail to switch group to gid as %d, aborting", + pidfile, gid); + } + + if (user && initgroups(user, gid) == -1) { + VLOG_FATAL("%s: fail to add supplementary group gid %d, aborting", + pidfile, gid); + } + + /* Change both real and effective uid and make sure they are set. */ + if (setreuid(uid, uid) == -1 || getuid() != uid || geteuid() != uid) { + VLOG_FATAL("%s: fail to switch to user %s, aborting", + pidfile, user); + } + } +} diff --git a/lib/daemon.h b/lib/daemon.h index 959341d..fb97cde 100644 --- a/lib/daemon.h +++ b/lib/daemon.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -76,6 +76,7 @@ void set_detach(void); void daemon_set_monitor(void); void set_no_chdir(void); void ignore_existing_pidfile(void); +void daemon_become_new_user(void); pid_t read_pidfile(const char *name); #else #define DAEMON_OPTION_ENUMS \ @@ -117,6 +118,12 @@ pid_t read_pidfile(const char *name); void control_handler(DWORD request); void set_pipe_handle(const char *pipe_handle); + +static inline void +daemon_become_new_user(void) +{ + /* Not implemented. */ +} #endif /* _WIN32 */ bool get_detach(void);
Added function to drop daemon's root privileges at run time by allowing it to run as a different user. Daemon can still start running as root. Each daemon's implementation can invoke this function when it is ready to drop the root privilege. Future patch will make use of this function. Signed-off-by: Andy Zhou <azhou@nicira.com> --- lib/daemon-unix.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- lib/daemon.h | 9 ++++++++- 2 files changed, 57 insertions(+), 2 deletions(-)