From 8b27330c8b3358e0e42c21d1f07cdcddb3d10e61 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 24 Jun 2017 09:15:22 +0000 Subject: [PATCH] Do not attempt to validate length of packets being emitted. This is a form of an uninitialized read bug; although safe it caused panics. In short, transmit buffers received from the network stack should be considered uninitialized (in practice they will often contain previously transmitted packets or parts thereof). Wrapping them with the only method we had (e.g. Ipv4Packet) treated the buffer as if it contained a valid incoming packet, which can easily fail with Error::Truncated. This commit splits every `fn new(buffer: T) -> Result` method on a `Packet` into three smaller ones: * `fn check_len(&self) -> Result<(), Error>`, purely a validator; * `fn new(T) -> Self`, purely a wrapper; * `fn new_checked(T) -> Result`, a validating wrapper. This makes it easy to process ingress packets (using `new_checked`), egress packets (using `new`), and, if needed, maintain the invariants at any point during packet construction (using `check_len`). Fixes #17. --- examples/ping.rs | 8 +++---- src/iface/ethernet.rs | 34 ++++++++++++++++------------- src/lib.rs | 3 +-- src/socket/raw.rs | 4 ++-- src/socket/tcp.rs | 12 +++++------ src/socket/udp.rs | 4 ++-- src/wire/arp.rs | 50 +++++++++++++++++++++++++++++-------------- src/wire/ethernet.rs | 31 ++++++++++++++++++++------- src/wire/icmpv4.rs | 49 ++++++++++++++++++++++++++++-------------- src/wire/ip.rs | 21 ++++++++++-------- src/wire/ipv4.rs | 48 ++++++++++++++++++++++++++++------------- src/wire/mod.rs | 47 +++++++++++++++++++++++++++++----------- src/wire/tcp.rs | 41 +++++++++++++++++++++++++---------- src/wire/udp.rs | 44 ++++++++++++++++++++++++++----------- 14 files changed, 264 insertions(+), 132 deletions(-) diff --git a/examples/ping.rs b/examples/ping.rs index 47467c9..a3b0ec0 100644 --- a/examples/ping.rs +++ b/examples/ping.rs @@ -84,9 +84,9 @@ fn main() { .send(ipv4_repr.buffer_len() + icmp_repr.buffer_len()) .unwrap(); - let mut ipv4_packet = Ipv4Packet::new(raw_payload).unwrap(); + let mut ipv4_packet = Ipv4Packet::new(raw_payload); ipv4_repr.emit(&mut ipv4_packet); - let mut icmp_packet = Icmpv4Packet::new(ipv4_packet.payload_mut()).unwrap(); + let mut icmp_packet = Icmpv4Packet::new(ipv4_packet.payload_mut()); icmp_repr.emit(&mut icmp_packet); waiting_queue.insert(seq_no, timestamp); @@ -96,11 +96,11 @@ fn main() { if socket.can_recv() { let payload = socket.recv().unwrap(); - let ipv4_packet = Ipv4Packet::new(payload).unwrap(); + let ipv4_packet = Ipv4Packet::new(payload); let ipv4_repr = Ipv4Repr::parse(&ipv4_packet).unwrap(); if ipv4_repr.src_addr == remote_addr && ipv4_repr.dst_addr == local_addr { - let icmp_packet = Icmpv4Packet::new(ipv4_packet.payload()).unwrap(); + let icmp_packet = Icmpv4Packet::new(ipv4_packet.payload()); let icmp_repr = Icmpv4Repr::parse(&icmp_packet); if let Ok(Icmpv4Repr::EchoReply { seq_no, data, .. }) = icmp_repr { diff --git a/src/iface/ethernet.rs b/src/iface/ethernet.rs index 092eaa0..eec3e9c 100644 --- a/src/iface/ethernet.rs +++ b/src/iface/ethernet.rs @@ -117,7 +117,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { // Now, receive any incoming packets. let rx_buffer = try!(self.device.receive()); - let eth_frame = try!(EthernetFrame::new(&rx_buffer)); + let eth_frame = try!(EthernetFrame::new_checked(&rx_buffer)); if !eth_frame.dst_addr().is_broadcast() && eth_frame.dst_addr() != self.hardware_addr { @@ -128,7 +128,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { match eth_frame.ethertype() { // Snoop all ARP traffic, and respond to ARP packets directed at us. EthernetProtocol::Arp => { - let arp_packet = try!(ArpPacket::new(eth_frame.payload())); + let arp_packet = try!(ArpPacket::new_checked(eth_frame.payload())); match try!(ArpRepr::parse(&arp_packet)) { // Respond to ARP requests aimed at us, and fill the ARP cache // from all ARP requests, including gratuitous. @@ -170,7 +170,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { // Handle IP packets directed at us. EthernetProtocol::Ipv4 => { - let ipv4_packet = try!(Ipv4Packet::new(eth_frame.payload())); + let ipv4_packet = try!(Ipv4Packet::new_checked(eth_frame.payload())); let ipv4_repr = try!(Ipv4Repr::parse(&ipv4_packet)); // Fill the ARP cache from IP header. @@ -195,7 +195,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { // Respond to ICMP packets. Ipv4Repr { protocol: IpProtocol::Icmp, src_addr, dst_addr, .. } => { - let icmp_packet = try!(Icmpv4Packet::new(ipv4_packet.payload())); + let icmp_packet = try!(Icmpv4Packet::new_checked(ipv4_packet.payload())); let icmp_repr = try!(Icmpv4Repr::parse(&icmp_packet)); match icmp_repr { // Respond to echo requests. @@ -251,7 +251,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { } if !handled && protocol == IpProtocol::Tcp { - let tcp_packet = try!(TcpPacket::new(ipv4_packet.payload())); + let tcp_packet = try!(TcpPacket::new_checked(ipv4_packet.payload())); if !tcp_packet.rst() { let tcp_reply_repr = TcpRepr { src_port: tcp_packet.dst_port(), @@ -312,12 +312,13 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { let frame_len = EthernetFrame::<&[u8]>::buffer_len($ip_repr.buffer_len() + $ip_repr.payload_len); $tx_buffer = try!(self.device.transmit(frame_len)); - $frame = try!(EthernetFrame::new(&mut $tx_buffer)); + $frame = EthernetFrame::new_checked(&mut $tx_buffer) + .expect("transmit frame too small"); $frame.set_src_addr(self.hardware_addr); $frame.set_dst_addr(dst_hardware_addr); $frame.set_ethertype(EthernetProtocol::Ipv4); - let mut ip_packet = try!(Ipv4Packet::new($frame.payload_mut())); + let mut ip_packet = Ipv4Packet::new($frame.payload_mut()); $ip_repr.emit(&mut ip_packet); ip_packet }) @@ -327,7 +328,8 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { Response::Arp(repr) => { let tx_len = EthernetFrame::<&[u8]>::buffer_len(repr.buffer_len()); let mut tx_buffer = try!(self.device.transmit(tx_len)); - let mut frame = try!(EthernetFrame::new(&mut tx_buffer)); + let mut frame = EthernetFrame::new_checked(&mut tx_buffer) + .expect("transmit frame too small"); frame.set_src_addr(self.hardware_addr); frame.set_dst_addr(match repr { ArpRepr::EthernetIpv4 { target_hardware_addr, .. } => target_hardware_addr, @@ -335,7 +337,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { }); frame.set_ethertype(EthernetProtocol::Arp); - let mut packet = try!(ArpPacket::new(frame.payload_mut())); + let mut packet = ArpPacket::new(frame.payload_mut()); repr.emit(&mut packet); Ok(()) @@ -345,7 +347,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { let mut tx_buffer; let mut frame; let mut ip_packet = ip_response!(tx_buffer, frame, ip_repr); - let mut icmp_packet = try!(Icmpv4Packet::new(ip_packet.payload_mut())); + let mut icmp_packet = Icmpv4Packet::new(ip_packet.payload_mut()); icmp_repr.emit(&mut icmp_packet); Ok(()) } @@ -354,7 +356,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { let mut tx_buffer; let mut frame; let mut ip_packet = ip_response!(tx_buffer, frame, ip_repr); - let mut tcp_packet = try!(TcpPacket::new(ip_packet.payload_mut())); + let mut tcp_packet = TcpPacket::new(ip_packet.payload_mut()); tcp_repr.emit(&mut tcp_packet, &IpAddress::Ipv4(ip_repr.src_addr), &IpAddress::Ipv4(ip_repr.dst_addr)); @@ -388,14 +390,15 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { let tx_len = EthernetFrame::<&[u8]>::buffer_len(repr.buffer_len() + payload.buffer_len()); let mut tx_buffer = try!(device.transmit(tx_len)); - let mut frame = try!(EthernetFrame::new(&mut tx_buffer)); + let mut frame = EthernetFrame::new_checked(&mut tx_buffer) + .expect("transmit frame too small"); frame.set_src_addr(src_hardware_addr); frame.set_dst_addr(dst_hardware_addr); frame.set_ethertype(EthernetProtocol::Ipv4); repr.emit(frame.payload_mut()); - let mut ip_packet = try!(Ipv4Packet::new(frame.payload_mut())); + let mut ip_packet = Ipv4Packet::new(frame.payload_mut()); payload.emit(&repr, ip_packet.payload_mut()); } @@ -418,12 +421,13 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> { let tx_len = EthernetFrame::<&[u8]>::buffer_len(payload.buffer_len()); let mut tx_buffer = try!(device.transmit(tx_len)); - let mut frame = try!(EthernetFrame::new(&mut tx_buffer)); + let mut frame = EthernetFrame::new_checked(&mut tx_buffer) + .expect("transmit frame too small"); frame.set_src_addr(src_hardware_addr); frame.set_dst_addr(EthernetAddress([0xff; 6])); frame.set_ethertype(EthernetProtocol::Arp); - let mut arp_packet = try!(ArpPacket::new(frame.payload_mut())); + let mut arp_packet = ArpPacket::new(frame.payload_mut()); payload.emit(&mut arp_packet); } } diff --git a/src/lib.rs b/src/lib.rs index 7396063..927256b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -112,8 +112,7 @@ mod parsing; /// The error type for the networking stack. #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum Error { - /// An incoming packet could not be parsed, or an outgoing packet could not be emitted - /// because a field was out of bounds for the underlying buffer. + /// An incoming packet could not be parsed. Truncated, /// An incoming packet could not be recognized and was dropped. /// E.g. a packet with an unknown EtherType. diff --git a/src/socket/raw.rs b/src/socket/raw.rs index 51c5ce9..c84981a 100644 --- a/src/socket/raw.rs +++ b/src/socket/raw.rs @@ -179,10 +179,10 @@ impl<'a, 'b> RawSocket<'a, 'b> { match self.ip_version { IpVersion::Ipv4 => { - let mut ipv4_packet = Ipv4Packet::new(packet_buf.as_mut())?; + let mut ipv4_packet = Ipv4Packet::new_checked(packet_buf.as_mut())?; ipv4_packet.fill_checksum(); - let ipv4_packet = Ipv4Packet::new(&*ipv4_packet.into_inner())?; + let ipv4_packet = Ipv4Packet::new(&*ipv4_packet.into_inner()); let raw_repr = RawRepr(ipv4_packet.payload()); let ipv4_repr = Ipv4Repr::parse(&ipv4_packet)?; emit(&IpRepr::Ipv4(ipv4_repr), &raw_repr) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index f0ab25f..215e06a 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -636,7 +636,7 @@ impl<'a> TcpSocket<'a> { if ip_repr.protocol() != IpProtocol::Tcp { return Err(Error::Rejected) } - let packet = try!(TcpPacket::new(&payload[..ip_repr.payload_len()])); + let packet = try!(TcpPacket::new_checked(&payload[..ip_repr.payload_len()])); let repr = try!(TcpRepr::parse(&packet, &ip_repr.src_addr(), &ip_repr.dst_addr())); // Reject packets with a wrong destination. @@ -1131,7 +1131,7 @@ impl<'a> IpPayload for TcpRepr<'a> { } fn emit(&self, ip_repr: &IpRepr, payload: &mut [u8]) { - let mut packet = TcpPacket::new(payload).expect("undersized payload"); + let mut packet = TcpPacket::new(payload); self.emit(&mut packet, &ip_repr.src_addr(), &ip_repr.dst_addr()) } } @@ -1216,7 +1216,7 @@ mod test { fn send(socket: &mut TcpSocket, timestamp: u64, repr: &TcpRepr) -> Result<(), Error> { trace!("send: {}", repr); let mut buffer = vec![0; repr.buffer_len()]; - let mut packet = TcpPacket::new(&mut buffer).unwrap(); + let mut packet = TcpPacket::new(&mut buffer); repr.emit(&mut packet, &REMOTE_IP, &LOCAL_IP); let ip_repr = IpRepr::Unspecified { src_addr: REMOTE_IP, @@ -1239,7 +1239,7 @@ mod test { buffer.resize(payload.buffer_len(), 0); payload.emit(&ip_repr, &mut buffer[..]); - let packet = TcpPacket::new(&buffer[..]).unwrap(); + let packet = TcpPacket::new(&buffer[..]); let repr = try!(TcpRepr::parse(&packet, &ip_repr.src_addr(), &ip_repr.dst_addr())); trace!("recv: {}", repr); Ok(f(Ok(repr))) @@ -2565,7 +2565,7 @@ mod test { s.dispatch(0, &limits, &mut |ip_repr, payload| { let mut buffer = vec![0; payload.buffer_len()]; payload.emit(&ip_repr, &mut buffer[..]); - let packet = TcpPacket::new(&buffer[..]).unwrap(); + let packet = TcpPacket::new(&buffer[..]); assert_eq!(packet.window_len(), 32767); Ok(()) }).unwrap(); @@ -2575,7 +2575,7 @@ mod test { s.dispatch(0, &limits, &mut |ip_repr, payload| { let mut buffer = vec![0; payload.buffer_len()]; payload.emit(&ip_repr, &mut buffer[..]); - let packet = TcpPacket::new(&buffer[..]).unwrap(); + let packet = TcpPacket::new(&buffer[..]); assert_eq!(packet.window_len(), 5920); Ok(()) }).unwrap(); diff --git a/src/socket/udp.rs b/src/socket/udp.rs index fe6a5de..b4dc56a 100644 --- a/src/socket/udp.rs +++ b/src/socket/udp.rs @@ -155,7 +155,7 @@ impl<'a, 'b> UdpSocket<'a, 'b> { payload: &[u8]) -> Result<(), Error> { if ip_repr.protocol() != IpProtocol::Udp { return Err(Error::Rejected) } - let packet = try!(UdpPacket::new(&payload[..ip_repr.payload_len()])); + let packet = try!(UdpPacket::new_checked(&payload[..ip_repr.payload_len()])); let repr = try!(UdpRepr::parse(&packet, &ip_repr.src_addr(), &ip_repr.dst_addr())); if repr.dst_port != self.endpoint.port { return Err(Error::Rejected) } @@ -202,7 +202,7 @@ impl<'a> IpPayload for UdpRepr<'a> { } fn emit(&self, repr: &IpRepr, payload: &mut [u8]) { - let mut packet = UdpPacket::new(payload).expect("undersized payload"); + let mut packet = UdpPacket::new(payload); self.emit(&mut packet, &repr.src_addr(), &repr.dst_addr()) } } diff --git a/src/wire/arp.rs b/src/wire/arp.rs index e415a7c..49392e2 100644 --- a/src/wire/arp.rs +++ b/src/wire/arp.rs @@ -62,23 +62,41 @@ mod field { } impl> Packet { - /// Wrap a buffer with an ARP packet. Returns an error if the buffer - /// is too small to contain one. - pub fn new(buffer: T) -> Result, Error> { - let len = buffer.as_ref().len(); + /// Imbue a raw octet buffer with ARP packet structure. + pub fn new(buffer: T) -> Packet { + Packet { buffer } + } + + /// Shorthand for a combination of [new] and [check_len]. + /// + /// [new]: #method.new + /// [check_len]: #method.check_len + pub fn new_checked(buffer: T) -> Result, Error> { + let packet = Self::new(buffer); + try!(packet.check_len()); + Ok(packet) + } + + /// Ensure that no accessor method will panic if called. + /// Returns `Err(Error::Truncated)` if the buffer is too short. + /// + /// The result of this check is invalidated by calling [set_hardware_len] or + /// [set_protocol_len]. + /// + /// [set_hardware_len]: #method.set_hardware_len + /// [set_protocol_len]: #method.set_protocol_len + pub fn check_len(&self) -> Result<(), Error> { + let len = self.buffer.as_ref().len(); if len < field::OPER.end { Err(Error::Truncated) + } else if len < field::TPA(self.hardware_len(), self.protocol_len()).end { + Err(Error::Truncated) } else { - let packet = Packet { buffer: buffer }; - if len < field::TPA(packet.hardware_len(), packet.protocol_len()).end { - Err(Error::Truncated) - } else { - Ok(packet) - } + Ok(()) } } - /// Consumes the packet, returning the underlying buffer. + /// Consume the packet, returning the underlying buffer. pub fn into_inner(self) -> T { self.buffer } @@ -336,7 +354,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent}; impl> PrettyPrint for Packet { fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter, indent: &mut PrettyIndent) -> fmt::Result { - match Packet::new(buffer) { + match Packet::new_checked(buffer) { Err(err) => write!(f, "{}({})\n", indent, err), Ok(frame) => write!(f, "{}{}\n", indent, frame) } @@ -360,7 +378,7 @@ mod test { #[test] fn test_deconstruct() { - let packet = Packet::new(&PACKET_BYTES[..]).unwrap(); + let packet = Packet::new(&PACKET_BYTES[..]); assert_eq!(packet.hardware_type(), Hardware::Ethernet); assert_eq!(packet.protocol_type(), Protocol::Ipv4); assert_eq!(packet.hardware_len(), 6); @@ -375,7 +393,7 @@ mod test { #[test] fn test_construct() { let mut bytes = vec![0; 28]; - let mut packet = Packet::new(&mut bytes).unwrap(); + let mut packet = Packet::new(&mut bytes); packet.set_hardware_type(Hardware::Ethernet); packet.set_protocol_type(Protocol::Ipv4); packet.set_hardware_len(6); @@ -404,7 +422,7 @@ mod test { #[test] fn test_parse() { - let packet = Packet::new(&PACKET_BYTES[..]).unwrap(); + let packet = Packet::new(&PACKET_BYTES[..]); let repr = Repr::parse(&packet).unwrap(); assert_eq!(repr, packet_repr()); } @@ -412,7 +430,7 @@ mod test { #[test] fn test_emit() { let mut bytes = vec![0; 28]; - let mut packet = Packet::new(&mut bytes).unwrap(); + let mut packet = Packet::new(&mut bytes); packet_repr().emit(&mut packet); assert_eq!(&packet.into_inner()[..], &PACKET_BYTES[..]); } diff --git a/src/wire/ethernet.rs b/src/wire/ethernet.rs index 9344bf1..1712172 100644 --- a/src/wire/ethernet.rs +++ b/src/wire/ethernet.rs @@ -91,14 +91,29 @@ mod field { } impl> Frame { - /// Wrap a buffer with an Ethernet frame. Returns an error if the buffer - /// is too small or too large to contain one. - pub fn new(buffer: T) -> Result, Error> { - let len = buffer.as_ref().len(); + /// Imbue a raw octet buffer with Ethernet frame structure. + pub fn new(buffer: T) -> Frame { + Frame { buffer } + } + + /// Shorthand for a combination of [new] and [check_len]. + /// + /// [new]: #method.new + /// [check_len]: #method.check_len + pub fn new_checked(buffer: T) -> Result, Error> { + let packet = Self::new(buffer); + try!(packet.check_len()); + Ok(packet) + } + + /// Ensure that no accessor method will panic if called. + /// Returns `Err(Error::Truncated)` if the buffer is too short. + pub fn check_len(&self) -> Result<(), Error> { + let len = self.buffer.as_ref().len(); if len < field::PAYLOAD.start { Err(Error::Truncated) } else { - Ok(Frame { buffer: buffer }) + Ok(()) } } @@ -194,7 +209,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent}; impl> PrettyPrint for Frame { fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter, indent: &mut PrettyIndent) -> fmt::Result { - let frame = match Frame::new(buffer) { + let frame = match Frame::new_checked(buffer) { Err(err) => return write!(f, "{}({})\n", indent, err), Ok(frame) => frame }; @@ -238,7 +253,7 @@ mod test { #[test] fn test_deconstruct() { - let frame = Frame::new(&FRAME_BYTES[..]).unwrap(); + let frame = Frame::new(&FRAME_BYTES[..]); assert_eq!(frame.dst_addr(), Address([0x01, 0x02, 0x03, 0x04, 0x05, 0x06])); assert_eq!(frame.src_addr(), Address([0x11, 0x12, 0x13, 0x14, 0x15, 0x16])); assert_eq!(frame.ethertype(), EtherType::Ipv4); @@ -248,7 +263,7 @@ mod test { #[test] fn test_construct() { let mut bytes = vec![0; 64]; - let mut frame = Frame::new(&mut bytes).unwrap(); + let mut frame = Frame::new(&mut bytes); frame.set_dst_addr(Address([0x01, 0x02, 0x03, 0x04, 0x05, 0x06])); frame.set_src_addr(Address([0x11, 0x12, 0x13, 0x14, 0x15, 0x16])); frame.set_ethertype(EtherType::Ipv4); diff --git a/src/wire/icmpv4.rs b/src/wire/icmpv4.rs index 312a830..f494432 100644 --- a/src/wire/icmpv4.rs +++ b/src/wire/icmpv4.rs @@ -184,23 +184,41 @@ mod field { } impl> Packet { - /// Wrap a buffer with an ICMPv4 packet. Returns an error if the buffer - /// is too small to contain one. - pub fn new(buffer: T) -> Result, Error> { - let len = buffer.as_ref().len(); + /// Imbue a raw octet buffer with ICMPv4 packet structure. + pub fn new(buffer: T) -> Packet { + Packet { buffer } + } + + /// Shorthand for a combination of [new] and [check_len]. + /// + /// [new]: #method.new + /// [check_len]: #method.check_len + pub fn new_checked(buffer: T) -> Result, Error> { + let packet = Self::new(buffer); + try!(packet.check_len()); + Ok(packet) + } + + /// Ensure that no accessor method will panic if called. + /// Returns `Err(Error::Truncated)` if the buffer is too short. + /// + /// The result of this check is invalidated by calling [set_header_len]. + /// + /// [set_header_len]: #method.set_header_len + pub fn check_len(&self) -> Result<(), Error> { + let len = self.buffer.as_ref().len(); if len < field::CHECKSUM.end { Err(Error::Truncated) } else { - let packet = Packet { buffer: buffer }; - if len < packet.header_len() { + if len < self.header_len() as usize { Err(Error::Truncated) } else { - Ok(packet) + Ok(()) } } } - /// Consumes the packet, returning the underlying buffer. + /// Consume the packet, returning the underlying buffer. pub fn into_inner(self) -> T { self.buffer } @@ -380,7 +398,7 @@ impl<'a> Repr<'a> { }, (Message::DstUnreachable, code) => { - let ip_packet = try!(Ipv4Packet::new(packet.data())); + let ip_packet = try!(Ipv4Packet::new_checked(packet.data())); let payload = &packet.data()[ip_packet.header_len() as usize..]; // RFC 792 requires exactly eight bytes to be returned. @@ -443,8 +461,7 @@ impl<'a> Repr<'a> { packet.set_msg_type(Message::DstUnreachable); packet.set_msg_code(reason.into()); - let mut ip_packet = Ipv4Packet::new(packet.data_mut()) - .expect("undersized data"); + let mut ip_packet = Ipv4Packet::new(packet.data_mut()); header.emit(&mut ip_packet); let mut payload = &mut ip_packet.into_inner()[header.buffer_len()..]; payload.copy_from_slice(&data[..]) @@ -496,7 +513,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent}; impl> PrettyPrint for Packet { fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter, indent: &mut PrettyIndent) -> fmt::Result { - let packet = match Packet::new(buffer) { + let packet = match Packet::new_checked(buffer) { Err(err) => return write!(f, "{}({})\n", indent, err), Ok(packet) => packet }; @@ -525,7 +542,7 @@ mod test { #[test] fn test_echo_deconstruct() { - let packet = Packet::new(&ECHO_PACKET_BYTES[..]).unwrap(); + let packet = Packet::new(&ECHO_PACKET_BYTES[..]); assert_eq!(packet.msg_type(), Message::EchoRequest); assert_eq!(packet.msg_code(), 0); assert_eq!(packet.checksum(), 0x8efe); @@ -538,7 +555,7 @@ mod test { #[test] fn test_echo_construct() { let mut bytes = vec![0; 12]; - let mut packet = Packet::new(&mut bytes).unwrap(); + let mut packet = Packet::new(&mut bytes); packet.set_msg_type(Message::EchoRequest); packet.set_msg_code(0); packet.set_echo_ident(0x1234); @@ -558,7 +575,7 @@ mod test { #[test] fn test_echo_parse() { - let packet = Packet::new(&ECHO_PACKET_BYTES[..]).unwrap(); + let packet = Packet::new(&ECHO_PACKET_BYTES[..]); let repr = Repr::parse(&packet).unwrap(); assert_eq!(repr, echo_packet_repr()); } @@ -567,7 +584,7 @@ mod test { fn test_echo_emit() { let repr = echo_packet_repr(); let mut bytes = vec![0; repr.buffer_len()]; - let mut packet = Packet::new(&mut bytes).unwrap(); + let mut packet = Packet::new(&mut bytes); repr.emit(&mut packet); assert_eq!(&packet.into_inner()[..], &ECHO_PACKET_BYTES[..]); } diff --git a/src/wire/ip.rs b/src/wire/ip.rs index 9ce2f14..4756895 100644 --- a/src/wire/ip.rs +++ b/src/wire/ip.rs @@ -267,9 +267,12 @@ impl IpRepr { /// This function panics if invoked on an unspecified representation. pub fn buffer_len(&self) -> usize { match self { - &IpRepr::Unspecified { .. } => panic!("unspecified IP representation"), - &IpRepr::Ipv4(repr) => repr.buffer_len(), - &IpRepr::__Nonexhaustive => unreachable!() + &IpRepr::Unspecified { .. } => + panic!("unspecified IP representation"), + &IpRepr::Ipv4(repr) => + repr.buffer_len(), + &IpRepr::__Nonexhaustive => + unreachable!() } } @@ -279,12 +282,12 @@ impl IpRepr { /// This function panics if invoked on an unspecified representation. pub fn emit + AsMut<[u8]>>(&self, buffer: T) { match self { - &IpRepr::Unspecified { .. } => panic!("unspecified IP representation"), - &IpRepr::Ipv4(repr) => { - let mut packet = Ipv4Packet::new(buffer).expect("undersized buffer"); - repr.emit(&mut packet) - } - &IpRepr::__Nonexhaustive => unreachable!() + &IpRepr::Unspecified { .. } => + panic!("unspecified IP representation"), + &IpRepr::Ipv4(repr) => + repr.emit(&mut Ipv4Packet::new(buffer)), + &IpRepr::__Nonexhaustive => + unreachable!() } } } diff --git a/src/wire/ipv4.rs b/src/wire/ipv4.rs index bc39f42..fed6b22 100644 --- a/src/wire/ipv4.rs +++ b/src/wire/ipv4.rs @@ -97,23 +97,41 @@ mod field { } impl> Packet { - /// Wrap a buffer with an IPv4 packet. Returns an error if the buffer - /// is too small to contain one. - pub fn new(buffer: T) -> Result, Error> { - let len = buffer.as_ref().len(); + /// Imbue a raw octet buffer with IPv4 packet structure. + pub fn new(buffer: T) -> Packet { + Packet { buffer } + } + + /// Shorthand for a combination of [new] and [check_len]. + /// + /// [new]: #method.new + /// [check_len]: #method.check_len + pub fn new_checked(buffer: T) -> Result, Error> { + let packet = Self::new(buffer); + try!(packet.check_len()); + Ok(packet) + } + + /// Ensure that no accessor method will panic if called. + /// Returns `Err(Error::Truncated)` if the buffer is too short. + /// + /// The result of this check is invalidated by calling [set_header_len]. + /// + /// [set_header_len]: #method.set_header_len + pub fn check_len(&self) -> Result<(), Error> { + let len = self.buffer.as_ref().len(); if len < field::DST_ADDR.end { Err(Error::Truncated) } else { - let packet = Packet { buffer: buffer }; - if len < packet.header_len() as usize { + if len < self.header_len() as usize { Err(Error::Truncated) } else { - Ok(packet) + Ok(()) } } } - /// Consumes the packet, returning the underlying buffer. + /// Consume the packet, returning the underlying buffer. pub fn into_inner(self) -> T { self.buffer } @@ -477,7 +495,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent}; impl> PrettyPrint for Packet { fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter, indent: &mut PrettyIndent) -> fmt::Result { - let (ip_repr, payload) = match Packet::new(buffer) { + let (ip_repr, payload) = match Packet::new_checked(buffer) { Err(err) => return write!(f, "{}({})\n", indent, err), Ok(ip_packet) => { try!(write!(f, "{}{}\n", indent, ip_packet)); @@ -493,7 +511,7 @@ impl> PrettyPrint for Packet { Protocol::Icmp => super::Icmpv4Packet::<&[u8]>::pretty_print(&payload, f, indent), Protocol::Udp => { - match super::UdpPacket::new(payload) { + match super::UdpPacket::new_checked(payload) { Err(err) => write!(f, "{}({})\n", indent, err), Ok(udp_packet) => { match super::UdpRepr::parse(&udp_packet, @@ -506,7 +524,7 @@ impl> PrettyPrint for Packet { } } Protocol::Tcp => { - match super::TcpPacket::new(payload) { + match super::TcpPacket::new_checked(payload) { Err(err) => write!(f, "{}({})\n", indent, err), Ok(tcp_packet) => { match super::TcpRepr::parse(&tcp_packet, @@ -544,7 +562,7 @@ mod test { #[test] fn test_deconstruct() { - let packet = Packet::new(&PACKET_BYTES[..]).unwrap(); + let packet = Packet::new(&PACKET_BYTES[..]); assert_eq!(packet.version(), 4); assert_eq!(packet.header_len(), 20); assert_eq!(packet.dscp(), 0); @@ -566,7 +584,7 @@ mod test { #[test] fn test_construct() { let mut bytes = vec![0; 30]; - let mut packet = Packet::new(&mut bytes).unwrap(); + let mut packet = Packet::new(&mut bytes); packet.set_version(4); packet.set_header_len(20); packet.set_dscp(0); @@ -607,7 +625,7 @@ mod test { #[test] fn test_parse() { - let packet = Packet::new(&REPR_PACKET_BYTES[..]).unwrap(); + let packet = Packet::new(&REPR_PACKET_BYTES[..]); let repr = Repr::parse(&packet).unwrap(); assert_eq!(repr, packet_repr()); } @@ -616,7 +634,7 @@ mod test { fn test_emit() { let repr = packet_repr(); let mut bytes = vec![0; repr.buffer_len() + REPR_PAYLOAD_BYTES.len()]; - let mut packet = Packet::new(&mut bytes).unwrap(); + let mut packet = Packet::new(&mut bytes); repr.emit(&mut packet); packet.payload_mut().copy_from_slice(&REPR_PAYLOAD_BYTES); assert_eq!(&packet.into_inner()[..], &REPR_PACKET_BYTES[..]); diff --git a/src/wire/mod.rs b/src/wire/mod.rs index a1a8a30..610501e 100644 --- a/src/wire/mod.rs +++ b/src/wire/mod.rs @@ -4,22 +4,41 @@ //! of functionality. //! //! * First, it provides functions to extract fields from sequences of octets, -//! and to insert fields into sequences of octets. This happens through the `Frame` -//! and `Packet` families of structures, e.g. [EthernetPacket](struct.EthernetPacket.html). +//! and to insert fields into sequences of octets. This happens `Packet` family of +//! structures, e.g. [EthernetFrame] or [Ipv4Packet]. //! * Second, in cases where the space of valid field values is much smaller than the space //! of possible field values, it provides a compact, high-level representation //! of packet data that can be parsed from and emitted into a sequence of octets. -//! This happens through the `Repr` family of enums, e.g. [ArpRepr](enum.ArpRepr.html). +//! This happens through the `Repr` family of structs and enums, e.g. [ArpRepr] or [Ipv4Repr]. // https://github.com/rust-lang/rust/issues/38739 //! //! -//! The functions in the `wire` module are designed for robustness and use together with -//! `-Cpanic=abort`. The accessor and parsing functions never panic. The setter and emission -//! functions only panic if the underlying buffer is too small. +//! [EthernetFrame]: struct.EthernetFrame.html +//! [Ipv4Packet]: struct.Ipv4Packet.html +//! [ArpRepr]: enum.ArpRepr.html +//! [Ipv4Repr]: struct.Ipv4Repr.html //! -//! The `Frame` and `Packet` families of data structures in the `wire` module do not perform -//! validation of received data except as needed to access the contents without panicking; -//! the `Repr` family does. +//! The functions in the `wire` module are designed for use together with `-Cpanic=abort`. +//! +//! The `Packet` family of data structures guarantees that, if the `Packet::check_len()` method +//! returned `Ok(())`, then no accessor or setter method will panic; however, the guarantee +//! provided by `Packet::check_len()` may no longer hold after changing certain fields, +//! which are listed in the documentation for the specific packet. +//! +//! The `Packet::new_checked` method is a shorthand for a combination of `Packet::new` and +//! `Packet::check_len`. +//! When parsing untrusted input, it is *necessary* to use `Packet::new_checked()`; +//! so long as the buffer is not modified, no accessor will fail. +//! When emitting output, though, it is *incorrect* to use `Packet::new_checked()`; +//! the length check is likely to succeed on a zeroed buffer, but fail on a buffer +//! filled with data from a previous packet, such as when reusing buffers, resulting +//! in nondeterministic panics with some network devices but not others. +//! The buffer length for emission is not calculated by the `Packet` layer. +//! +//! In the `Repr` family of data structures, the `Repr::parse()` method never panics +//! as long as `Packet::new_checked()` (or `Packet::check_len()`) has succeeded, and +//! the `Repr::emit()` method never panics as long as the underlying buffer is exactly +//! `Repr::buffer_len()` octets long. //! //! # Examples //! @@ -34,14 +53,16 @@ let repr = Ipv4Repr { protocol: IpProtocol::Tcp, payload_len: 10 }; -let mut buffer = vec![0; repr.buffer_len() + 10]; +let mut buffer = vec![0; repr.buffer_len() + repr.payload_len]; { // emission - let mut packet = Ipv4Packet::new(&mut buffer).unwrap(); + let mut packet = Ipv4Packet::new(&mut buffer); repr.emit(&mut packet); } { // parsing - let packet = Ipv4Packet::new(&buffer).unwrap(); - let parsed = Ipv4Repr::parse(&packet).unwrap(); + let packet = Ipv4Packet::new_checked(&buffer) + .expect("truncated packet"); + let parsed = Ipv4Repr::parse(&packet) + .expect("malformed packet"); assert_eq!(repr, parsed); } ``` diff --git a/src/wire/tcp.rs b/src/wire/tcp.rs index 4bf6cc8..7cdeb19 100644 --- a/src/wire/tcp.rs +++ b/src/wire/tcp.rs @@ -97,18 +97,37 @@ mod field { } impl> Packet { - /// Wrap a buffer with a TCP packet. Returns an error if the buffer - /// is too small to contain one. - pub fn new(buffer: T) -> Result, Error> { - let len = buffer.as_ref().len(); + /// Imbue a raw octet buffer with TCP packet structure. + pub fn new(buffer: T) -> Packet { + Packet { buffer } + } + + /// Shorthand for a combination of [new] and [check_len]. + /// + /// [new]: #method.new + /// [check_len]: #method.check_len + pub fn new_checked(buffer: T) -> Result, Error> { + let packet = Self::new(buffer); + try!(packet.check_len()); + Ok(packet) + } + + /// Ensure that no accessor method will panic if called. + /// Returns `Err(Error::Truncated)` if the buffer is too short. + /// + /// The result of this check is invalidated by calling [set_header_len]. + /// + /// [set_header_len]: #method.set_header_len + pub fn check_len(&self) -> Result<(), Error> { + let len = self.buffer.as_ref().len(); if len < field::URGENT.end { Err(Error::Truncated) } else { - Ok(Packet { buffer: buffer }) + Ok(()) } } - /// Consumes the packet, returning the underlying buffer. + /// Consume the packet, returning the underlying buffer. pub fn into_inner(self) -> T { self.buffer } @@ -761,7 +780,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent}; impl> PrettyPrint for Packet { fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter, indent: &mut PrettyIndent) -> fmt::Result { - match Packet::new(buffer) { + match Packet::new_checked(buffer) { Err(err) => write!(f, "{}({})\n", indent, err), Ok(packet) => write!(f, "{}{}\n", indent, packet) } @@ -793,7 +812,7 @@ mod test { #[test] fn test_deconstruct() { - let packet = Packet::new(&PACKET_BYTES[..]).unwrap(); + let packet = Packet::new(&PACKET_BYTES[..]); assert_eq!(packet.src_port(), 48896); assert_eq!(packet.dst_port(), 80); assert_eq!(packet.seq_number(), SeqNumber(0x01234567)); @@ -816,7 +835,7 @@ mod test { #[test] fn test_construct() { let mut bytes = vec![0; PACKET_BYTES.len()]; - let mut packet = Packet::new(&mut bytes).unwrap(); + let mut packet = Packet::new(&mut bytes); packet.set_src_port(48896); packet.set_dst_port(80); packet.set_seq_number(SeqNumber(0x01234567)); @@ -860,7 +879,7 @@ mod test { #[test] fn test_parse() { - let packet = Packet::new(&SYN_PACKET_BYTES[..]).unwrap(); + let packet = Packet::new(&SYN_PACKET_BYTES[..]); let repr = Repr::parse(&packet, &SRC_ADDR.into(), &DST_ADDR.into()).unwrap(); assert_eq!(repr, packet_repr()); } @@ -869,7 +888,7 @@ mod test { fn test_emit() { let repr = packet_repr(); let mut bytes = vec![0; repr.buffer_len()]; - let mut packet = Packet::new(&mut bytes).unwrap(); + let mut packet = Packet::new(&mut bytes); repr.emit(&mut packet, &SRC_ADDR.into(), &DST_ADDR.into()); assert_eq!(&packet.into_inner()[..], &SYN_PACKET_BYTES[..]); } diff --git a/src/wire/udp.rs b/src/wire/udp.rs index 94580af..510c633 100644 --- a/src/wire/udp.rs +++ b/src/wire/udp.rs @@ -27,23 +27,41 @@ mod field { } impl> Packet { - /// Wrap a buffer with an UDP packet. Returns an error if the buffer - /// is too small to contain one. - pub fn new(buffer: T) -> Result, Error> { - let len = buffer.as_ref().len(); + /// Imbue a raw octet buffer with UDP packet structure. + pub fn new(buffer: T) -> Packet { + Packet { buffer } + } + + /// Shorthand for a combination of [new] and [check_len]. + /// + /// [new]: #method.new + /// [check_len]: #method.check_len + pub fn new_checked(buffer: T) -> Result, Error> { + let packet = Self::new(buffer); + try!(packet.check_len()); + Ok(packet) + } + + /// Ensure that no accessor method will panic if called. + /// Returns `Err(Error::Truncated)` if the buffer is too short. + /// + /// The result of this check is invalidated by calling [set_header_len]. + /// + /// [set_header_len]: #method.set_header_len + pub fn check_len(&self) -> Result<(), Error> { + let len = self.buffer.as_ref().len(); if len < field::CHECKSUM.end { Err(Error::Truncated) } else { - let packet = Packet { buffer: buffer }; - if len < packet.len() as usize { + if len < self.len() as usize { Err(Error::Truncated) } else { - Ok(packet) + Ok(()) } } } - /// Consumes the packet, returning the underlying buffer. + /// Consume the packet, returning the underlying buffer. pub fn into_inner(self) -> T { self.buffer } @@ -234,7 +252,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent}; impl> PrettyPrint for Packet { fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter, indent: &mut PrettyIndent) -> fmt::Result { - match Packet::new(buffer) { + match Packet::new_checked(buffer) { Err(err) => write!(f, "{}({})\n", indent, err), Ok(packet) => write!(f, "{}{}\n", indent, packet) } @@ -259,7 +277,7 @@ mod test { #[test] fn test_deconstruct() { - let packet = Packet::new(&PACKET_BYTES[..]).unwrap(); + let packet = Packet::new(&PACKET_BYTES[..]); assert_eq!(packet.src_port(), 48896); assert_eq!(packet.dst_port(), 53); assert_eq!(packet.len(), 12); @@ -271,7 +289,7 @@ mod test { #[test] fn test_construct() { let mut bytes = vec![0; 12]; - let mut packet = Packet::new(&mut bytes).unwrap(); + let mut packet = Packet::new(&mut bytes); packet.set_src_port(48896); packet.set_dst_port(53); packet.set_len(12); @@ -291,7 +309,7 @@ mod test { #[test] fn test_parse() { - let packet = Packet::new(&PACKET_BYTES[..]).unwrap(); + let packet = Packet::new(&PACKET_BYTES[..]); let repr = Repr::parse(&packet, &SRC_ADDR.into(), &DST_ADDR.into()).unwrap(); assert_eq!(repr, packet_repr()); } @@ -300,7 +318,7 @@ mod test { fn test_emit() { let repr = packet_repr(); let mut bytes = vec![0; repr.buffer_len()]; - let mut packet = Packet::new(&mut bytes).unwrap(); + let mut packet = Packet::new(&mut bytes); repr.emit(&mut packet, &SRC_ADDR.into(), &DST_ADDR.into()); assert_eq!(&packet.into_inner()[..], &PACKET_BYTES[..]); }