ARP fixes

src/iface/interface.rs

@@ -952,43 +952,12 @@ impl<'a> InterfaceInner<'a> {
  #[cfg(feature = "proto-ipv4")]
  EthernetProtocol::Ipv4 => {
  let ipv4_packet = Ipv4Packet::new_checked(eth_frame.payload())?;
- if eth_frame.src_addr().is_unicast() && ipv4_packet.src_addr().is_unicast() {
- // Fill the neighbor cache from IP header of unicast frames.
- let ip_addr = IpAddress::Ipv4(ipv4_packet.src_addr());
- if self.in_same_network(&ip_addr) {
- self.neighbor_cache.as_mut().unwrap().fill(
- ip_addr,
- eth_frame.src_addr(),
- cx.now,
- );
- }
- }
-
  self.process_ipv4(cx, sockets, &ipv4_packet)
  .map(|o| o.map(EthernetPacket::Ip))
  }
  #[cfg(feature = "proto-ipv6")]
  EthernetProtocol::Ipv6 => {
  let ipv6_packet = Ipv6Packet::new_checked(eth_frame.payload())?;
- if eth_frame.src_addr().is_unicast() && ipv6_packet.src_addr().is_unicast() {
- // Fill the neighbor cache from IP header of unicast frames.
- let ip_addr = IpAddress::Ipv6(ipv6_packet.src_addr());
- if self.in_same_network(&ip_addr)
- && self
- .neighbor_cache
- .as_mut()
- .unwrap()
- .lookup(&ip_addr, cx.now)
- .found()
- {
- self.neighbor_cache.as_mut().unwrap().fill(
- ip_addr,
- eth_frame.src_addr(),
- cx.now,
- );
- }
- }
-
  self.process_ipv6(cx, sockets, &ipv6_packet)
  .map(|o| o.map(EthernetPacket::Ip))
  }
@@ -1030,29 +999,46 @@ impl<'a> InterfaceInner<'a> {
  let arp_repr = ArpRepr::parse(&arp_packet)?;
 
  match arp_repr {
- // Respond to ARP requests aimed at us, and fill the ARP cache from all ARP
- // requests and replies, to minimize the chance that we have to perform
- // an explicit ARP request.
  ArpRepr::EthernetIpv4 {
  operation,
  source_hardware_addr,
  source_protocol_addr,
  target_protocol_addr,
  ..
  } => {
- if source_protocol_addr.is_unicast() && source_hardware_addr.is_unicast() {
- self.neighbor_cache.as_mut().unwrap().fill(
- source_protocol_addr.into(),
- source_hardware_addr,
- timestamp,
- );
- } else {
- // Discard packets with non-unicast source addresses.
- net_debug!("non-unicast source address");
+ // Only process ARP packets for us.
+ if !self.has_ip_addr(target_protocol_addr) {
+ return Ok(None);
+ }
+
+ // Only process REQUEST and RESPONSE.
+ if let ArpOperation::Unknown(_) = operation {
+ net_debug!("arp: unknown operation code");
  return Err(Error::Malformed);
  }
 
- if operation == ArpOperation::Request && self.has_ip_addr(target_protocol_addr) {
+ // Discard packets with non-unicast source addresses.
+ if !source_protocol_addr.is_unicast() || !source_hardware_addr.is_unicast() {
+ net_debug!("arp: non-unicast source address");
+ return Err(Error::Malformed);
+ }
+
+ if !self.in_same_network(&IpAddress::Ipv4(source_protocol_addr)) {
+ net_debug!("arp: source IP address not in same network as us");
+ return Err(Error::Malformed);
+ }
+
+ // Fill the ARP cache from any ARP packet aimed at us (both request or response).
+ // We fill from requests too because if someone is requesting our address they
+ // are probably going to talk to us, so we avoid having to request their address
+ // when we later reply to them.
+ self.neighbor_cache.as_mut().unwrap().fill(
+ source_protocol_addr.into(),
+ source_hardware_addr,
+ timestamp,
+ );
+
+ if operation == ArpOperation::Request {
  Ok(Some(EthernetPacket::Arp(ArpRepr::EthernetIpv4 {
  operation: ArpOperation::Reply,
  source_hardware_addr: self.ethernet_addr.unwrap(),
@@ -2885,15 +2871,15 @@ mod test {
  Ok(None)
  );
 
- // Ensure the address of the requestor was entered in the cache
+ // Ensure the address of the requestor was NOT entered in the cache
  assert_eq!(
  iface.inner.lookup_hardware_addr(
  &cx,
  MockTxToken,
  &IpAddress::Ipv4(Ipv4Address([0x7f, 0x00, 0x00, 0x01])),
  &IpAddress::Ipv4(remote_ip_addr)
  ),
- Ok((remote_hw_addr, MockTxToken))
+ Err(Error::Unaddressable)
  );
  }