Skip to content

Commit

Permalink
gsepacket: return an error if the BBFRAME is malformed
Browse files Browse the repository at this point in the history
This changes the API so that an error can be returned instead of
panicking if the BBFRAME is malformed. In the CLI application we
unwrap, because we know that the BBFRAME has been validated.

Signed-off-by: Daniel Estévez <[email protected]>
  • Loading branch information
daniestevez committed Nov 1, 2023
1 parent 1222fc5 commit 74d7690
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 12 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ lazy_static = "1.4"
libc = { version = "0.2", optional = true }
log = "0.4"
num_enum = "0.6"
thiserror = "1"
tun-tap = { version = "0.1", default-features = false, optional = true }

[dev-dependencies]
Expand Down
4 changes: 3 additions & 1 deletion src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ impl<D: BBFrameReceiver> AppLoop<D> {
}
}
};
for pdu in self.gsepacket_defrag.defragment(&bbframe) {
// the BBFRAME was validated by bbframe_recv, so we can unwrap here
let pdus = self.gsepacket_defrag.defragment(&bbframe).unwrap();
for pdu in pdus {
if let Err(err) = self.tun.send(pdu.data()) {
log::error!("could not write packet to TUN device: {err}");
}
Expand Down
44 changes: 33 additions & 11 deletions src/gsepacket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use super::gseheader::{GSEHeader, Label};
use bytes::Bytes;
use crc::Digest;
use std::collections::HashMap;
use thiserror::Error;

/// GSE Packet.
///
Expand All @@ -21,6 +22,14 @@ pub struct GSEPacket {
data: Bytes,
}

/// GSE protocol error.
#[derive(Error, Debug, Copy, Clone, Eq, PartialEq, Hash)]
pub enum GSEError {
/// The BBFRAME is shorter than the BBHEADER length.
#[error("the BBFRAME is shorter than the BBHEADER length")]
BBFrameShort,
}

lazy_static::lazy_static! {
static ref CRC32: crc::Crc<u32> = crc::Crc::<u32>::new(&crc::CRC_32_MPEG_2);
}
Expand Down Expand Up @@ -56,10 +65,16 @@ impl GSEPacket {
/// This function returns an iterator that returns the GSE Packets contained
/// in the BBFRAME. The iterator stops when the end of the BBFRAME is
/// reached or when a malformed GSE Packet is found.
pub fn split_bbframe(bbframe: &BBFrame) -> impl Iterator<Item = GSEPacket> {
///
/// The function returns an error if the BBFRAME is malformed. For instance,
/// if the BBFRAME length is shorter than the BBHEADER length.
pub fn split_bbframe(bbframe: &BBFrame) -> Result<impl Iterator<Item = GSEPacket>, GSEError> {
if bbframe.len() < BBHeader::LEN {
return Err(GSEError::BBFrameShort);
}
let mut remain = bbframe.slice(BBHeader::LEN..);
let mut label = None;
std::iter::from_fn(move || {
Ok(std::iter::from_fn(move || {
if let Some(packet) = GSEPacket::from_bytes(&remain, label.as_ref()) {
log::debug!("extracted GSE Packet with header {}", packet.header());
log::trace!("GSE Packet data field {:?}", packet.data());
Expand All @@ -72,7 +87,7 @@ impl GSEPacket {
log::debug!("no more GSE Packets in BBFRAME");
None
}
})
}))
}

/// Gives the length of the GSE Packet in bytes.
Expand Down Expand Up @@ -184,8 +199,14 @@ impl GSEPacketDefrag {
///
/// This function returns an iterator that produces all the PDUs that can be
/// completed with the GSE Packets found in the BBFRAME.
pub fn defragment(&mut self, bbframe: &BBFrame) -> impl Iterator<Item = PDU> + '_ {
GSEPacket::split_bbframe(bbframe).flat_map(|packet| self.defrag_packet(&packet))
///
/// The function returns an error if the BBFRAME is malformed. For instance,
/// if the BBFRAME length is shorter than the BBHEADER length.
pub fn defragment(
&mut self,
bbframe: &BBFrame,
) -> Result<impl Iterator<Item = PDU> + '_, GSEError> {
Ok(GSEPacket::split_bbframe(bbframe)?.flat_map(|packet| self.defrag_packet(&packet)))
}

fn defrag_packet(&mut self, packet: &GSEPacket) -> Option<PDU> {
Expand Down Expand Up @@ -328,7 +349,7 @@ mod test {
fn defrag_single_packet() {
let bbframe = Bytes::copy_from_slice(&SINGLE_PACKET);
let mut defrag = GSEPacketDefrag::new();
let pdus: Vec<_> = defrag.defragment(&bbframe).collect();
let pdus: Vec<_> = defrag.defragment(&bbframe).unwrap().collect();
assert_eq!(pdus.len(), 1);
let pdu = &pdus[0];
assert_eq!(&pdu.data()[..], &SINGLE_PACKET[20..]);
Expand All @@ -353,14 +374,15 @@ mod proptests {

proptest! {
#[test]
#[ignore = "this makes the defragmenter panic at the moment"]
fn defrag_garbage(garbage_bbframes in garbage()) {
let mut defrag = GSEPacketDefrag::new();
for bbframe in &garbage_bbframes {
for pdu in defrag.defragment(&bbframe) {
pdu.data();
pdu.protocol_type();
pdu.label();
if let Ok(pdus) = defrag.defragment(&bbframe) {
for pdu in pdus {
pdu.data();
pdu.protocol_type();
pdu.label();
}
}
}
}
Expand Down

0 comments on commit 74d7690

Please sign in to comment.