]> git.ipfire.org Git - thirdparty/iptables.git/commitdiff
nft: Fix for useless meta expressions in rule
authorPhil Sutter <phil@nwl.cc>
Wed, 6 Sep 2023 14:32:47 +0000 (16:32 +0200)
committerPhil Sutter <phil@nwl.cc>
Thu, 14 Sep 2023 10:20:11 +0000 (12:20 +0200)
A relict of legacy iptables' mandatory matching on interfaces and IP
addresses is support for the '-i +' notation, basically a "match any
input interface". Trying to make things better than its predecessor,
iptables-nft boldly optimizes that nop away - not entirely though, the
meta expression loading the interface name was left in place. While not
a problem (apart from pointless overhead) in current HEAD, v1.8.7 would
trip over this as a following cmp expression (for another match) was
incorrectly linked to that stale meta expression, loading strange values
into the respective interface name field.

While being at it, merge and generalize the functions into a common one
for use with ebtables' NFT_META_BRI_(I|O)IFNAME matches, too.

Fixes: 0a8635183edd0 ("xtables-compat: ignore '+' interface name")
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1702
Signed-off-by: Phil Sutter <phil@nwl.cc>
extensions/libebt_standard.t
extensions/libip6t_standard.t
extensions/libxt_standard.t
iptables/nft-arp.c
iptables/nft-bridge.c
iptables/nft-ipv4.c
iptables/nft-ipv6.c
iptables/nft-shared.c
iptables/nft-shared.h

index 97cb3baaf6d21548c573f987e27061f2461df5b8..370a12f4a2cec5184b2aedc59e9c8d383bde6425 100644 (file)
 -o foobar;=;FAIL
 --logical-in br0;=;OK
 --logical-out br1;=;FAIL
+-i + -d 00:0f:ee:d0:ba:be;-d 00:0f:ee:d0:ba:be;OK
+-i + -p ip;-p IPv4;OK
+--logical-in + -d 00:0f:ee:d0:ba:be;-d 00:0f:ee:d0:ba:be;OK
+--logical-in + -p ip;-p IPv4;OK
 :FORWARD
 -i foobar;=;OK
 -o foobar;=;OK
index a528af10ea152b71f26f36aef0387286ebb9bff8..0c559cc5021f6f9d7eb1f34cf38287bc5cb0aba7 100644 (file)
@@ -3,3 +3,6 @@
 ! -d ::;! -d ::/128;OK
 ! -s ::;! -s ::/128;OK
 -s ::/64;=;OK
+:INPUT
+-i + -d c0::fe;-d c0::fe/128;OK
+-i + -p tcp;-p tcp;OK
index 6ed978e442b80fe4ec57beaef63fb236e2f7b6d6..7c83cfa3ba232ab2fe24a2664604b213c20c065f 100644 (file)
@@ -24,3 +24,5 @@
 :FORWARD
 --protocol=tcp --source=1.2.3.4 --destination=5.6.7.8/32 --in-interface=eth0 --out-interface=eth1 --jump=ACCEPT;-s 1.2.3.4/32 -d 5.6.7.8/32 -i eth0 -o eth1 -p tcp -j ACCEPT;OK
 -ptcp -s1.2.3.4 -d5.6.7.8/32 -ieth0 -oeth1 -jACCEPT;-s 1.2.3.4/32 -d 5.6.7.8/32 -i eth0 -o eth1 -p tcp -j ACCEPT;OK
