Strange UDP binding behavior (SO_BINDTODEVICE)

Strange UDP binding behavior (SO_BINDTODEVICE)

[Kevin Dwyer]

[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]

Hello,

We have come across something that may be a bug, unless this behavior
was intentional.

The problem can be simulated by creating a socket, setting
SO_BINDTODEVICE, and binding to a port.  Then, in a separate process we
attempt to bind to the same port but without the SO_BINDTODEVICE option.
The expected behavior is to get EINVAL because the port is already
bound by a prior call.  However, it succeeds, and the second process
steals the first process' packets.

The likely code in question resides in net/ipv4/udp.c:

        for (sk2 = udp_hash[snum & (UDP_HTABLE_SIZE - 1)];
             sk2 != NULL;
             sk2 = sk2->next) {
                if (sk2->num == snum &&
                    sk2 != sk &&
                    sk2->bound_dev_if == sk->bound_dev_if &&
                    (!sk2->rcv_saddr ||
                     !sk->rcv_saddr ||
                     sk2->rcv_saddr == sk->rcv_saddr) &&
                    (!sk2->reuse || !sk->reuse))
                        goto fail;
        }

The condition (sk2->bound_dev_if == sk->bound_dev_if) will fail because
sk2->bound_dev_if will be the ifindex of the interface we bound to, and
sk->bound_dev_if will be 0, since we didn't bind to a specific
interface.

Lars Ellenberg suggests something like:
|       (!sk2->bound_dev_if ||
|        !sk->bound_dev_if ||
|        sk2->bound_dev_if == sk->bound_dev_if) &&

Which on its face appears to clear the bug.  I don't see any obvious
downsides to it either, but this is why I'm here.

So, is this intentional or a bug?

Thanks.

-- 
- kpd
"If at first you don't succeed, redefine success." - Anonymous

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Strange UDP binding behavior (SO_BINDTODEVICE)

[Casey Carter]

Kevin Dwyer wrote:

>Hello,
>
>We have come across something that may be a bug, unless this behavior
>was intentional.
>
>The problem can be simulated by creating a socket, setting
>SO_BINDTODEVICE, and binding to a port.  Then, in a separate process we
>attempt to bind to the same port but without the SO_BINDTODEVICE option.
>The expected behavior is to get EINVAL because the port is already
>bound by a prior call.  However, it succeeds, and the second process
>steals the first process' packets.
>
>The likely code in question resides in net/ipv4/udp.c:
>
>        for (sk2 = udp_hash[snum & (UDP_HTABLE_SIZE - 1)];
>             sk2 != NULL;
>             sk2 = sk2->next) {
>                if (sk2->num == snum &&
>                    sk2 != sk &&
>                    sk2->bound_dev_if == sk->bound_dev_if &&
>                    (!sk2->rcv_saddr ||
>                     !sk->rcv_saddr ||
>                     sk2->rcv_saddr == sk->rcv_saddr) &&
>                    (!sk2->reuse || !sk->reuse))
>                        goto fail;
>        }
>
>The condition (sk2->bound_dev_if == sk->bound_dev_if) will fail because
>sk2->bound_dev_if will be the ifindex of the interface we bound to, and
>sk->bound_dev_if will be 0, since we didn't bind to a specific
>interface.
>
>Lars Ellenberg suggests something like:
>|       (!sk2->bound_dev_if ||
>|        !sk->bound_dev_if ||
>|        sk2->bound_dev_if == sk->bound_dev_if) &&
>
>Which on its face appears to clear the bug.  I don't see any obvious
>downsides to it either, but this is why I'm here.
>
>So, is this intentional or a bug?
>
>Thanks.
>
>  
>
This is not a bug, it's a feature!  It is possible to use multiple 
sockets with the same port number bound to different interfaces (where 
"no interface" is one possible choice).  The UDP packet delivery code 
favors the socket that is bound to the interface through which packets 
are received. So, if I am only interested in packets for port 6666 on 
eth1, then I am prioritized ahead of the socket bound to "no interface" 
port 6666.  The only way you can "steal" packets is to create sockets 
and SO_BINDTODEVICE them "below" another process's non-interface-bound 
socket.  This is a clear security issue, and is the main reason for 
restricting SO_BINDEVICE to users with CAP_NET_ADMIN.

-- 
Casey Carter
Casey@Carter.net
ccarter@cs.uiuc.edu
AIM: cartec69

Re: Strange UDP binding behavior (SO_BINDTODEVICE)

[Kevin Dwyer]

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

On Mon, 06 Oct 2003 17:50:53 -0500
Casey Carter <Casey@Carter.net> wrote:

> This is not a bug, it's a feature!  It is possible to use multiple 
> sockets with the same port number bound to different interfaces (where
> "no interface" is one possible choice).  The UDP packet delivery code 
> favors the socket that is bound to the interface through which packets
> are received. So, if I am only interested in packets for port 6666 on 
> eth1, then I am prioritized ahead of the socket bound to "no
> interface" port 6666.  

Well, I would buy that as reasonable, acceptable behavior, but I think
the reverse is true.

