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 BB8DE5A005E for ; Fri, 14 Oct 2022 04:57:01 +0200 (CEST) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4MpWJ33n0Tz4xGx; Fri, 14 Oct 2022 13:56:51 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1665716211; bh=5iL5Z4RXFFRu0fy0XXcnwUspYs2EFh2aRgVJouc+Ko0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Ve1PL/t/ygpzc6XtEyNa7NUrEPflQlt29NjI79yLlW/6cdHbfZAdMitMyjxQlQMoj Y65BvN+scR5S1saSuYiIW2G7kvRieVWsEjOQ8ylTPre5xkbW2HbfRWdn80P9Bez0a4 89Icf/rANmwNJRk22yJwuEnmMFi3oJP7WNZJ0I9U= Date: Fri, 14 Oct 2022 13:56:37 +1100 From: David Gibson To: Stefano Brivio Subject: Re: Alas for CAP_NET_BIND_SERVICE Message-ID: References: <20221012075432.09e33625@elisabeth> <20221012124707.70755587@elisabeth> <20221013125017.195769cf@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bJjZp84tck4UjGpx" Content-Disposition: inline In-Reply-To: <20221013125017.195769cf@elisabeth> Message-ID-Hash: 4QFCJRZRDTG3XCSR4DAAF7EAL7JP2D7N X-Message-ID-Hash: 4QFCJRZRDTG3XCSR4DAAF7EAL7JP2D7N 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: --bJjZp84tck4UjGpx Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Oct 13, 2022 at 12:50:17PM +0200, Stefano Brivio wrote: > On Wed, 12 Oct 2022 12:47:07 +0200 > Stefano Brivio wrote: >=20 > > [...] > > > > We currently need to process port configuration in a second step for > > two reasons: > >=20 > > - we might bind ports in the detached namespace (-T, -U) > >=20 > > - one between IPv4 and IPv6 support could be administratively disabled > > (operationally, who cares, we'll just fail to bind if that's the > > case) > >=20 > > ...but for init/host facing ports (now: "inbound"), we don't care about > > the detached namespace, and we could simply call conf_ports() for -t > > and -u in a second step after the main loop. Sure, if we continue like > > this, we'll end up with O(n=B2) option handling, but right now it > > doesn't look that bad to me. > >=20 > > I would give it a shot after I'm done reviewing your patchset (it > > should also look clearer after that) and re-spinning my recent ones, > > unless you see something wrong with it. >=20 > So, I have a draft (minus man page changes), a bit more involved than I > wanted but still not adding much. >=20 > It's based on your patchset, so I can't really test it with Podman > because of that new issue I'm facing with filesystem-bound namespaces, > but it passes our tests, and it works with low ports too. >=20 > I would try to figure out that other issue before posting it properly, > here it comes anyway: Looks reasonable. > diff --git a/conf.c b/conf.c > index a22ef48..e1f42f1 100644 > --- a/conf.c > +++ b/conf.c > @@ -1530,6 +1530,11 @@ void conf(struct ctx *c, int argc, char **argv) > } > } while (name !=3D -1); > =20 > + if (v4_only && v6_only) { > + err("Options ipv4-only and ipv6-only are mutually exclusive"); > + usage(argv[0]); > + } > + > ret =3D conf_ugid(runas, &uid, &gid); > if (ret) > usage(argv[0]); > @@ -1539,6 +1544,30 @@ void conf(struct ctx *c, int argc, char **argv) > logfile, logsize); > } > =20 > + nl_sock_init(c, false); > + if (!v6_only) > + c->ifi4 =3D conf_ip4(ifi, &c->ip4, c->mac); > + if (!v4_only) > + c->ifi6 =3D conf_ip6(ifi, &c->ip6, c->mac); > + if (!c->ifi4 && !c->ifi6) { > + err("External interface not usable"); > + exit(EXIT_FAILURE); > + } > + > + /* Inbound port options can be parsed now (after IPv4/IPv6 settings) */ > + optind =3D 1; > + do { > + struct port_fwd *fwd; > + > + name =3D getopt_long(argc, argv, optstring, options, NULL); > + > + if ((name =3D=3D 't' && (fwd =3D &c->tcp.fwd_in)) || > + (name =3D=3D 'u' && (fwd =3D &c->udp.fwd_in.f))) { > + if (!optarg || conf_ports(c, name, optarg, fwd)) > + usage(argv[0]); > + } > + } while (name !=3D -1); > + > if (c->mode =3D=3D MODE_PASTA) { > if (conf_pasta_ns(&netns_only, userns, netns, > optind, argc, argv) < 0) > @@ -1561,50 +1590,20 @@ void conf(struct ctx *c, int argc, char **argv) > } > } > =20 > - if (nl_sock_init(c)) { > - err("Failed to get netlink socket"); > - exit(EXIT_FAILURE); > - } > - > - if (v4_only && v6_only) { > - err("Options ipv4-only and ipv6-only are mutually exclusive"); > - usage(argv[0]); > - } > - if (!v6_only) > - c->ifi4 =3D conf_ip4(ifi, &c->ip4, c->mac); > - if (!v4_only) > - c->ifi6 =3D conf_ip6(ifi, &c->ip6, c->mac); > - if (!c->ifi4 && !c->ifi6) { > - err("External interface not usable"); > - exit(EXIT_FAILURE); > - } > + if (c->mode =3D=3D MODE_PASTA) > + nl_sock_init(c, true); > =20 > - /* Now we can process port configuration options */ > + /* ...and outbound port options now that namespaces are set up. */ > optind =3D 1; > do { > - struct port_fwd *fwd =3D NULL; > + struct port_fwd *fwd; > =20 > name =3D getopt_long(argc, argv, optstring, options, NULL); > - switch (name) { > - case 't': > - case 'u': > - case 'T': > - case 'U': > - if (name =3D=3D 't') > - fwd =3D &c->tcp.fwd_in; > - else if (name =3D=3D 'T') > - fwd =3D &c->tcp.fwd_out; > - else if (name =3D=3D 'u') > - fwd =3D &c->udp.fwd_in.f; > - else if (name =3D=3D 'U') > - fwd =3D &c->udp.fwd_out.f; > =20 > + if ((name =3D=3D 'T' && (fwd =3D &c->tcp.fwd_out)) || > + (name =3D=3D 'U' && (fwd =3D &c->udp.fwd_out.f))) { > if (!optarg || conf_ports(c, name, optarg, fwd)) > usage(argv[0]); > - > - break; > - default: > - break; > } > } while (name !=3D -1); > =20 > diff --git a/netlink.c b/netlink.c > index 6e5a96b..3ee4d42 100644 > --- a/netlink.c > +++ b/netlink.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -71,25 +72,28 @@ ns: > } > =20 > /** > - * nl_sock_init() - Call nl_sock_init_do() and check for failures > + * nl_sock_init() - Call nl_sock_init_do(), won't return on failure > * @c: Execution context > - * > - * Return: -EIO if sockets couldn't be set up, 0 otherwise > + * @ns: Get socket in namespace, not in init > */ > -int nl_sock_init(const struct ctx *c) > +void nl_sock_init(const struct ctx *c, bool ns) > { > - if (c->mode =3D=3D MODE_PASTA) { > + if (c->mode =3D=3D MODE_PASTA && ns) { > NS_CALL(nl_sock_init_do, c); > if (nl_sock_ns =3D=3D -1) > - return -EIO; > + goto fail; > } else { > nl_sock_init_do(NULL); > } > =20 > if (nl_sock =3D=3D -1) > - return -EIO; > + goto fail; > =20 > - return 0; > + return; > + > +fail: > + err("Failed to get netlink socket"); > + exit(EXIT_FAILURE); > } > =20 > /** > diff --git a/netlink.h b/netlink.h > index 5ce5037..3c991cc 100644 > --- a/netlink.h > +++ b/netlink.h > @@ -6,7 +6,7 @@ > #ifndef NETLINK_H > #define NETLINK_H > =20 > -int nl_sock_init(const struct ctx *c); > +void nl_sock_init(const struct ctx *c, bool ns); > unsigned int nl_get_ext_if(sa_family_t af); > void nl_route(int ns, unsigned int ifi, sa_family_t af, void *gw); > void nl_addr(int ns, unsigned int ifi, sa_family_t af, > diff --git a/tap.c b/tap.c > index 77e513c..8b6d9bc 100644 > --- a/tap.c > +++ b/tap.c > @@ -30,6 +30,7 @@ > #include > #include > #include > +#include > #include > #include > #include >=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 --bJjZp84tck4UjGpx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEoULxWu4/Ws0dB+XtgypY4gEwYSIFAmNIz94ACgkQgypY4gEw YSI+FA/9FeNmS2qAMoLCXJnYyU0SmTgdTOosXNiI1bIpAPo+5GlXFH+HK8DFGW2N ZlPNwnH0vtHt48sxuPd8AHWVxBw6fysEsqkYCK6OMYpSXLph84KpOqvSu4lQf1kv tuqE2JgvULDHP9moKpGyweqlUxf4Vek84z2nY/CqTxcEWMAHAP4bu/mwLx/jk2yG zEkIWLYNbyiqdVYhYpNDc35Wn6NPS4ZvVmIaaLMpO3mT09F79IGzWQtJFRdPts8w tqNlgjRxpg33tP/xLWX346jy5vbA49SqIHmrGZoGBbhRQiQCnARIRdurANrm3iyD 6qxvUNBIaE35mgGlpIVteHCVpOnU1Xg/qf7vKNbe0BLxt8VXec0Ozerc9/BfH4Kx oW5sUq6c2yNycmrZoT31zhaLJjBQpk8gWeSzgRHC2Zu+8ekAycQmHWJg6Q3lXnNB vpTxuzxslJqSwMRDhkNB++Ts1J0JHy4EZbJONJN7eLRw26oaG41mTkjdwUAG44CE ljp3hoQd7pmsS5kyO9wfNBISevcTIRfb0EH0L8clrzRVXm/21m2NdyMqq/EA2Ksx GT5N+40NkUQcdDCQxyGR3NvZA2Kfe3EFyM4pzoe6xphdgrlpV+dni3Vit9kug/V7 PQ5RJM/pXNhyRG9c2/1QHD/cGkgVg2pIx7r0VQ0EG72zGaOUI/k= =BQyB -----END PGP SIGNATURE----- --bJjZp84tck4UjGpx--