- dev - passt

[PATCH v2] conf: Add --runas option, changing to given UID and GID if started as root
by Stefano Brivio 19 May '22

19 May '22

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. We can't call check_root() before we process --runas option, but we shouldn't call it after we set up a new user namespace (when pasta is invoked without passing an existing namespace): move that to conf(). Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Fix a typo in usage() (missing "); at the end of line) and move check_root() before pasta_start_ns(), because if we created the namespace we actually need to have UID 0 there. conf.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ passt.1 | 7 ++++++ passt.c | 46 ------------------------------------- passt.h | 5 +++++ util.c | 52 ++++++++++++++++++++++++++++++++++++++++++ util.h | 1 + 6 files changed, 135 insertions(+), 46 deletions(-) diff --git a/conf.c b/conf.c index 0baf4fa..ddad9a3 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"); @@ -1298,6 +1366,8 @@ void conf(struct ctx *c, int argc, char **argv) } } while (name != -1); + check_root(c); + if (c->mode == MODE_PASTA && optind + 1 == argc) { ret = conf_ns_opt(c, nsdir, userns, argv[optind]); if (ret == -ENOENT) 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..dd0229a 100644 --- a/passt.c +++ b/passt.c @@ -46,8 +46,6 @@ #include <sys/stat.h> #include <sys/prctl.h> #include <stddef.h> -#include <pwd.h> -#include <grp.h> #include <netinet/udp.h> #include <netinet/tcp.h> #include <netinet/if_ether.h> @@ -190,49 +188,6 @@ static void seccomp(const struct ctx *c) } } -/** - * check_root() - Warn if root in init, exit if we can't drop to nobody - */ -static void check_root(void) -{ - const char root_uid_map[] = " 0 0 4294967295"; - struct passwd *pw; - char buf[BUFSIZ]; - int fd; - - if (getuid() && geteuid()) - return; - - if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) - return; - - if (read(fd, buf, BUFSIZ) != sizeof(root_uid_map) || - strncmp(buf, root_uid_map, sizeof(root_uid_map) - 1)) { - close(fd); - return; - } - - close(fd); - - fprintf(stderr, "Don't run this as root. Changing to nobody...\n"); -#ifndef GLIBC_NO_STATIC_NSS - 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; -#else - (void)pw; -#endif - - fprintf(stderr, "Can't change to user/group nobody, exiting"); - exit(EXIT_FAILURE); -} - /** * sandbox() - Unshare IPC, mount, PID, UTS, and user namespaces, "unmount" root * @@ -336,7 +291,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; 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; diff --git a/util.c b/util.c index 9afd2a5..7ffd9d1 100644 --- a/util.c +++ b/util.c @@ -33,6 +33,8 @@ #include <string.h> #include <time.h> #include <errno.h> +#include <pwd.h> +#include <grp.h> #include <linux/capability.h> @@ -532,6 +534,56 @@ void drop_caps(void) } } +/** + * check_root() - Check if root in init ns, exit if we can't drop to user + */ +void check_root(struct ctx *c) +{ + const char root_uid_map[] = " 0 0 4294967295"; + struct passwd *pw; + char buf[BUFSIZ]; + int fd; + + if (getuid() && geteuid()) + return; + + if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) + return; + + if (read(fd, buf, BUFSIZ) != sizeof(root_uid_map) || + strncmp(buf, root_uid_map, sizeof(root_uid_map) - 1)) { + close(fd); + return; + } + + close(fd); + + 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); + } + + c->uid = pw->pw_uid; + c->gid = pw->pw_gid; +#else + (void)pw; + + /* Common value for 'nobody', not really specified */ + c->uid = c->gid = 65534; +#endif + } + + if (!setgid(c->gid) && !setuid(c->uid)) + return; + + fprintf(stderr, "Can't change user/group, exiting"); + exit(EXIT_FAILURE); +} + /** * ns_enter() - Enter configured user (unless already joined) and network ns * @c: Execution context diff --git a/util.h b/util.h index f85a87a..2b68acf 100644 --- a/util.h +++ b/util.h @@ -217,6 +217,7 @@ char *line_read(char *buf, size_t len, int fd); void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns, uint8_t *map, uint8_t *exclude); void drop_caps(void); +void check_root(struct ctx *c); int ns_enter(const struct ctx *c); void write_pidfile(int fd, pid_t pid); int __daemon(int pidfile_fd, int devnull_fd); -- 2.35.1

1 0

[PATCH v2] udp: Ignore bogus -Wstringop-overread for write() from gcc 12.1
by Stefano Brivio 19 May '22

19 May '22

With current OpenSUSE Tumbleweed on aarch64 (gcc-12-1.3.aarch64) and on x86_64 (gcc-12-1.4.x86_64), but curiously not on armv7hl (gcc-12-1.3.armv7hl), gcc warns about using the _pointer_ to the 802.3 header to write the whole frame to the tap descriptor: reading between 62 and 4294967357 bytes from a region of size 14 which is bogus: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103483 Probably declaring udp_sock_fill_data_v{4,6}() as noinline would "fix" this, but that's on the data path, so I'd rather not. Use a gcc pragma instead. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: LLVM 13.0.1 doesn't know about -Wstringop-overread, and clang-tidy warns about the unknown diagnostic pragma: make it conditional udp.c | 4 ++++ util.h | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/udp.c b/udp.c index f425d14..98e3eaa 100644 --- a/udp.c +++ b/udp.c @@ -714,8 +714,10 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n, b->uh.len = htons(udp4_l2_mh_sock[n].msg_len + sizeof(b->uh)); if (c->mode == MODE_PASTA) { + PRAGMA_STRINGOP_OVERREAD_IGNORE if (write(c->fd_tap, &b->eh, sizeof(b->eh) + ip_len) < 0) debug("tap write: %s", strerror(errno)); + PRAGMA_STRINGOP_OVERREAD_IGNORE_POP pcap((char *)&b->eh, sizeof(b->eh) + ip_len); return; @@ -813,8 +815,10 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n, b->ip6h.hop_limit = 255; if (c->mode == MODE_PASTA) { + PRAGMA_STRINGOP_OVERREAD_IGNORE if (write(c->fd_tap, &b->eh, sizeof(b->eh) + ip_len) < 0) debug("tap write: %s", strerror(errno)); + PRAGMA_STRINGOP_OVERREAD_IGNORE_POP pcap((char *)&b->eh, sizeof(b->eh) + ip_len); return; diff --git a/util.h b/util.h index f85a87a..3ccd208 100644 --- a/util.h +++ b/util.h @@ -102,6 +102,24 @@ enum { (void *)(arg)); \ } while (0) + +#ifdef __has_warning +# if __has_warning("-Wstringop-overread") +# define PRAGMA_STRINGOP_OVERREAD_IGNORE \ + _Pragma("GCC diagnostic ignored \"-Wstringop-overread\"") +# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP \ + _Pragma("GCC diagnostic pop") +# else +# define PRAGMA_STRINGOP_OVERREAD_IGNORE +# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP +# endif +#else +# define PRAGMA_STRINGOP_OVERREAD_IGNORE \ + _Pragma("GCC diagnostic ignored \"-Wstringop-overread\"") +# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP \ + _Pragma("GCC diagnostic pop") +#endif + #if __BYTE_ORDER == __BIG_ENDIAN #define L2_BUF_ETH_IP4_INIT \ { \ -- 2.35.1

1 0

[PATCH v2] conf: Add --runas option, changing to given UID and GID if started as root
by Stefano Brivio 19 May '22

19 May '22

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. We can't call check_root() before we process --runas option, but we shouldn't call it after we set up a new user namespace (when pasta is invoked without passing an existing namespace): move that to conf(). Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Fix a typo in usage() (missing "); at the end of line) and move check_root() before pasta_start_ns(), because if we created the namespace we actually need to have UID 0 there. conf.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ passt.1 | 7 ++++++ passt.c | 46 ------------------------------------- passt.h | 5 +++++ util.c | 52 ++++++++++++++++++++++++++++++++++++++++++ util.h | 1 + 6 files changed, 135 insertions(+), 46 deletions(-) diff --git a/conf.c b/conf.c index 0baf4fa..ddad9a3 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"); @@ -1298,6 +1366,8 @@ void conf(struct ctx *c, int argc, char **argv) } } while (name != -1); + check_root(c); + if (c->mode == MODE_PASTA && optind + 1 == argc) { ret = conf_ns_opt(c, nsdir, userns, argv[optind]); if (ret == -ENOENT) 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..dd0229a 100644 --- a/passt.c +++ b/passt.c @@ -46,8 +46,6 @@ #include <sys/stat.h> #include <sys/prctl.h> #include <stddef.h> -#include <pwd.h> -#include <grp.h> #include <netinet/udp.h> #include <netinet/tcp.h> #include <netinet/if_ether.h> @@ -190,49 +188,6 @@ static void seccomp(const struct ctx *c) } } -/** - * check_root() - Warn if root in init, exit if we can't drop to nobody - */ -static void check_root(void) -{ - const char root_uid_map[] = " 0 0 4294967295"; - struct passwd *pw; - char buf[BUFSIZ]; - int fd; - - if (getuid() && geteuid()) - return; - - if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) - return; - - if (read(fd, buf, BUFSIZ) != sizeof(root_uid_map) || - strncmp(buf, root_uid_map, sizeof(root_uid_map) - 1)) { - close(fd); - return; - } - - close(fd); - - fprintf(stderr, "Don't run this as root. Changing to nobody...\n"); -#ifndef GLIBC_NO_STATIC_NSS - 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; -#else - (void)pw; -#endif - - fprintf(stderr, "Can't change to user/group nobody, exiting"); - exit(EXIT_FAILURE); -} - /** * sandbox() - Unshare IPC, mount, PID, UTS, and user namespaces, "unmount" root * @@ -336,7 +291,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; 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; diff --git a/util.c b/util.c index 9afd2a5..7ffd9d1 100644 --- a/util.c +++ b/util.c @@ -33,6 +33,8 @@ #include <string.h> #include <time.h> #include <errno.h> +#include <pwd.h> +#include <grp.h> #include <linux/capability.h> @@ -532,6 +534,56 @@ void drop_caps(void) } } +/** + * check_root() - Check if root in init ns, exit if we can't drop to user + */ +void check_root(struct ctx *c) +{ + const char root_uid_map[] = " 0 0 4294967295"; + struct passwd *pw; + char buf[BUFSIZ]; + int fd; + + if (getuid() && geteuid()) + return; + + if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) + return; + + if (read(fd, buf, BUFSIZ) != sizeof(root_uid_map) || + strncmp(buf, root_uid_map, sizeof(root_uid_map) - 1)) { + close(fd); + return; + } + + close(fd); + + 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); + } + + c->uid = pw->pw_uid; + c->gid = pw->pw_gid; +#else + (void)pw; + + /* Common value for 'nobody', not really specified */ + c->uid = c->gid = 65534; +#endif + } + + if (!setgid(c->gid) && !setuid(c->uid)) + return; + + fprintf(stderr, "Can't change user/group, exiting"); + exit(EXIT_FAILURE); +} + /** * ns_enter() - Enter configured user (unless already joined) and network ns * @c: Execution context diff --git a/util.h b/util.h index f85a87a..2b68acf 100644 --- a/util.h +++ b/util.h @@ -217,6 +217,7 @@ char *line_read(char *buf, size_t len, int fd); void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns, uint8_t *map, uint8_t *exclude); void drop_caps(void); +void check_root(struct ctx *c); int ns_enter(const struct ctx *c); void write_pidfile(int fd, pid_t pid); int __daemon(int pidfile_fd, int devnull_fd); -- 2.35.1

1 0

[PATCH] udp: Ignore bogus -Wstringop-overread for write() from gcc 12.1
by Stefano Brivio 19 May '22

19 May '22

With current OpenSUSE Tumbleweed on aarch64 (gcc-12-1.3.aarch64) and on x86_64 (gcc-12-1.4.x86_64), but curiously not on armv7hl (gcc-12-1.3.armv7hl), gcc warns about using the _pointer_ to the 802.3 header to write the whole frame to the tap descriptor: reading between 62 and 4294967357 bytes from a region of size 14 which is bogus: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103483 Probably declaring udp_sock_fill_data_v{4,6}() as noinline would "fix" this, but that's on the data path, so I'd rather not. Use a gcc pragma instead. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- udp.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/udp.c b/udp.c index f425d14..dd7119e 100644 --- a/udp.c +++ b/udp.c @@ -714,8 +714,10 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n, b->uh.len = htons(udp4_l2_mh_sock[n].msg_len + sizeof(b->uh)); if (c->mode == MODE_PASTA) { +#pragma GCC diagnostic ignored "-Wstringop-overread" if (write(c->fd_tap, &b->eh, sizeof(b->eh) + ip_len) < 0) debug("tap write: %s", strerror(errno)); +#pragma GCC diagnostic pop pcap((char *)&b->eh, sizeof(b->eh) + ip_len); return; @@ -813,8 +815,10 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n, b->ip6h.hop_limit = 255; if (c->mode == MODE_PASTA) { +#pragma GCC diagnostic ignored "-Wstringop-overread" if (write(c->fd_tap, &b->eh, sizeof(b->eh) + ip_len) < 0) debug("tap write: %s", strerror(errno)); +#pragma GCC diagnostic pop pcap((char *)&b->eh, sizeof(b->eh) + ip_len); return; -- 2.35.1

1 0

[PATCH] qrap.1: Clarify it takes a qemu command, not a path
by Stefano Brivio 19 May '22

19 May '22

Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- qrap.1 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qrap.1 b/qrap.1 index 8b1f863..cc73f1e 100644 --- a/qrap.1 +++ b/qrap.1 @@ -19,10 +19,10 @@ A running instance of \fBpasst\fR(1) is probed checking for UNIX domain sockets, answering a dummy ARP request, with names starting from \fI/tmp/passt_1.socket\fR up to \fI/tmp/passt_64.socket\fR. -If first and second arguments are not a socket number and a path, that is, +If first and second arguments are not a socket number and a command, that is, respectively, \fIFDNUM\fR and \fIQEMU_CMD\fR, \fBqrap\fR will try to locate a -qemu binary and patch the command line to specify a network device and a -\fInetdev\fR back-end for usage with \fBpasst\fR(1). +qemu executable in \fBPATH\fR and patch the command line to specify a network +device and a \fInetdev\fR back-end for usage with \fBpasst\fR(1). If \fBqrap\fR patches the command line, it will remove any potentially conflicting network device, that is, any \fI-netdev\fR or \fI-net\fR option, or -- 2.35.1

2 1

[PATCH] Add basic .gitignore files
by Stefano Brivio 19 May '22

19 May '22

From: David Gibson <david(a)gibson.dropbear.id.au> Ignore various files generated during build or test. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- .gitignore | 8 ++++++++ test/.gitignore | 2 ++ 2 files changed, 10 insertions(+) create mode 100644 .gitignore create mode 100644 test/.gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..89d9919 --- /dev/null +++ b/.gitignore @@ -0,0 +1,8 @@ +*~ +/passt +/passt.avx2 +/pasta +/pasta.avx2 +/qrap +/pasta.1 +/seccomp.h diff --git a/test/.gitignore b/test/.gitignore new file mode 100644 index 0000000..edb2221 --- /dev/null +++ b/test/.gitignore @@ -0,0 +1,2 @@ +perf.js +test_logs/ -- 2.35.1

2 1

[PATCH] conf: Add --runas option, changing to given UID and GID if started as root
by Stefano Brivio 19 May '22

19 May '22

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

2 1

[PATCH 0/3] tests: A few fixes for demos
by Stefano Brivio 19 May '22

19 May '22

This series fixes issues that came with the previous test clean-up series, or that were made visible by it. Stefano Brivio (3): demo/pasta: Fix bad sleep directive tests: Don't count number of test units for demos tests: Don't check exit code for every command in demo mode test/demo/pasta | 2 +- test/lib/term | 3 +++ test/lib/test | 6 ++++-- 3 files changed, 8 insertions(+), 3 deletions(-) -- 2.35.1

2 6

[PATCH 00/12] Tests fixes and clean-ups
by Stefano Brivio 18 May '22

18 May '22

This series, mostly from David, cleans up a number of atrocities in the test suite, makes it more observable by reporting test failures whenever a command fails, and a bit more robust by improving filtering in pane_parse(). Further, the whole script now fails if an unhandled failure occurs. A few assorted fixes are included as well. David Gibson (9): tests: Remove unused XVFB variable tests: Add some debugging output for the test scripts themselves tests: Don't use tmux update-environment tests: Don't globally set tmux default-shell tests: Improve control character filtering in pane_parse tests: Don't ignore errors during script tests: Add pane_status command to check for success of issued commands tests: Simplify *tools commands using pane_status tests: Simplify explicit checks for command success Stefano Brivio (3): tests: Update mbuto git URLs test/perf/pasta_udp: Drop redundant assignment of ::1 to loopback interface test/run: Return 0 from run(), exit value already reflects failures test/build/static_checkers | 6 ++-- test/demo/passt | 2 +- test/distro/debian | 33 +++++++------------ test/distro/opensuse | 18 ++++------- test/distro/ubuntu | 15 +++------ test/icmp/passt_in_ns | 4 --- test/lib/setup | 59 ++++++++++++++++------------------ test/lib/term | 30 ++++++++++++++---- test/lib/test | 65 ++++++++++++++++++-------------------- test/perf/pasta_udp | 1 - test/run | 7 ++-- 11 files changed, 108 insertions(+), 132 deletions(-) -- 2.35.1

2 14

[PATCH 00/16] Fix issues reported by Coverity
by Stefano Brivio 05 Apr '22

05 Apr '22

Most of these are formal issues with no actual effect, some are false positives, but it looks sensible to fix all of them and there's also an interesting finding in udp_timer(). Stefano Brivio (16): treewide: Invalid type in argument to printf format specifier, CWE-686 passt: Ignoring number of bytes read, CWE-252 tcp: False "Untrusted loop bound" positive, CWE-606 treewide: Unchecked return value from library, CWE-252 tap: Resource leak, CWE-404 conf, packet: Operands don't affect result, CWE-569 passt: Improper use of negative value (CWE-394) treewide: Argument cannot be negative, CWE-687 conf: False "Assign instead of compare" positive, CWE-481 conf, tap: False "Buffer not null terminated" positives, CWE-170 tcp: Dereference null return value, CWE-476 tcp_splice: Logically dead code, CWE-561 tcp, tcp_splice: False "Negative array index read" positives, CWE-129 tcp: False "Out-of-bounds read" positive, CWE-125 udp: Out-of-bounds read, CWE-125 in udp_timer() arch: Pointer to local outside scope, CWE-562 arch.c | 10 +++--- conf.c | 15 +++++---- icmp.c | 13 +++++--- netlink.c | 40 ++++++++++++++--------- packet.c | 8 ++--- passt.c | 24 ++++++++++---- pasta.c | 25 +++++---------- pcap.c | 6 ++-- qrap.c | 15 ++++++--- tap.c | 35 +++++++++++++------- tcp.c | 75 ++++++++++++++++++++++++++----------------- tcp_splice.c | 91 ++++++++++++++++++++++++++++++++++------------------ udp.c | 5 +-- util.c | 11 ++++--- util.h | 9 ++++++ 15 files changed, 238 insertions(+), 144 deletions(-) -- 2.35.1

1 16