diff mbox series

batman-adv: allow snooping gratuitous ARP Replies

Message ID 20190214155100.10070-1-linus.luessing@c0d3.blue (mailing list archive)
State Changes Requested
Delegated to: Simon Wunderlich
Headers
Series batman-adv: allow snooping gratuitous ARP Replies |

Commit Message

Linus Lüssing Feb. 14, 2019, 3:51 p.m. UTC 
  A gratuitous ARP Reply has the following format (example):

Sender MAC: 00:12:34:56:78:9A
Sender IP:  192.168.2.3
Target MAC: FF:FF:FF:FF:FF:FF
Target IP:  192.168.2.3

A gratuitous ARP Reply is commonly used to update an ARP table in the
network in an unsolicited way. Here, the host with the MAC address
00:12:34:56:78:9A announces that it is now the owner of 192.168.2.3.
Gratuitous ARP Replies are usually used for redundancy or for IP address
handovers between hosts.

So far, gratuitous ARP Replies were ignored for DAT processing as it
contains a broadcast MAC address. This patch changes this and allows
snooping such ARP messages, too.

Special care needs to be taken with the target MAC, to not accidentally
add this broadcast MAC to the DAT cache.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>

---
Gratuitous ARP Replies were ignored since this commit:

ab361a9ccc5 ("batman-adv: filter ARP packets with invalid MAC addresses in DAT")

Daniel Ehlers is currently working on a distributed DHCP server for mesh
networks. For cases like these it would be helpful if userspace programs
were able to add entries to the DAT, too. Sending gratuitous ARP Replies
would be one easy way for userspace tools to do so.

This patch was verified in VMs with gratuitous ARP Replies generated via
"mausezahn". Sending such packets with a 00:00:00:00:00:00 ethernet
frame destination address even allows updating the DAT without actually
broadcasting the original frame into the mesh.

[0]: https://github.com/sargon/ddhcpd,
[1]: https://media.freifunk.net/search/?q=ddhcp
---
 net/batman-adv/distributed-arp-table.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Marek Lindner Feb. 14, 2019, 4:16 p.m. UTC | #1 
On Thursday, 14 February 2019 23:51:00 HKT Linus Lüssing wrote:
> So far, gratuitous ARP Replies were ignored for DAT processing as it
> contains a broadcast MAC address. This patch changes this and allows
> snooping such ARP messages, too.

