Skip to content

Commit

Permalink
rfc7871: fix ECS addr truncation and add strict check
Browse files Browse the repository at this point in the history 
The code panics when the prefix length is 0(minus overflow) or at
byte boundary(shift overflow), fixed it and added more tests.

Added a strict check in parser, according to RFC, it should return
form error when there are non-zero bits beyond the prefix length.
  • Loading branch information
@xofyarg
xofyarg committed May 26, 2021
1 parent 15d0b7d commit 836e453
Showing 1 changed file with 154 additions and 73 deletions.
227 changes: 154 additions & 73 deletions src/base/opt/rfc7871.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ use super::super::iana::OptionCode;
use super::super::message_builder::OptBuilder;
use super::super::net::IpAddr;
use super::super::octets::{
Compose, FormError, OctetsBuilder, Parse, ParseError, Parser, ShortBuf
Compose, FormError, OctetsBuilder, Parse, ParseError, Parser, ShortBuf,
};
use super::CodeOptData;


//------------ ClientSubnet --------------------------------------------------

const ERR_ADDR_LEN: &str = "invalid address length in client subnet option";

#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct ClientSubnet {
source_prefix_len: u8,
Expand All @@ -22,26 +23,39 @@ impl ClientSubnet {
pub fn new(
source_prefix_len: u8,
scope_prefix_len: u8,
addr: IpAddr
addr: IpAddr,
) -> ClientSubnet {
ClientSubnet { source_prefix_len, scope_prefix_len, addr }
let source_prefix_len = normalize_prefix_len(addr, source_prefix_len);
let scope_prefix_len = normalize_prefix_len(addr, scope_prefix_len);
let (addr, _) = addr_apply_mask(addr, source_prefix_len);

ClientSubnet {
source_prefix_len,
scope_prefix_len,
addr,
}
}

pub fn push<Target: OctetsBuilder>(
builder: &mut OptBuilder<Target>,
source_prefix_len: u8,
scope_prefix_len: u8,
addr: IpAddr
addr: IpAddr,
) -> Result<(), ShortBuf> {
builder.push(&Self::new(source_prefix_len, scope_prefix_len, addr))
}

pub fn source_prefix_len(&self) -> u8 { self.source_prefix_len }
pub fn scope_prefix_len(&self) -> u8 { self.scope_prefix_len }
pub fn addr(&self) -> IpAddr { self.addr }
pub fn source_prefix_len(&self) -> u8 {
self.source_prefix_len
}
pub fn scope_prefix_len(&self) -> u8 {
self.scope_prefix_len
}
pub fn addr(&self) -> IpAddr {
self.addr
}
}


//--- Parse and Compose

