Check connect address in NETLINK

15 messages, 3 authors, 2004-07-05 · open the first message on its own page

Check connect address in NETLINK

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: 2004-06-28 23:14:39

Hi:

The recent thread on NLMSG_OK has reminded me about an old problem
with NETLINK.

The problem is that any user on the system can launch a DoS attack on
any NETLINK application by flooding its NETLINK address with packets.
This will easily fill up the receive queue of the destination
application and therefore cause legitimate packets from the kernel
or elsewhere to be dropped.

The solution seems simple.  We already have a connect(2) call for
NETLINK sockets.  So why don't we check the connected address of
the destination socket against the address of the sender before
putting the packet on the queue?

Any comments before I go ahead and code it?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Check connect address in NETLINK

From: David S. Miller <hidden>
Date: 2004-06-29 00:30:39

On Tue, 29 Jun 2004 09:14:39 +1000
Herbert Xu [off-list ref] wrote:
The solution seems simple.  We already have a connect(2) call for
NETLINK sockets.  So why don't we check the connected address of
the destination socket against the address of the sender before
putting the packet on the queue?

Any comments before I go ahead and code it?
This really won't break any existing legitimate cases?
Are you sure?

Re: Check connect address in NETLINK

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: 2004-06-29 02:09:18

On Mon, Jun 28, 2004 at 05:30:39PM -0700, David S. Miller wrote:
This really won't break any existing legitimate cases?
Are you sure?
I would've thought that it shouldn't break anything.  But let me
have a look around and get back to you.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Check connect address in NETLINK

From: Alexey Kuznetsov <hidden>
Date: 2004-06-29 08:22:52

Hello!
The solution seems simple.  We already have a connect(2) call for
NETLINK sockets.  So why don't we check the connected address of
the destination socket against the address of the sender before
putting the packet on the queue?
Do you mean the restriction sort of made in AF_UNIX SOCK_DGRAM:
a connected socket receives messages only from its destination?

I think this is safe.

It was not done because netlink sockets were expected to listen
for broadcasts, so that this kind of protection would be not useful
and even harmful. But taking into account that inter-application
communication is not used, only kernel sends broadcasts and applications
talking to kernel will receive such broadcasts, because they are connected
to kernel.

The troube is that pid of kernel socket used to be 0, so that
applications connected to kernel are not connected in technical sense. :-)
Apparently, to implement this we have to add some kind of flag
marking connected sockets.

Alexey

Re: Check connect address in NETLINK

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: 2004-06-29 08:45:52

On Tue, Jun 29, 2004 at 12:22:52PM +0400, Alexey Kuznetsov wrote:
Do you mean the restriction sort of made in AF_UNIX SOCK_DGRAM:
a connected socket receives messages only from its destination?
Exactly.  Another example would be UDP over IP.
It was not done because netlink sockets were expected to listen
for broadcasts, so that this kind of protection would be not useful
and even harmful. But taking into account that inter-application
communication is not used, only kernel sends broadcasts and applications
talking to kernel will receive such broadcasts, because they are connected
to kernel.
I've had a look in the various NETLINK applications that I know of,
including quagga/iproute/iptables and all the stuff that I wrote, 
none of them does a connect at all.

So it should be harmless to introduce this new semantics.
The troube is that pid of kernel socket used to be 0, so that
applications connected to kernel are not connected in technical sense. :-)
That's kind of a good thing since it means that existing applications
are less likely to call connect(2) :)
Apparently, to implement this we have to add some kind of flag
marking connected sockets.
Or we can set the disconnected pid to a negative value since POSIX
requires pid_t to be signed.  I see that you've reserved everything
between -4096 and 0.  So perhaps we can pick -1?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Check connect address in NETLINK

From: Alexey Kuznetsov <hidden>
Date: 2004-06-29 11:14:33

Hello!
Or we can set the disconnected pid to a negative value since POSIX
requires pid_t to be signed.  I see that you've reserved everything
between -4096 and 0.  So perhaps we can pick -1?
I think we can.

Alexey

Re: Check connect address in NETLINK

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: 2004-06-29 11:18:33

On Tue, Jun 29, 2004 at 03:14:33PM +0400, Alexey Kuznetsov wrote:
quoted
Or we can set the disconnected pid to a negative value since POSIX
requires pid_t to be signed.  I see that you've reserved everything
between -4096 and 0.  So perhaps we can pick -1?
I think we can.
Great.  I'll code it up then.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Check connect address in NETLINK

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: 2004-06-30 11:27:51