> Gratuitous ARP Replies were ignored since this commit:
> ab361a9ccc5 ("batman-adv: filter ARP packets with invalid MAC addresses in 
DAT")

You're kind enough to mention when the filter was introduced but fail to 
explain why the filter introduced in the past was has outlived its usefulness 
or how the new behavior addresses the  previous concerns.

Cheers,
Marek
Linus Lüssing Feb. 14, 2019, 5:49 p.m. UTC | #2 
On Fri, Feb 15, 2019 at 12:16:31AM +0800, Marek Lindner wrote:
> On Thursday, 14 February 2019 23:51:00 HKT Linus Lüssing wrote:
> > So far, gratuitous ARP Replies were ignored for DAT processing as it
> > contains a broadcast MAC address. This patch changes this and allows
> > snooping such ARP messages, too.
> 
> > Gratuitous ARP Replies were ignored since this commit:
> > ab361a9ccc5 ("batman-adv: filter ARP packets with invalid MAC addresses in 
> DAT")
> 
> You're kind enough to mention when the filter was introduced but fail to 
> explain why the filter introduced in the past was has outlived its usefulness 
> or how the new behavior addresses the  previous concerns.

That patch added filtering for both zero and broadcast MAC
addresses. While the original premise is correct - we do not want
those addresses in the DAT, the assumption that a broadcast MAC
address as ARP target MAC address were invalid, is wrong.
Gratuitous ARP Replies are valid packets.

So that patch was a bit too strict in that regard, I think. For
gratuitous ARP it's enough to ignore the (Target MAC/Target IP)
pair. Snooping the (Sender MAC/Sender IP) should be fine.
Antonio Quartulli Feb. 15, 2019, 6:36 a.m. UTC | #3 
Hi,

On 15/02/2019 03:49, Linus Lüssing wrote:
> On Fri, Feb 15, 2019 at 12:16:31AM +0800, Marek Lindner wrote:
>> On Thursday, 14 February 2019 23:51:00 HKT Linus Lüssing wrote:
>>> So far, gratuitous ARP Replies were ignored for DAT processing as it
>>> contains a broadcast MAC address. This patch changes this and allows
>>> snooping such ARP messages, too.
>>
>>> Gratuitous ARP Replies were ignored since this commit:
>>> ab361a9ccc5 ("batman-adv: filter ARP packets with invalid MAC addresses in 
>> DAT")
>>
>> You're kind enough to mention when the filter was introduced but fail to 
>> explain why the filter introduced in the past was has outlived its usefulness 
>> or how the new behavior addresses the  previous concerns.
> 
> That patch added filtering for both zero and broadcast MAC
> addresses. While the original premise is correct - we do not want
> those addresses in the DAT, the assumption that a broadcast MAC
> address as ARP target MAC address were invalid, is wrong.
> Gratuitous ARP Replies are valid packets.
> 
> So that patch was a bit too strict in that regard, I think. For
> gratuitous ARP it's enough to ignore the (Target MAC/Target IP)
> pair. Snooping the (Sender MAC/Sender IP) should be fine.
> 

Is there any situation where an OS would reject a gracious ARP? Or are
they always blindly accepted and processed accordingly?
If they have any protection against any misuse, I guess batman-adv
should try to do the same.

Cheers,
Linus Lüssing Feb. 15, 2019, 5:45 p.m. UTC | #4 
Hi Antonio,

On Fri, Feb 15, 2019 at 04:36:08PM +1000, Antonio Quartulli wrote:
> Is there any situation where an OS would reject a gracious ARP? Or are
> they always blindly accepted and processed accordingly?
> If they have any protection against any misuse, I guess batman-adv
> should try to do the same.

I have played some more with gratuitous ARPs and this is what I
could find out:

sysfs, /proc/sys/net/ipv4/conf/<iface>/*arp*:

drop_gratuitous_arp = 0 (default)
arp_accept = 0 (default)
-> no new address added via gratuitous ARP
   cmp.: https://elixir.bootlin.com/linux/v4.20.10/source/net/ipv4/arp.c#L872
-> but existing entries are updated

drop_gratuitous_arp = 1:
-> grat. ARP ignored completely
   cmp.: https://elixir.bootlin.com/linux/v4.20.10/source/net/ipv4/arp.c#L776

drop_gratuitous_arp = 0 (default)
arp_accept = 1
-> grat. ARP Reply updates existing entries and creates new ones


Also, I noticed that using a zero MAC address for the ethernet header
destination did not work. Even if setting promisc-mode on the veth
in its extra network namespace interface, the neighbor table would
not be populated. Using the broadcast MAC for the ethernet header
(and arp_accept = 1) worked, though. But would defeat the idea
of using gratuitous ARP to populate the DHT without flooded
messages. I wasn't able to spot where the ethernet destination is
checked yet.

Regards, Linus
diff mbox series

Patch

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index 310a4f35..c8923c7d 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -373,9 +373,12 @@  batadv_dat_entry_hash_find(struct batadv_priv *bat_priv, __be32 ip,
 static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip,
 				 u8 *mac_addr, unsigned short vid)
 {
-	struct batadv_dat_entry *dat_entry;
+	struct batadv_dat_entry *dat_entry = NULL;
 	int hash_added;
 
+	if (is_multicast_ether_addr(mac_addr))
+		goto out;
+
 	dat_entry = batadv_dat_entry_hash_find(bat_priv, ip, vid);
 	/* if this entry is already known, just update it */
 	if (dat_entry) {
@@ -1117,8 +1120,7 @@  static u16 batadv_arp_get_type(struct batadv_priv *bat_priv,
 	/* don't care about the destination MAC address in ARP requests */
 	if (arphdr->ar_op != htons(ARPOP_REQUEST)) {
 		hw_dst = batadv_arp_hw_dst(skb, hdr_size);
-		if (is_zero_ether_addr(hw_dst) ||
-		    is_multicast_ether_addr(hw_dst))
+		if (is_zero_ether_addr(hw_dst))
 			goto out;
 	}