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=JsvqW/gZ; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 635B95A026E for ; Mon, 20 Apr 2026 18:49:01 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1776703738; 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:autocrypt:autocrypt; bh=UnqD37QqFMO7S4xXFAicDwx4FQydH+uR+VbqNLiPUhI=; b=JsvqW/gZclzJ3LwUZQ4mLChW2AMtoAAanhKY0R4tY+fq5ZiWKQNbgw/we0+uPiZislLT+6 1AqmV2C6wKxbp1AkcLyd/r1Tf96i50FyeoE4XWLKCtbgHNZoFH4wjIMl9pci4I66Zba7hZ H2YIQaKroPgN+oGkNG9m9Bh+UTeco7w= Received: from mail-pl1-f198.google.com (mail-pl1-f198.google.com [209.85.214.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-433-aE4iZxJaOvGYf-efAT27zg-1; Mon, 20 Apr 2026 12:48:56 -0400 X-MC-Unique: aE4iZxJaOvGYf-efAT27zg-1 X-Mimecast-MFC-AGG-ID: aE4iZxJaOvGYf-efAT27zg_1776703736 Received: by mail-pl1-f198.google.com with SMTP id d9443c01a7336-2b45cd0bb96so33361625ad.3 for ; Mon, 20 Apr 2026 09:48:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776703736; x=1777308536; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:to:subject:user-agent:mime-version:date :message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=UnqD37QqFMO7S4xXFAicDwx4FQydH+uR+VbqNLiPUhI=; b=qFx53NqX9zjfuzWQc+UdNQhp5+41Vm42sR/j5DdTf1oouuFhtCa7BxNyOTsc/cXKoA 9ZiRGVkEdewaAqT0Xx8DCzjd0wOdoVq2Nxv9nB22n3/Q7b7iD97QMJ5C6DF4G0pPp4xD jDBHB3tYFoxRhs5hlAEpVBbHN0K8pEq5PrYja+Rgc5BHjaZNWQszm8CjLurX9cN1So6z n5+/eZZ1Zk/JHv2bciNSGrR8iQAjl6Wj01J2l/QVRZof0coq6Q1i4bDVhxxLe3uFgG7r /Gll7OyhF2Giiw5ssglCPLJSxgJoXzummVtNZZant2+JGIkgkjmaK7FhZf0QRrdxhfp+ Ku7g== X-Forwarded-Encrypted: i=1; AFNElJ/+3H2td1yLhSxmMBqRBm54e5V9ZtarJR7ohPXauHQ+dBubpypbWoaIH95d2yJkfPt2lRHKE7+gVWQ=@passt.top X-Gm-Message-State: AOJu0Yy9fYYXjUUQ/0djrD2bh1qGEMDbuNpqFHOgcc6s7qI64z61HZoW olmnpeC7D7EczhPpwEDyNczPeAlCjFsJMo8mmgqg5chCnZQo+NeilWJROviZWe0QH88URlj6aCL RXC2ux1zMxw13i3fDKOaCjW9I6GbQX5GTwg0sCocOmNLRbwzIaiQIqxR/f2Q/Bg== X-Gm-Gg: AeBDiet+gts07cnoFUWMCcwcK+KAZ1h0OSDP8dIO+v5c88wLuirxZs210lutTWmWSr1 5LPY2MHqueJxkKftMDypf8aOIkOpiSLn2mtl3B4tpsvJ9fGct1nIQV+qvhDXaV6XpJqyGE/5sLV 7ruExgiQ2SCjFFVTAdN+wqLtZB6tV8kb9dj5sKFxwaKkwJ6thik73Ir6CRJ4lt+3zztp206/CyE 1mZelpld+X/Ztb36r37v36wucABYoC/lqKP/S/S2+JdCjHl/jIwiCwRynYr23ehbWsWWDnJhjol oABcJYD3pAHjJSoojNYRPVBvLDlsvRaMw12aOHwZfCZQ2fFI+00ggZq9OE9vdctXE12gTkBEZWK 6Ne8mUfwcdi6eyK8WX01E5Q+SHEsf248nDnUwEFGzI33lVaFRAFg2sU8ygs7/QigSMA== X-Received: by 2002:a17:903:1a88:b0:2ae:46b9:c653 with SMTP id d9443c01a7336-2b5f9f3c5b3mr159809335ad.33.1776703735500; Mon, 20 Apr 2026 09:48:55 -0700 (PDT) X-Received: by 2002:a17:903:1a88:b0:2ae:46b9:c653 with SMTP id d9443c01a7336-2b5f9f3c5b3mr159808925ad.33.1776703734828; Mon, 20 Apr 2026 09:48:54 -0700 (PDT) Received: from [192.168.100.100] (82-64-211-94.subs.proxad.net. [82.64.211.94]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2b5fab3bd2bsm114748525ad.74.2026.04.20.09.48.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 20 Apr 2026 09:48:54 -0700 (PDT) Message-ID: <10b179ed-fdab-4306-a92a-4dfb104a942f@redhat.com> Date: Mon, 20 Apr 2026 18:48:48 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 07/11] conf, fwd: Stricter rule checking in fwd_rule_add() To: David Gibson , passt-dev@passt.top, Stefano Brivio References: <20260417050520.102247-1-david@gibson.dropbear.id.au> <20260417050520.102247-8-david@gibson.dropbear.id.au> From: Laurent Vivier Autocrypt: addr=lvivier@redhat.com; keydata= xsFNBFYFJhkBEAC2me7w2+RizYOKZM+vZCx69GTewOwqzHrrHSG07MUAxJ6AY29/+HYf6EY2 WoeuLWDmXE7A3oJoIsRecD6BXHTb0OYS20lS608anr3B0xn5g0BX7es9Mw+hV/pL+63EOCVm SUVTEQwbGQN62guOKnJJJfphbbv82glIC/Ei4Ky8BwZkUuXd7d5NFJKC9/GDrbWdj75cDNQx UZ9XXbXEKY9MHX83Uy7JFoiFDMOVHn55HnncflUncO0zDzY7CxFeQFwYRbsCXOUL9yBtqLer Ky8/yjBskIlNrp0uQSt9LMoMsdSjYLYhvk1StsNPg74+s4u0Q6z45+l8RAsgLw5OLtTa+ePM JyS7OIGNYxAX6eZk1+91a6tnqfyPcMbduxyBaYXn94HUG162BeuyBkbNoIDkB7pCByed1A7q q9/FbuTDwgVGVLYthYSfTtN0Y60OgNkWCMtFwKxRaXt1WFA5ceqinN/XkgA+vf2Ch72zBkJL RBIhfOPFv5f2Hkkj0MvsUXpOWaOjatiu0fpPo6Hw14UEpywke1zN4NKubApQOlNKZZC4hu6/ 8pv2t4HRi7s0K88jQYBRPObjrN5+owtI51xMaYzvPitHQ2053LmgsOdN9EKOqZeHAYG2SmRW LOxYWKX14YkZI5j/TXfKlTpwSMvXho+efN4kgFvFmP6WT+tPnwARAQABzSNMYXVyZW50IFZp dmllciA8bHZpdmllckByZWRoYXQuY29tPsLBeAQTAQIAIgUCVgVQgAIbAwYLCQgHAwIGFQgC CQoLBBYCAwECHgECF4AACgkQ8ww4vT8vvjwpgg//fSGy0Rs/t8cPFuzoY1cex4limJQfReLr SJXCANg9NOWy/bFK5wunj+h/RCFxIFhZcyXveurkBwYikDPUrBoBRoOJY/BHK0iZo7/WQkur 6H5losVZtrotmKOGnP/lJYZ3H6OWvXzdz8LL5hb3TvGOP68K8Bn8UsIaZJoeiKhaNR0sOJyI YYbgFQPWMHfVwHD/U+/gqRhD7apVysxv5by/pKDln1I5v0cRRH6hd8M8oXgKhF2+rAOL7gvh jEHSSWKUlMjC7YwwjSZmUkL+TQyE18e2XBk85X8Da3FznrLiHZFHQ/NzETYxRjnOzD7/kOVy gKD/o7asyWQVU65mh/ECrtjfhtCBSYmIIVkopoLaVJ/kEbVJQegT2P6NgERC/31kmTF69vn8 uQyW11Hk8tyubicByL3/XVBrq4jZdJW3cePNJbTNaT0d/bjMg5zCWHbMErUib2Nellnbg6bc 2HLDe0NLVPuRZhHUHM9hO/JNnHfvgiRQDh6loNOUnm9Iw2YiVgZNnT4soUehMZ7au8PwSl4I KYE4ulJ8RRiydN7fES3IZWmOPlyskp1QMQBD/w16o+lEtY6HSFEzsK3o0vuBRBVp2WKnssVH qeeV01ZHw0bvWKjxVNOksP98eJfWLfV9l9e7s6TaAeySKRRubtJ+21PRuYAxKsaueBfUE7ZT 7zfOwU0EVgUmGQEQALxSQRbl/QOnmssVDxWhHM5TGxl7oLNJms2zmBpcmlrIsn8nNz0rRyxT 460k2niaTwowSRK8KWVDeAW6ZAaWiYjLlTunoKwvF8vP3JyWpBz0diTxL5o+xpvy/Q6YU3BN efdq8Vy3rFsxgW7mMSrI/CxJ667y8ot5DVugeS2NyHfmZlPGE0Nsy7hlebS4liisXOrN3jFz asKyUws3VXek4V65lHwB23BVzsnFMn/bw/rPliqXGcwl8CoJu8dSyrCcd1Ibs0/Inq9S9+t0 VmWiQWfQkz4rvEeTQkp/VfgZ6z98JRW7S6l6eophoWs0/ZyRfOm+QVSqRfFZdxdP2PlGeIFM C3fXJgygXJkFPyWkVElr76JTbtSHsGWbt6xUlYHKXWo+xf9WgtLeby3cfSkEchACrxDrQpj+ Jt/JFP+q997dybkyZ5IoHWuPkn7uZGBrKIHmBunTco1+cKSuRiSCYpBIXZMHCzPgVDjk4viP brV9NwRkmaOxVvye0vctJeWvJ6KA7NoAURplIGCqkCRwg0MmLrfoZnK/gRqVJ/f6adhU1oo6 z4p2/z3PemA0C0ANatgHgBb90cd16AUxpdEQmOCmdNnNJF/3Zt3inzF+NFzHoM5Vwq6rc1JP jfC3oqRLJzqAEHBDjQFlqNR3IFCIAo4SYQRBdAHBCzkM4rWyRhuVABEBAAHCwV8EGAECAAkF AlYFJhkCGwwACgkQ8ww4vT8vvjwg9w//VQrcnVg3TsjEybxDEUBm8dBmnKqcnTBFmxN5FFtI WlEuY8+YMiWRykd8Ln9RJ/98/ghABHz9TN8TRo2b6WimV64FmlVn17Ri6FgFU3xNt9TTEChq AcNg88eYryKsYpFwegGpwUlaUaaGh1m9OrTzcQy+klVfZWaVJ9Nw0keoGRGb8j4XjVpL8+2x OhXKrM1fzzb8JtAuSbuzZSQPDwQEI5CKKxp7zf76J21YeRrEW4WDznPyVcDTa+tz++q2S/Bp P4W98bXCBIuQgs2m+OflERv5c3Ojldp04/S4NEjXEYRWdiCxN7ca5iPml5gLtuvhJMSy36gl U6IW9kn30IWuSoBpTkgV7rLUEhh9Ms82VWW/h2TxL8enfx40PrfbDtWwqRID3WY8jLrjKfTd R3LW8BnUDNkG+c4FzvvGUs8AvuqxxyHbXAfDx9o/jXfPHVRmJVhSmd+hC3mcQ+4iX5bBPBPM oDqSoLt5w9GoQQ6gDVP2ZjTWqwSRMLzNr37rJjZ1pt0DCMMTbiYIUcrhX8eveCJtY7NGWNyx FCRkhxRuGcpwPmRVDwOl39MB3iTsRighiMnijkbLXiKoJ5CDVvX5yicNqYJPKh5MFXN1bvsB kmYiStMRbrD0HoY1kx5/VozBtc70OU0EB8Wrv9hZD+Ofp0T3KOr1RUHvCZoLURfFhSQ= In-Reply-To: <20260417050520.102247-8-david@gibson.dropbear.id.au> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: zk-ylbMcJn8rALP3Ww-hMSqGLubZa-zRUrai6SC1adE_1776703736 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: 5QE6HKTJRHHKMNL4TFET43T2UB3BIQNP X-Message-ID-Hash: 5QE6HKTJRHHKMNL4TFET43T2UB3BIQNP X-MailFrom: lvivier@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: On 4/17/26 07:05, 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 ecc3a342..3b373b22 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); > - } > - We remove the die() here but we keep the "assert(first != 0)" in conf_ports_range_except(), so the user can trigger it with "-t 0" before the call to fwd_rule_add(). > 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..aa966731 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 attempts to map from port 0"); > + return -EINVAL; > + } > + if (!new->to || (new->to + new->last - new->first) < new->to) { Why do we need the second part? We know new->first < new->last and this cannot overflow as values are uint16_t and arithmetic uses int. FWIW: (gdb) print (unsigned short)65535 $1 = 65535 (gdb) print (unsigned short)65536 $2 = 0 (gdb) print (unsigned short)65535 + (unsigned short)1 $3 = 65536 > + warn("Forwarding rule attempts to map to port 0"); > + 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; > } >