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

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

[Stefano Brivio]

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

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>
---
 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;
-- 
@@ -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;
-- 
2.35.1

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

[David Gibson]

[-- 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 --]