ike: add support to log attributes as objects

This is an opt-in feature to log IKE attributes as objects for better
handling of duplicate key values. Added as an option, as this breaks
the Suricata 7 and 8 log format for IKE, but is the default in
Suricata 9.

Ticket: OISF#7923

Author: jasonish

doc/userguide/output/eve/eve-json-output.rst

@@ -315,6 +315,17 @@ YAML::
             # Default: all.
             #types: [a, aaaa, cname, mx, ns, ptr, txt]
 
+IKE
+~~~
+
+YAML::
+
+  - ike:
+      # Enable logging attributes as objects instead of
+      # giving duplicates an index suffix. This will be
+      # the default in Suricata 9.0.
+      #object-attributes: false
+
 TLS
 ~~~
 

doc/userguide/upgrade.rst

@@ -34,6 +34,21 @@ also check all the new features that have been added but are not covered by
 this guide. Those features are either not enabled by default or require
 dedicated new configuration.
 
+Upgrading to 8.0.2
+------------------
+
+Logging Changes
+~~~~~~~~~~~~~~~
+
+- Duplicate field names in IKE attributes has been removed. Before 8.0
+  it was possible to see duplicate field names like
+  ``sa_life_duration``, while some parsers ignore the duplicates,
+  strict parsers will fail on duplicates. To address this, duplicates
+  will now be given an index suffix, for example: `sa_life_duration`,
+  `sa_life_duration_1`. See
+  https://redmine.openinfosecfoundation.org/issues/7902 for more
+  information.
+
 Upgrading to 8.0.1
 ------------------
 

rust/src/ike/logger.rs

@@ -26,6 +26,7 @@ use std::collections::HashMap;
 use std::convert::TryFrom;
 
 const LOG_EXTENDED: u32 = 0x01;
+const LOG_IKE_ATTRIBUTE_OBJECTS: u32 = 0x02;
 
 fn add_attributes(transform: &Vec<SaAttribute>, js: &mut JsonBuilder) -> Result<(), JsonError> {
     let mut logged: HashMap<String, usize> = HashMap::new();
@@ -52,6 +53,30 @@ fn add_attributes(transform: &Vec<SaAttribute>, js: &mut JsonBuilder) -> Result<
     Ok(())
 }
 
+/// Add IKE attributes as objects.
+///
+/// This function does not open the array, it should be opened by the
+/// caller.
+///
+/// This is the default add_attributes function in Suricata 9.
+fn add_attributes_as_objects(transform: &Vec<SaAttribute>, js: &mut JsonBuilder) -> Result<(), JsonError> {
+    for attribute in transform {
+        js.start_object()?;
+        js.set_string("key", &attribute.attribute_type.to_string())?;
+        js.set_string("value", &attribute.attribute_value.to_string())?;
+
+        if let Some(v) = attribute.numeric_value {
+            js.set_uint("raw", v as u64)?;
+        } else if let Some(v) = &attribute.hex_value {
+            js.set_string("raw", v)?;
+        }
+
+        js.close()?;
+    }
+
+    Ok(())
+}
+
 fn log_ike(
     state: &IKEState, tx: &IKETransaction, flags: u32, jb: &mut JsonBuilder,
 ) -> Result<(), JsonError> {
@@ -79,15 +104,25 @@ fn log_ike(
     if tx.ike_version == 1 {
         if state.ikev1_container.server.nb_transforms > 0 {
             // log the first transform as the chosen one
-            add_attributes(&state.ikev1_container.server.transform, jb)?;
+            if flags & LOG_IKE_ATTRIBUTE_OBJECTS == LOG_IKE_ATTRIBUTE_OBJECTS {
+                jb.open_array("attributes")?;
+                add_attributes_as_objects(&state.ikev1_container.server.transform, jb)?;
+                jb.close()?;
+            } else {
+                add_attributes(&state.ikev1_container.server.transform, jb)?;
+            }
         }
         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 &tx.hdr.ikev1_transforms {
-                jb.start_object()?;
-                add_attributes(server_transform, jb)?;
-                jb.close()?;
+                if flags & LOG_IKE_ATTRIBUTE_OBJECTS == LOG_IKE_ATTRIBUTE_OBJECTS {
+                    add_attributes_as_objects(server_transform, jb)?;
+                } else {
+                    jb.start_object()?;
+                    add_attributes(server_transform, jb)?;
+                    jb.close()?;
+                }
             }
             jb.close()?;
         }
@@ -120,15 +155,15 @@ fn log_ike(
     jb.close()?;
 
     if tx.ike_version == 1 {
-        log_ikev1(state, tx, jb)?;
+        log_ikev1(state, tx, flags, jb)?;
     } else if tx.ike_version == 2 {
         log_ikev2(tx, jb)?;
     }
     jb.close()?;
     return Ok(());
 }
 
-fn log_ikev1(state: &IKEState, tx: &IKETransaction, jb: &mut JsonBuilder) -> Result<(), JsonError> {
+fn log_ikev1(state: &IKEState, tx: &IKETransaction, flags: u32, jb: &mut JsonBuilder) -> Result<(), JsonError> {
     jb.open_object("ikev1")?;
 
     if let Some(doi) = state.ikev1_container.domain_of_interpretation {
@@ -163,9 +198,13 @@ fn log_ikev1(state: &IKEState, tx: &IKETransaction, jb: &mut JsonBuilder) -> Res
         if tx.direction == Direction::ToServer && !tx.hdr.ikev1_transforms.is_empty() {
             jb.open_array("proposals")?;
             for client_transform in &tx.hdr.ikev1_transforms {
-                jb.start_object()?;
-                add_attributes(client_transform, jb)?;
-                jb.close()?;
+                if flags & LOG_IKE_ATTRIBUTE_OBJECTS == LOG_IKE_ATTRIBUTE_OBJECTS {
+                    add_attributes_as_objects(client_transform, jb)?;
+                } else {
+                    jb.start_object()?;
+                    add_attributes(client_transform, jb)?;
+                    jb.close()?;
+                }
             }
             jb.close()?; // proposals
         }

src/output-json-ike.c

@@ -49,8 +49,9 @@
 
 #include "rust.h"
 
-#define LOG_IKE_DEFAULT  0
-#define LOG_IKE_EXTENDED (1 << 0)
+#define LOG_IKE_DEFAULT           BIT_U32(0)
+#define LOG_IKE_EXTENDED          BIT_U32(1)
+#define LOG_IKE_ATTRIBUTE_OBJECTS BIT_U32(2)
 
 typedef struct LogIKEFileCtx_ {
     uint32_t flags;
@@ -132,6 +133,11 @@ static OutputInitResult OutputIKELogInitSub(SCConfNode *conf, OutputCtx *parent_
         }
     }
 
+    const char *objects = SCConfNodeLookupChildValue(conf, "object-attributes");
+    if (objects && SCConfValIsTrue(objects)) {
+        ikelog_ctx->flags |= LOG_IKE_ATTRIBUTE_OBJECTS;
+    }
+
     output_ctx->data = ikelog_ctx;
     output_ctx->DeInit = OutputIKELogDeInitCtxSub;
 

suricata.yaml.in

@@ -326,7 +326,9 @@ outputs:
             #types: [file, tree_connect, negotiate, dcerpc, create,
             #  session_setup, ioctl, rename, set_file_path_info, generic]
         - tftp
-        - ike
+        - ike:
+            # Log attributes as object (future Suricata 9.0 behavior)
+            #object-attributes: false
         - dcerpc
         - krb5
         - bittorrent-dht