| Message ID | 20230305121817.2234734-6-jeremy@azazel.net |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | Support for shifted port-ranges in NAT | expand |
Jeremy Sowden <jeremy@azazel.net> wrote: > index 5c29915ab028..0517a3efb259 100644 > --- a/net/netfilter/nft_nat.c > +++ b/net/netfilter/nft_nat.c > @@ -25,6 +25,7 @@ struct nft_nat { > u8 sreg_addr_max; > u8 sreg_proto_min; > u8 sreg_proto_max; > + u8 sreg_proto_base; > enum nf_nat_manip_type type:8; > u8 family; > u16 flags; > @@ -58,6 +59,8 @@ static void nft_nat_setup_proto(struct nf_nat_range2 *range, > nft_reg_load16(®s->data[priv->sreg_proto_min]); > range->max_proto.all = (__force __be16) > nft_reg_load16(®s->data[priv->sreg_proto_max]); > + range->base_proto.all = (__force __be16) > + nft_reg_load16(®s->data[priv->sreg_proto_base]); Hmmm! See below. > - plen = sizeof_field(struct nf_nat_range, min_proto.all); > + plen = sizeof_field(struct nf_nat_range2, min_proto.all); > if (tb[NFTA_NAT_REG_PROTO_MIN]) { > err = nft_parse_register_load(tb[NFTA_NAT_REG_PROTO_MIN], > &priv->sreg_proto_min, plen); > @@ -239,6 +243,16 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr, > plen); > if (err < 0) > return err; > + > + if (tb[NFTA_NAT_REG_PROTO_BASE]) { > + err = nft_parse_register_load > + (tb[NFTA_NAT_REG_PROTO_BASE], > + &priv->sreg_proto_base, plen); > + if (err < 0) > + return err; > + > + priv->flags |= NF_NAT_RANGE_PROTO_OFFSET; So sreg_proto_base is only set if tb[NFTA_NAT_REG_PROTO_BASE] gets passed. So, I would expect that all accesses to priv->sreg_proto_base are guarded with a 'if (priv->sreg_proto_base)' check. > @@ -286,7 +300,9 @@ static int nft_nat_dump(struct sk_buff *skb, > if (nft_dump_register(skb, NFTA_NAT_REG_PROTO_MIN, > priv->sreg_proto_min) || > nft_dump_register(skb, NFTA_NAT_REG_PROTO_MAX, > - priv->sreg_proto_max)) > + priv->sreg_proto_max) || > + nft_dump_register(skb, NFTA_NAT_REG_PROTO_BASE, > + priv->sreg_proto_base)) sreg_proto_min/max are only dumped when set, so NFTA_NAT_REG_PROTO_BASE should not be dumped unconditionally either?
On 2023-03-07, at 13:27:51 +0100, Florian Westphal wrote: > Jeremy Sowden <jeremy@azazel.net> wrote: > > index 5c29915ab028..0517a3efb259 100644 > > --- a/net/netfilter/nft_nat.c > > +++ b/net/netfilter/nft_nat.c > > @@ -25,6 +25,7 @@ struct nft_nat { > > u8 sreg_addr_max; > > u8 sreg_proto_min; > > u8 sreg_proto_max; > > + u8 sreg_proto_base; > > enum nf_nat_manip_type type:8; > > u8 family; > > u16 flags; > > @@ -58,6 +59,8 @@ static void nft_nat_setup_proto(struct nf_nat_range2 *range, > > nft_reg_load16(®s->data[priv->sreg_proto_min]); > > range->max_proto.all = (__force __be16) > > nft_reg_load16(®s->data[priv->sreg_proto_max]); > > + range->base_proto.all = (__force __be16) > > + nft_reg_load16(®s->data[priv->sreg_proto_base]); > > Hmmm! See below. > > > - plen = sizeof_field(struct nf_nat_range, min_proto.all); > > + plen = sizeof_field(struct nf_nat_range2, min_proto.all); > > if (tb[NFTA_NAT_REG_PROTO_MIN]) { > > err = nft_parse_register_load(tb[NFTA_NAT_REG_PROTO_MIN], > > &priv->sreg_proto_min, plen); > > @@ -239,6 +243,16 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr, > > plen); > > if (err < 0) > > return err; > > + > > + if (tb[NFTA_NAT_REG_PROTO_BASE]) { > > + err = nft_parse_register_load > > + (tb[NFTA_NAT_REG_PROTO_BASE], > > + &priv->sreg_proto_base, plen); > > + if (err < 0) > > + return err; > > + > > + priv->flags |= NF_NAT_RANGE_PROTO_OFFSET; > > So sreg_proto_base is only set if tb[NFTA_NAT_REG_PROTO_BASE] gets > passed. > > So, I would expect that all accesses to priv->sreg_proto_base are > guarded with a 'if (priv->sreg_proto_base)' check. > > > @@ -286,7 +300,9 @@ static int nft_nat_dump(struct sk_buff *skb, > > if (nft_dump_register(skb, NFTA_NAT_REG_PROTO_MIN, > > priv->sreg_proto_min) || > > nft_dump_register(skb, NFTA_NAT_REG_PROTO_MAX, > > - priv->sreg_proto_max)) > > + priv->sreg_proto_max) || > > + nft_dump_register(skb, NFTA_NAT_REG_PROTO_BASE, > > + priv->sreg_proto_base)) > > sreg_proto_min/max are only dumped when set, so NFTA_NAT_REG_PROTO_BASE > should not be dumped unconditionally either? > Agreed. Will fix. J.
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h index ff677f3a6cad..af6032720c78 100644 --- a/include/uapi/linux/netfilter/nf_tables.h +++ b/include/uapi/linux/netfilter/nf_tables.h @@ -1432,6 +1432,7 @@ enum nft_nat_types { * @NFTA_NAT_REG_PROTO_MIN: source register of proto range start (NLA_U32: nft_registers) * @NFTA_NAT_REG_PROTO_MAX: source register of proto range end (NLA_U32: nft_registers) * @NFTA_NAT_FLAGS: NAT flags (see NF_NAT_RANGE_* in linux/netfilter/nf_nat.h) (NLA_U32) + * @NFTA_NAT_REG_PROTO_BASE: source register of proto range base offset (NLA_U32: nft_registers) */ enum nft_nat_attributes { NFTA_NAT_UNSPEC, @@ -1442,6 +1443,7 @@ enum nft_nat_attributes { NFTA_NAT_REG_PROTO_MIN, NFTA_NAT_REG_PROTO_MAX, NFTA_NAT_FLAGS, + NFTA_NAT_REG_PROTO_BASE, __NFTA_NAT_MAX }; #define NFTA_NAT_MAX (__NFTA_NAT_MAX - 1) diff --git a/net/netfilter/nft_nat.c b/net/netfilter/nft_nat.c index 5c29915ab028..0517a3efb259 100644 --- a/net/netfilter/nft_nat.c +++ b/net/netfilter/nft_nat.c @@ -25,6 +25,7 @@ struct nft_nat { u8 sreg_addr_max; u8 sreg_proto_min; u8 sreg_proto_max; + u8 sreg_proto_base; enum nf_nat_manip_type type:8; u8 family; u16 flags; @@ -58,6 +59,8 @@ static void nft_nat_setup_proto(struct nf_nat_range2 *range, nft_reg_load16(®s->data[priv->sreg_proto_min]); range->max_proto.all = (__force __be16) nft_reg_load16(®s->data[priv->sreg_proto_max]); + range->base_proto.all = (__force __be16) + nft_reg_load16(®s->data[priv->sreg_proto_base]); } static void nft_nat_setup_netmap(struct nf_nat_range2 *range, @@ -126,13 +129,14 @@ static void nft_nat_eval(const struct nft_expr *expr, } static const struct nla_policy nft_nat_policy[NFTA_NAT_MAX + 1] = { - [NFTA_NAT_TYPE] = { .type = NLA_U32 }, - [NFTA_NAT_FAMILY] = { .type = NLA_U32 }, - [NFTA_NAT_REG_ADDR_MIN] = { .type = NLA_U32 }, - [NFTA_NAT_REG_ADDR_MAX] = { .type = NLA_U32 }, - [NFTA_NAT_REG_PROTO_MIN] = { .type = NLA_U32 }, - [NFTA_NAT_REG_PROTO_MAX] = { .type = NLA_U32 }, - [NFTA_NAT_FLAGS] = { .type = NLA_U32 }, + [NFTA_NAT_TYPE] = { .type = NLA_U32 }, + [NFTA_NAT_FAMILY] = { .type = NLA_U32 }, + [NFTA_NAT_REG_ADDR_MIN] = { .type = NLA_U32 }, + [NFTA_NAT_REG_ADDR_MAX] = { .type = NLA_U32 }, + [NFTA_NAT_REG_PROTO_MIN] = { .type = NLA_U32 }, + [NFTA_NAT_REG_PROTO_MAX] = { .type = NLA_U32 }, + [NFTA_NAT_REG_PROTO_BASE] = { .type = NLA_U32 }, + [NFTA_NAT_FLAGS] = { .type = NLA_U32 }, }; static int nft_nat_validate(const struct nft_ctx *ctx, @@ -195,10 +199,10 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr, switch (family) { case NFPROTO_IPV4: - alen = sizeof_field(struct nf_nat_range, min_addr.ip); + alen = sizeof_field(struct nf_nat_range2, min_addr.ip); break; case NFPROTO_IPV6: - alen = sizeof_field(struct nf_nat_range, min_addr.ip6); + alen = sizeof_field(struct nf_nat_range2, min_addr.ip6); break; default: if (tb[NFTA_NAT_REG_ADDR_MIN]) @@ -226,7 +230,7 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr, priv->flags |= NF_NAT_RANGE_MAP_IPS; } - plen = sizeof_field(struct nf_nat_range, min_proto.all); + plen = sizeof_field(struct nf_nat_range2, min_proto.all); if (tb[NFTA_NAT_REG_PROTO_MIN]) { err = nft_parse_register_load(tb[NFTA_NAT_REG_PROTO_MIN], &priv->sreg_proto_min, plen); @@ -239,6 +243,16 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr, plen); if (err < 0) return err; + + if (tb[NFTA_NAT_REG_PROTO_BASE]) { + err = nft_parse_register_load + (tb[NFTA_NAT_REG_PROTO_BASE], + &priv->sreg_proto_base, plen); + if (err < 0) + return err; + + priv->flags |= NF_NAT_RANGE_PROTO_OFFSET; + } } else { priv->sreg_proto_max = priv->sreg_proto_min; } @@ -286,7 +300,9 @@ static int nft_nat_dump(struct sk_buff *skb, if (nft_dump_register(skb, NFTA_NAT_REG_PROTO_MIN, priv->sreg_proto_min) || nft_dump_register(skb, NFTA_NAT_REG_PROTO_MAX, - priv->sreg_proto_max)) + priv->sreg_proto_max) || + nft_dump_register(skb, NFTA_NAT_REG_PROTO_BASE, + priv->sreg_proto_base)) goto nla_put_failure; }
Commit 2eb0f624b709 ("netfilter: add NAT support for shifted portmap ranges") introduced support for shifting port-ranges in NAT. This allows one to redirect packets intended for one port to another in a range in such a way that the new port chosen has the same offset in the range as the original port had from a specified base value. For example, by using the base value 2000, one could redirect packets intended for 10.0.0.1:2000-3000 to 10.10.0.1:12000-13000 so that the old and new ports were at the same offset in their respective ranges, i.e.: 10.0.0.1:2345 -> 10.10.0.1:12345 However, while support for this was added to the common NAT infra- structure, only the xt_nat module was updated to make use of it. This commit updates the nft_nat module to allow shifted port-ranges to be used by nftables. In contrast to xt_nat, where shifting is only available for DNAT, both DNAT and SNAT are supported. Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=970672 Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1501 Signed-off-by: Jeremy Sowden <jeremy@azazel.net> --- include/uapi/linux/netfilter/nf_tables.h | 2 ++ net/netfilter/nft_nat.c | 38 +++++++++++++++++------- 2 files changed, 29 insertions(+), 11 deletions(-)