We are binding specifically to say, eth0 on port 694.  Someone comes
along and binds to "no interface" on port 694 and trumps our socket. 
Isn't that the opposite of what you describe?

-- 
- kpd
"If at first you don't succeed, redefine success." - Anonymous

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Strange UDP binding behavior (SO_BINDTODEVICE)

[Casey Carter]

Kevin Dwyer wrote:

>On Mon, 06 Oct 2003 17:50:53 -0500
>Casey Carter <Casey@Carter.net> wrote:
>
>  
>
>>This is not a bug, it's a feature!  It is possible to use multiple 
>>sockets with the same port number bound to different interfaces (where
>>"no interface" is one possible choice).  The UDP packet delivery code 
>>favors the socket that is bound to the interface through which packets
>>are received. So, if I am only interested in packets for port 6666 on 
>>eth1, then I am prioritized ahead of the socket bound to "no
>>interface" port 6666.  
>>    
>>
>
>Well, I would buy that as reasonable, acceptable behavior, but I think
>the reverse is true.
>
>We are binding specifically to say, eth0 on port 694.  Someone comes
>along and binds to "no interface" on port 694 and trumps our socket. 
>Isn't that the opposite of what you describe?
>  
>
Taking a look at the UDP delivery code in net/ipv4/udp.c (version 
2.6.0-test6) we see:

struct sock *udp_v4_lookup_longway(u32 saddr, u16 sport, u32 daddr, u16 dport, int dif)

{

    struct sock *sk, *result = NULL;

    struct hlist_node *node;

    unsigned short hnum = ntohs(dport);

    int badness = -1;


    sk_for_each(sk, node, &udp_hash[hnum & (UDP_HTABLE_SIZE - 1)]) {

        struct inet_opt *inet = inet_sk(sk);


        if (inet->num == hnum && !ipv6_only_sock(sk)) {

            int score = (sk->sk_family == PF_INET ? 1 : 0);

            if (inet->rcv_saddr) {

                if (inet->rcv_saddr != daddr)

                    continue;

                score+=2;

            }

            if (inet->daddr) {

                if (inet->daddr != saddr)

                    continue;

                score+=2;

            }

            if (inet->dport) {

                if (inet->dport != sport)

                    continue;

                score+=2;

            }

            if (sk->sk_bound_dev_if) {

                if (sk->sk_bound_dev_if != dif)

                    continue;

                score+=2;

            }

            if(score == 9) {

                result = sk;

                break;

            } else if(score > badness) {

                result = sk;

                badness = score;

            }

        }

    }

    return result;

}

This code picks a UDP socket to deliver to, amongst those with the 
correct destination port number.  It does so by ranking each socket on a 
scale from 0 to 9 and picking the first socket with the best score.   
Since the score increments are all 2, this code weights equally a socket 
that is bound to the correct interface and (for example) a socket bound 
to no interface but bound to the specific IP address the packet is 
addressed to.  IMHO, the delivery should weigh sk_bound_dev_if much more 
strongly (7 instead of 2), so that if-bound sockets are always favored 
over non-if-bound.  I would be happy to submit the (trivial) patch to do 
so if the networking gurus agree?

-- 
Casey Carter
Casey@Carter.net
ccarter@cs.uiuc.edu
AIM: cartec69

Re: Strange UDP binding behavior (SO_BINDTODEVICE)

[Kevin Dwyer]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Tue, 07 Oct 2003 15:51:57 -0500
Casey Carter <ccarter@cs.uiuc.edu> wrote:

> IMHO, the delivery should weigh sk_bound_dev_if much more strongly (7
> instead of 2), so that if-bound sockets are always favored over
> non-if-bound.  I would be happy to submit the (trivial) patch to do so
> if the networking gurus agree?

Any possibility of getting this behavior into 2.4 as well?  Albeit,
without the scoring since that's obviously a new concept introduced by
2.6. (Which I prefer; well done.)

I confess that I don't know who to talk to about such a change.

-- 
- kpd
"If at first you don't succeed, redefine success." - Anonymous

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Strange UDP binding behavior (SO_BINDTODEVICE)

[Casey Carter]

Kevin Dwyer wrote:

>On Tue, 07 Oct 2003 15:51:57 -0500
>Casey Carter <ccarter@cs.uiuc.edu> wrote:
>
>  
>
>>IMHO, the delivery should weigh sk_bound_dev_if much more strongly (7
>>instead of 2), so that if-bound sockets are always favored over
>>non-if-bound.  I would be happy to submit the (trivial) patch to do so
>>if the networking gurus agree?
>>    
>>
>
>Any possibility of getting this behavior into 2.4 as well?  Albeit,
>without the scoring since that's obviously a new concept introduced by
>2.6. (Which I prefer; well done.)
>
>I confess that I don't know who to talk to about such a change.
>  
>
There is actually a similar ranking algorithm in 2.4, with a similar 
fix. I'll work up a patch and resubmit to netdev in hopes that one of 
the gurus will pick it up.

-- 
Casey Carter
Casey@Carter.net
ccarter@cs.uiuc.edu
AIM: cartec69