Netdev List
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: herbert@gondor.hengli.com.au
Cc: netdev@vger.kernel.org
Subject: unintended ipv4 broadcast policy change
Date: Wed, 22 Jun 2011 16:39:35 -0700 (PDT)	[thread overview]
Message-ID: <20110622.163935.2248705300315908767.davem@davemloft.net> (raw)


The context is that I'm looking into cleaning up up the mess we have
wrt. DHCP listening on packet sockets and (in some cases) seeing every
packet that hits the system.

Anyways, back in 2007 this commit was made:

commit 8030f54499925d073a88c09f30d5d844fb1b3190
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Thu Feb 22 01:53:47 2007 +0900

    [IPV4] devinet: Register inetdev earlier.

So now every net device registered has inetdev_init() called on it.

This has a subtle policy side effect that has some interesting
implications.  The route input slow path has this check:

	/* IP on this device is disabled. */

	if (!in_dev)
		goto out;

But now this will never, ever, be true.

Which means that previously we would not accept even broadcast
or multicast packets on an interface that hasn't had at least
one IP address configured.

Now we will.

I think we have a hard decision to make.  One option is to
fix the input routing check, by changing it to test if the
ipv4 address list is empty.

The second option is to remove the check entirely and keep the
new behavior.

This subtle new behavior is interesting because it means that
a DHCP client could be implemented entirely with plain UDP
sockets.

Contrary to previous discussions I see no code in the ISC dhcp
client that needs to get at the MAC address.  It merely wants
it there so that it's packet parsing engine can skip over it
to get at the ipv4/UDP bits.

In fact the stock ISC dhcp code has a bug in that it creates
it's AF_PACKET socket with SOCK_PACKET as the type, which causes
the BPF filter it registers to never be used.  It should be
using SOCK_RAW as the type.  It seems to use SOCK_PACKET in
order to use the name based socket address in it's bind()
call.

As a side effect of an unrelated change, this bug got fixed in
Fedora/RHEL (it wants access to the tpacket aux data in order to see
the checksum flags, this is not available with SOCK_PACKET type
AF_PACKET sockets).

But debian definitely still has this bug.  On debian, as a result,
every packet received gets parsed.

Actually this bug is more serious, since the code in ISC dhcp
elides the UDP protocol and port validation if it is compiled
with the BPF code in place (which it is).  This means it's
parsing garbage most of the time.

It also creates the packet socket with ETH_P_ALL, making this
an even more severe issue.

             reply	other threads:[~2011-06-22 23:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-22 23:39 David Miller [this message]
2011-06-23  0:41 ` unintended ipv4 broadcast policy change Herbert Xu
2011-06-23  2:41   ` David Miller
2011-06-23  5:41     ` David Miller
2011-06-23  0:42 ` David Miller
2011-06-23 15:16 ` Stephen Hemminger
2011-06-23 20:45   ` David Miller
2011-06-23 21:01     ` Stephen Hemminger
2011-06-23 21:08       ` David Miller
2011-06-24 17:01         ` Stephen Hemminger
2011-06-24 19:54           ` David Miller
2011-06-25  0:27             ` Herbert Xu
2011-06-25  0:28               ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110622.163935.2248705300315908767.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=herbert@gondor.hengli.com.au \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox