]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
ike: log ikev1 tx fields instead of state ones
authorPhilippe Antoine <pantoine@oisf.net>
Sun, 24 Jul 2022 20:18:29 +0000 (22:18 +0200)
committerVictor Julien <vjulien@oisf.net>
Tue, 9 Aug 2022 08:52:10 +0000 (10:52 +0200)
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

rust/src/ike/ike.rs
rust/src/ike/ikev1.rs
rust/src/ike/logger.rs

index 2d975c6dab6614a4d6d1c3287ff6dcffdcbb6901..53b91bbd8b8d91a48f592889530a65aa3054d311 100644 (file)
@@ -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<NotifyType>,
@@ -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![],
index 7d76ed0e8714a31d8935deab424841749aa50d08..d3be063c69653bbf0fd18a63dd6e01c6842d1b3d 100644 (file)
@@ -40,27 +40,27 @@ pub struct IkeV1Header {
 pub struct Ikev1ParticipantData {
     pub key_exchange: String,
     pub nonce: String,
-    pub vendor_ids: HashSet<String>,
-    /// nested Vec, outer Vec per Proposal/Transform, inner Vec has the list of attributes.
-    pub transforms: Vec<Vec<SaAttribute>>,
+    pub nb_transforms: u64,
+    pub transform: Vec<SaAttribute>,
 }
 
 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<String>,
-        transforms: &Vec<Vec<SaAttribute>>,
+        &mut self, key_exchange: &String, nonce: &String, transforms: &Vec<Vec<SaAttribute>>,
     ) {
         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,
                     );
                 }
index b326cdec0720c948673ee3e08d986db894bee273..042fdae4cd07b55c4afc64b40581f8db01512c4c 100644 (file)
@@ -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()?;