[PATCH] IPv6: optionaly validate RAs on raw sockets

[PATCH] IPv6: optionaly validate RAs on raw sockets

[Remi Denis-Courmont]

ICMPv6 Router Advertisements may now contain informations that is
mostly of interest to userland. This currently mostly consists of
recursive DNS server addresses (though one should expect other
stuff to come).

This patch adds a setsockopt to ICMPv6 sockets to only deliver Router
Advertisements if they pass the same set of checks as the kernel
IPv6 autoconfiguration does, so that userland can parse attributes it
is interested safely (otherwise, it would get and parse advertisements
that the kernel rejected).

N.B.: not too sure about the option name though...

Signed-off-by: Remi Denis-Courmont <rdenis@simphalempin.com>

diff --git a/include/linux/icmpv6.h b/include/linux/icmpv6.h
index 7c5e981..8c96822 100644
--- a/include/linux/icmpv6.h
+++ b/include/linux/icmpv6.h
@@ -139,6 +139,7 @@ static inline struct icmp6hdr *icmp6_hdr(const struct sk_buff *skb)
  */
 
 #define ICMPV6_FILTER			1
+#define ICMPV6_VALID_ADVERT		2
 
 /*
  *	ICMPV6 filter
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 648bd1f..af72f02 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -343,7 +343,9 @@ struct ipv6_pinfo {
 struct raw6_sock {
 	/* inet_sock has to be the first member of raw6_sock */
 	struct inet_sock	inet;
-	__u32			checksum;	/* perform checksum */
+	__u16			unused;
+	__u8			valid_advert;	/* deliver valid RAs only */
+	__u8			checksum;	/* perform checksum */
 	__u32			offset;		/* checksum offset  */
 	struct icmp6_filter	filter;
 	/* ipv6_pinfo has to be the last member of raw6_sock, see inet6_sk_generic */
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index a58459a..da6cb50 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -127,12 +127,35 @@ static __inline__ int icmpv6_filter(struct sock *sk, struct sk_buff *skb)
 
 	if (pskb_may_pull(skb, sizeof(struct icmp6hdr))) {
 		__u32 *data = &rp->filter.data[0];
-		int bit_nr;
+		int type;
 
 		icmph = (struct icmp6hdr *) skb->data;
-		bit_nr = icmph->icmp6_type;
+		type = icmph->icmp6_type;
 
-		return (data[bit_nr >> 5] & (1 << (bit_nr & 31))) != 0;
+		if (data[type >> 5] & (1 << (type & 31)))
+			return 1;
+
+		/* Userland only wants valid router advertisements? */
+		if (type == NDISC_ROUTER_ADVERTISEMENT &&
+		    rp->valid_advert) {
+			struct inet6_dev *idev;
+
+			if (!pskb_may_pull(skb, sizeof(struct ra_msg)) ||
+			    !(ipv6_addr_type(&ipv6_hdr(skb)->saddr) &
+			      IPV6_ADDR_LINKLOCAL))
+				return 1;
+
+		        idev = in6_dev_get(skb->dev);
+		        if (!idev)
+		                return 1;
+
+			/* Ignore RA when routing */
+			if (idev->cnf.forwarding || !idev->cnf.accept_ra) {
+				in6_dev_put(idev);
+				return 1;
+			}
+			in6_dev_put(idev);
+		}
 	}
 	return 0;
 }
