From: Philippe Antoine Date: Sun, 24 Jul 2022 20:18:29 +0000 (+0200) Subject: ike: log ikev1 tx fields instead of state ones X-Git-Tag: suricata-7.0.0-beta1~301 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3de735ae7020da4e9ea42f12676609a9ec637d1a;p=thirdparty%2Fsuricata.git ike: log ikev1 tx fields instead of state ones As state fields can grow abitrarily, and this can lead to DOS by quadratic complexity (CPU time and disk space) Adds a direction field to retain all the information in the transaction. Also checks array vendor_ids had at least one element before logging it. Ticket: #5455 --- diff --git a/rust/src/ike/ike.rs b/rust/src/ike/ike.rs index 2d975c6dab..53b91bbd8b 100644 --- a/rust/src/ike/ike.rs +++ b/rust/src/ike/ike.rs @@ -97,6 +97,7 @@ pub struct IKETransaction { tx_id: u64, pub ike_version: u8, + pub direction: Direction, pub hdr: IkeHeaderWrapper, pub payload_types: IkePayloadWrapper, pub notify_types: Vec, @@ -119,6 +120,7 @@ impl IKETransaction { IKETransaction { tx_id: 0, ike_version: 0, + direction: Direction::ToServer, hdr: IkeHeaderWrapper::new(), payload_types: Default::default(), notify_types: vec![], diff --git a/rust/src/ike/ikev1.rs b/rust/src/ike/ikev1.rs index 7d76ed0e87..d3be063c69 100644 --- a/rust/src/ike/ikev1.rs +++ b/rust/src/ike/ikev1.rs @@ -40,27 +40,27 @@ pub struct IkeV1Header { pub struct Ikev1ParticipantData { pub key_exchange: String, pub nonce: String, - pub vendor_ids: HashSet, - /// nested Vec, outer Vec per Proposal/Transform, inner Vec has the list of attributes. - pub transforms: Vec>, + pub nb_transforms: u64, + pub transform: Vec, } impl Ikev1ParticipantData { pub fn reset(&mut self) { self.key_exchange.clear(); self.nonce.clear(); - self.vendor_ids.clear(); - self.transforms.clear(); + self.nb_transforms = 0; + self.transform.clear(); } pub fn update( - &mut self, key_exchange: &String, nonce: &String, vendor_ids: &Vec, - transforms: &Vec>, + &mut self, key_exchange: &String, nonce: &String, transforms: &Vec>, ) { self.key_exchange = key_exchange.clone(); self.nonce = nonce.clone(); - self.vendor_ids.extend(vendor_ids.iter().cloned()); - self.transforms.extend(transforms.iter().cloned()); + if self.nb_transforms == 0 && transforms.len() > 0 { + self.transform.extend(transforms[0].iter().cloned()); + } + self.nb_transforms += transforms.len() as u64; } } @@ -77,6 +77,7 @@ pub fn handle_ikev1( let mut tx = state.new_tx(); tx.ike_version = 1; + tx.direction = direction; tx.hdr.spi_initiator = format!("{:016x}", isakmp_header.init_spi); tx.hdr.spi_responder = format!("{:016x}", isakmp_header.resp_spi); tx.hdr.maj_ver = isakmp_header.maj_ver; @@ -126,13 +127,12 @@ pub fn handle_ikev1( state.ikev1_container.client.update( &to_hex(tx.hdr.ikev1_header.key_exchange.as_ref()), &to_hex(tx.hdr.ikev1_header.nonce.as_ref()), - &tx.hdr.ikev1_header.vendor_ids, &tx.hdr.ikev1_transforms, ); } else { - if state.ikev1_container.server.transforms.len() <= 1 - && state.ikev1_container.server.transforms.len() - + tx.hdr.ikev1_transforms.len() + if state.ikev1_container.server.nb_transforms <= 1 + && state.ikev1_container.server.nb_transforms + + tx.hdr.ikev1_transforms.len() as u64 > 1 { SCLogDebug!("More than one chosen server proposal"); @@ -142,7 +142,6 @@ pub fn handle_ikev1( state.ikev1_container.server.update( &to_hex(tx.hdr.ikev1_header.key_exchange.as_ref()), &to_hex(tx.hdr.ikev1_header.nonce.as_ref()), - &tx.hdr.ikev1_header.vendor_ids, &tx.hdr.ikev1_transforms, ); } diff --git a/rust/src/ike/logger.rs b/rust/src/ike/logger.rs index b326cdec07..042fdae4cd 100644 --- a/rust/src/ike/logger.rs +++ b/rust/src/ike/logger.rs @@ -17,6 +17,7 @@ use super::ike::{IKEState, IKETransaction}; use super::ipsec_parser::IKEV2_FLAG_INITIATOR; +use crate::core::Direction; use crate::ike::parser::{ExchangeType, IsakmpPayloadType, SaAttribute}; use crate::jsonbuilder::{JsonBuilder, JsonError}; use num_traits::FromPrimitive; @@ -73,14 +74,14 @@ fn log_ike( } if tx.ike_version == 1 { - if state.ikev1_container.server.transforms.len() > 0 { + if state.ikev1_container.server.nb_transforms > 0 { // log the first transform as the chosen one - add_attributes(&state.ikev1_container.server.transforms[0], jb)?; + add_attributes(&state.ikev1_container.server.transform, jb)?; } - if state.ikev1_container.server.transforms.len() > 1 { + if tx.direction == Direction::ToClient && tx.hdr.ikev1_transforms.len() > 1 { // in case we have multiple server transforms log them in a list jb.open_array("server_proposals")?; - for server_transform in &state.ikev1_container.server.transforms { + for server_transform in &tx.hdr.ikev1_transforms { jb.start_object()?; add_attributes(server_transform, jb)?; jb.close()?; @@ -156,13 +157,15 @@ fn log_ikev1(state: &IKEState, tx: &IKETransaction, jb: &mut JsonBuilder) -> Res } } - jb.open_array("proposals")?; - for client_transform in &state.ikev1_container.client.transforms { - jb.start_object()?; - add_attributes(client_transform, jb)?; - jb.close()?; + if tx.direction == Direction::ToServer && tx.hdr.ikev1_transforms.len() > 0 { + jb.open_array("proposals")?; + for client_transform in &tx.hdr.ikev1_transforms { + jb.start_object()?; + add_attributes(client_transform, jb)?; + jb.close()?; + } + jb.close()?; // proposals } - jb.close()?; // proposals jb.close()?; // client // server data @@ -187,16 +190,13 @@ fn log_ikev1(state: &IKEState, tx: &IKETransaction, jb: &mut JsonBuilder) -> Res } jb.close()?; // server - jb.open_array("vendor_ids")?; - for vendor in state - .ikev1_container - .client - .vendor_ids - .union(&state.ikev1_container.server.vendor_ids) - { - jb.append_string(vendor)?; + if tx.hdr.ikev1_header.vendor_ids.len() > 0 { + jb.open_array("vendor_ids")?; + for vendor in &tx.hdr.ikev1_header.vendor_ids { + jb.append_string(vendor)?; + } + jb.close()?; // vendor_ids } - jb.close()?; // vendor_ids } jb.close()?;