From: Florian Westphal Date: Sun, 16 Mar 2025 13:10:26 +0000 (+0100) Subject: netlink_delinerize: add more restrictions on meta nfproto removal X-Git-Tag: v1.1.2~42 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7b3ee497040ff8efb131c566e1c6b466e16f45cc;p=thirdparty%2Fnftables.git 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 Reviewed-by: Pablo Neira Ayuso --- diff --git a/src/ct.c b/src/ct.c index 4d71a4b0..71ebb948 100644 --- 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 ae1ee53f..f7c10fb5 100644 --- 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 5312b328..8a7b1555 100644 --- 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 223ac9e7..155eecc5 100644 --- 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 f7a2ef27..216dad2b 100644 --- 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 ]