From a47e04124f7056f0aad0bb48bd80332689a86e68 Mon Sep 17 00:00:00 2001 From: jrx Date: Wed, 4 Dec 2024 09:54:51 +0100 Subject: [PATCH] review changes --- src/decoders/decoder.rs | 2 +- src/decoders/dns.rs | 128 +++++++++++++++------------------------- 2 files changed, 49 insertions(+), 81 deletions(-) diff --git a/src/decoders/decoder.rs b/src/decoders/decoder.rs index 63edd59..0717b42 100644 --- a/src/decoders/decoder.rs +++ b/src/decoders/decoder.rs @@ -137,7 +137,7 @@ pub(crate) fn check_objects( match message_value { Ok(value) => value, Err(e) => { - log::error!("[macos-unifiedlogs] Failed to parse DNS header counts. Error: {e:?}"); + log::error!("[macos-unifiedlogs] Failed to decode log object. Error: {e:?}"); e.to_string() } } diff --git a/src/decoders/dns.rs b/src/decoders/dns.rs index b275445..0d05d84 100644 --- a/src/decoders/dns.rs +++ b/src/decoders/dns.rs @@ -14,8 +14,8 @@ use byteorder::{BigEndian, WriteBytesExt}; use log::{error, warn}; use nom::{ bytes::complete::take, - combinator::{fail, iterator, map_parser, verify}, - multi::fold_many0, + combinator::{fail, map, map_parser, verify}, + multi::{fold_many0, many0}, number::complete::{be_u128, be_u16, be_u32, be_u8, le_u32}, sequence::tuple, IResult, @@ -227,95 +227,46 @@ fn parse_svcb_alpn(mut input: &[u8]) -> nom::IResult<&[u8], String> { } /// Parse the IPs -// fn parse_svcb_ip(data: &[u8]) -> nom::IResult<&[u8], String> { -// const IPV4: u16 = 4; -// const IPV6: u16 = 6; - -// let mut dns_data = data; - -// let mut ipv4_addrs = vec![]; -// let mut ipv6_addrs = vec![]; - -// // IPs can either be IPv4 or/and IPv6 -// while !dns_data.is_empty() { -// let (remaining_dns_data, (ip_version, ip_size)) = -// tuple((verify(be_u16, |val| *val == IPV4 || *val == IPV6), be_u16))(dns_data)?; - -// let (remaining_dns_data, ip_data) = take(ip_size)(remaining_dns_data)?; - -// dns_data = remaining_dns_data; - -// // There can be multiple IPs -// match ip_version { -// IPV4 => { -// let (_, mut addrs) = -// many0(map(be_u32, |raw| Ipv4Addr::from(raw).to_string()))(ip_data)?; -// ipv4_addrs.append(&mut addrs); -// } -// IPV6 => { -// let (_, mut addrs) = -// many0(map(be_u128, |raw| Ipv6Addr::from(raw).to_string()))(ip_data)?; -// ipv6_addrs.append(&mut addrs); -// } -// _ => {} -// }; -// } - -// let message = format!( -// "ipv4 hint:{}, ipv6 hint:{}", -// ipv4_addrs.join(","), -// ipv6_addrs.join(",") -// ); -// Ok((dns_data, message)) -// } - -fn parse_svcb_ip(mut input: &[u8]) -> nom::IResult<&[u8], String> { +fn parse_svcb_ip(data: &[u8]) -> nom::IResult<&[u8], String> { const IPV4: u16 = 4; const IPV6: u16 = 6; - fn ipv4_parser(input: &[u8]) -> nom::IResult<&[u8], Ipv4Addr> { - let (i, raw) = be_u32(input)?; - Ok((i, Ipv4Addr::from(raw))) - } - - fn ipv6_parser(input: &[u8]) -> nom::IResult<&[u8], Ipv6Addr> { - let (i, raw) = be_u128(input)?; - Ok((i, Ipv6Addr::from(raw))) - } + let mut dns_data = data; - let mut ipv4s = String::with_capacity(2 * 16); // let's reserve max space for 2 IPV4 addresses - let mut ipv6s = String::with_capacity(2 * 40); // let's reserve max space for 8 IPV6 addresses + let mut ipv4_addrs = vec![]; + let mut ipv6_addrs = vec![]; // IPs can either be IPv4 or/and IPv6 - while !input.is_empty() { - let (i, ip_version) = verify(be_u16, |val| *val == IPV4 || *val == IPV6)(input)?; - let (i, ip_size) = be_u16(i)?; - let (i, ip_data) = take(ip_size)(i)?; - input = i; + while !dns_data.is_empty() { + let (remaining_dns_data, (ip_version, ip_size)) = + tuple((verify(be_u16, |val| *val == IPV4 || *val == IPV6), be_u16))(dns_data)?; - if ip_version == IPV4 { - let mut iter = iterator(ip_data, ipv4_parser); - for ip in iter.into_iter() { - if !ipv4s.is_empty() { - ipv4s.push(','); - } - write!(ipv4s, "{}", ip).ok(); // ignore errors on write in String + let (remaining_dns_data, ip_data) = take(ip_size)(remaining_dns_data)?; + + dns_data = remaining_dns_data; + + // There can be multiple IPs + match ip_version { + IPV4 => { + let (_, mut addrs) = + many0(map(be_u32, |raw| Ipv4Addr::from(raw).to_string()))(ip_data)?; + ipv4_addrs.append(&mut addrs); } - iter.finish()?; - } else if ip_version == IPV6 { - let mut iter = iterator(ip_data, ipv6_parser); - for ip in iter.into_iter() { - if !ipv6s.is_empty() { - ipv6s.push(','); - } - write!(ipv6s, "{}", ip).ok(); // ignore errors on write in String + IPV6 => { + let (_, mut addrs) = + many0(map(be_u128, |raw| Ipv6Addr::from(raw).to_string()))(ip_data)?; + ipv6_addrs.append(&mut addrs); } - iter.finish()?; - } + _ => {} + }; } - let message = format!("ipv4 hint:{ipv4s}, ipv6 hint:{ipv6s}"); - Ok((input, message)) + let message = format!( + "ipv4 hint:{}, ipv6 hint:{}", + ipv4_addrs.join(","), + ipv6_addrs.join(",") + ); + Ok((dns_data, message)) } /// Get the MAC Address from the log data @@ -718,6 +669,23 @@ mod tests { assert_eq!(result, "ipv4 hint:104.16.148.64,104.16.149.64, ipv6 hint:2606:4700::6810:9440,2606:4700::6810:9540"); } + #[test] + fn test_parse_svcb_ip_should_not_infine_loop() { + let test_data = [ + 0, 4, 0, 4, 104, 16, 148, 64, // 104.16.148.64 + 0, 6, 0, 16, 38, 6, 71, 0, 0, 0, 0, 0, 0, 0, 0, 0, 104, 16, 148, 64, // 2606:4700::6810:9440 + 0, 42, 0, 0 // [invalid data] infinite loop (consuming too much) in the previous version + ]; + + let result = parse_svcb_ip(&test_data); + assert!(result.is_err()); + + // // Previous version would have this behavior : + // let (rest, result) = parse_svcb_ip(&test_data).unwrap(); + // assert_eq!(rest, &[] as &[u8]); + // assert_eq!(result, "ipv4 hint:104.16.148.64, ipv6 hint:2606:4700::6810:9440,"); + } + #[test] fn test_get_dns_mac_addr() { let test_data = "AAAAAAAA";