impl<Ref: AsRef<[u8]>> Parse<Ref> for ClientSubnet {
Expand All @@ -56,49 +70,60 @@ impl<Ref: AsRef<[u8]>> Parse<Ref> for ClientSubnet {
// | IPv6 address, depending on FAMILY, which MUST be truncated to
// | the number of bits indicated by the SOURCE PREFIX-LENGTH field,
// | padding with 0 bits to pad to the end of the last octet needed.
let (prefix_bytes, mask) =
prefix_bytes(usize::from(source_prefix_len));
let prefix_bytes = prefix_bytes(usize::from(source_prefix_len));

let addr = match family {
1 => {
let mut buf = [0; 4];
if prefix_bytes > buf.len() {
return Err(
FormError::new(
"invalid address length in client subnet option"
).into()
);
return Err(ParseError::form_error(ERR_ADDR_LEN));
}
parser.parse_buf(&mut buf[..prefix_bytes])?;
if let Some(e) = buf.get_mut(prefix_bytes - 1) {
*e &= mask;
parser
.parse_buf(&mut buf[..prefix_bytes])
.map_err(|_| ParseError::form_error(ERR_ADDR_LEN))?;

if parser.remaining() != 0 {
return Err(ParseError::form_error(ERR_ADDR_LEN));
}

IpAddr::from(buf)
}
2 => {
let mut buf = [0; 16];
if prefix_bytes > buf.len() {
return Err(
FormError::new(
"invalid address length in client subnet option"
).into()
);
return Err(ParseError::form_error(ERR_ADDR_LEN));
}
parser.parse_buf(&mut buf[..prefix_bytes])?;
if let Some(e) = buf.get_mut(prefix_bytes - 1) {
*e &= mask;
parser
.parse_buf(&mut buf[..prefix_bytes])
.map_err(|_| ParseError::form_error(ERR_ADDR_LEN))?;

if parser.remaining() != 0 {
return Err(ParseError::form_error(ERR_ADDR_LEN));
}

IpAddr::from(buf)
}
_ => {
return Err(
FormError::new(
"invalid client subnet address family"
).into()
return Err(FormError::new(
"invalid client subnet address family",
)
.into())
}
};
Ok(ClientSubnet::new(source_prefix_len, scope_prefix_len, addr))

// If the trailing bits beyond prefix length are not zero,
// return form error.
let (addr, modified) = addr_apply_mask(addr, source_prefix_len);
if modified {
return Err(ParseError::form_error(ERR_ADDR_LEN));
}

// no need to pass the normalizer in constructor again
Ok(ClientSubnet {
source_prefix_len,
scope_prefix_len,
addr,
})
}

fn skip(parser: &mut Parser<Ref>) -> Result<(), ParseError> {
Expand All @@ -111,42 +136,94 @@ impl<Ref: AsRef<[u8]>> Parse<Ref> for ClientSubnet {
impl Compose for ClientSubnet {
fn compose<T: OctetsBuilder>(
&self,
target: &mut T
target: &mut T,
) -> Result<(), ShortBuf> {
let (prefix_bytes, mask) =
prefix_bytes(self.source_prefix_len as usize);
let prefix_bytes = prefix_bytes(self.source_prefix_len as usize);
target.append_all(|target| match self.addr {
IpAddr::V4(addr) => {
1u16.compose(target)?;
self.source_prefix_len.compose(target)?;
self.scope_prefix_len.compose(target)?;
let mut array = addr.octets();
if let Some(e) = array.get_mut(prefix_bytes - 1) {
*e &= mask;
let array = addr.octets();
if prefix_bytes > array.len() {
return Err(ShortBuf);
}
target.append_slice(&array[..prefix_bytes])
}
IpAddr::V6(addr) => {
2u16.compose(target)?;
self.source_prefix_len.compose(target)?;
self.scope_prefix_len.compose(target)?;
let mut array = addr.octets();
if let Some(e) = array.get_mut(prefix_bytes - 1) {
*e &= mask;
let array = addr.octets();
if prefix_bytes > array.len() {
return Err(ShortBuf);
}
target.append_slice(&array[..prefix_bytes])
}
})
}
}

fn prefix_bytes(bits: usize) -> (usize, u8) {
let n = (bits + 7) / 8;
let mask = match 8 - (bits % 8) {
0 => 0xff,
n => 0xff << n,
fn prefix_bytes(bits: usize) -> usize {
(bits + 7) / 8
}

// Apply a prefix bit mask indicated by its length to the provided
// buffer, clear rest of the buffer which is not covered by the mask.
// Reture whether or not the buffer has been modified.
fn apply_bit_mask(buf: &mut [u8], mask: usize) -> bool {
let mut modified = false;

// skip full bytes covered by prefix length
let mut p = mask / 8;
if p >= buf.len() {
return modified;
}

// clear extra bits in a byte
let bits = mask % 8;
if bits != 0 {
if buf[p].trailing_zeros() < (8 - bits) as u32 {
buf[p] &= 0xff << (8 - bits);
modified = true;
}
p += 1;
}

// clear the rest bytes
while p < buf.len() {
if buf[p] != 0 {
buf[p] = 0;
modified = true;
}
p += 1;
}

modified
}

fn addr_apply_mask(addr: IpAddr, len: u8) -> (IpAddr, bool) {
match addr {
IpAddr::V4(a) => {
let mut array = a.octets();
let m = apply_bit_mask(&mut array, len as usize);
(array.into(), m)
}
IpAddr::V6(a) => {
let mut array = a.octets();
let m = apply_bit_mask(&mut array, len as usize);
(array.into(), m)
}
}
}

fn normalize_prefix_len(addr: IpAddr, len: u8) -> u8 {
let max = match addr {
IpAddr::V4(_) => 32,
IpAddr::V6(_) => 128,
};
(n, mask)

core::cmp::min(len, max)
}

//--- CodeOptData
Expand All @@ -159,31 +236,35 @@ impl CodeOptData for ClientSubnet {
mod tests {
use super::*;
use crate::base::octets::Octets512;
use core::convert::TryFrom;

#[test]
fn truncate_bits() {
// no extra truncation
let csub = ClientSubnet::new(23, 0, "192.0.2.0".parse().unwrap());
let mut buf = Octets512::new();
csub.compose(&mut buf).unwrap();
assert_eq!(buf.as_ref(), [0, 1, 23, 0, 192, 0, 2].as_ref());

let parsed =
ClientSubnet::parse(&mut Parser::from_ref(buf.as_ref())).unwrap();
assert_eq!(parsed, csub);

// with extra truncation
let opt_bytes = [0, 1, 22, 0, 192, 0, 2];
let csub = ClientSubnet::parse(&mut Parser::from_ref(
Octets512::try_from(opt_bytes.as_ref()).unwrap(),
))
.unwrap();
assert_eq!(csub.addr(), "192.0.0.0".parse::<IpAddr>().unwrap());

let csub = ClientSubnet::new(22, 0, "192.0.2.0".parse().unwrap());
let mut buf = Octets512::new();
csub.compose(&mut buf).unwrap();
assert_eq!(buf.as_ref(), [0, 1, 22, 0, 192, 0, 0].as_ref());

macro_rules! check {
($name:ident, $addr:expr, $prefix:expr, $exp:expr, $ok:expr) => {
#[test]
fn $name() {
let addr = $addr.parse().unwrap();
let opt = ClientSubnet::new($prefix, 0, addr);
assert_eq!(opt.addr(), $exp.parse::<IpAddr>().unwrap());

// Check parse by mangling the addr in option to
// generate maybe invalid buffer.
let mut opt_ = opt.clone();
opt_.addr = addr;
let mut buf = Octets512::new();

opt_.compose(&mut buf).unwrap();
match ClientSubnet::parse(&mut Parser::from_ref(&buf)) {
Ok(v) => assert_eq!(opt, v),
Err(_) => assert!(!$ok),
}
}
};
}

check!(prefix_at_boundary_v4, "192.0.2.0", 24, "192.0.2.0", true);
check!(prefix_at_boundary_v6, "2001:db8::", 32, "2001:db8::", true);
check!(prefix_no_truncation, "192.0.2.0", 23, "192.0.2.0", true);
check!(prefix_need_truncation, "192.0.2.0", 22, "192.0.0.0", false);
check!(prefix_min, "192.0.2.0", 0, "0.0.0.0", true);
check!(prefix_max, "192.0.2.0", 32, "192.0.2.0", true);
check!(prefix_too_long, "192.0.2.0", 100, "192.0.2.0", false);
}

0 comments on commit 836e453

Please sign in to comment.