On Mon, Sep 14, 2015 at 03:54:05PM -0700, Andy Zhou wrote:
> Added functions to drop daemon's root privileges at run time by
> allowing it to run as a different user. Daemons all start
> running as root, then drop to the user by invoking
> daemon_become_new_user() function when they are ready to drop
> root privileges.
>
> Future patch will make use of this function.
>
> Signed-off-by: Andy Zhou <[email protected]>
Thanks for working on this. I'm glad to see some progress in basic
security.
Many of these functions mark non-pointer parameters as "const". That's
unusual and not very useful; it doesn't promise the caller anything in
the same way that, say, declaring a parameter as type "const char *"
promises that the function won't modify the caller's string.
> +static bool
> +gid_equal(const gid_t expected, const gid_t value)
> +{
Usually I'd write this:
> + return (expected == -1) ? true : expected == value;
as just:
return expected == -1 || expected == value;
> +}
s/effecitve/effective/ in the following function:
> +static bool
> +gid_verify(const gid_t real, const gid_t effecitve, const gid_t saved)
> +{
> + gid_t r, e, s;
Here, I'd check the return value of getresgid() and abort if it fails
(or I guess you could just return false):
> + getresgid(&r, &e, &s);
> + return (gid_equal(real, r) &&
> + gid_equal(effecitve, e) && gid_equal(saved, s));
> +}
> +
> +static void
> +daemon_switch_group(const gid_t real, const gid_t effective,
> + const gid_t saved)
> +{
> + if ((setresgid(real, effective, saved) == -1) ||
> + !gid_verify(real, effective, saved)) {
> + VLOG_FATAL("%s: fail to switch group to gid as %d, aborting",
> + pidfile, gid);
> + }
> +}
> +
> +static bool
> +uid_equal(const uid_t expected, const uid_t value)
> +{
I'd usually write this with || as for gid_equal:
> + return (expected == -1) ? true : expected == value;
> +}
> +
> +static bool
> +uid_verify(const uid_t real, const uid_t effective, const uid_t saved)
> +{
> + uid_t r, e, s;
Return value check here too:
> + getresuid(&r, &e, &s);
> + return (uid_equal(real, r) &&
> + uid_equal(effective, e) && uid_equal(saved, s));
> +}
> +
> +static void
> +daemon_switch_user(const uid_t real, const uid_t effective, const uid_t
> saved,
> + const char *user)
> +{
> + if ((setresuid(real, effective, saved) == -1) ||
> + !uid_verify(real, effective, saved)) {
> + VLOG_FATAL("%s: fail to switch user to %s, aborting",
> + pidfile, user);
> + }
> +}
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
