netlink_delinerize: add more restrictions on meta nfproto removal

We can't remove 'meta nfproto' dependencies for all cases.
Its removed for ip/ip6 families, this works fine.

But for others, e.g. inet, removal is not as simple.
For example

   meta nfproto ipv4 ct protocol tcp

is listed as 'ct protocol tcp', even when this is uses in the inet
table.

Meta L4PROTO removal checks were correct, but refactor this
into a helper function to split meta/ct checks from the common
calling function.

Ct check was lacking, we need to examine ct keys more closely
to figure out if they need to retain the network protocol depenency
or not.  Elide for NFT_CT_SRC/DST and its variants, as those imply
the network protocol to use, all others must keep it as-is.

Also extend test coverage for this.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1783
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/ct.c b/src/ct.c
index 4d71a4b0103b9080be4e0af959364e2467164523..71ebb948389361282960cd243887459307595ccb 100644 (file)
--- a/src/ct.c
+++ b/src/ct.c
@@ -456,7 +456,11 @@ struct expr *ct_expr_alloc(const struct location *loc, enum nft_ct_keys key,
 
        switch (key) {
        case NFT_CT_SRC:
+       case NFT_CT_SRC_IP:
+       case NFT_CT_SRC_IP6:
        case NFT_CT_DST:
+       case NFT_CT_DST_IP:
+       case NFT_CT_DST_IP6:
                expr->ct.base = PROTO_BASE_NETWORK_HDR;
                break;
        case NFT_CT_PROTO_SRC:

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index ae1ee53f6e7c6a2d5452c83baebe0bae1b36dd2d..f7c10fb5af316a1a85c55c37eeadbe84e26c0422 100644 (file)
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2250,17 +2250,71 @@ static bool __meta_dependency_may_kill(const struct expr *dep, uint8_t *nfproto)
        return false;
 }
 
+static bool ct_may_dependency_kill(unsigned int meta_nfproto,
+                                  const struct expr *ct)
+{
+       assert(ct->etype == EXPR_CT);
+
+       switch (ct->ct.key) {
+       case NFT_CT_DST:
+       case NFT_CT_SRC:
+               switch (ct->len) {
+               case 32:
+                       return meta_nfproto == NFPROTO_IPV4;
+               case 128:
+                       return meta_nfproto == NFPROTO_IPV6;
+               default:
+                       break;
+               }
+               return false;
+       case NFT_CT_DST_IP:
+       case NFT_CT_SRC_IP:
+               return meta_nfproto == NFPROTO_IPV4;
+       case NFT_CT_DST_IP6:
+       case NFT_CT_SRC_IP6:
+               return meta_nfproto == NFPROTO_IPV6;
+       default:
+               break;
+       }
+
+       return false;
+}
+
+static bool meta_may_dependency_kill(uint8_t nfproto, const struct expr *meta, const struct expr *v)
+{
+       uint8_t l4proto;
+
+       if (meta->meta.key != NFT_META_L4PROTO)
+               return true;
+
+       if (v->etype != EXPR_VALUE || v->len != 8)
+               return false;
+
+       l4proto = mpz_get_uint8(v->value);
+
+       switch (l4proto) {
+       case IPPROTO_ICMP:
+               return nfproto == NFPROTO_IPV4;
+       case IPPROTO_ICMPV6:
+               return nfproto == NFPROTO_IPV6;
+       default:
+               break;
+       }
+
+       return false;
+}
+
 /* We have seen a protocol key expression that restricts matching at the network
  * base, leave it in place since this is meaningful in bridge, inet and netdev
  * families. Exceptions are ICMP and ICMPv6 where this code assumes that can
  * only happen with IPv4 and IPv6.
  */
-static bool meta_may_dependency_kill(struct payload_dep_ctx *ctx,
+static bool ct_meta_may_dependency_kill(struct payload_dep_ctx *ctx,
                                     unsigned int family,
                                     const struct expr *expr)
 {
-       uint8_t l4proto, nfproto = NFPROTO_UNSPEC;
        struct expr *dep = payload_dependency_get(ctx, PROTO_BASE_NETWORK_HDR);
+       uint8_t nfproto = NFPROTO_UNSPEC;
 
        if (!dep)
                return true;
@@ -2280,23 +2334,15 @@ static bool meta_may_dependency_kill(struct payload_dep_ctx *ctx,
                return true;
        }
 
-       if (expr->left->meta.key != NFT_META_L4PROTO)
-               return true;
-
-       l4proto = mpz_get_uint8(expr->right->value);
-
-       switch (l4proto) {
-       case IPPROTO_ICMP:
-       case IPPROTO_ICMPV6:
-               break;
+       switch (expr->left->etype) {
+       case EXPR_META:
+               return meta_may_dependency_kill(nfproto, expr->left, expr->right);
+       case EXPR_CT:
+               return ct_may_dependency_kill(nfproto, expr->left);
        default:
-               return false;
+               break;
        }
 
-       if ((nfproto == NFPROTO_IPV4 && l4proto == IPPROTO_ICMPV6) ||
-           (nfproto == NFPROTO_IPV6 && l4proto == IPPROTO_ICMP))
-               return false;
-
        return true;
 }
 
@@ -2322,8 +2368,8 @@ static void ct_meta_common_postprocess(struct rule_pp_ctx *ctx,
 
                if (base < PROTO_BASE_TRANSPORT_HDR) {
                        if (payload_dependency_exists(&dl->pdctx, base) &&
-                           meta_may_dependency_kill(&dl->pdctx,
-                                                    dl->pctx.family, expr))
+                           ct_meta_may_dependency_kill(&dl->pdctx,
+                                                       dl->pctx.family, expr))
                                payload_dependency_release(&dl->pdctx, base);
 
                        if (left->flags & EXPR_F_PROTOCOL)

diff --git a/tests/py/inet/ct.t b/tests/py/inet/ct.t
index 5312b328aa9180ded59bca2c20445fcf75abefbe..8a7b1555bf40f618f6b1db6a8a16e2fdcb3f4071 100644 (file)
--- a/tests/py/inet/ct.t
+++ b/tests/py/inet/ct.t
@@ -3,11 +3,16 @@
 
 *inet;test-inet;input
 
+# dependency should be removed
 meta nfproto ipv4 ct original saddr 1.2.3.4;ok;ct original ip saddr 1.2.3.4
 ct original ip6 saddr ::1;ok
 
 ct original ip daddr 1.2.3.4 accept;ok
 
+# dependency must not be removed
+meta nfproto ipv4 ct mark 0x00000001;ok
+meta nfproto ipv6 ct protocol 6;ok
+
 # missing protocol context
 ct original saddr ::1;fail
 

diff --git a/tests/py/inet/ct.t.json b/tests/py/inet/ct.t.json
index 223ac9e7575f9ccbf6c2a8fdc4b6c60cc05ab032..155eecc5fa086e0a4322fbdb4fd607a24d513c96 100644 (file)
--- a/tests/py/inet/ct.t.json
+++ b/tests/py/inet/ct.t.json
@@ -58,3 +58,54 @@
     }
 ]
 
+# meta nfproto ipv4 ct mark 0x00000001
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "nfproto"
+                }
+            },
+            "op": "==",
+            "right": "ipv4"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "ct": {
+                    "key": "mark"
+                }
+            },
+            "op": "==",
+            "right": 1
+        }
+    }
+]
+
+# meta nfproto ipv6 ct protocol 6
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "nfproto"
+                }
+            },
+            "op": "==",
+            "right": "ipv6"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "ct": {
+                    "key": "protocol"
+                }
+            },
+            "op": "==",
+            "right": 6
+        }
+    }
+]

diff --git a/tests/py/inet/ct.t.payload b/tests/py/inet/ct.t.payload
index f7a2ef27274a3e201896da3252df2ab88b39adb3..216dad2bb531c40d86e713465afc287cbc021e49 100644 (file)
--- a/tests/py/inet/ct.t.payload
+++ b/tests/py/inet/ct.t.payload
@@ -15,3 +15,17 @@ inet test-inet input
   [ ct load dst_ip => reg 1 , dir original ]
   [ cmp eq reg 1 0x04030201 ]
   [ immediate reg 0 accept ]
+
+# meta nfproto ipv4 ct mark 0x00000001
+inet test-inet input
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ ct load mark => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+
+# meta nfproto ipv6 ct protocol 6
+inet test-inet input
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ ct load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]