Merge lp:~mpontillo/maas/fix-bogus-mac-in-moved-event--bug-1609194 into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Fix bug where ARP parser considers the all-zeroes MAC address to be valid.

[Blake Rouse (blake-rouse)]

review: Needs Information

[Blake Rouse (blake-rouse)]

Sounds good.

review: Approve

=== modified file 'src/provisioningserver/utils/arp.py'
--- src/provisioningserver/utils/arp.py	2016-08-09 00:14:06 +0000
+++ src/provisioningserver/utils/arp.py	2016-08-25 16:11:01 +0000
@@ -183,17 +183,20 @@
             # This is an ARP request.
             # We can find a binding in the (source_eui, source_ip)
             source_ip = self.source_ip
-            if int(source_ip) != 0:
+            source_eui = self.source_eui
+            if int(source_ip) != 0 and int(source_eui) != 0:
                 yield (source_ip, self.source_eui)
         elif self.operation == 2:
             # This is an ARP reply.
             # We can find a binding in both the (source_eui, source_ip) and
             # the (target_eui, target_ip).
             source_ip = self.source_ip
+            source_eui = self.source_eui
             target_ip = self.target_ip
-            if int(source_ip) != 0:
+            target_eui = self.target_eui
+            if int(source_ip) != 0 and int(source_eui) != 0:
                 yield (source_ip, self.source_eui)
-            if int(target_ip) != 0:
+            if int(target_ip) != 0 and int(target_eui) != 0:
                 yield (target_ip, self.target_eui)
 
     def write(self, out=sys.stdout):

=== modified file 'src/provisioningserver/utils/tests/test_arp.py'
--- src/provisioningserver/utils/tests/test_arp.py	2016-08-09 00:14:06 +0000
+++ src/provisioningserver/utils/tests/test_arp.py	2016-08-25 16:11:01 +0000
@@ -217,6 +217,38 @@
         self.assertItemsEqual(
             arp.bindings(), [(IPAddress(pkt_sender_ip), EUI(pkt_sender_mac))])
 
+    def test__bindings__skips_null_source_eui_for_request(self):
+        pkt_sender_mac = '00:00:00:00:00:00'
+        pkt_sender_ip = '192.168.0.1'
+        pkt_target_ip = '192.168.0.2'
+        pkt_target_mac = '00:00:00:00:00:00'
+        arp = ARP(make_arp_packet(
+            pkt_sender_ip, pkt_sender_mac, pkt_target_ip, pkt_target_mac,
+            op=ARP_OPERATION.REQUEST))
+        self.assertItemsEqual(arp.bindings(), [])
+
+    def test__bindings__skips_null_source_eui_in_reply(self):
+        pkt_sender_mac = '00:00:00:00:00:00'
+        pkt_sender_ip = '192.168.0.1'
+        pkt_target_ip = '192.168.0.2'
+        pkt_target_mac = '02:03:04:05:06:07'
+        arp = ARP(make_arp_packet(
+            pkt_sender_ip, pkt_sender_mac, pkt_target_ip, pkt_target_mac,
+            op=ARP_OPERATION.REPLY))
+        self.assertItemsEqual(
+            arp.bindings(), [(IPAddress(pkt_target_ip), EUI(pkt_target_mac))])
+
+    def test__bindings__skips_null_target_eui_in_reply(self):
+        pkt_sender_mac = '01:02:03:04:05:06'
+        pkt_sender_ip = '192.168.0.1'
+        pkt_target_ip = '192.168.0.2'
+        pkt_target_mac = '00:00:00:00:00:00'
+        arp = ARP(make_arp_packet(
+            pkt_sender_ip, pkt_sender_mac, pkt_target_ip, pkt_target_mac,
+            op=ARP_OPERATION.REPLY))
+        self.assertItemsEqual(
+            arp.bindings(), [(IPAddress(pkt_sender_ip), EUI(pkt_sender_mac))])
+
 
 class TestUpdateBindingsAndGetEvent(MAASTestCase):