From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=R3bOlv9K; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id 5149F5A0265 for ; Thu, 16 Apr 2026 00:05:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1776290703; 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=lnUOmuhbCpM/tRBknDJgGkNMYiirQlTBOyVPXf0js+8=; b=R3bOlv9K8cHuI2LxOR787s62wKjl2kom0mgS9mIY0eB0ERaHLWlUz+8DVKhC0NseXcOO95 A+c3k6KynYKrbF5exturHvzhLn4Y+MQAayhAQcRUH5zf69TxuBMn/KE3jThGVMaiZoFav6 NDLXyroDkbPmU6qpcqwEf8pS40Pn8Ms= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-96-fSIOpiZbPeeuimFQZP6XOQ-1; Wed, 15 Apr 2026 18:05:02 -0400 X-MC-Unique: fSIOpiZbPeeuimFQZP6XOQ-1 X-Mimecast-MFC-AGG-ID: fSIOpiZbPeeuimFQZP6XOQ_1776290701 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-488c768a9a9so37876505e9.1 for ; Wed, 15 Apr 2026 15:05:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776290700; x=1776895500; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=lnUOmuhbCpM/tRBknDJgGkNMYiirQlTBOyVPXf0js+8=; b=H+2nHi0Luziay5oEKzuwzSP2RYe/P93G5jGEb5JAHwXTDt6oXDPNGG9x37ILdpCs2B Xqy5Ygq6so5XpSXYb1PQmhM03HSh7NvC2OjwGr8wdJene8RciaCKGqFub/WurjT10rsZ R/5bSDTUnHQaMEHiETc+047o9bYxRB/51fuXF3XAs92EnnEPwH9Rh3+PFtDycLxDXRhc jjCib4urCYZJf0wHL3Ovi0x/7avYVMd0igz0yOQGzpzkUBnt21+SAWdQrMvX1SfDTap6 amZj0MWS0gilVN4wiSGQWL/z/No93e5ggBuw+MWhk/EkxNj+eiFS0+bdT8EHpuRuzXwe ABDA== X-Gm-Message-State: AOJu0YwNLjSLy4OjYJ1cRSM4zwglArA0FnOVjonGZfFz4EcUr/EK0UEq mieMJv3JdfOFmU8oXh+HK4cMBQe5mceQZMOVGVyXAsHwMXfb9jxvO+EmDKWDNKYhEVGlL9DWioc F26+gkjlEQPTgm7s2Or1H91585F5IdDvvRE3aGy3DVUnc46A4L+7LVHp/QhRfJQ== X-Gm-Gg: AeBDiesCeXetcimo3DGD1AoNJXTHeL+Fuc3LKi4LH1LJ0RaL+nAD9hbBxvx7RCeKliw MKotov+H//sQzqSRF7dd0aIYbpbpmGoKTJYd2yc6pWkgHCnebZpGiaq/PVezH2TZgXZl0D72Lva SdhdlXfP3TpSlByfz0he9HXm8FCaVN9cU4aAPzbMkrpbhGDCmu5A+3L+Gn1QBhILM8MatrYhc4x seTxrzHN2Jhbhyp6HpZ1rKb+DlMwsegPRFN9ivuRRXofbsl8jSa0j9GknJm+yKMnHdolFdUQ9yb gqtWAAv1MY5G64DtK3GXxU4AJe5dXwFHrI31uI7A/6dlFCAPypm3cCxNlYk6qWsrj/HVpOiqFvO bRC3wI40j4MbOHDeWNMi8pd0uOM9pAcOo X-Received: by 2002:a05:600c:628b:b0:487:1fbf:e0bb with SMTP id 5b1f17b1804b1-488d680851fmr310401485e9.6.1776290699924; Wed, 15 Apr 2026 15:04:59 -0700 (PDT) X-Received: by 2002:a05:600c:628b:b0:487:1fbf:e0bb with SMTP id 5b1f17b1804b1-488d680851fmr310401315e9.6.1776290699455; Wed, 15 Apr 2026 15:04:59 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-488f58165e1sm1048035e9.2.2026.04.15.15.04.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Apr 2026 15:04:58 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2 21/23] conf, fwd: Stricter rule checking in fwd_rule_add() Message-ID: <20260416000457.55ecdbec@elisabeth> In-Reply-To: <20260410010309.736855-22-david@gibson.dropbear.id.au> References: <20260410010309.736855-1-david@gibson.dropbear.id.au> <20260410010309.736855-22-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Thu, 16 Apr 2026 00:04:58 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: PPBVnx7ntVBMPNnlmL7--7nVLkj88HHzo1qkphYRkRs_1776290701 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: NMJCALRPHO74XQGHFLYWSEQV4IIIXYN4 X-Message-ID-Hash: NMJCALRPHO74XQGHFLYWSEQV4IIIXYN4 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.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: On Fri, 10 Apr 2026 11:03:07 +1000 David Gibson wrote: > Although fwd_rule_add() performs some sanity checks on the rule it is > given, there are invalid rules we don't check for, assuming that its > callers will do that. > > That won't be enough when we can get rules inserted by a dynamic update > client without going through the existing parsing code. So, add stricter > checks to fwd_rule_add(), which is now possible thanks to the capabilities > bits in the struct fwd_table. Where those duplicate existing checks in the > callers, remove the old copies. > > Signed-off-by: David Gibson > --- > conf.c | 19 ------------------- > fwd.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++----- > 2 files changed, 46 insertions(+), 24 deletions(-) > > diff --git a/conf.c b/conf.c > index b7a4459e..31627209 100644 > --- a/conf.c > +++ b/conf.c > @@ -310,10 +310,6 @@ static void conf_ports_spec(struct fwd_table *fwd, uint8_t proto, > if (p != ep) /* Garbage after the ranges */ > goto bad; > > - if (orig_range.first == 0) { > - die("Can't forward port 0 included in '%s'", spec); > - } > - > conf_ports_range_except(fwd, proto, addr, ifname, > orig_range.first, orig_range.last, > exclude, > @@ -356,11 +352,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) > return; > } > > - if (proto == IPPROTO_TCP && !(fwd->caps & FWD_CAP_TCP)) > - die("TCP port forwarding requested but TCP is disabled"); > - if (proto == IPPROTO_UDP && !(fwd->caps & FWD_CAP_UDP)) > - die("UDP port forwarding requested but UDP is disabled"); > - > strncpy(buf, optarg, sizeof(buf) - 1); > > if ((spec = strchr(buf, '/'))) { > @@ -405,16 +396,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) > addr = NULL; > } > > - if (addr) { > - if (!(fwd->caps & FWD_CAP_IPV4) && inany_v4(addr)) { > - die("IPv4 is disabled, can't use -%c %s", > - optname, optarg); > - } else if (!(fwd->caps & FWD_CAP_IPV6) && !inany_v4(addr)) { > - die("IPv6 is disabled, can't use -%c %s", > - optname, optarg); > - } > - } > - > if (optname == 'T' || optname == 'U') { > assert(!addr && !ifname); > > diff --git a/fwd.c b/fwd.c > index c7fd1a9d..98b04d0c 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -367,17 +367,58 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) > new->first, new->last); > return -EINVAL; > } > + if (!new->first) { > + warn("Forwarding rule atttempts to map from port 0"); > + return -EINVAL; > + } > + if (!new->to || (new->to + new->last - new->first) < new->to) { > + warn("Forwarding rule atttempts to map to port 0"); Nit: attempts > + return -EINVAL; > + } > if (new->flags & ~allowed_flags) { > warn("Rule has invalid flags 0x%hhx", > new->flags & ~allowed_flags); > return -EINVAL; > } > - if (new->flags & FWD_DUAL_STACK_ANY && > - !inany_equals(&new->addr, &inany_any6)) { > - char astr[INANY_ADDRSTRLEN]; > + if (new->flags & FWD_DUAL_STACK_ANY) { > + if (!inany_equals(&new->addr, &inany_any6)) { > + char astr[INANY_ADDRSTRLEN]; > > - warn("Dual stack rule has non-wildcard address %s", > - inany_ntop(&new->addr, astr, sizeof(astr))); > + warn("Dual stack rule has non-wildcard address %s", > + inany_ntop(&new->addr, astr, sizeof(astr))); > + return -EINVAL; > + } > + if (!(fwd->caps & FWD_CAP_IPV4)) { > + warn("Dual stack forward, but IPv4 not enabled"); > + return -EINVAL; > + } > + if (!(fwd->caps & FWD_CAP_IPV6)) { > + warn("Dual stack forward, but IPv6 not enabled"); > + return -EINVAL; > + } > + } else { > + if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) { > + warn("IPv4 forward, but IPv4 not enabled"); > + return -EINVAL; > + } > + if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) { > + warn("IPv6 forward, but IPv6 not enabled"); > + return -EINVAL; > + } > + } > + if (new->proto == IPPROTO_TCP) { > + if (!(fwd->caps & FWD_CAP_TCP)) { > + warn("Can't add TCP forwarding rule, TCP not enabled"); > + return -EINVAL; > + } > + } else if (new->proto == IPPROTO_UDP) { > + if (!(fwd->caps & FWD_CAP_UDP)) { > + warn("Can't add UDP forwarding rule, UDP not enabled"); > + return -EINVAL; > + } > + } else { > + warn("Unsupported protocol 0x%hhx (%s) for forwarding rule", > + new->proto, ipproto_name(new->proto)); > return -EINVAL; > } > -- Stefano