From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 12CFD5A005E for ; Thu, 13 Oct 2022 12:50:25 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1665658225; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YpVlcko+JecU/xAorOefB4JZLaCokdrYhA2tV+ml0l8=; b=InGBB6YHQuSE+iB/fW4OcILbSwuIqvRIddXmNmyrJeMF8THWO4jDHslsBgJXthej+ymy12 Ke4trrN8149gPB5oxLm3tB4zGA9F5LKAxitBnq6Ftyl4n78yEOVGBz75Q59PszgcOEDejd MzgYxprCCmpbuA8rVy0+PHymSsdo9L4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-154-pPE9Y0UFOsSXNJLEcdyBfA-1; Thu, 13 Oct 2022 06:50:21 -0400 X-MC-Unique: pPE9Y0UFOsSXNJLEcdyBfA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 66134101A54E; Thu, 13 Oct 2022 10:50:21 +0000 (UTC) Received: from maya.cloud.tilaa.com (ovpn-208-3.brq.redhat.com [10.40.208.3]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BB22140C2064; Thu, 13 Oct 2022 10:50:20 +0000 (UTC) Date: Thu, 13 Oct 2022 12:50:17 +0200 From: Stefano Brivio To: David Gibson Subject: Re: Alas for CAP_NET_BIND_SERVICE Message-ID: <20221013125017.195769cf@elisabeth> In-Reply-To: <20221012124707.70755587@elisabeth> References: <20221012075432.09e33625@elisabeth> <20221012124707.70755587@elisabeth> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: BTBQVZFJMWL4W7FFBH5YLGRWLHOR7RAZ X-Message-ID-Hash: BTBQVZFJMWL4W7FFBH5YLGRWLHOR7RAZ X-MailFrom: sbrivio@redhat.com 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: On Wed, 12 Oct 2022 12:47:07 +0200 Stefano Brivio wrote: > [...] > > 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=C2=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. So, I have a draft (minus man page changes), a bit more involved than I wanted but still not adding much. 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. I would try to figure out that other issue before posting it properly, here it comes anyway: 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) =09=09} =09} while (name !=3D -1); =20 +=09if (v4_only && v6_only) { +=09=09err("Options ipv4-only and ipv6-only are mutually exclusive"); +=09=09usage(argv[0]); +=09} + =09ret =3D conf_ugid(runas, &uid, &gid); =09if (ret) =09=09usage(argv[0]); @@ -1539,6 +1544,30 @@ void conf(struct ctx *c, int argc, char **argv) =09=09=09 logfile, logsize); =09} =20 +=09nl_sock_init(c, false); +=09if (!v6_only) +=09=09c->ifi4 =3D conf_ip4(ifi, &c->ip4, c->mac); +=09if (!v4_only) +=09=09c->ifi6 =3D conf_ip6(ifi, &c->ip6, c->mac); +=09if (!c->ifi4 && !c->ifi6) { +=09=09err("External interface not usable"); +=09=09exit(EXIT_FAILURE); +=09} + +=09/* Inbound port options can be parsed now (after IPv4/IPv6 settings) */ +=09optind =3D 1; +=09do { +=09=09struct port_fwd *fwd; + +=09=09name =3D getopt_long(argc, argv, optstring, options, NULL); + +=09=09if ((name =3D=3D 't' && (fwd =3D &c->tcp.fwd_in)) || +=09=09 (name =3D=3D 'u' && (fwd =3D &c->udp.fwd_in.f))) { +=09=09=09if (!optarg || conf_ports(c, name, optarg, fwd)) +=09=09=09=09usage(argv[0]); +=09=09} +=09} while (name !=3D -1); + =09if (c->mode =3D=3D MODE_PASTA) { =09=09if (conf_pasta_ns(&netns_only, userns, netns, =09=09=09=09 optind, argc, argv) < 0) @@ -1561,50 +1590,20 @@ void conf(struct ctx *c, int argc, char **argv) =09=09} =09} =20 -=09if (nl_sock_init(c)) { -=09=09err("Failed to get netlink socket"); -=09=09exit(EXIT_FAILURE); -=09} - -=09if (v4_only && v6_only) { -=09=09err("Options ipv4-only and ipv6-only are mutually exclusive"); -=09=09usage(argv[0]); -=09} -=09if (!v6_only) -=09=09c->ifi4 =3D conf_ip4(ifi, &c->ip4, c->mac); -=09if (!v4_only) -=09=09c->ifi6 =3D conf_ip6(ifi, &c->ip6, c->mac); -=09if (!c->ifi4 && !c->ifi6) { -=09=09err("External interface not usable"); -=09=09exit(EXIT_FAILURE); -=09} +=09if (c->mode =3D=3D MODE_PASTA) +=09=09nl_sock_init(c, true); =20 -=09/* Now we can process port configuration options */ +=09/* ...and outbound port options now that namespaces are set up. */ =09optind =3D 1; =09do { -=09=09struct port_fwd *fwd =3D NULL; +=09=09struct port_fwd *fwd; =20 =09=09name =3D getopt_long(argc, argv, optstring, options, NULL); -=09=09switch (name) { -=09=09case 't': -=09=09case 'u': -=09=09case 'T': -=09=09case 'U': -=09=09=09if (name =3D=3D 't') -=09=09=09=09fwd =3D &c->tcp.fwd_in; -=09=09=09else if (name =3D=3D 'T') -=09=09=09=09fwd =3D &c->tcp.fwd_out; -=09=09=09else if (name =3D=3D 'u') -=09=09=09=09fwd =3D &c->udp.fwd_in.f; -=09=09=09else if (name =3D=3D 'U') -=09=09=09=09fwd =3D &c->udp.fwd_out.f; =20 +=09=09if ((name =3D=3D 'T' && (fwd =3D &c->tcp.fwd_out)) || +=09=09 (name =3D=3D 'U' && (fwd =3D &c->udp.fwd_out.f))) { =09=09=09if (!optarg || conf_ports(c, name, optarg, fwd)) =09=09=09=09usage(argv[0]); - -=09=09=09break; -=09=09default: -=09=09=09break; =09=09} =09} 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:=09=09Execution context - * - * Return: -EIO if sockets couldn't be set up, 0 otherwise + * @ns:=09=09Get socket in namespace, not in init */ -int nl_sock_init(const struct ctx *c) +void nl_sock_init(const struct ctx *c, bool ns) { -=09if (c->mode =3D=3D MODE_PASTA) { +=09if (c->mode =3D=3D MODE_PASTA && ns) { =09=09NS_CALL(nl_sock_init_do, c); =09=09if (nl_sock_ns =3D=3D -1) -=09=09=09return -EIO; +=09=09=09goto fail; =09} else { =09=09nl_sock_init_do(NULL); =09} =20 =09if (nl_sock =3D=3D -1) -=09=09return -EIO; +=09=09goto fail; =20 -=09return 0; +=09return; + +fail: +=09err("Failed to get netlink socket"); +=09exit(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 Stefano