All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] netfilter: nf_conntrack_icmpv6: find conntrack related to ICMPv6 redirect packet
@ 2016-01-13 13:42 Alin Nastac
  2016-01-13 16:43 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 4+ messages in thread
From: Alin Nastac @ 2016-01-13 13:42 UTC (permalink / raw)
  To: netfilter-devel

---
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c | 77 ++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index 660bc10..699848a 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -144,7 +144,7 @@ static bool icmpv6_new(struct nf_conn *ct, const struct sk_buff *skb,
 static int
 icmpv6_error_message(struct net *net, struct nf_conn *tmpl,
 		     struct sk_buff *skb,
-		     unsigned int icmp6off,
+		     unsigned int inneripv6off,
 		     enum ip_conntrack_info *ctinfo,
 		     unsigned int hooknum)
 {
@@ -157,9 +157,7 @@ icmpv6_error_message(struct net *net, struct nf_conn *tmpl,
 
 	/* Are they talking about one of our connections? */
 	if (!nf_ct_get_tuplepr(skb,
-			       skb_network_offset(skb)
-				+ sizeof(struct ipv6hdr)
-				+ sizeof(struct icmp6hdr),
+			       inneripv6off,
 			       PF_INET6, net, &origtuple)) {
 		pr_debug("icmpv6_error: Can't get tuple\n");
 		return -NF_ACCEPT;
@@ -227,9 +225,78 @@ icmpv6_error(struct net *net, struct nf_conn *tmpl,
 		nf_conntrack_get(skb->nfct);
 		return NF_ACCEPT;
 	}
+	dataoff += sizeof(struct icmp6hdr);
 
 	/* is not error message ? */
-	if (icmp6h->icmp6_type >= 128)
+	if (icmp6h->icmp6_type == NDISC_REDIRECT) {
+		const struct in6_addr *dst;
+		struct in6_addr _dst;
+		const struct nd_opt_hdr *opt;
+		struct nd_opt_hdr _opt;
+		const struct ipv6hdr *iph;
+		struct ipv6hdr _iph;
+
+		/* skip target address */
+		dataoff += sizeof(_dst);
+
+		/* read destination address */
+		dst = skb_header_pointer(skb, dataoff, sizeof(_dst), &_dst);
+		if (dst == NULL) {
+			if (LOG_INVALID(net, IPPROTO_ICMPV6))
+			nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+				      "nf_ct_icmpv6: short redirect packet ");
+			return -NF_ACCEPT;
+		}
+		if (ipv6_addr_is_multicast(dst)) {
+			if (LOG_INVALID(net, IPPROTO_ICMPV6))
+			nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+				      "nf_ct_icmpv6: redirect destination address is multicast ");
+			return -NF_ACCEPT;
+		}
+		dataoff += sizeof(_dst);
+
+		/* find redirected header */
+		while (1) {
+			opt = skb_header_pointer(skb, dataoff, sizeof(_opt), &_opt);
+			if (opt == NULL) {
+				if (LOG_INVALID(net, IPPROTO_ICMPV6))
+				nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+					      "nf_ct_icmpv6: invalid redirect option ");
+				return -NF_ACCEPT;
+			}
+			if (opt->nd_opt_len == 0) {
+				if (LOG_INVALID(net, IPPROTO_ICMPV6))
+				nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+					      "nf_ct_icmpv6: invalid redirect option length ");
+				return -NF_ACCEPT;
+			}
+
+			if (opt->nd_opt_type == ND_OPT_REDIRECT_HDR) {
+				dataoff += 8;
+				break;
+			}
+
+			dataoff += opt->nd_opt_len << 3;
+		}
+
+		/* read redirect header */
+		iph = skb_header_pointer(skb, dataoff, sizeof(_iph), &_iph);
+		if (iph == NULL) {
+			if (LOG_INVALID(net, IPPROTO_ICMPV6))
+			nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+				      "nf_ct_icmpv6: short redirect header ");
+			return -NF_ACCEPT;
+		}
+
+		/* validate destination address */
+		if (!ipv6_addr_equal(&iph->daddr, dst)) {
+			if (LOG_INVALID(net, IPPROTO_ICMPV6))
+			nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+				      "nf_ct_icmpv6: redirect destination address not matching destination address of redirect header ");
+			return -NF_ACCEPT;
+		}
+	}
+	else if (icmp6h->icmp6_type >= 128)
 		return NF_ACCEPT;
 
 	return icmpv6_error_message(net, tmpl, skb, dataoff, ctinfo, hooknum);
-- 
1.7.12.4


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

* Re: [PATCH] netfilter: nf_conntrack_icmpv6: find conntrack related to ICMPv6 redirect packet
  2016-01-13 13:42 [PATCH] netfilter: nf_conntrack_icmpv6: find conntrack related to ICMPv6 redirect packet Alin Nastac
@ 2016-01-13 16:43 ` Pablo Neira Ayuso
  2016-01-14  7:41   ` Alin Năstac
  0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2016-01-13 16:43 UTC (permalink / raw)
  To: Alin Nastac; +Cc: netfilter-devel

I'd suggest you submit this patch with a proper description.

Thanks.

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

* Re: [PATCH] netfilter: nf_conntrack_icmpv6: find conntrack related to ICMPv6 redirect packet
  2016-01-13 16:43 ` Pablo Neira Ayuso
@ 2016-01-14  7:41   ` Alin Năstac
  0 siblings, 0 replies; 4+ messages in thread
From: Alin Năstac @ 2016-01-14  7:41 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Wed, Jan 13, 2016 at 5:43 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> I'd suggest you submit this patch with a proper description.
>
> Thanks.

This patch will modify the conntrack state created by ICMPv6 redirect
packets from INVALID (as it is implemented now, skb->nfct remains
NULL) to RELATED (like in ICMPv6 errors case). In IPv4 case, ICMP
redirects are treated the same way as ICMP errors, so there is no
issue. Probably ICMPv6 redirects were not handled because their
parsing is not as straightforward as ICMPv6 errors.

I tested it on an older version of kernel, but since
nf_conntrack_proto_icmpv6.c remained basically the same, I think the
issue would be reproducible even on latest version of kernel.

Cheers,
Alin

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

* [PATCH] netfilter: nf_conntrack_icmpv6: find conntrack related to ICMPv6 redirect packet
@ 2016-01-13 14:34 Alin Nastac
  0 siblings, 0 replies; 4+ messages in thread
From: Alin Nastac @ 2016-01-13 14:34 UTC (permalink / raw)
  To: netfilter-devel

---
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c | 77 ++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index 660bc10..699848a 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -144,7 +144,7 @@ static bool icmpv6_new(struct nf_conn *ct, const struct sk_buff *skb,
 static int
 icmpv6_error_message(struct net *net, struct nf_conn *tmpl,
 		     struct sk_buff *skb,
-		     unsigned int icmp6off,
+		     unsigned int inneripv6off,
 		     enum ip_conntrack_info *ctinfo,
 		     unsigned int hooknum)
 {
@@ -157,9 +157,7 @@ icmpv6_error_message(struct net *net, struct nf_conn *tmpl,
 
 	/* Are they talking about one of our connections? */
 	if (!nf_ct_get_tuplepr(skb,
-			       skb_network_offset(skb)
-				+ sizeof(struct ipv6hdr)
-				+ sizeof(struct icmp6hdr),
+			       inneripv6off,
 			       PF_INET6, net, &origtuple)) {
 		pr_debug("icmpv6_error: Can't get tuple\n");
 		return -NF_ACCEPT;
@@ -227,9 +225,78 @@ icmpv6_error(struct net *net, struct nf_conn *tmpl,
 		nf_conntrack_get(skb->nfct);
 		return NF_ACCEPT;
 	}
+	dataoff += sizeof(struct icmp6hdr);
 
 	/* is not error message ? */
-	if (icmp6h->icmp6_type >= 128)
+	if (icmp6h->icmp6_type == NDISC_REDIRECT) {
+		const struct in6_addr *dst;
+		struct in6_addr _dst;
+		const struct nd_opt_hdr *opt;
+		struct nd_opt_hdr _opt;
+		const struct ipv6hdr *iph;
+		struct ipv6hdr _iph;
+
+		/* skip target address */
+		dataoff += sizeof(_dst);
+
+		/* read destination address */
+		dst = skb_header_pointer(skb, dataoff, sizeof(_dst), &_dst);
+		if (dst == NULL) {
+			if (LOG_INVALID(net, IPPROTO_ICMPV6))
+			nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+				      "nf_ct_icmpv6: short redirect packet ");
+			return -NF_ACCEPT;
+		}
+		if (ipv6_addr_is_multicast(dst)) {
+			if (LOG_INVALID(net, IPPROTO_ICMPV6))
+			nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+				      "nf_ct_icmpv6: redirect destination address is multicast ");
+			return -NF_ACCEPT;
+		}
+		dataoff += sizeof(_dst);
+
+		/* find redirected header */
+		while (1) {
+			opt = skb_header_pointer(skb, dataoff, sizeof(_opt), &_opt);
+			if (opt == NULL) {
+				if (LOG_INVALID(net, IPPROTO_ICMPV6))
+				nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+					      "nf_ct_icmpv6: invalid redirect option ");
+				return -NF_ACCEPT;
+			}
+			if (opt->nd_opt_len == 0) {
+				if (LOG_INVALID(net, IPPROTO_ICMPV6))
+				nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+					      "nf_ct_icmpv6: invalid redirect option length ");
+				return -NF_ACCEPT;
+			}
+
+			if (opt->nd_opt_type == ND_OPT_REDIRECT_HDR) {
+				dataoff += 8;
+				break;
+			}
+
+			dataoff += opt->nd_opt_len << 3;
+		}
+
+		/* read redirect header */
+		iph = skb_header_pointer(skb, dataoff, sizeof(_iph), &_iph);
+		if (iph == NULL) {
+			if (LOG_INVALID(net, IPPROTO_ICMPV6))
+			nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+				      "nf_ct_icmpv6: short redirect header ");
+			return -NF_ACCEPT;
+		}
+
+		/* validate destination address */
+		if (!ipv6_addr_equal(&iph->daddr, dst)) {
+			if (LOG_INVALID(net, IPPROTO_ICMPV6))
+			nf_log_packet(net, PF_INET6, 0, skb, NULL, NULL, NULL,
+				      "nf_ct_icmpv6: redirect destination address not matching destination address of redirect header ");
+			return -NF_ACCEPT;
+		}
+	}
+	else if (icmp6h->icmp6_type >= 128)
 		return NF_ACCEPT;
 
 	return icmpv6_error_message(net, tmpl, skb, dataoff, ctinfo, hooknum);
-- 
1.7.12.4


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

end of thread, other threads:[~2016-01-14  7:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-13 13:42 [PATCH] netfilter: nf_conntrack_icmpv6: find conntrack related to ICMPv6 redirect packet Alin Nastac
2016-01-13 16:43 ` Pablo Neira Ayuso
2016-01-14  7:41   ` Alin Năstac
2016-01-13 14:34 Alin Nastac

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.