netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net/ipv6/netfilter/ip6t_NPT: rewrite addresses in ICMPv6 original packet
@ 2020-07-20 13:17 Michael Zhou
  2020-07-29 20:43 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Zhou @ 2020-07-20 13:17 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Michael Zhou

Detect and rewrite a prefix embedded in an ICMPv6 original packet that was
rewritten by a corresponding DNPT/SNPT rule so it will be recognised by
the host that sent the original packet.

Example

Rules in effect on the 1:2:3:4::/64 + 5:6:7:8::/64 side router:
* SNPT src-pfx 1:2:3:4::/64 dst-pfx 5:6:7:8::/64
* DNPT src-pfx 5:6:7:8::/64 dst-pfx 1:2:3:4::/64

No rules on the 9:a:b:c::/64 side.

1. 1:2:3:4::1 sends UDP packet to 9:a:b:c::1
2. Router applies SNPT changing src to 5:6:7:8::ffef::1
3. 9:a:b:c::1 receives packet with (src 5:6:7:8::ffef::1 dst 9:a:b:c::1)
	and replies with ICMPv6 port unreachable to 5:6:7:8::ffef::1,
	including original packet (src 5:6:7:8::ffef::1 dst 9:a:b:c::1)
4. Router forwards ICMPv6 packet with (src 9:a:b:c::1 dst 5:6:7:8::ffef::1)
	including original packet (src 5:6:7:8::ffef::1 dst 9:a:b:c::1)
	and applies DNPT changing dst to 1:2:3:4::1
5. 1:2:3:4::1 receives ICMPv6 packet with (src 9:a:b:c::1 dst 1:2:3:4::1)
	including original packet (src 5:6:7:8::ffef::1 dst 9:a:b:c::1).
	It doesn't recognise the original packet as the src doesn't
	match anything it originally sent

With this change, at step 4, DNPT will also rewrite the original packet
src to 1:2:3:4::1, so at step 5, 1:2:3:4::1 will recognise the ICMPv6
error and provide feedback to the application properly.

Conversely, SNPT will help when ICMPv6 errors are sent from the
translated network.

1. 9:a:b:c::1 sends UDP packet to 5:6:7:8::ffef::1
2. Router applies DNPT changing dst to 1:2:3:4::1
3. 1:2:3:4::1 receives packet with (src 9:a:b:c::1 dst 1:2:3:4::1)
	and replies with ICMPv6 port unreachable to 9:a:b:c::1
	including original packet (src 9:a:b:c::1 dst 1:2:3:4::1)
4. Router forwards ICMPv6 packet with (src 1:2:3:4::1 dst 9:a:b:c::1)
	including original packet (src 9:a:b:c::1 dst 1:2:3:4::1)
	and applies SNPT changing src to 5:6:7:8::ffef::1
5. 9:a:b:c::1 receives ICMPv6 packet with
	(src 5:6:7:8::ffef::1 dst 9:a:b:c::1) including
	original packet (src 9:a:b:c::1 dst 1:2:3:4::1).
	It doesn't recognise the original packet as the dst doesn't
	match anything it already sent

The change to SNPT means the ICMPv6 original packet dst will be
rewritten to 5:6:7:8::ffef::1 in step 4, allowing the error to be
properly recognised in step 5.

Signed-off-by: Michael Zhou <mzhou@cse.unsw.edu.au>
---
 net/ipv6/netfilter/ip6t_NPT.c | 37 +++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/net/ipv6/netfilter/ip6t_NPT.c b/net/ipv6/netfilter/ip6t_NPT.c
index 9ee077bf4f49..b25e786607ed 100644
--- a/net/ipv6/netfilter/ip6t_NPT.c
+++ b/net/ipv6/netfilter/ip6t_NPT.c
@@ -77,16 +77,42 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 	return true;
 }
 
+static struct ipv6hdr *ip6t_npt_icmpv6_bounced_ipv6hdr(struct sk_buff *skb)
+{
+	if (ipv6_hdr(skb)->nexthdr != IPPROTO_ICMPV6)
+		return NULL;
+
+	if (!icmpv6_is_err(icmp6_hdr(skb)->icmp6_type))
+		return NULL;
+
+	if ((const unsigned char *)&icmp6_hdr(skb)[1] + sizeof(struct ipv6hdr) >
+			skb_tail_pointer(skb))
+		return NULL;
+
+	return (struct ipv6hdr *)&icmp6_hdr(skb)[1];
+}
+
 static unsigned int
 ip6t_snpt_tg(struct sk_buff *skb, const struct xt_action_param *par)
 {
 	const struct ip6t_npt_tginfo *npt = par->targinfo;
+	struct ipv6hdr *bounced_hdr;
+	struct in6_addr bounced_pfx;
 
 	if (!ip6t_npt_map_pfx(npt, &ipv6_hdr(skb)->saddr)) {
 		icmpv6_send(skb, ICMPV6_PARAMPROB, ICMPV6_HDR_FIELD,
 			    offsetof(struct ipv6hdr, saddr));
 		return NF_DROP;
 	}
+
+	/* rewrite dst addr of bounced packet which was sent to dst range */
+	bounced_hdr = ip6t_npt_icmpv6_bounced_ipv6hdr(skb);
+	if (bounced_hdr) {
+		ipv6_addr_prefix(&bounced_pfx, &bounced_hdr->daddr, npt->src_pfx_len);
+		if (ipv6_addr_cmp(&bounced_pfx, &npt->src_pfx.in6) == 0)
+			ip6t_npt_map_pfx(npt, &bounced_hdr->daddr);
+	}
+
 	return XT_CONTINUE;
 }
 
