Re: [PATCH] conf: Add --runas option, changing to given UID and GID if started as root

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 8025 bytes --]

On Wed, May 18, 2022 at 07:18:08PM +0200, Stefano Brivio wrote:
> On some systems, user and group "nobody" might not be available. The
> new --runas option allows to override the default "nobody" choice if
> started as root.
> 
> Now that we allow this, drop the initgroups() call that was used to
> add any additional groups for the given user, as that might now
> grant unnecessarily broad permissions. For instance, several
> distributions have a "kvm" group to allow regular user access to
> /dev/kvm, and we don't need that in passt or pasta.
> 
> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>

Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>

> ---
>  conf.c  | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  passt.1 |  7 ++++++
>  passt.c | 36 ++++++++++++++++++------------
>  passt.h |  5 +++++
>  4 files changed, 102 insertions(+), 14 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 0baf4fa..2e6e3c4 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -22,6 +22,8 @@
>  #include <sys/stat.h>
>  #include <libgen.h>
>  #include <limits.h>
> +#include <grp.h>
> +#include <pwd.h>
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <unistd.h>
> @@ -614,6 +616,9 @@ static void usage(const char *name)
>  	info(   "    default: run in background if started from a TTY");
>  	info(   "  -e, --stderr		Log to stderr too");
>  	info(   "    default: log to system logger only if started from a TTY");
> +	info(   "  --runas UID|UID:GID 	Use given UID, GID if started as root"
> +	info(   "    UID and GID can be numeric, or login and group names");
> +	info(   "    default: drop to user \"nobody\"");
>  	info(   "  -h, --help		Display this help message and exit");
>  
>  	if (strstr(name, "pasta")) {
> @@ -837,6 +842,57 @@ dns6:
>  	}
>  }
>  
> +/**
> + * conf_runas() - Handle --runas: look up desired UID and GID
> + * @opt:	Passed option value
> + * @uid:	User ID, set on return if valid
> + * @gid:	Group ID, set on return if valid
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gid)
> +{
> +	char ubuf[LOGIN_NAME_MAX], gbuf[LOGIN_NAME_MAX], *endptr;
> +	struct passwd *pw;
> +	struct group *gr;
> +
> +	/* NOLINTNEXTLINE(cert-err34-c): 2 if conversion succeeds */
> +	if (sscanf(opt, "%u:%u", uid, gid) == 2 && uid && gid)
> +		return 0;
> +
> +	*uid = strtol(opt, &endptr, 0);
> +	if (!*endptr && (*gid = *uid))
> +		return 0;
> +
> +#ifdef GLIBC_NO_STATIC_NSS
> +	(void)ubuf;
> +	(void)gbuf;
> +	(void)pw;
> +
> +	return -EINVAL;
> +#else
> +	/* NOLINTNEXTLINE(cert-err34-c): 2 if conversion succeeds */
> +	if (sscanf(opt, "%" STR(LOGIN_NAME_MAX) "[^:]:"
> +			"%" STR(LOGIN_NAME_MAX) "s", ubuf, gbuf) == 2) {
> +		pw = getpwnam(ubuf);
> +		if (!pw || !(*uid = pw->pw_uid))
> +			return -ENOENT;
> +
> +		gr = getgrnam(gbuf);
> +		if (!gr || !(*gid = gr->gr_gid))
> +			return -ENOENT;
> +
> +		return 0;
> +	}
> +
> +	pw = getpwnam(ubuf);
> +	if (!pw || !(*uid = pw->pw_uid) || !(*gid = pw->pw_gid))
> +		return -ENOENT;
> +
> +	return 0;
> +#endif /* !GLIBC_NO_STATIC_NSS */
> +}
> +
>  /**
>   * conf() - Process command-line arguments and set configuration
>   * @c:		Execution context
> @@ -889,6 +945,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  		{"dns-forward",	required_argument,	NULL,		9 },
>  		{"no-netns-quit", no_argument,		NULL,		10 },
>  		{"trace",	no_argument,		NULL,		11 },
> +		{"runas",	required_argument,	NULL,		12 },
>  		{ 0 },
>  	};
>  	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
> @@ -1032,6 +1089,17 @@ void conf(struct ctx *c, int argc, char **argv)
>  
>  			c->trace = c->debug = c->foreground = 1;
>  			break;
> +		case 12:
> +			if (c->uid || c->gid) {
> +				err("Multiple --runas options given");
> +				usage(argv[0]);
> +			}
> +
> +			if (conf_runas(optarg, &c->uid, &c->gid)) {
> +				err("Invalid --runas option: %s", optarg);
> +				usage(argv[0]);
> +			}
> +			break;
>  		case 'd':
>  			if (c->debug) {
>  				err("Multiple --debug options given");
> diff --git a/passt.1 b/passt.1
> index cdca3e9..d3af916 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -95,6 +95,13 @@ Log to standard error too.
>  Default is to log to system logger only, if started from an interactive
>  terminal, and to both system logger and standard error otherwise.
>  
> +.TP
> +.BR \-\-runas " " \fIUID\fR|\fIUID:GID\fR|\fILOGIN\fR|\fILOGIN:GROUP\fR
> +If started as root, change to given UID and corresponding group if UID is given,
> +or to given UID and given GID if both are given. Alternatively, login name, or
> +login name and group name can be passed.
> +Default is to change to user \fInobody\fR if started as root.
> +
>  .TP
>  .BR \-h ", " \-\-help
>  Display a help message and exit.
> diff --git a/passt.c b/passt.c
> index e5064f8..0732176 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -191,9 +191,9 @@ static void seccomp(const struct ctx *c)
>  }
>  
>  /**
> - * check_root() - Warn if root in init, exit if we can't drop to nobody
> + * check_root() - Check if root in init ns, exit if we can't drop to user
>   */
> -static void check_root(void)
> +static void check_root(struct ctx *c)
>  {
>  	const char root_uid_map[] = "         0          0 4294967295";
>  	struct passwd *pw;
> @@ -214,22 +214,29 @@ static void check_root(void)
>  
>  	close(fd);
>  
> -	fprintf(stderr, "Don't run this as root. Changing to nobody...\n");
> +	if (!c->uid) {
> +		fprintf(stderr, "Don't run as root. Changing to nobody...\n");
>  #ifndef GLIBC_NO_STATIC_NSS
> -	pw = getpwnam("nobody");
> -	if (!pw) {
> -		perror("getpwnam");
> -		exit(EXIT_FAILURE);
> -	}
> +		pw = getpwnam("nobody");
> +		if (!pw) {
> +			perror("getpwnam");
> +			exit(EXIT_FAILURE);
> +		}
>  
> -	if (!initgroups(pw->pw_name, pw->pw_gid) &&
> -	    !setgid(pw->pw_gid) && !setuid(pw->pw_uid))
> -		return;
> +		c->uid = pw->pw_uid;
> +		c->gid = pw->pw_gid;
>  #else
> -	(void)pw;
> +		(void)pw;
> +
> +		/* Common value for 'nobody', not really specified */
> +		c->uid = c->gid = 65534;
>  #endif
> +	}
>  
> -	fprintf(stderr, "Can't change to user/group nobody, exiting");
> +	if (!setgid(c->gid) && !setuid(c->uid))
> +		return;
> +
> +	fprintf(stderr, "Can't change user/group, exiting");
>  	exit(EXIT_FAILURE);
>  }
>  
> @@ -336,7 +343,6 @@ int main(int argc, char **argv)
>  
>  	arch_avx2_exec(argv);
>  
> -	check_root();
>  	drop_caps();
>  
>  	c.pasta_userns_fd = c.pasta_netns_fd = c.fd_tap = c.fd_tap_listen = -1;
> @@ -391,6 +397,8 @@ int main(int argc, char **argv)
>  	conf(&c, argc, argv);
>  	trace_init(c.trace);
>  
> +	check_root(&c);
> +
>  	if (!c.debug && (c.stderr || isatty(fileno(stdout))))
>  		__openlog(log_name, LOG_PERROR, LOG_DAEMON);
>  
> diff --git a/passt.h b/passt.h
> index 69e334d..e541341 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -106,6 +106,8 @@ enum passt_modes {
>   * @sock_path:		Path for UNIX domain socket
>   * @pcap:		Path for packet capture file
>   * @pid_file:		Path to PID file, empty string if not configured
> + * @uid:		UID we should drop to, if started as root
> + * @gid:		GID we should drop to, if started as root
>   * @pasta_netns_fd:	File descriptor for network namespace in pasta mode
>   * @pasta_userns_fd:	Descriptor for user namespace to join, -1 once joined
>   * @netns_only:		In pasta mode, don't join or create a user namespace
> @@ -170,6 +172,9 @@ struct ctx {
>  	char pcap[PATH_MAX];
>  	char pid_file[PATH_MAX];
>  
> +	uid_t uid;
> +	uid_t gid;
> +
>  	int pasta_netns_fd;
>  	int pasta_userns_fd;
>  	int netns_only;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]