From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from imap.gmail.com [173.194.76.109] by localhost with POP3 (fetchmail-6.3.26) for (single-drop); Thu, 23 May 2024 12:10:55 +0200 (CEST) Received: by 2002:a05:6a11:2489:b0:55f:c3c0:ed08 with SMTP id sg9csp1205767pxb; Thu, 23 May 2024 03:10:40 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCVJZNB/s7+NNcbivvwb/QXEq35DOcpwH0M5KXMotTB7qMzWe1Ih610YXECyFCIMERpvq42uOJxb/yjPK7LtSWzDB/LpjUXgZHw= X-Google-Smtp-Source: AGHT+IGlzL/28IAb3CJBgZhADozqOEeXdcHp+7cH0CcLnDCZ7tRqsvPijr5hId6n8R4cA4hx9yG6 X-Received: by 2002:a05:622a:508:b0:43a:b1bf:4762 with SMTP id d75a77b69052e-43f9e0a184bmr52474301cf.5.1716459040716; Thu, 23 May 2024 03:10:40 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1716459040; cv=none; d=google.com; s=arc-20160816; b=Ohcggr9mOph6hGFED2KXDkEOfyg/Qqe/UUP59dwYJrNTtTSn3bEfbcXEENn9BYMGuc fELNCZKLGLs8VZk2HMWW8AAyUw2G4FdzOYBguB27Kenuqy80f2celEL6Sp15ku7G2IrZ DpwMHMmuTTcx0P1IfXw+gvaaeVI282fjdAPM+oR67cC1yCYpqR7MoCsNSZsYIf5QeadE mUkjchsreRUIAhINeQbBPhHytDfcgRu2i4UriR8g4gIxnbphBlKLL0wr4QyGxIYexg4U V3+/FyipGdrIloiXoZBnS0Y5r9wrcqfFQXOuVw0MhRcnCcQ+LrKmu/bRby1qzi+CMztM 3/NA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-disposition:user-agent:in-reply-to:mime-version:references :message-id:subject:cc:to:from:date:delivered-to; bh=m6Rnf3t2/4U+pq9OeRFWkBkPr5eiMDzyZ74+iNvZi/8=; fh=sa22/YzBg7gBlP1vWMAMO0ID50cEEzmrnuJNDeV9zT4=; b=U0vSqtvXbs7eFqqq2Bqbfy+6NhSjXJPmMdsVX7kJPQAb0VeUmYgxraVyo70C+6k4M0 yjQiGUgYKGiEmhwbrfqbj/HA+7qEfcqwGWN/S0/vpzHQOprZ7mzY/OI983OK3VIvOdXN cACS5qe0wsIekn4YMwiey6o84yBK4/4c9I8tKWDYkQ6YzL4O+u0U8/Ybo+XZe0/92kyR syAv3PLKBS2OMoyGkzakQViUUscUU4A7JTvmzYYEEI5lt/paTtF4Q9kzMuPtacprBNRj veixSHksStP6zwan261sEffb+3cM/a/5kAcFthgIeGPVBxSidZ5UbAOcVSpG770TlFLk cmGQ==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; gateway.spf=pass (google.com: domain gapps.redhat.com configured 205.139.110.120 as internal address) smtp.mailfrom=rjones@redhat.com smtp.remote-ip=205.139.110.120 policy.d=gapps.redhat.com Return-Path: Received: from us-smtp-inbound-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com. [205.139.110.120]) by mx.google.com with ESMTPS id d75a77b69052e-43df54b54d8si57941201cf.209.2024.05.23.03.10.40 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 May 2024 03:10:40 -0700 (PDT) Received-SPF: pass (google.com: domain gapps.redhat.com configured 205.139.110.120 as internal address) Authentication-Results: mx.google.com; gateway.spf=pass (google.com: domain gapps.redhat.com configured 205.139.110.120 as internal address) smtp.mailfrom=rjones@redhat.com smtp.remote-ip=205.139.110.120 policy.d=gapps.redhat.com Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-398-mpwFFqyNOPqqEbdWbfk_2w-1; Thu, 23 May 2024 06:10:39 -0400 X-MC-Unique: mpwFFqyNOPqqEbdWbfk_2w-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id B6CD53C025B2 for ; Thu, 23 May 2024 10:10:38 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id B35F12026D37; Thu, 23 May 2024 10:10:38 +0000 (UTC) Received: from localhost (unknown [10.42.28.23]) by smtp.corp.redhat.com (Postfix) with ESMTP id DFD862026D68; Thu, 23 May 2024 10:10:37 +0000 (UTC) Date: Thu, 23 May 2024 11:10:36 +0100 From: "Richard W.M. Jones" To: Stefano Brivio Cc: passt-dev@passt.top, David Gibson , Minxi Hou Subject: Re: [PATCH 7/8] conf, passt, tap: Open socket and PID files before switching UID/GID Message-ID: <20240523101036.GW4345@redhat.com> References: <20240522205911.261325-1-sbrivio@redhat.com> <20240522205911.261325-8-sbrivio@redhat.com> MIME-Version: 1.0 In-Reply-To: <20240522205911.261325-8-sbrivio@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline List-Id: On Wed, May 22, 2024 at 10:59:10PM +0200, Stefano Brivio wrote: > Otherwise, if the user runs us as root, and gives us paths that are > only accessible by root, we'll fail to open them, which might in turn > encourage users to change permissions or ownerships: definitely a bad > idea in terms of security. > > Reported-by: Minxi Hou > Reported-by: Richard W.M. Jones > Signed-off-by: Stefano Brivio > --- > conf.c | 17 ++++++++++++++++- > passt.c | 10 ++++------ > passt.h | 4 ++++ > tap.c | 7 +++---- > tap.h | 1 + > 5 files changed, 28 insertions(+), 11 deletions(-) > > diff --git a/conf.c b/conf.c > index 2e0d909..f62a5eb 100644 > --- a/conf.c > +++ b/conf.c > @@ -38,6 +38,7 @@ > #include "ip.h" > #include "passt.h" > #include "netlink.h" > +#include "tap.h" > #include "udp.h" > #include "tcp.h" > #include "pasta.h" > @@ -1093,7 +1094,7 @@ static void conf_ugid(char *runas, uid_t *uid, gid_t *gid) > return; > > /* ...otherwise use nobody:nobody */ > - warn("Started as root. Changing to nobody..."); > + warn("Started as root, will change to nobody."); > { > #ifndef GLIBC_NO_STATIC_NSS > const struct passwd *pw; > @@ -1113,6 +1114,18 @@ static void conf_ugid(char *runas, uid_t *uid, gid_t *gid) > } > } > > +/** > + * conf_open_files() - Open files as requested by configuration > + * @c: Execution context > + */ > +static void conf_open_files(struct ctx *c) > +{ > + if (c->mode == MODE_PASST && c->fd_tap == -1) > + c->fd_tap_listen = tap_sock_unix_open(c->sock_path); > + > + c->pidfile_fd = pidfile_open(c->pid_file); > +} > + > /** > * conf() - Process command-line arguments and set configuration > * @c: Execution context > @@ -1712,6 +1725,8 @@ void conf(struct ctx *c, int argc, char **argv) > else if (optind != argc) > die("Extra non-option argument: %s", argv[optind]); > > + conf_open_files(c); /* Before any possible setuid() / setgid() */ > + > isolate_user(uid, gid, !netns_only, userns, c->mode); > > if (c->pasta_conf_ns) > diff --git a/passt.c b/passt.c > index e2446fc..a8c4cd3 100644 > --- a/passt.c > +++ b/passt.c > @@ -199,9 +199,9 @@ void exit_handler(int signal) > */ > int main(int argc, char **argv) > { > - int nfds, i, devnull_fd = -1, pidfile_fd; > struct epoll_event events[EPOLL_EVENTS]; > char *log_name, argv0[PATH_MAX], *name; > + int nfds, i, devnull_fd = -1; > struct ctx c = { 0 }; > struct rlimit limit; > struct timespec now; > @@ -211,7 +211,7 @@ int main(int argc, char **argv) > > isolate_initial(); > > - c.pasta_netns_fd = c.fd_tap = -1; > + c.pasta_netns_fd = c.fd_tap = c.pidfile_fd = -1; > > sigemptyset(&sa.sa_mask); > sa.sa_flags = 0; > @@ -299,8 +299,6 @@ int main(int argc, char **argv) > } > } > > - pidfile_fd = pidfile_open(c.pid_file); > - > if (isolate_prefork(&c)) > die("Failed to sandbox process, exiting"); > > @@ -308,9 +306,9 @@ int main(int argc, char **argv) > __openlog(log_name, 0, LOG_DAEMON); > > if (!c.foreground) > - __daemon(pidfile_fd, devnull_fd); > + __daemon(c.pidfile_fd, devnull_fd); > else > - pidfile_write(pidfile_fd, getpid()); > + pidfile_write(c.pidfile_fd, getpid()); > > if (pasta_child_pid) > kill(pasta_child_pid, SIGUSR1); > diff --git a/passt.h b/passt.h > index bc58d64..3e50612 100644 > --- a/passt.h > +++ b/passt.h > @@ -185,6 +185,7 @@ struct ip6_ctx { > * @sock_path: Path for UNIX domain socket > * @pcap: Path for packet capture file > * @pid_file: Path to PID file, empty string if not configured > + * @pidfile_fd: File descriptor for PID file, -1 if none > * @pasta_netns_fd: File descriptor for network namespace in pasta mode > * @no_netns_quit: In pasta mode, don't exit if fs-bound namespace is gone > * @netns_base: Base name for fs-bound namespace, if any, in pasta mode > @@ -234,7 +235,10 @@ struct ctx { > int nofile; > char sock_path[UNIX_PATH_MAX]; > char pcap[PATH_MAX]; > + > char pid_file[PATH_MAX]; > + int pidfile_fd; > + > int one_off; > > int pasta_netns_fd; > diff --git a/tap.c b/tap.c > index c9f580e..2ea0849 100644 > --- a/tap.c > +++ b/tap.c > @@ -1100,7 +1100,7 @@ restart: > * > * Return: socket descriptor on success, won't return on failure > */ > -static int tap_sock_unix_open(char *sock_path) > +int tap_sock_unix_open(char *sock_path) > { > int fd = socket(AF_UNIX, SOCK_STREAM, 0); > struct sockaddr_un addr = { > @@ -1144,7 +1144,7 @@ static int tap_sock_unix_open(char *sock_path) > if (i == UNIX_SOCK_MAX) > die("UNIX socket bind: %s", strerror(errno)); > > - info("UNIX domain socket bound at %s\n", addr.sun_path); > + info("UNIX domain socket bound at %s", addr.sun_path); > if (!*sock_path) > memcpy(sock_path, addr.sun_path, UNIX_PATH_MAX); > > @@ -1167,7 +1167,7 @@ static void tap_sock_unix_init(struct ctx *c) > ev.data.u64 = ref.u64; > epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap_listen, &ev); > > - info("You can now start qemu (>= 7.2, with commit 13c6be96618c):"); > + info("\nYou can now start qemu (>= 7.2, with commit 13c6be96618c):"); > info(" kvm ... -device virtio-net-pci,netdev=s -netdev stream,id=s,server=off,addr.type=unix,addr.path=%s", > c->sock_path); > info("or qrap, for earlier qemu versions:"); > @@ -1318,7 +1318,6 @@ void tap_sock_init(struct ctx *c) > } > > if (c->mode == MODE_PASST) { > - c->fd_tap_listen = tap_sock_unix_open(c->sock_path); > tap_sock_unix_init(c); > > /* In passt mode, we don't know the guest's MAC address until it > diff --git a/tap.h b/tap.h > index d146d2f..2285a87 100644 > --- a/tap.h > +++ b/tap.h > @@ -68,6 +68,7 @@ void tap_handler_pasta(struct ctx *c, uint32_t events, > const struct timespec *now); > void tap_handler_passt(struct ctx *c, uint32_t events, > const struct timespec *now); > +int tap_sock_unix_open(char *sock_path); > void tap_sock_init(struct ctx *c); > > #endif /* TAP_H */ Seems sensible. The code to open the pidfile definitely moves to the new function and the new function is called just before passt isolates itself in the new namespace. Acked-by: Richard W.M. Jones Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html