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 67EED5A027B for ; Wed, 14 Feb 2024 12:09:55 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707908994; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BaFFFauo1VNSxfbPXbfjEPofC9MrnP3nihMbUCL1GrU=; b=U7imzJgX9i58DqMu7qdF1gVmP9zNif1MgsUXztwR04Ov7uj4rFoVe++DLrNsjPRb/hrOky mdZqkajL7iwKAn9AjPi25d+y9a/51S27mWYmWLUe1LV6jBray3GrJTX/aAJG5JmX49Aq5j /Gd9qpJrFhIQLlCfa6PNSJqj8DojMRs= Received: from mail-lf1-f71.google.com (mail-lf1-f71.google.com [209.85.167.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-648-VMYxjKy7MC6ZChWlR4OUrA-1; Wed, 14 Feb 2024 06:09:52 -0500 X-MC-Unique: VMYxjKy7MC6ZChWlR4OUrA-1 Received: by mail-lf1-f71.google.com with SMTP id 2adb3069b0e04-5118731c991so2558339e87.2 for ; Wed, 14 Feb 2024 03:09:52 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707908991; x=1708513791; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=BaFFFauo1VNSxfbPXbfjEPofC9MrnP3nihMbUCL1GrU=; b=WZf5n1N5Wd/5NGVmrh+TcLtUjdFkIqodg1PfRS8S5U6qMcPUjWeUGw4ro8ipXwAUD0 Vu/hMRrMiZK8OiU6qZdlyrT4KNrYi6o3mBSj9K4nxOdXjKREngu30rpMHlFe7ye0nx4l nD0ZTsXPN2hP8zsZG0E474FKhPOEuvafH8npeTR7RcldsC5lf6Dn+5CHZYJ79pcxr2nV yNWaH+Bndjn0Ha2SsH04U4JCFKkvZgRzADjlN9SUZ86UD68mGbVRKBPZ9Ei+RW9yn/wZ TvdpSSaZlaE1iwVavYZi2IJwNWUjypVFRi/IG2SyN1Hgv5Y+2WRPRB+Hwr2El75N5XPU M3eA== X-Forwarded-Encrypted: i=1; AJvYcCUtAM9ZbJwbERr6CYGcDUQ93mIUVZep5IcDPN9nmMKovzdtHUkF+MGSRSyRXpqtafwWI0Q/LJq+w4KEdGdzjdMlLkiO X-Gm-Message-State: AOJu0YwktF7hpdxPF26lNeKQ2fr9m/2yHy/+PG24Zt5gyYrnSogi2xU4 KdDqYDYhF+WrEyFfHgA5uOTfG8B0kjqaNLyXLYV8hUvit+S6fujTS0bwbZ4Pc1V5CqE7GGjVdcN GVhE8RvNDZ+rkF6oBeZGU75W978OGzCYhdq1om5Huscz5h9VshA== X-Received: by 2002:a05:6512:b84:b0:511:ac56:2595 with SMTP id b4-20020a0565120b8400b00511ac562595mr844089lfv.2.1707908991153; Wed, 14 Feb 2024 03:09:51 -0800 (PST) X-Google-Smtp-Source: AGHT+IG1ikWDozn7PLssRxZMxGh5Rbi4hSI0k9xNnFaO9XchOpQTaQk+31Jd2Zg60k28KmGox3FCng== X-Received: by 2002:a05:6512:b84:b0:511:ac56:2595 with SMTP id b4-20020a0565120b8400b00511ac562595mr844066lfv.2.1707908990693; Wed, 14 Feb 2024 03:09:50 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCUoG28WiSZfN8sYssywshFl+pXGYIOndjIeJ242aKXSpR0BEGy68UrO2j7Is1sG6PRXfTqOfvdE8tDjhKPLlfccEbBL Received: from [192.168.188.25] ([80.243.52.136]) by smtp.gmail.com with ESMTPSA id ck5-20020a0564021c0500b00561a727f1bfsm3048497edb.29.2024.02.14.03.09.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 14 Feb 2024 03:09:50 -0800 (PST) Message-ID: Date: Wed, 14 Feb 2024 12:09:49 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] conf, passt.1: Exit if we can't bind a forwarded port, except for -[tu] all To: Stefano Brivio , passt-dev@passt.top References: <20240214091538.3995295-1-sbrivio@redhat.com> From: Paul Holzinger In-Reply-To: <20240214091538.3995295-1-sbrivio@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Message-ID-Hash: TEVNHXDORRD7QN7UPAYKXVS4MO3QCOQR X-Message-ID-Hash: TEVNHXDORRD7QN7UPAYKXVS4MO3QCOQR X-MailFrom: pholzing@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 X-Mailman-Version: 3.3.8 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: Thanks Stefano, On 14/02/2024 10:15, Stefano Brivio wrote: > ...or similar, that is, if only excluded ranges are given (implying > we'll forward any other available port). In that case, we'll usually > forward large sets of ports, and it might be inconvenient for the > user to skip excluding single ports that are already taken. > > The existing behaviour, that is, exiting only if we fail to bind all > the ports for one given forwarding option, turns out to be > problematic for several aspects raised by Paul: > > - Podman merges ranges anyway, so we might fail to bind all the ports > from a specific range given by the user, but we'll not fail anyway > because Podman merges it with another one where we succeed to bind > at least one port. At the same time, there should be no semantic > difference between multiple ranges given by a single option and > multiple ranges given as multiple options: it's unexpected and > not documented > > - the user might actually rely on a given port to be forwarded to a > given container or a virtual machine, and if connections are > forwarded to an unrelated process, this might raise security > concerns > > - given that we can try and fail to bind multiple ports before > exiting (in case we can't bind any), we don't have a specific error > code we can return to the user, so we don't give the user helpful > indication as to why we couldn't bind ports. > > Exit as soon as we fail to create or bind a socket for a given > forwarded port, and report the actual error. > > Keep the current behaviour, however, in case the user wants to > forward all the (available) ports for a given protocol, or all the > ports with excluded ranges only. There, it's more reasonable that > the user is expecting partial failures, and it's probably convenient > that we continue with the ports we could forward. > > Update the manual page to reflect the new behaviour, and the old > behaviour too in the cases where we keep it. > > Suggested-by: Paul Holzinger > Link: https://github.com/containers/podman/pull/21563#issuecomment-1937024642 > Signed-off-by: Stefano Brivio Tested-by: Paul Holzinger > --- > conf.c | 36 +++++++++++------------------------- > passt.1 | 15 ++++++++++++--- > 2 files changed, 23 insertions(+), 28 deletions(-) > > diff --git a/conf.c b/conf.c > index 5e15b66..f68f5d3 100644 > --- a/conf.c > +++ b/conf.c > @@ -119,6 +119,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > bool exclude_only = true, bound_one = false; > uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; > sa_family_t af = AF_UNSPEC; > + unsigned i; > int ret; > > if (!strcmp(optarg, "none")) { > @@ -141,8 +142,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > } > > if (!strcmp(optarg, "all")) { > - unsigned i; > - > if (fwd->mode) > goto mode_conflict; > > @@ -171,7 +170,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > } > > if (!bound_one) > - goto bind_fail; > + goto bind_all_fail; > > return; > } > @@ -221,7 +220,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > p = spec; > do { > struct port_range xrange; > - unsigned i; > > if (*p != '~') { > /* Not an exclude range, parse later */ > @@ -244,8 +242,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > } while ((p = next_chunk(p, ','))); > > if (exclude_only) { > - unsigned i; > - > for (i = 0; i < PORT_EPHEMERAL_MIN; i++) { > if (bitmap_isset(exclude, i)) > continue; > @@ -271,7 +267,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > } > > if (!bound_one) > - goto bind_fail; > + goto bind_all_fail; > > return; > } > @@ -280,7 +276,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > p = spec; > do { > struct port_range orig_range, mapped_range; > - unsigned i; > > if (*p == '~') > /* Exclude range, already parsed */ > @@ -314,28 +309,16 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > > fwd->delta[i] = mapped_range.first - orig_range.first; > > - if (optname == 't') { > + ret = 0; > + if (optname == 't') > ret = tcp_sock_init(c, af, addr, ifname, i); > - if (ret == -ENFILE || ret == -EMFILE) > - goto enfile; > - if (!ret) > - bound_one = true; > - } else if (optname == 'u') { > + else if (optname == 'u') > ret = udp_sock_init(c, 0, af, addr, ifname, i); > - if (ret == -ENFILE || ret == -EMFILE) > - goto enfile; > - if (!ret) > - bound_one = true; > - } else { > - /* No way to check in advance for -T and -U */ > - bound_one = true; > - } > + if (ret) > + goto bind_fail; > } > } while ((p = next_chunk(p, ','))); > > - if (!bound_one) > - goto bind_fail; > - > return; > enfile: > die("Can't open enough sockets for port specifier: %s", optarg); > @@ -344,6 +327,9 @@ bad: > mode_conflict: > die("Port forwarding mode '%s' conflicts with previous mode", optarg); > bind_fail: > + die("Failed to bind port %u (%s) for option '-%c %s', exiting", > + i, strerror(-ret), optname, optarg); > +bind_all_fail: > die("Failed to bind any port for '-%c %s', exiting", optname, optarg); > } > > diff --git a/passt.1 b/passt.1 > index cc678ed..dc2d719 100644 > --- a/passt.1 > +++ b/passt.1 > @@ -355,7 +355,8 @@ Don't forward any ports > .TP > .BR all > Forward all unbound, non-ephemeral ports, as permitted by current capabilities. > -For low (< 1024) ports, see \fBNOTES\fR. > +For low (< 1024) ports, see \fBNOTES\fR. No failures are reported for > +unavailable ports, unless no ports could be forwarded at all. > > .TP > .BR ports > @@ -364,7 +365,11 @@ optionally, with target ports after \fI:\fR, if they differ. Specific addresses > can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited > to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports > and ranges can be excluded by an additional specification prefixed by \fI~\fR. > -Specifying excluded ranges only implies that all other ports are forwarded. > + > +Specifying excluded ranges only implies that all other ports are forwarded. In > +this case, no failures are reported for unavailable ports, unless no ports could > +be forwarded at all. > + > Examples: > .RS > .TP > @@ -447,7 +452,11 @@ optionally, with target ports after \fI:\fR, if they differ. Specific addresses > can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited > to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports > and ranges can be excluded by an additional specification prefixed by \fI~\fR. > -Specifying excluded ranges only implies that all other ports are forwarded. > + > +Specifying excluded ranges only implies that all other ports are forwarded. In > +this case, no failures are reported for unavailable ports, unless no ports could > +be forwarded at all. > + > Examples: > .RS > .TP