@@ -94,12 +120,23 @@ static unsigned int
 ip6t_dnpt_tg(struct sk_buff *skb, const struct xt_action_param *par)
 {
 	const struct ip6t_npt_tginfo *npt = par->targinfo;
+	struct ipv6hdr *bounced_hdr;
+	struct in6_addr bounced_pfx;
 
 	if (!ip6t_npt_map_pfx(npt, &ipv6_hdr(skb)->daddr)) {
 		icmpv6_send(skb, ICMPV6_PARAMPROB, ICMPV6_HDR_FIELD,
 			    offsetof(struct ipv6hdr, daddr));
 		return NF_DROP;
 	}
+
+	/* rewrite src addr of bounced packet which was sent from dst range */
+	bounced_hdr = ip6t_npt_icmpv6_bounced_ipv6hdr(skb);
+	if (bounced_hdr) {
+		ipv6_addr_prefix(&bounced_pfx, &bounced_hdr->saddr, npt->src_pfx_len);
+		if (ipv6_addr_cmp(&bounced_pfx, &npt->src_pfx.in6) == 0)
+			ip6t_npt_map_pfx(npt, &bounced_hdr->saddr);
+	}
+
 	return XT_CONTINUE;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] net/ipv6/netfilter/ip6t_NPT: rewrite addresses in ICMPv6 original packet
  2020-07-20 13:17 [PATCH] net/ipv6/netfilter/ip6t_NPT: rewrite addresses in ICMPv6 original packet Michael Zhou
@ 2020-07-29 20:43 ` Pablo Neira Ayuso
  2020-07-30 22:14   ` TEST " Michael Zhou
  0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2020-07-29 20:43 UTC (permalink / raw)
  To: Michael Zhou; +Cc: netfilter-devel

Hi,

On Mon, Jul 20, 2020 at 11:17:01PM +1000, Michael Zhou wrote:
> Detect and rewrite a prefix embedded in an ICMPv6 original packet that was
> rewritten by a corresponding DNPT/SNPT rule so it will be recognised by
> the host that sent the original packet.

Thanks for submitting your patch, a few comments below.

