On Wed, Mar 19, 2025 at 12:18:51PM +0100, Florian Westphal wrote: > We can't remove 'meta nfproto' dependencies for all cases, e.g. inet. > > meta nfproto ipv4 ct protocol tcp > > is listed as 'ct protocol tcp'. > > meta l4proto removal checks were correct, but refactor this into a helper > function to split meta/ct checks from the caller. > > We need to examine ct keys more closely to figure out if the dependency > can be inferred or not: only NFT_CT_SRC/DST and its variants imply the > network protocol to use. All other keys must keep the l3 dependency. > > Also extend test coverage. > > Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1783 > Signed-off-by: Florian Westphal <fw@xxxxxxxxx> Reviewed-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> nitpick: s/dependeny/dependency, mangle before applying, thanks. Thanks. > --- > 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 6793464859ca..022b4282c520 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 ae14065c00d6..ab8254e2a99c 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 5312b328aa91..f2dbba89a703 100644 > --- a/tests/py/inet/ct.t > +++ b/tests/py/inet/ct.t > @@ -3,11 +3,16 @@ > > *inet;test-inet;input > > +# dependeny 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 > > +# dependeny 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 223ac9e7575f..155eecc5fa08 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 f7a2ef27274a..216dad2bb531 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.48.1 > >
