netdev - Re: ipv4/udp: Verify multicast group is ours in upd_v4_early

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150730213225.GA11294@sbohrermbp13-local.rgmadvisors.com>
Date:	Thu, 30 Jul 2015 16:32:25 -0500
From:	Shawn Bohrer <shawn.bohrer@...il.com>
To:	Kenneth Klette Jonassen <kennetkl@....uio.no>
Cc:	sbohrer@...advisors.com, netdev <netdev@...r.kernel.org>,
	Yurij.Plotnikov@...etlabs.ru, David Miller <davem@...emloft.net>,
	Ståle Kristoffersen <stalk@...dgetech.tv>
Subject: Re: ipv4/udp: Verify multicast group is ours in upd_v4_early_demux()

On Thu, Jul 30, 2015 at 10:26:08PM +0200, Kenneth Klette Jonassen wrote:
> Commit 6e54030 breaks the IP_MULTICAST_ALL socket option. There is
> already a check in ip_mc_sf_allow() that should do the filtering you
> claim to fix. Was it considered?
>
> Commit message:
> 421b3885bf6d56391297844f43fb7154a6396e12 "udp: ipv4: Add udp early
> demux" introduced a regression that allowed sockets bound to INADDR_ANY
> to receive packets from multicast groups that the socket had not joined...
> 
> man ip(7):
> IP_MULTICAST_ALL (since Linux 2.6.31)
> This option can be used to modify the delivery policy of
> multicast messages to sockets bound to the wildcard INADDR_ANY
> address.  The argument is a boolean integer (defaults to 1).
> If set to 1, the socket will receive messages from all the
> groups that have been joined globally on the whole system.
> Otherwise, it will deliver messages only from the groups that
> have been explicitly joined…

Do you have an application or test case that is now broken after this
commit?  Or is this just a theoretical breakage based off of my commit
message?  If you have a simple example test case can you share it?
Also if this breaks your application but you don't have a test case
can you describe the breakage?  Is IP_MULTICAST_ALL 0 or 1?  Are you
not receiving packets or receiving packets you don't expect to
receive?

I admittedly haven't tried to create a test case yet, but looking over
the code I'm not sure how this could have broken IP_MULTICAST_ALL.
Commit 6e54030 added a call to ip_check_mc_rcu() which as far as I can
tell checks to see that a received multicast packet was a mc address
joined on that interface.  Without this check it is possible to
receive multicast packets for an address that hasn't been joined on
that machine at all.  Which was the bug I was fixing.

The ip_mc_sf_allow() check comes next, and is still there.  It should
continue to provide the behavior you quoted from the man page.  It
isn't/wasn't sufficient to use IP_MULTICAST_ALL as a fix for the
original bug because as I mentioned above you could receive packets
that nothing on the machine had joined.

--
Shawn
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html