@@ -877,13 +900,26 @@ do_confirm:
 static int rawv6_seticmpfilter(struct sock *sk, int level, int optname,
 			       char __user *optval, int optlen)
 {
+	struct raw6_sock *rp = raw6_sk(sk);
+
 	switch (optname) {
 	case ICMPV6_FILTER:
 		if (optlen > sizeof(struct icmp6_filter))
 			optlen = sizeof(struct icmp6_filter);
-		if (copy_from_user(&raw6_sk(sk)->filter, optval, optlen))
+		if (copy_from_user(&rp->filter, optval, optlen))
 			return -EFAULT;
 		return 0;
+	case ICMPV6_VALID_ADVERT: {
+		int val;
+
+		if (optlen != sizeof(int))
+			return -EINVAL;
+		if (copy_from_user(&val, optval, sizeof(int)))
+			return -EFAULT;
+		/* -1 resets to default, which is actually 0 */
+		rp->valid_advert = (val > 0);
+		return 0;
+	}
 	default:
 		return -ENOPROTOOPT;
 	}
@@ -894,25 +930,38 @@ static int rawv6_seticmpfilter(struct sock *sk, int level, int optname,
 static int rawv6_geticmpfilter(struct sock *sk, int level, int optname,
 			       char __user *optval, int __user *optlen)
 {
+	struct raw6_sock *rp = raw6_sk(sk);
 	int len;
 
+	if (get_user(len, optlen))
+		return -EFAULT;
+	if (len < 0)
+		return -EINVAL;
+
 	switch (optname) {
 	case ICMPV6_FILTER:
-		if (get_user(len, optlen))
-			return -EFAULT;
-		if (len < 0)
-			return -EINVAL;
 		if (len > sizeof(struct icmp6_filter))
 			len = sizeof(struct icmp6_filter);
-		if (put_user(len, optlen))
+		if (copy_to_user(optval, &rp->filter, len))
 			return -EFAULT;
-		if (copy_to_user(optval, &raw6_sk(sk)->filter, len))
+		break;
+	case ICMPV6_VALID_ADVERT: {
+		int val;
+
+		if (len < sizeof(int))
+			return -EINVAL;
+		val = rp->valid_advert;
+		len = sizeof(int);
+		if (copy_to_user(optval, &val, sizeof(int)))
 			return -EFAULT;
-		return 0;
+		break;
+	}
 	default:
 		return -ENOPROTOOPT;
 	}
 
+	if (put_user(len, optlen))
+		return -EFAULT;
 	return 0;
 }
 

-- 
Rémi Denis-Courmont
http://www.remlab.net/

Re: [PATCH] IPv6: optionaly validate RAs on raw sockets

[YOSHIFUJI Hideaki / 吉藤英明]

In article <200707102111.18824@auguste.remlab.net> (at Tue, 10 Jul 2007 21:11:17 +0300), Remi Denis-Courmont <rdenis@simphalempin.com> says:

> ICMPv6 Router Advertisements may now contain informations that is
> mostly of interest to userland. This currently mostly consists of
> recursive DNS server addresses (though one should expect other
> stuff to come).

I really do not want to have such non-standard API in kernel.

--yoshfuji

Re: [PATCH] IPv6: optionaly validate RAs on raw sockets

[Rémi Denis-Courmont]

On Wednesday 11 July 2007 15:29:16 YOSHIFUJI Hideaki / 吉藤英明 wrote:
> In article <200707102111.18824@auguste.remlab.net> (at Tue, 10 Jul 2007 
21:11:17 +0300), Remi Denis-Courmont <rdenis@simphalempin.com> says:
> > ICMPv6 Router Advertisements may now contain informations that is
> > mostly of interest to userland. This currently mostly consists of
> > recursive DNS server addresses (though one should expect other
> > stuff to come).
>
> I really do not want to have such non-standard API in kernel.

I can only think of a very limited set of ways to extract options from RAs 
that the kernel currently ignores:

1) parse everything in kernel addrconf.c
2) validate RA in kernel, parse userland options in userland
3) parse everything in userland
4) do not support any option of interest to userland ever
5) userland and kernel do their own cooking separately

netdev folks already rejected (1) earlier. You just rejected (2) this instant. 
(3) implies removing addrconf from the kernel completely, which does not 
sound good, besides being a big waste. (4) means Linux is unusable on IPv6 
networks. And it's already been pointed out (5) was not safe/secure (userland 
may end up accepting something when it should not).

I might be missing something because I am a notoriously arrogant moron but it 
looks like Linux IPv6 is in a dead-end for the time being :-(

What do you propose then?

-- 
Rémi Denis-Courmont

Re: [PATCH] IPv6: optionaly validate RAs on raw sockets

[David Stevens]

I think #2 in your list is the right choice, and that has nothing to do 
with adding a
non-standard option (which I completely agree is a bad idea).

It looked like you're just checking if the machine is acting as a router 
or not and
if it comes from a link-local address; is that right? Of course, lots of 
apps already
check for "am I a router" and they don't require a new socket option. (!) 
See everything
in the quagga package, for example. And checking the address type in a app 
is
trivial.

The previous discussion about "validation" was talking about RA's that are 
forged,
so don't pass IPsec authentication checks. I don't see any reason at all 
to deliver those
to an application (ever), so no non-standard socket option required there. 
I don't know
if those are currently delivered on raw sockets or not, but if they are, I 
think it's
reasonable to have a patch that clones them only after authentication 
rather than before.

Prior discussion used FUD about some monitoring apps needing to see forged 
RA's.
I don't think there really are apps that need to see forged RA's, but if 
they really
want everything, they should use bpf or the like, just as they would need 
to do to
receive, for example, packets with invalid checksums.

                                                                +-DLS

Re: [PATCH] IPv6: optionaly validate RAs on raw sockets

[Vlad Yasevich]

Rémi Denis-Courmont wrote:
> On Wednesday 11 July 2007 15:29:16 YOSHIFUJI Hideaki / 吉藤英明 wrote:
>> In article <200707102111.18824@auguste.remlab.net> (at Tue, 10 Jul 2007 
> 21:11:17 +0300), Remi Denis-Courmont <rdenis@simphalempin.com> says:
>>> ICMPv6 Router Advertisements may now contain informations that is
>>> mostly of interest to userland. This currently mostly consists of
>>> recursive DNS server addresses (though one should expect other
>>> stuff to come).
>> I really do not want to have such non-standard API in kernel.
> 
> I can only think of a very limited set of ways to extract options from RAs 
> that the kernel currently ignores:
> 
> 1) parse everything in kernel addrconf.c
> 2) validate RA in kernel, parse userland options in userland