On Tue, Jun 29, 2004 at 09:18:33PM +1000, herbert wrote:
quoted
quoted
Or we can set the disconnected pid to a negative value since POSIX
requires pid_t to be signed.  I see that you've reserved everything
between -4096 and 0.  So perhaps we can pick -1?
Actually that doesn't quite work.  Users are allowed to bind to any
non-zero address including -1.  Besides, we already have sock->sk_state
and socket->state which are perfect for this.

So here is a patch to disallow sending unicast messages to connected
sockets from addresses other than the one that it is connected to.

I've tested it with a locally patched Openswan and it works as
intended by stopping me from sending bogus messages to it and
still allowing kernel messages to go through.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Check connect address in NETLINK

From: Alexey Kuznetsov <hidden>
Date: 2004-06-30 12:00:45

Hello!
+	if (sock->sk_socket->state == SS_CONNECTED &&
+	    nlk->dst_pid != nlk_sk(ssk)->pid) {
No-no-no! sock->sk_socket can be NULL at this point.

You can use sock->sk_state =  TCP_ESTABLISHED, forxample.

Alexey

Re: Check connect address in NETLINK

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: 2004-06-30 12:08:28

On Wed, Jun 30, 2004 at 04:00:45PM +0400, Alexey Kuznetsov wrote:
quoted
+	if (sock->sk_socket->state == SS_CONNECTED &&
+	    nlk->dst_pid != nlk_sk(ssk)->pid) {
No-no-no! sock->sk_socket can be NULL at this point.

You can use sock->sk_state =  TCP_ESTABLISHED, forxample.
OK.  Can you give me a code path that allows sk_socket to be NULL
at this point?

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Check connect address in NETLINK

From: Alexey Kuznetsov <hidden>
Date: 2004-06-30 12:14:20

Hello!
OK.  Can you give me a code path that allows sk_socket to be NULL
at this point?
cpu 0:				cpu1 (or just preempted cpu)

sk = netlink_lookup(...);
				... closing sk
				netlink_release() clears sk_socket

use sk->sk_socket. Oops.

Alexey

Re: Check connect address in NETLINK

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: 2004-06-30 12:40:50

On Wed, Jun 30, 2004 at 04:14:20PM +0400, Alexey Kuznetsov wrote:
cpu 0:				cpu1 (or just preempted cpu)

sk = netlink_lookup(...);
				... closing sk
				netlink_release() clears sk_socket

use sk->sk_socket. Oops.
Thanks for the example.

Here is a version that uses sk_state instead.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Check connect address in NETLINK

From: David S. Miller <hidden>
Date: 2004-06-30 22:36:06

Why don't you combine the two "ERR_PTR(-ECONNREFUSED)" tests
into one test like:

	if ((nlk->pid == 0 && !nlk->data_ready) ||
	    (sock->sk_state == NELTINK_CONNECTED &&
	     nlk->dst_pid != nlk_sk(ssk)->pid)) {
		sock_put(sock);
		return ERR_PTR(-ECONNREFUSED);
	}

so we don't have two copies of the "sock_put(); return ERR_PTR()"
thing emitted by the compiler?

Re: Check connect address in NETLINK

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: 2004-06-30 23:01:47

On Wed, Jun 30, 2004 at 03:36:06PM -0700, David S. Miller wrote:
Why don't you combine the two "ERR_PTR(-ECONNREFUSED)" tests
into one test like:

	if ((nlk->pid == 0 && !nlk->data_ready) ||
	    (sock->sk_state == NELTINK_CONNECTED &&
	     nlk->dst_pid != nlk_sk(ssk)->pid)) {
		sock_put(sock);
		return ERR_PTR(-ECONNREFUSED);
	}

so we don't have two copies of the "sock_put(); return ERR_PTR()"
thing emitted by the compiler?
Well at least under i386, gcc (3.3.4) is smart enough to merge these
common exit paths.

But yes we could merge them.  What about the following incremental
patch?
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Check connect address in NETLINK

From: David S. Miller <hidden>
Date: 2004-07-05 22:46:39

On Wed, 30 Jun 2004 22:40:50 +1000
Herbert Xu [off-list ref] wrote:
On Wed, Jun 30, 2004 at 04:14:20PM +0400, Alexey Kuznetsov wrote:
quoted
cpu 0:				cpu1 (or just preempted cpu)

sk = netlink_lookup(...);
				... closing sk
				netlink_release() clears sk_socket

use sk->sk_socket. Oops.
Thanks for the example.

Here is a version that uses sk_state instead.
Applied, thanks Herbert.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help