From: Giuseppe Longo Date: Fri, 4 Oct 2024 06:56:24 +0000 (+0200) Subject: sdp: stringify structured fields X-Git-Tag: suricata-8.0.0-beta1~110 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b481705ff827f5237a9b16666247e48a1751cff6;p=thirdparty%2Fsuricata.git sdp: stringify structured fields The current parser implementations take a field, such as connection data, and split it into subfields for a specific structure (e.g., struct ConnectionData). However, following this approach requires several sticky buffers to match the whole field, which can make a rule a bit verbose and doesn't offer any advantage for matching specific parts of a field. With this patch, a single line is still split into pieces if it makes sense for parsing purposes, but these pieces are then reassembled into a single string. This way, only one sticky buffer is needed to match the entire field. Ticket #7291 --- diff --git a/rust/src/sdp/logger.rs b/rust/src/sdp/logger.rs index f5d3b6fdd0..2951d772f8 100644 --- a/rust/src/sdp/logger.rs +++ b/rust/src/sdp/logger.rs @@ -19,22 +19,12 @@ use crate::jsonbuilder::{JsonBuilder, JsonError}; -use super::parser::{ConnectionData, MediaDescription, SdpMessage}; +use super::parser::{MediaDescription, SdpMessage}; pub fn sdp_log(msg: &SdpMessage, js: &mut JsonBuilder) -> Result<(), JsonError> { js.open_object("sdp")?; - let origin = format!( - "{} {} {} {} {} {}", - &msg.origin.username, - &msg.origin.sess_id, - &msg.origin.sess_version, - &msg.origin.nettype, - &msg.origin.addrtype, - &msg.origin.unicast_address - ); - - js.set_string("origin", &origin)?; + js.set_string("origin", &msg.origin)?; js.set_string("session_name", &msg.session_name)?; if let Some(session_info) = &msg.session_info { @@ -50,7 +40,7 @@ pub fn sdp_log(msg: &SdpMessage, js: &mut JsonBuilder) -> Result<(), JsonError> js.set_string("phone_number", phone_number)?; } if let Some(conn_data) = &msg.connection_data { - log_connection_data(conn_data, js)?; + js.set_string("connection_data", conn_data)?; } if let Some(bws) = &msg.bandwidths { log_bandwidth(bws, js)?; @@ -82,17 +72,7 @@ fn log_media_description( js.open_array("media_descriptions")?; for m in media { js.start_object()?; - let port = if let Some(num_ports) = m.number_of_ports { - format!("{}/{}", m.port, num_ports) - } else { - format!("{}", m.port) - }; - let mut media = format!("{} {} {}", &m.media, &port, &m.proto); - for f in &m.fmt { - media = format!("{} {}", media, f); - } - js.set_string("media", &media)?; - + js.set_string("media", &m.media)?; if let Some(session_info) = &m.session_info { js.set_string("media_info", session_info)?; }; @@ -100,7 +80,7 @@ fn log_media_description( log_bandwidth(bws, js)?; } if let Some(conn_data) = &m.connection_data { - log_connection_data(conn_data, js)?; + js.set_string("connection_data", conn_data)?; } if let Some(enc_key) = &m.encryption_key { js.set_string("encryption_key", enc_key)?; @@ -127,24 +107,6 @@ fn log_bandwidth(bws: &Vec, js: &mut JsonBuilder) -> Result<(), JsonErro Ok(()) } -fn log_connection_data(conn_data: &ConnectionData, js: &mut JsonBuilder) -> Result<(), JsonError> { - let mut conn = format!( - "{} {} {}", - &conn_data.nettype, - &conn_data.addrtype, - &conn_data.connection_address.to_string() - ); - if let Some(ttl) = conn_data.ttl { - conn = format!("{}/{}", conn, ttl); - js.set_uint("ttl", ttl as u64)?; - } - if let Some(num_addrs) = conn_data.number_of_addresses { - conn = format!("{}/{}", conn, num_addrs); - } - js.set_string("connection_data", &conn)?; - Ok(()) -} - fn log_attributes(attrs: &Vec, js: &mut JsonBuilder) -> Result<(), JsonError> { if !attrs.is_empty() { js.open_array("attributes")?; diff --git a/rust/src/sdp/parser.rs b/rust/src/sdp/parser.rs index 2a501b4a32..2f2baed9fc 100644 --- a/rust/src/sdp/parser.rs +++ b/rust/src/sdp/parser.rs @@ -40,13 +40,13 @@ use std::str::FromStr; #[derive(Debug)] pub struct SdpMessage { pub version: u32, - pub origin: OriginField, + pub origin: String, pub session_name: String, pub session_info: Option, pub uri: Option, pub email: Option, pub phone_number: Option, - pub connection_data: Option, + pub connection_data: Option, pub bandwidths: Option>, pub time: String, pub repeat_time: Option, @@ -56,34 +56,11 @@ pub struct SdpMessage { pub media_description: Option>, } -#[derive(Debug)] -pub struct OriginField { - pub username: String, - pub sess_id: String, - pub sess_version: String, - pub nettype: String, - pub addrtype: String, - pub unicast_address: String, -} - -#[derive(Debug)] -pub struct ConnectionData { - pub nettype: String, - pub addrtype: String, - pub connection_address: IpAddr, - pub ttl: Option, - pub number_of_addresses: Option, -} - #[derive(Debug)] pub struct MediaDescription { pub media: String, - pub port: u16, - pub number_of_ports: Option, - pub proto: String, - pub fmt: Vec, pub session_info: Option, - pub connection_data: Option, + pub connection_data: Option, pub bandwidths: Option>, pub encryption_key: Option, pub attributes: Option>, @@ -208,7 +185,7 @@ fn parse_version_line(i: &[u8]) -> IResult<&[u8], u32> { Ok((i, 0)) } -fn parse_origin_line(i: &[u8]) -> IResult<&[u8], OriginField> { +fn parse_origin_line(i: &[u8]) -> IResult<&[u8], String> { let (i, _) = tag("o=")(i)?; let (i, username) = map_res(take_while(is_token_char), std::str::from_utf8)(i)?; let (i, _) = space1(i)?; @@ -223,17 +200,12 @@ fn parse_origin_line(i: &[u8]) -> IResult<&[u8], OriginField> { let (i, unicast_address) = map_res(take_till(is_line_ending), std::str::from_utf8)(i)?; let (i, _) = line_ending(i)?; - Ok(( - i, - OriginField { - username: username.to_string(), - sess_id: sess_id.to_string(), - sess_version: sess_version.to_string(), - nettype: nettype.to_string(), - addrtype: addrtype.to_string(), - unicast_address: unicast_address.to_string(), - }, - )) + let origin_line = format!( + "{} {} {} {} {} {}", + username, sess_id, sess_version, nettype, addrtype, unicast_address + ); + + Ok((i, origin_line)) } fn parse_session_name(i: &[u8]) -> IResult<&[u8], String> { @@ -257,7 +229,7 @@ fn parse_uri(i: &[u8]) -> IResult<&[u8], String> { Ok((i, uri.to_string())) } -fn parse_connection_data(i: &[u8]) -> IResult<&[u8], ConnectionData> { +fn parse_connection_data(i: &[u8]) -> IResult<&[u8], String> { let (i, _) = tag("c=")(i)?; let (i, nettype) = map_res(take_while(is_alphabetic), std::str::from_utf8)(i)?; let (i, _) = space1(i)?; @@ -286,16 +258,20 @@ fn parse_connection_data(i: &[u8]) -> IResult<&[u8], ConnectionData> { _ => (None, None), }; - Ok(( - i, - ConnectionData { - nettype: nettype.to_string(), - addrtype: addrtype.to_string(), - connection_address, - ttl, - number_of_addresses, - }, - )) + let mut connection_data = format!( + "{} {} {}", + &nettype, + &addrtype, + &connection_address.to_string() + ); + if let Some(ttl) = ttl { + connection_data = format!("{}/{}", connection_data, ttl); + } + if let Some(num_addrs) = number_of_addresses { + connection_data = format!("{}/{}", connection_data, num_addrs); + } + + Ok((i, connection_data)) } fn parse_email(i: &[u8]) -> IResult<&[u8], String> { @@ -464,23 +440,29 @@ fn parse_media_description(i: &[u8]) -> IResult<&[u8], MediaDescription> { let (i, encryption_key) = opt(parse_encryption_key)(i)?; let (i, attributes) = opt(parse_attributes)(i)?; - let port = match port.parse::() { + let port: u16 = match port.parse::() { Ok(p) => p, - Err(_) => return Err(Err::Error(make_error(i, ErrorKind::HexDigit))) + Err(_) => return Err(Err::Error(make_error(i, ErrorKind::HexDigit))), }; - let number_of_ports = match number_of_ports { + let number_of_ports: Option = match number_of_ports { Some(num_str) => num_str.parse().ok(), None => None, }; + let port = if let Some(num_ports) = number_of_ports { + format!("{}/{}", port, num_ports) + } else { + format!("{}", port) + }; + let mut media_str = format!("{} {} {}", &media, &port, &proto); + let fmt: Vec = fmt.into_iter().map(String::from).collect(); + for f in &fmt { + media_str = format!("{} {}", media_str, f); + } Ok(( i, MediaDescription { - media, - port, - number_of_ports, - proto, - fmt: fmt.into_iter().map(String::from).collect(), + media: media_str, session_info, connection_data, bandwidths, @@ -506,12 +488,7 @@ mod tests { let buf: &[u8] = "o=Clarent 120386 120387 IN IP4 200.57.7.196\r\n".as_bytes(); let (_, o) = parse_origin_line(buf).expect("parsing failed"); - assert_eq!(o.username, "Clarent"); - assert_eq!(o.sess_id, "120386"); - assert_eq!(o.sess_version, "120387"); - assert_eq!(o.nettype, "IN"); - assert_eq!(o.addrtype, "IP4"); - assert_eq!(o.unicast_address, "200.57.7.196"); + assert_eq!(o, "Clarent 120386 120387 IN IP4 200.57.7.196"); } #[test] @@ -543,14 +520,7 @@ mod tests { let buf: &[u8] = "c=IN IP4 224.2.36.42/127\r\n".as_bytes(); let (_, c) = parse_connection_data(buf).expect("parsing failed"); - assert_eq!(c.nettype, "IN"); - assert_eq!(c.addrtype, "IP4"); - assert_eq!( - c.connection_address, - IpAddr::from_str("224.2.36.42").unwrap() - ); - assert_eq!(c.ttl, Some(127)); - assert_eq!(c.number_of_addresses, None); + assert_eq!(c, "IN IP4 224.2.36.42/127"); } #[test] @@ -558,11 +528,7 @@ mod tests { let buf: &[u8] = "c=IN IP6 FF15::101/3\r\n".as_bytes(); let (_, c) = parse_connection_data(buf).expect("parsing failed"); - assert_eq!(c.nettype, "IN"); - assert_eq!(c.addrtype, "IP6"); - assert_eq!(c.connection_address, IpAddr::from_str("FF15::101").unwrap()); - assert_eq!(c.ttl, None); - assert_eq!(c.number_of_addresses, Some(3)); + assert_eq!(c, "IN IP6 ff15::101/3"); } #[test] @@ -570,14 +536,7 @@ mod tests { let buf: &[u8] = "c=IN IP4 224.2.36.42/127/2\r\n".as_bytes(); let (_, c) = parse_connection_data(buf).expect("parsing failed"); - assert_eq!(c.nettype, "IN"); - assert_eq!(c.addrtype, "IP4"); - assert_eq!( - c.connection_address, - IpAddr::from_str("224.2.36.42").unwrap() - ); - assert_eq!(c.ttl, Some(127)); - assert_eq!(c.number_of_addresses, Some(2)); + assert_eq!(c, "IN IP4 224.2.36.42/127/2"); } #[test] @@ -593,11 +552,7 @@ mod tests { let buf: &[u8] = "c=IN IP4 8.8.8.8\r\n".as_bytes(); let (_, c) = parse_connection_data(buf).expect("parsing failed"); - assert_eq!(c.nettype, "IN"); - assert_eq!(c.addrtype, "IP4"); - assert_eq!(c.connection_address, IpAddr::from_str("8.8.8.8").unwrap()); - assert_eq!(c.ttl, None); - assert_eq!(c.number_of_addresses, None); + assert_eq!(c, "IN IP4 8.8.8.8"); } #[test] @@ -605,11 +560,7 @@ mod tests { let buf: &[u8] = "c=IN IP6 FF15::101\r\n".as_bytes(); let (_, c) = parse_connection_data(buf).expect("parsing failed"); - assert_eq!(c.nettype, "IN"); - assert_eq!(c.addrtype, "IP6"); - assert_eq!(c.connection_address, IpAddr::from_str("FF15::101").unwrap()); - assert_eq!(c.ttl, None); - assert_eq!(c.number_of_addresses, None); + assert_eq!(c, "IN IP6 ff15::101"); } #[test] @@ -682,12 +633,7 @@ mod tests { fn test_media_line() { let buf: &[u8] = "m=audio 40392 RTP/AVP 8 0\r\n".as_bytes(); let (_, m) = parse_media_description(buf).expect("parsing failed"); - assert_eq!(m.media, "audio"); - assert_eq!(m.port, 40392); - assert_eq!(m.number_of_ports, None); - assert_eq!(m.proto, "RTP/AVP"); - assert_eq!(m.fmt.first().unwrap(), "8"); - assert_eq!(m.fmt.get(1).unwrap(), "0"); + assert_eq!(m.media, "audio 40392 RTP/AVP 8 0"); } #[test]