Yes, #2 is the right option.  However, I agree that passing such info with
non-standard socket option is not ideal.

Since you asked for another idea, how about using netlink to send _validated_ RA
information to interested parties?

-vlad

Re: [PATCH] IPv6: optionaly validate RAs on raw sockets

[David Stevens]

> Since you asked for another idea, how about using netlink to send 
_validated_ RA
> information to interested parties?
> 
> -vlad

        That sounds like a good idea to me (FWIW),
though I also still think a simple raw-socket
application would do it just fine, possibly with
no kernel modification at all.
        But since the kernel wouldn't be maintaining
the DNS info, which was my real objection to the
original version,  netlink would work well too.

                                                +-DLS

Re: [PATCH] IPv6: optionaly validate RAs on raw sockets

[Rémi Denis-Courmont]

Le mercredi 11 juillet 2007, David Stevens a écrit :
>         That sounds like a good idea to me (FWIW),
> though I also still think a simple raw-socket
> application would do it just fine, possibly with
> no kernel modification at all.
>         But since the kernel wouldn't be maintaining
> the DNS info, which was my real objection to the
> original version,  netlink would work well too.

One remaining corner case is NFS/IPv6 root, whereby userland won't have 
a chance to start before the network, and hence may miss the solicited 
RA. Or would it? By default, the next unsolicited RA can be anytime 
from now to after 10 minutes, so that's not sufficient. I wouldn't 
personnaly care, but...

-- 
Rémi Denis-Courmont
http://www.remlab.net/

Re: [PATCH] IPv6: optionaly validate RAs on raw sockets

[Vlad Yasevich]

Rémi Denis-Courmont wrote:
> Le mercredi 11 juillet 2007, David Stevens a écrit :
>>         That sounds like a good idea to me (FWIW),
>> though I also still think a simple raw-socket
>> application would do it just fine, possibly with
>> no kernel modification at all.
>>         But since the kernel wouldn't be maintaining
>> the DNS info, which was my real objection to the
>> original version,  netlink would work well too.
> 
> One remaining corner case is NFS/IPv6 root, whereby userland won't have 
> a chance to start before the network, and hence may miss the solicited 
> RA. Or would it? By default, the next unsolicited RA can be anytime 
> from now to after 10 minutes, so that's not sufficient. I wouldn't 
> personnaly care, but...
> 

You've got the same issue with the socket option approach.

-vlad

Re: [PATCH] IPv6: optionaly validate RAs on raw sockets

[David Miller]

From: Rémi_Denis-Courmont <rdenis@simphalempin.com>
Date: Wed, 11 Jul 2007 19:19:11 +0300

> Le mercredi 11 juillet 2007, David Stevens a écrit :
> >         That sounds like a good idea to me (FWIW),
> > though I also still think a simple raw-socket
> > application would do it just fine, possibly with
> > no kernel modification at all.
> >         But since the kernel wouldn't be maintaining
> > the DNS info, which was my real objection to the
> > original version,  netlink would work well too.
> 
> One remaining corner case is NFS/IPv6 root, whereby userland won't have 
> a chance to start before the network, and hence may miss the solicited 
> RA. Or would it? By default, the next unsolicited RA can be anytime 
> from now to after 10 minutes, so that's not sufficient. I wouldn't 
> personnaly care, but...

We already have cases like that with network device firmware that
has to be loaded in from the filesystem in userspace, and the
answer is to use a properly populated initrd.

Same goes for things like this.

That's the fact of life these days, like it or not.

Re: [PATCH] IPv6: optionaly validate RAs on raw sockets

[James Morris]

On Wed, 11 Jul 2007, David Miller wrote:

> > One remaining corner case is NFS/IPv6 root, whereby userland won't have 
> > a chance to start before the network, and hence may miss the solicited 
> > RA. Or would it? By default, the next unsolicited RA can be anytime 
> > from now to after 10 minutes, so that's not sufficient. I wouldn't 
> > personnaly care, but...
> 
> We already have cases like that with network device firmware that
> has to be loaded in from the filesystem in userspace, and the
> answer is to use a properly populated initrd.
> 
> Same goes for things like this.
> 
> That's the fact of life these days, like it or not.

Same story for NFS root when using strong authentication -- something has 
to be running in userland to manage that.



- James
-- 
James Morris
<jmorris@namei.org>