From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 951695A026A for ; Thu, 13 Oct 2022 11:37:28 +0200 (CEST) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Mp4Dg1WC8z4xH2; Thu, 13 Oct 2022 20:37:23 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1665653843; bh=a4bSdRa34XMTqNqCO+uOP3lIJ6J3m/vdEPGam/ukdmk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=QF/CrU9xO5cEc/1JiMTHfeTm/oOmT9SaI0cIAD5y4m+6jqIo1CQt5PWp0d9kr3Tjs EdXjtqeP2MXcpZUq8EJFl9DH8vZh7/kmiu686is3LdSKzBzRCSDFBQCEs24MVQXaPW hmuBRjwBOUaQwXOYiUd6H8kex0zLvbtmdHtvLzOo= Date: Thu, 13 Oct 2022 20:36:49 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 09/10] isolation: Only configure UID/GID mappings in userns when spawning shell Message-ID: References: <20221011054018.1449506-1-david@gibson.dropbear.id.au> <20221011054018.1449506-10-david@gibson.dropbear.id.au> <20221013041829.252980e1@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="lHLZqwSjscrjNaNs" Content-Disposition: inline In-Reply-To: <20221013041829.252980e1@elisabeth> Message-ID-Hash: 74EBYBCBVBWEKW73TJKGRLJCMW42MW3U X-Message-ID-Hash: 74EBYBCBVBWEKW73TJKGRLJCMW42MW3U X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: <> Archived-At: List-Archive: <> List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --lHLZqwSjscrjNaNs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Oct 13, 2022 at 04:18:29AM +0200, Stefano Brivio wrote: > On Tue, 11 Oct 2022 16:40:17 +1100 > David Gibson wrote: >=20 > > When in passt mode, or pasta mode spawning a command, we create a userns > > for ourselves. This is used both to isolate the pasta/passt process it= self > > and to run the spawned command, if any. > >=20 > > Since eed17a47 "Handle userns isolation and dropping root at the same t= ime" > > we've handled both cases the same, configuring the UID and GID mappings= in > > the new userns to map whichever UID we're running as to root within the > > userns. > >=20 > > This mapping is desirable when spawning a shell or other command, so th= at > > the user gets a root shell with reasonably clear abilities within the > > userns and netns. It's not necessarily essential, though. When not > > spawning a shell, it doesn't really have any purpose: passt itself does= n't > > need to be root and can operate fine with an unmapped user (using some = of > > the capabilities we get when entering the userns instead). > >=20 > > Configuring the uid_map can cause problems if passt is running with any > > capabilities in the initial namespace, such as CAP_NET_BIND_SERVICE to > > allow it to forward low ports. In this case the kernel makes files in > > /proc/pid owned by root rather than the starting user to prevent the us= er > > from interfering with the operation of the capability-enhanced process. > > This includes uid_map meaning we are not able to write to it. > >=20 > > Whether this behaviour is correct in the kernel is debatable, but in any > > case we might as well avoid problems by only initializing the user mapp= ings > > when we really want them. > >=20 > > Signed-off-by: David Gibson > > --- > > conf.c | 3 ++- > > isolation.c | 13 ------------- > > pasta.c | 16 ++++++++++++++-- > > pasta.h | 3 ++- > > 4 files changed, 18 insertions(+), 17 deletions(-) > >=20 > > diff --git a/conf.c b/conf.c > > index 1537dbf..b7661b6 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -1478,7 +1478,8 @@ void conf(struct ctx *c, int argc, char **argv) > > if (*netns) { > > pasta_open_ns(c, netns); > > } else { > > - pasta_start_ns(c, argc - optind, argv + optind); > > + pasta_start_ns(c, uid, gid, > > + argc - optind, argv + optind); > > } > > } > > =20 > > diff --git a/isolation.c b/isolation.c > > index e1a024d..b94226d 100644 > > --- a/isolation.c > > +++ b/isolation.c > > @@ -207,9 +207,6 @@ void isolate_initial(void) > > */ > > void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *u= serns) > > { > > - char uidmap[BUFSIZ]; > > - char gidmap[BUFSIZ]; > > - > > /* First set our UID & GID in the original namespace */ > > if (setgroups(0, NULL)) { > > /* If we don't have CAP_SETGID, this will EPERM */ > > @@ -261,16 +258,6 @@ void isolate_user(uid_t uid, gid_t gid, bool use_u= serns, const char *userns) > > err("Couldn't create user namespace: %s", strerror(errno)); > > exit(EXIT_FAILURE); > > } > > - > > - /* Configure user and group mappings */ > > - snprintf(uidmap, BUFSIZ, "0 %u 1", uid); > > - snprintf(gidmap, BUFSIZ, "0 %u 1", gid); > > - > > - if (write_file("/proc/self/uid_map", uidmap) || > > - write_file("/proc/self/setgroups", "deny") || > > - write_file("/proc/self/gid_map", gidmap)) { > > - warn("Couldn't configure user namespace"); > > - } > > } > > =20 > > /** > > diff --git a/pasta.c b/pasta.c > > index 0ab2fe4..9666fed 100644 > > --- a/pasta.c > > +++ b/pasta.c > > @@ -166,7 +166,6 @@ static int pasta_setup_ns(void *arg) > > { > > const struct pasta_setup_ns_arg *a > > =3D (const struct pasta_setup_ns_arg *)arg; > > - >=20 > Unrelated. Removed. > > if (write_file("/proc/sys/net/ipv4/ping_group_range", "0 0")) > > warn("Cannot set ping_group_range, ICMP requests might fail"); > > =20 > > @@ -179,16 +178,20 @@ static int pasta_setup_ns(void *arg) > > /** > > * pasta_start_ns() - Fork command in new namespace if target ns is no= t given > > * @c: Execution context > > + * @uid: UID we're running as in the init namespace > > + * @gid: GID we're running as in the init namespace > > * @argc: Number of arguments for spawned command > > * @argv: Command to spawn and arguments > > */ > > -void pasta_start_ns(struct ctx *c, int argc, char *argv[]) > > +void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, > > + int argc, char *argv[]) > > { > > struct pasta_setup_ns_arg arg =3D { > > .exe =3D argv[0], > > .argv =3D argv, > > }; > > char *sh_argv[] =3D { NULL, NULL }; > > + char uidmap[BUFSIZ], gidmap[BUFSIZ]; >=20 > Should go at the beginning for the usual semi-silly reason. Done. > > char ns_fn_stack[NS_FN_STACK_SIZE]; > > char sh_arg0[PATH_MAX + 1]; > > =20 > > @@ -196,7 +199,16 @@ void pasta_start_ns(struct ctx *c, int argc, char = *argv[]) > > if (!c->debug) > > c->quiet =3D 1; > > =20 > > + /* Configure user and group mappings */ > > + snprintf(uidmap, BUFSIZ, "0 %u 1", uid); > > + snprintf(gidmap, BUFSIZ, "0 %u 1", gid); > > =20 > > + if (write_file("/proc/self/uid_map", uidmap) || > > + write_file("/proc/self/setgroups", "deny") || > > + write_file("/proc/self/gid_map", gidmap)) { > > + warn("Couldn't configure user mappings"); > > + } > > +=09 >=20 > Excess whitespace. Fixed. > > if (argc =3D=3D 0) { > > arg.exe =3D getenv("SHELL"); > > if (!arg.exe) > > diff --git a/pasta.h b/pasta.h > > index 02df1f6..a8b9893 100644 > > --- a/pasta.h > > +++ b/pasta.h > > @@ -7,7 +7,8 @@ > > #define PASTA_H > > =20 > > void pasta_open_ns(struct ctx *c, const char *netns); > > -void pasta_start_ns(struct ctx *c, int argc, char *argv[]); > > +void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, > > + int argc, char *argv[]); > > void pasta_ns_conf(struct ctx *c); > > void pasta_child_handler(int signal); > > int pasta_netns_quit_init(struct ctx *c); >=20 --=20 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 --lHLZqwSjscrjNaNs Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEoULxWu4/Ws0dB+XtgypY4gEwYSIFAmNH3CUACgkQgypY4gEw YSLNSQ//dHbFIQFiQnYocT/xwvrr5kvjDw9A+wFjScEtrZYSxAhPaRMtmW79DLwR Pv3NT/1jllqJBytxprZyRB6omK2p8oBCsjnsVyG6rSPz8yb0nvVqHJ49Bh8P3Jdt nJbS13YeU6I8xYPaqu8yT9skJQbFpMswrPaxw8QtDNi2+OPEWZiA0M3GGXqFnLtj ZoQhhOyUkemYEdB020Vt8ssSKfUhGOOJjCBeBoP3ZvWxQEkEi6ZbuX9aT9grKGwF CAhXzBr0PGq1403t/crzGkS03u8WsHVJ7iBsd4FWNIHiQNY8KYj+QE7KDUD3YjxD m2wtdgGvqHvOjz01MsQ0y2OB91k858spk5L4183gPF/MHtf6YlDLe2UygEZbx2iI 3U7Xo35qgn/myk6UGXQb2pWTrGE12rYHS2uKvui3BehfgLRSTN5Lg0DWQvkp3i2W F4xxZfnYRZ73kzi9GJ+QRJ7qoa/STKXvpevexrIbr2y/SfET2S8sAlen2qIAAOBm WVguEvQuQmsNKDukTeRLk9S5qpRf3MzSnk6G8wgqHWgXPJD7HpVEc4++QDgop3nB EUuqeECpJUNKM8vQhltEJo/Wz5MrgIS7LUWBiOAICTjKtZm5Cq68laiXLRHHJuqC DUQaX/2Gg6SGskEy2dyFq7hMKuBFEpNYo8xlqanf7nAVpk1JGxE= =qVm3 -----END PGP SIGNATURE----- --lHLZqwSjscrjNaNs--