> Example
> 
> Rules in effect on the 1:2:3:4::/64 + 5:6:7:8::/64 side router:
> * SNPT src-pfx 1:2:3:4::/64 dst-pfx 5:6:7:8::/64
> * DNPT src-pfx 5:6:7:8::/64 dst-pfx 1:2:3:4::/64
> 
> No rules on the 9:a:b:c::/64 side.
> 
> 1. 1:2:3:4::1 sends UDP packet to 9:a:b:c::1
> 2. Router applies SNPT changing src to 5:6:7:8::ffef::1
> 3. 9:a:b:c::1 receives packet with (src 5:6:7:8::ffef::1 dst 9:a:b:c::1)
> 	and replies with ICMPv6 port unreachable to 5:6:7:8::ffef::1,
> 	including original packet (src 5:6:7:8::ffef::1 dst 9:a:b:c::1)
> 4. Router forwards ICMPv6 packet with (src 9:a:b:c::1 dst 5:6:7:8::ffef::1)
> 	including original packet (src 5:6:7:8::ffef::1 dst 9:a:b:c::1)
> 	and applies DNPT changing dst to 1:2:3:4::1
> 5. 1:2:3:4::1 receives ICMPv6 packet with (src 9:a:b:c::1 dst 1:2:3:4::1)
> 	including original packet (src 5:6:7:8::ffef::1 dst 9:a:b:c::1).
> 	It doesn't recognise the original packet as the src doesn't
> 	match anything it originally sent
> 
> With this change, at step 4, DNPT will also rewrite the original packet
> src to 1:2:3:4::1, so at step 5, 1:2:3:4::1 will recognise the ICMPv6
> error and provide feedback to the application properly.
> 
> Conversely, SNPT will help when ICMPv6 errors are sent from the
> translated network.
> 
> 1. 9:a:b:c::1 sends UDP packet to 5:6:7:8::ffef::1
> 2. Router applies DNPT changing dst to 1:2:3:4::1
> 3. 1:2:3:4::1 receives packet with (src 9:a:b:c::1 dst 1:2:3:4::1)
> 	and replies with ICMPv6 port unreachable to 9:a:b:c::1
> 	including original packet (src 9:a:b:c::1 dst 1:2:3:4::1)
> 4. Router forwards ICMPv6 packet with (src 1:2:3:4::1 dst 9:a:b:c::1)
> 	including original packet (src 9:a:b:c::1 dst 1:2:3:4::1)
> 	and applies SNPT changing src to 5:6:7:8::ffef::1
> 5. 9:a:b:c::1 receives ICMPv6 packet with
> 	(src 5:6:7:8::ffef::1 dst 9:a:b:c::1) including
> 	original packet (src 9:a:b:c::1 dst 1:2:3:4::1).
> 	It doesn't recognise the original packet as the dst doesn't
> 	match anything it already sent
> 
> The change to SNPT means the ICMPv6 original packet dst will be
> rewritten to 5:6:7:8::ffef::1 in step 4, allowing the error to be
> properly recognised in step 5.
> 
> Signed-off-by: Michael Zhou <mzhou@cse.unsw.edu.au>
> ---
>  net/ipv6/netfilter/ip6t_NPT.c | 37 +++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/net/ipv6/netfilter/ip6t_NPT.c b/net/ipv6/netfilter/ip6t_NPT.c
> index 9ee077bf4f49..b25e786607ed 100644
> --- a/net/ipv6/netfilter/ip6t_NPT.c
> +++ b/net/ipv6/netfilter/ip6t_NPT.c
> @@ -77,16 +77,42 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
>  	return true;
>  }
>  
> +static struct ipv6hdr *ip6t_npt_icmpv6_bounced_ipv6hdr(struct sk_buff *skb)
> +{
> +	if (ipv6_hdr(skb)->nexthdr != IPPROTO_ICMPV6)
> +		return NULL;
> +
> +	if (!icmpv6_is_err(icmp6_hdr(skb)->icmp6_type))
> +		return NULL;
> +
> +	if ((const unsigned char *)&icmp6_hdr(skb)[1] + sizeof(struct ipv6hdr) >
> +			skb_tail_pointer(skb))
> +		return NULL;
> +
> +	return (struct ipv6hdr *)&icmp6_hdr(skb)[1];

This ICMPv6 header might fall withing the non-linear data of the
skbuff.

BTW, does rfc6296 describes what to do with icmp traffic?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* TEST Re: [PATCH] net/ipv6/netfilter/ip6t_NPT: rewrite addresses in ICMPv6 original packet
  2020-07-29 20:43 ` Pablo Neira Ayuso
@ 2020-07-30 22:14   ` Michael Zhou
  2020-07-30 22:28     ` Florian Westphal
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Zhou @ 2020-07-30 22:14 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Thanks for the comments.

On Wed, 29 Jul 2020 22:43:23 +0200
Pablo Neira Ayuso <pablo@netfilter.org> wrote:

> This ICMPv6 header might fall withing the non-linear data of the
> skbuff.

Might you be able to point me to an example of how to handle and test
this? So far in my testing it has always been in the linear data.

> BTW, does rfc6296 describes what to do with icmp traffic?

Unfortunately not. Do you think this functionality should be an
optional flag or be part of a different target to maintain conformance
with the RFC?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: TEST Re: [PATCH] net/ipv6/netfilter/ip6t_NPT: rewrite addresses in ICMPv6 original packet
  2020-07-30 22:14   ` TEST " Michael Zhou
@ 2020-07-30 22:28     ` Florian Westphal
  0 siblings, 0 replies; 4+ messages in thread
From: Florian Westphal @ 2020-07-30 22:28 UTC (permalink / raw)
  To: Michael Zhou; +Cc: Pablo Neira Ayuso, netfilter-devel

Michael Zhou <mzhou@cse.unsw.edu.au> wrote:
> Thanks for the comments.
> 
> On Wed, 29 Jul 2020 22:43:23 +0200
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> 
> > This ICMPv6 header might fall withing the non-linear data of the
> > skbuff.
> 
> Might you be able to point me to an example of how to handle and test
> this? So far in my testing it has always been in the linear data.

Look at skb_header_pointer() function and other users of it.

> > BTW, does rfc6296 describes what to do with icmp traffic?
> 
> Unfortunately not. Do you think this functionality should be an
> optional flag or be part of a different target to maintain conformance
> with the RFC?

Handling it automatically seems sane to me.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-07-30 22:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-20 13:17 [PATCH] net/ipv6/netfilter/ip6t_NPT: rewrite addresses in ICMPv6 original packet Michael Zhou
2020-07-29 20:43 ` Pablo Neira Ayuso
2020-07-30 22:14   ` TEST " Michael Zhou
2020-07-30 22:28     ` Florian Westphal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).