+-i + -d 1.2.3.4;-d 1.2.3.4/32;OK
+-i + -p tcp;-p tcp;OK
index 9868966a0368860bd4ae3327b36c617d6e1d8cbd..aed39ebdd516677d8cb4c46955d9bf7081994d64 100644 (file)
@@ -49,12 +49,12 @@ static int nft_arp_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
 
        if (fw->arp.iniface[0] != '\0') {
                op = nft_invflags2cmp(fw->arp.invflags, IPT_INV_VIA_IN);
-               add_iniface(h, r, fw->arp.iniface, op);
+               add_iface(h, r, fw->arp.iniface, NFT_META_IIFNAME, op);
        }
 
        if (fw->arp.outiface[0] != '\0') {
                op = nft_invflags2cmp(fw->arp.invflags, IPT_INV_VIA_OUT);
-               add_outiface(h, r, fw->arp.outiface, op);
+               add_iface(h, r, fw->arp.outiface, NFT_META_OIFNAME, op);
        }
 
        if (fw->arp.arhrd != 0 ||
index 391a8ab723c1cb173fd3aeddc0cf0a825a9b804c..d9a8ad2b0f373a6c8bee85afb81af403b54642af 100644 (file)
@@ -65,36 +65,6 @@ static void ebt_print_mac_and_mask(const unsigned char *mac, const unsigned char
                xtables_print_mac_and_mask(mac, mask);
 }
 
-static void add_logical_iniface(struct nft_handle *h, struct nftnl_rule *r,
-                               char *iface, uint32_t op)
-{
-       int iface_len;
-       uint8_t reg;
-
-       iface_len = strlen(iface);
-
-       add_meta(h, r, NFT_META_BRI_IIFNAME, &reg);
-       if (iface[iface_len - 1] == '+')
-               add_cmp_ptr(r, op, iface, iface_len - 1, reg);
-       else
-               add_cmp_ptr(r, op, iface, iface_len + 1, reg);
-}
-
-static void add_logical_outiface(struct nft_handle *h, struct nftnl_rule *r,
-                                char *iface, uint32_t op)
-{
-       int iface_len;
-       uint8_t reg;
-
-       iface_len = strlen(iface);
-
-       add_meta(h, r, NFT_META_BRI_OIFNAME, &reg);
-       if (iface[iface_len - 1] == '+')
-               add_cmp_ptr(r, op, iface, iface_len - 1, reg);
-       else
-               add_cmp_ptr(r, op, iface, iface_len + 1, reg);
-}
-
 static int add_meta_broute(struct nftnl_rule *r)
 {
        struct nftnl_expr *expr;
@@ -180,22 +150,22 @@ static int nft_bridge_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
 
        if (fw->in[0] != '\0') {
                op = nft_invflags2cmp(fw->invflags, EBT_IIN);
-               add_iniface(h, r, fw->in, op);
+               add_iface(h, r, fw->in, NFT_META_IIFNAME, op);
        }
 
        if (fw->out[0] != '\0') {
                op = nft_invflags2cmp(fw->invflags, EBT_IOUT);
-               add_outiface(h, r, fw->out, op);
+               add_iface(h, r, fw->out, NFT_META_OIFNAME, op);
        }
 
        if (fw->logical_in[0] != '\0') {
                op = nft_invflags2cmp(fw->invflags, EBT_ILOGICALIN);
-               add_logical_iniface(h, r, fw->logical_in, op);
+               add_iface(h, r, fw->logical_in, NFT_META_BRI_IIFNAME, op);
        }
 
        if (fw->logical_out[0] != '\0') {
                op = nft_invflags2cmp(fw->invflags, EBT_ILOGICALOUT);
-               add_logical_outiface(h, r, fw->logical_out, op);
+               add_iface(h, r, fw->logical_out, NFT_META_BRI_OIFNAME, op);
        }
 
        if ((fw->bitmask & EBT_NOPROTO) == 0) {
index 2f10220edd509c61047e2903066b60b03172cdee..75912847aea3e2d6c4bcbc87c21de4031cbd5b65 100644 (file)
@@ -51,12 +51,12 @@ static int nft_ipv4_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
 
        if (cs->fw.ip.iniface[0] != '\0') {
                op = nft_invflags2cmp(cs->fw.ip.invflags, IPT_INV_VIA_IN);
-               add_iniface(h, r, cs->fw.ip.iniface, op);
+               add_iface(h, r, cs->fw.ip.iniface, NFT_META_IIFNAME, op);
        }
 
        if (cs->fw.ip.outiface[0] != '\0') {
                op = nft_invflags2cmp(cs->fw.ip.invflags, IPT_INV_VIA_OUT);
-               add_outiface(h, r, cs->fw.ip.outiface, op);
+               add_iface(h, r, cs->fw.ip.outiface, NFT_META_OIFNAME, op);
        }
 
        if (cs->fw.ip.proto != 0) {
index d53f87c1d26e33fc2e1022d8cb70bc6ca872ca65..5aef365b79f2a7dd7d3c7b289319aa4e34013f3e 100644 (file)
@@ -54,12 +54,12 @@ static int nft_ipv6_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
 
        if (cs->fw6.ipv6.iniface[0] != '\0') {
                op = nft_invflags2cmp(cs->fw6.ipv6.invflags, IPT_INV_VIA_IN);
-               add_iniface(h, r, cs->fw6.ipv6.iniface, op);
+               add_iface(h, r, cs->fw6.ipv6.iniface, NFT_META_IIFNAME, op);
        }
 
        if (cs->fw6.ipv6.outiface[0] != '\0') {
                op = nft_invflags2cmp(cs->fw6.ipv6.invflags, IPT_INV_VIA_OUT);
-               add_outiface(h, r, cs->fw6.ipv6.outiface, op);
+               add_iface(h, r, cs->fw6.ipv6.outiface, NFT_META_OIFNAME, op);
        }
 
        if (cs->fw6.ipv6.proto != 0) {
index 34ca9d16569d0a329095518c964d9bbb8466c9c5..6775578b1e36b233b4087502edba5358e9176ccb 100644 (file)
@@ -147,44 +147,29 @@ void add_cmp_u32(struct nftnl_rule *r, uint32_t val, uint32_t op, uint8_t sreg)
        add_cmp_ptr(r, op, &val, sizeof(val), sreg);
 }
 
-void add_iniface(struct nft_handle *h, struct nftnl_rule *r,
-                char *iface, uint32_t op)
+void add_iface(struct nft_handle *h, struct nftnl_rule *r,
+              char *iface, uint32_t key, uint32_t op)
 {
-       int iface_len;
+       int iface_len = strlen(iface);
        uint8_t reg;
 
-       iface_len = strlen(iface);
 
-       add_meta(h, r, NFT_META_IIFNAME, &reg);
        if (iface[iface_len - 1] == '+') {
-               if (iface_len > 1)
-                       add_cmp_ptr(r, op, iface, iface_len - 1, reg);
-               else if (op != NFT_CMP_EQ)
-                       add_cmp_ptr(r, NFT_CMP_EQ, "INVAL/D",
-                                   strlen("INVAL/D") + 1, reg);
+               if (iface_len > 1) {
+                       iface_len -= 1;
+               } else if (op != NFT_CMP_EQ) {
+                       op = NFT_CMP_EQ;
+                       iface = "INVAL/D";
+                       iface_len = strlen(iface) + 1;
+               } else {
+                       return; /* -o + */
+               }
        } else {
-               add_cmp_ptr(r, op, iface, iface_len + 1, reg);
+               iface_len += 1;
        }
-}
-
-void add_outiface(struct nft_handle *h, struct nftnl_rule *r,
-                 char *iface, uint32_t op)
-{
-       int iface_len;
-       uint8_t reg;
 
-       iface_len = strlen(iface);
-
-       add_meta(h, r, NFT_META_OIFNAME, &reg);
-       if (iface[iface_len - 1] == '+') {
-               if (iface_len > 1)
-                       add_cmp_ptr(r, op, iface, iface_len - 1, reg);
-               else if (op != NFT_CMP_EQ)
-                       add_cmp_ptr(r, NFT_CMP_EQ, "INVAL/D",
-                                   strlen("INVAL/D") + 1, reg);
-       } else {
-               add_cmp_ptr(r, op, iface, iface_len + 1, reg);
-       }
+       add_meta(h, r, key, &reg);
+       add_cmp_ptr(r, op, iface, iface_len, reg);
 }
 
 void add_addr(struct nft_handle *h, struct nftnl_rule *r,
index 4f47058d2ec5cb821179fe0b7ba83ac85a5a12e2..51d1e4609a3b63d983e49f8410edab58eb7620e0 100644 (file)
@@ -95,8 +95,8 @@ void add_cmp_ptr(struct nftnl_rule *r, uint32_t op, void *data, size_t len, uint
 void add_cmp_u8(struct nftnl_rule *r, uint8_t val, uint32_t op, uint8_t sreg);
 void add_cmp_u16(struct nftnl_rule *r, uint16_t val, uint32_t op, uint8_t sreg);
 void add_cmp_u32(struct nftnl_rule *r, uint32_t val, uint32_t op, uint8_t sreg);
-void add_iniface(struct nft_handle *h, struct nftnl_rule *r, char *iface, uint32_t op);
-void add_outiface(struct nft_handle *h, struct nftnl_rule *r, char *iface, uint32_t op);
+void add_iface(struct nft_handle *h, struct nftnl_rule *r,
+              char *iface, uint32_t key, uint32_t op);
 void add_addr(struct nft_handle *h, struct nftnl_rule *r, enum nft_payload_bases base, int offset,
              void *data, void *mask, size_t len, uint32_t op);
 void add_proto(struct nft_handle *h, struct nftnl_rule *r, int offset, size_t len,