From 58ca69f04b3fb73aa8cc580a46de957400ad9064 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Sun, 16 Mar 2025 14:10:26 +0100 Subject: [PATCH] netlink_delinerize: add more restrictions on meta nfproto removal commit 7b3ee497040ff8efb131c566e1c6b466e16f45cc upstream. 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 --- src/ct.c | 4 ++ src/netlink_delinearize.c | 82 +++++++++++++++++++++++++++++--------- tests/py/inet/ct.t | 5 +++ tests/py/inet/ct.t.json | 51 ++++++++++++++++++++++++ tests/py/inet/ct.t.payload | 14 +++++++ 5 files changed, 138 insertions(+), 18 deletions(-) diff --git a/src/ct.c b/src/ct.c index a17d848fc..9541d54f4 100644 --- a/src/ct.c +++ b/src/ct.c @@ -449,7 +449,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 8fba23535..990032f73 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -2070,17 +2070,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; @@ -2100,23 +2154,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; } @@ -2141,8 +2187,8 @@ static void ct_meta_common_postprocess(struct rule_pp_ctx *ctx, if (base < PROTO_BASE_TRANSPORT_HDR) { if (payload_dependency_exists(&ctx->pdctx, base) && - meta_may_dependency_kill(&ctx->pdctx, - ctx->pctx.family, expr)) + ct_meta_may_dependency_kill(&ctx->pdctx, + ctx->pctx.family, expr)) payload_dependency_release(&ctx->pdctx, base); if (left->flags & EXPR_F_PROTOCOL) diff --git a/tests/py/inet/ct.t b/tests/py/inet/ct.t index 5312b328a..8a7b1555b 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 223ac9e75..155eecc5f 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 f7a2ef272..216dad2bb 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 ] -- 2.47.2