From 010e55beeda23e3e5e9ae0657be89d767118f3b0 Mon Sep 17 00:00:00 2001 From: Dan Robertson Date: Sat, 21 Apr 2018 00:23:48 +0000 Subject: [PATCH] Fix ICMPv6 checksum function The ICMPv6 checksum calculation requires the pseudo header. Closes: #196 Approved by: whitequark --- src/iface/ethernet.rs | 12 +++++---- src/wire/icmpv6.rs | 59 ++++++++++++++++++++++++++++--------------- src/wire/ndisc.rs | 11 +++++--- 3 files changed, 52 insertions(+), 30 deletions(-) diff --git a/src/iface/ethernet.rs b/src/iface/ethernet.rs index 6f46f70..c14ac3f 100644 --- a/src/iface/ethernet.rs +++ b/src/iface/ethernet.rs @@ -735,12 +735,13 @@ impl<'b, 'c> InterfaceInner<'b, 'c> { } #[cfg(feature = "proto-ipv6")] - fn process_icmpv6<'frame>(&self, _sockets: &mut SocketSet, ip_repr: IpRepr, - ip_payload: &'frame [u8]) -> Result> + fn process_icmpv6<'frame>(&mut self, _sockets: &mut SocketSet, + ip_repr: IpRepr, ip_payload: &'frame [u8]) -> Result> { let icmp_packet = Icmpv6Packet::new_checked(ip_payload)?; let checksum_caps = self.device_capabilities.checksum.clone(); - let icmp_repr = Icmpv6Repr::parse(&icmp_packet, &checksum_caps)?; + let icmp_repr = Icmpv6Repr::parse(&ip_repr.src_addr(), &ip_repr.dst_addr(), + &icmp_packet, &checksum_caps)?; match icmp_repr { // Respond to echo requests. @@ -971,8 +972,9 @@ impl<'b, 'c> InterfaceInner<'b, 'c> { #[cfg(feature = "proto-ipv6")] Packet::Icmpv6((ipv6_repr, icmpv6_repr)) => { self.dispatch_ip(tx_token, timestamp, IpRepr::Ipv6(ipv6_repr), - |_ip_repr, payload| { - icmpv6_repr.emit(&mut Icmpv6Packet::new(payload), &checksum_caps); + |ip_repr, payload| { + icmpv6_repr.emit(&ip_repr.src_addr(), &ip_repr.dst_addr(), + &mut Icmpv6Packet::new(payload), &checksum_caps); }) } #[cfg(feature = "socket-raw")] diff --git a/src/wire/icmpv6.rs b/src/wire/icmpv6.rs index 7d18ad2..1e3a37b 100644 --- a/src/wire/icmpv6.rs +++ b/src/wire/icmpv6.rs @@ -4,7 +4,7 @@ use byteorder::{ByteOrder, NetworkEndian}; use {Error, Result}; use phy::ChecksumCapabilities; use super::ip::checksum; -use super::{Ipv6Packet, Ipv6Repr}; +use super::{IpAddress, IpProtocol, Ipv6Packet, Ipv6Repr}; use super::NdiscRepr; enum_with_unknown! { @@ -337,11 +337,15 @@ impl> Packet { /// /// # Fuzzing /// This function always returns `true` when fuzzing. - pub fn verify_checksum(&self) -> bool { + pub fn verify_checksum(&self, src_addr: &IpAddress, dst_addr: &IpAddress) -> bool { if cfg!(fuzzing) { return true } let data = self.buffer.as_ref(); - checksum::data(data) == !0 + checksum::combine(&[ + checksum::pseudo_header(src_addr, dst_addr, IpProtocol::Icmpv6, + data.len() as u32), + checksum::data(data) + ]) == !0 } } @@ -417,11 +421,15 @@ impl + AsMut<[u8]>> Packet { } /// Compute and fill in the header checksum. - pub fn fill_checksum(&mut self) { + pub fn fill_checksum(&mut self, src_addr: &IpAddress, dst_addr: &IpAddress) { self.set_checksum(0); let checksum = { let data = self.buffer.as_ref(); - !checksum::data(data) + !checksum::combine(&[ + checksum::pseudo_header(src_addr, dst_addr, IpProtocol::Icmpv6, + data.len() as u32), + checksum::data(data) + ]) }; self.set_checksum(checksum) } @@ -483,7 +491,8 @@ pub enum Repr<'a> { impl<'a> Repr<'a> { /// Parse an Internet Control Message Protocol version 6 packet and return /// a high-level representation. - pub fn parse(packet: &Packet<&'a T>, checksum_caps: &ChecksumCapabilities) + pub fn parse(src_addr: &IpAddress, dst_addr: &IpAddress, + packet: &Packet<&'a T>, checksum_caps: &ChecksumCapabilities) -> Result> where T: AsRef<[u8]> + ?Sized { fn create_packet_from_payload<'a, T>(packet: &Packet<&'a T>) @@ -503,7 +512,9 @@ impl<'a> Repr<'a> { Ok((payload, repr)) } // Valid checksum is expected. - if checksum_caps.icmpv6.rx() && !packet.verify_checksum() { return Err(Error::Checksum) } + if checksum_caps.icmpv6.rx() && !packet.verify_checksum(src_addr, dst_addr) { + return Err(Error::Checksum) + } match (packet.msg_type(), packet.msg_code()) { (Message::DstUnreachable, code) => { @@ -580,7 +591,8 @@ impl<'a> Repr<'a> { /// Emit a high-level representation into an Internet Control Message Protocol version 6 /// packet. - pub fn emit(&self, packet: &mut Packet<&mut T>, checksum_caps: &ChecksumCapabilities) + pub fn emit(&self, src_addr: &IpAddress, dst_addr: &IpAddress, + packet: &mut Packet<&mut T>, checksum_caps: &ChecksumCapabilities) where T: AsRef<[u8]> + AsMut<[u8]> + ?Sized { fn emit_contained_packet(buffer: &mut [u8], header: Ipv6Repr, data: &[u8]) { let mut ip_packet = Ipv6Packet::new(buffer); @@ -646,7 +658,7 @@ impl<'a> Repr<'a> { } if checksum_caps.icmpv6.tx() { - packet.fill_checksum() + packet.fill_checksum(src_addr, dst_addr); } else { // make sure we get a consistently zeroed checksum, since implementations might rely on it packet.set_checksum(0); @@ -657,10 +669,11 @@ impl<'a> Repr<'a> { #[cfg(test)] mod test { use wire::{Ipv6Address, Ipv6Repr, IpProtocol}; + use wire::ip::test::{MOCK_IP_ADDR_1, MOCK_IP_ADDR_2}; use super::*; static ECHO_PACKET_BYTES: [u8; 12] = - [0x80, 0x00, 0x16, 0xfe, + [0x80, 0x00, 0x19, 0xb3, 0x12, 0x34, 0xab, 0xcd, 0xaa, 0x00, 0x00, 0xff]; @@ -668,7 +681,7 @@ mod test { [0xaa, 0x00, 0x00, 0xff]; static PKT_TOO_BIG_BYTES: [u8; 60] = - [0x02, 0x00, 0x0d, 0x44, + [0x02, 0x00, 0x0f, 0xc9, 0x00, 0x00, 0x05, 0xdc, 0x60, 0x00, 0x00, 0x00, 0x00, 0x0c, 0x11, 0x40, @@ -737,11 +750,11 @@ mod test { let packet = Packet::new(&ECHO_PACKET_BYTES[..]); assert_eq!(packet.msg_type(), Message::EchoRequest); assert_eq!(packet.msg_code(), 0); - assert_eq!(packet.checksum(), 0x16fe); + assert_eq!(packet.checksum(), 0x19b3); assert_eq!(packet.echo_ident(), 0x1234); assert_eq!(packet.echo_seq_no(), 0xabcd); assert_eq!(packet.payload(), &ECHO_PACKET_PAYLOAD[..]); - assert_eq!(packet.verify_checksum(), true); + assert_eq!(packet.verify_checksum(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2), true); assert!(!packet.msg_type().is_error()); } @@ -754,14 +767,15 @@ mod test { packet.set_echo_ident(0x1234); packet.set_echo_seq_no(0xabcd); packet.payload_mut().copy_from_slice(&ECHO_PACKET_PAYLOAD[..]); - packet.fill_checksum(); + packet.fill_checksum(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2); assert_eq!(&packet.into_inner()[..], &ECHO_PACKET_BYTES[..]); } #[test] fn test_echo_repr_parse() { let packet = Packet::new(&ECHO_PACKET_BYTES[..]); - let repr = Repr::parse(&packet, &ChecksumCapabilities::default()).unwrap(); + let repr = Repr::parse(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2, + &packet, &ChecksumCapabilities::default()).unwrap(); assert_eq!(repr, echo_packet_repr()); } @@ -770,7 +784,8 @@ mod test { let repr = echo_packet_repr(); let mut bytes = vec![0xa5; repr.buffer_len()]; let mut packet = Packet::new(&mut bytes); - repr.emit(&mut packet, &ChecksumCapabilities::default()); + repr.emit(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2, + &mut packet, &ChecksumCapabilities::default()); assert_eq!(&packet.into_inner()[..], &ECHO_PACKET_BYTES[..]); } @@ -779,10 +794,10 @@ mod test { let packet = Packet::new(&PKT_TOO_BIG_BYTES[..]); assert_eq!(packet.msg_type(), Message::PktTooBig); assert_eq!(packet.msg_code(), 0); - assert_eq!(packet.checksum(), 0x0d44); + assert_eq!(packet.checksum(), 0x0fc9); assert_eq!(packet.pkt_too_big_mtu(), 1500); assert_eq!(packet.payload(), &PKT_TOO_BIG_IP_PAYLOAD[..]); - assert_eq!(packet.verify_checksum(), true); + assert_eq!(packet.verify_checksum(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2), true); assert!(packet.msg_type().is_error()); } @@ -794,14 +809,15 @@ mod test { packet.set_msg_code(0); packet.set_pkt_too_big_mtu(1500); packet.payload_mut().copy_from_slice(&PKT_TOO_BIG_IP_PAYLOAD[..]); - packet.fill_checksum(); + packet.fill_checksum(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2); assert_eq!(&packet.into_inner()[..], &PKT_TOO_BIG_BYTES[..]); } #[test] fn test_too_big_repr_parse() { let packet = Packet::new(&PKT_TOO_BIG_BYTES[..]); - let repr = Repr::parse(&packet, &ChecksumCapabilities::default()).unwrap(); + let repr = Repr::parse(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2, + &packet, &ChecksumCapabilities::default()).unwrap(); assert_eq!(repr, too_big_packet_repr()); } @@ -810,7 +826,8 @@ mod test { let repr = too_big_packet_repr(); let mut bytes = vec![0xa5; repr.buffer_len()]; let mut packet = Packet::new(&mut bytes); - repr.emit(&mut packet, &ChecksumCapabilities::default()); + repr.emit(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2, + &mut packet, &ChecksumCapabilities::default()); assert_eq!(&packet.into_inner()[..], &PKT_TOO_BIG_BYTES[..]); } } diff --git a/src/wire/ndisc.rs b/src/wire/ndisc.rs index 2c13fb3..4996df7 100644 --- a/src/wire/ndisc.rs +++ b/src/wire/ndisc.rs @@ -454,9 +454,10 @@ mod test { use phy::ChecksumCapabilities; use super::*; use wire::Icmpv6Repr; + use wire::ip::test::{MOCK_IP_ADDR_1, MOCK_IP_ADDR_2}; static ROUTER_ADVERT_BYTES: [u8; 24] = - [0x86, 0x00, 0xa7, 0x35, + [0x86, 0x00, 0xa9, 0xde, 0x40, 0x80, 0x03, 0x84, 0x00, 0x00, 0x03, 0x84, 0x00, 0x00, 0x03, 0x84, @@ -504,14 +505,15 @@ mod test { packet.set_reachable_time(Duration::from_millis(900)); packet.set_retrans_time(Duration::from_millis(900)); packet.payload_mut().copy_from_slice(&SOURCE_LINK_LAYER_OPT[..]); - packet.fill_checksum(); + packet.fill_checksum(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2); assert_eq!(&packet.into_inner()[..], &ROUTER_ADVERT_BYTES[..]); } #[test] fn test_router_advert_repr_parse() { let packet = Packet::new(&ROUTER_ADVERT_BYTES[..]); - assert_eq!(Icmpv6Repr::parse(&packet, &ChecksumCapabilities::default()).unwrap(), + assert_eq!(Icmpv6Repr::parse(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2, + &packet, &ChecksumCapabilities::default()).unwrap(), create_repr()); } @@ -519,7 +521,8 @@ mod test { fn test_router_advert_repr_emit() { let mut bytes = vec![0x2a; 24]; let mut packet = Packet::new(&mut bytes[..]); - create_repr().emit(&mut packet, &ChecksumCapabilities::default()); + create_repr().emit(&MOCK_IP_ADDR_1, &MOCK_IP_ADDR_2, + &mut packet, &ChecksumCapabilities::default()); assert_eq!(&packet.into_inner()[..], &ROUTER_ADVERT_BYTES[..]); } }