Skip to content

ike: log attributes as objects - v2 #13927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

ike: log attributes as objects - v2 #13927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9e96afd
ike/detect: info log message should be debug
jasonish Sep 25, 2025
4b12773
ike: log attributes as objects
jasonish Sep 23, 2025
aef5561
doc: upgrade notes for changes to ike output
jasonish Sep 25, 2025
f861c3f
scripts/evedoc.py: handle union types in schema
jasonish Sep 25, 2025
52482af
ike: don't log empty server objects
jasonish Sep 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions doc/userguide/upgrade.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ 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 9.0.0
------------------

Logging Changes
~~~~~~~~~~~~~~~
- The format of IKEv1 proposal attributes has been changed to handle
duplicate attribute types. See :ref:`IKE logging changes
<9.0-ike-logging-changes>`

Upgrading to 8.0.1
------------------

Expand Down
139 changes: 139 additions & 0 deletions doc/userguide/upgrade/9.0-logging-changes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
:orphan: Referenced from upgrade notes, not a toctree

Suricata 9.0 Logging Changes
############################

.. _9.0-ike-logging-changes:

IKE
***

IKE attributes are now logged as an array of objects instead of a map
keyed by the attribute type. This allows for multiple attributes of
the same type to be logged.

The affected field names include:

* alg_auth
* alg_auth_raw
* alg_dh
* alf_dh_raw
* alg_enc
* alg_enc_raw
* alg_hash
* alg_hash_raw
* sa_key_length
* sa_key_length_raw
* sa_life_duration
* sa_life_duration_raw
* sa_life_type
* sa_life_type_raw

Example - Attributes in "ike" object
====================================

**Suricata 8.0**

.. code-block:: json

"ike": {
"alg_enc": "EncAesCbc",
"alg_enc_raw": 7,
"sa_key_length": "Unknown",
"sa_key_length_raw": 128
}

**Suricata 9.0**

.. code-block:: json

"ike": {
"attributes": [
{
"key": "alg_enc",
"value": "EncAesCbc",
"raw": 7
},
{
"key": "sa_key_length",
"value": "Unknown",
"raw": 128
}
]
}

Example - Client Proposal
=========================

**Suricata 8.0**

.. code-block:: json

"ikev1": {
"client": {
"proposals": [
{
"alg_enc": "EncAesCbc",
"alg_enc_raw": 7,
"sa_key_length": "Unknown",
"sa_key_length_raw": 128,
"alg_hash": "HashSha",
"alg_hash_raw": 2,
"alg_dh": "GroupAlternate1024BitModpGroup",
"alg_dh_raw": 2,
"alg_auth": "AuthPreSharedKey",
"alg_auth_raw": 1,
"sa_life_type": "LifeTypeSeconds",
"sa_life_type_raw": 1,
"sa_life_duration": "Unknown",
"sa_life_duration_raw": 86400
}
]
}
}

**Suricata 9.0**

.. code-block:: json

"ikev1": {
"client": {
"proposals": [
{
"key": "alg_enc",
"value": "EncAesCbc",
"raw": 7
},
{
"key": "sa_key_length",
"value": "Unknown",
"raw": 128
},
{
"key": "alg_hash",
"value": "HashSha",
"raw": 2
},
{
"key": "alg_dh",
"value": "GroupAlternate1024BitModpGroup",
"raw": 2
},
{
"key": "alg_auth",
"value": "AuthPreSharedKey",
"raw": 1
},
{
"key": "sa_life_type",
"value": "LifeTypeSeconds",
"raw": 1
},
{
"key": "sa_life_duration",
"value": "Unknown",
"raw": 86400
}
]
}
}
121 changes: 44 additions & 77 deletions etc/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2459,29 +2459,33 @@
"type": "object",
"additionalProperties": false,
"properties": {
"alg_auth": {
"type": "string"
},
"alg_auth_raw": {
"type": "integer"
},
"alg_dh": {
"type": "string"
},
"alg_dh_raw": {
"type": "integer"
},
"alg_enc": {
"type": "string"
},
"alg_enc_raw": {
"type": "integer"
},
"alg_hash": {
"type": "string"
},
"alg_hash_raw": {
"type": "integer"
"attributes": {
"type": "array",
"minItems": 1,
"items": {
"type": "object",
"additionalProperties": false,
"properties": {
"key": {
"type": "string",
"enum": [
"alg_auth",
"alg_dh",
"alg_enc",
"alg_hash",
"sa_key_length",
"sa_life_duration",
"sa_life_type"
]
},
"raw": {
"type": ["string", "number"]
},
"value": {
"type": "string"
}
}
}
},
"exchange_type": {
"type": "integer",
Expand Down Expand Up @@ -2531,47 +2535,23 @@
"type": "object",
"additionalProperties": false,
"properties": {
"alg_auth": {
"type": "string"
},
"alg_auth_raw": {
"type": "integer"
},
"alg_dh": {
"type": "string"
},
"alg_dh_raw": {
"type": "integer"
},
"alg_enc": {
"type": "string"
},
"alg_enc_raw": {
"type": "integer"
},
"alg_hash": {
"type": "string"
},
"alg_hash_raw": {
"type": "integer"
},
"sa_key_length": {
"type": "string"
},
"sa_key_length_raw": {
"type": "integer"
},
"sa_life_duration": {
"type": "string"
"key": {
"type": "string",
"enum": [
"alg_auth",
"alg_dh",
"alg_enc",
"alg_hash",
"sa_key_length",
"sa_life_duration",
"sa_life_type"
]
},
"sa_life_duration_raw": {
"type": "integer"
"raw": {
"type": ["string", "number"]
},
"sa_life_type": {
"value": {
"type": "string"
},
"sa_life_type_raw": {
"type": "integer"
}
}
}
Expand All @@ -2587,6 +2567,7 @@
"server": {
"type": "object",
"additionalProperties": false,
"minProperties": 1,
"properties": {
"key_exchange_payload": {
"type": "string"
Expand Down Expand Up @@ -2642,22 +2623,8 @@
"role": {
"type": "string"
},
"sa_key_length": {
"type": "string"
},
"sa_key_length_raw": {
"type": "integer"
},
"sa_life_duration": {
"type": "string"
},
"sa_life_duration_raw": {
"type": "integer"
},
"sa_life_type": {
"type": "string"
},
"sa_life_type_raw": {
"version": {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be a little confusing with the 2 versions below.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think we discussed this before for other records.

suricata_record_version:2
srv:2

suricata:{
   version:2
}

other ideas?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not, srv, thats a little ambiguous in DNS events. So I think we need something completely unambiguous.

suricata_record_version works, but I 'd prefer the object approach.

"suricata": {
  "version": 2
}

could work, but we've also talked about adding the Suricata version to each record before, and this should go in a similar object, but would have a different schema, so something like:

"suricata": {
    "version": "8.0.2"
    "major": 8
}

or something like that, but that doesn't work as well for the "ike" record format.

Ideally the suricata object would have the same schema no matter where, but not strictly necessary, so I could be swayed either way.

I've also really come to appreciate how Elastic underscore prefixes fields that are part of their system, and not the payload, and that might be worth exploring as well, something like:

"ike": {
    "_v": 2   // IKE record version (could also use _rv, or _sv for schema version)
}

Other systems will use a @ prefix which is fine as well, just a little less friendly when working with parsed JSON in Javascript.

Any of these jump out at you? Repel you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of easily marking and identifying fields that are not part of the payload. And, for what you've said, tend to prefer the underscore approach.

Could it happen that we have both record version and schema version, for some development decision?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it happen that we have both record version and schema version, for some development decision?

No. It better not. We might have a top level schema version. But really the Suricata version can serve that role just as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even the "suricata" object is not future proof. There could be a Suricata protocol out there in the future. So "_suricata" would be the better object name.

"desription": "The version of the IKE log record (not IKE version)",
"type": "integer"
},
"version_major": {
Expand Down
2 changes: 1 addition & 1 deletion rust/src/ike/detect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ pub extern "C" fn SCIkeStateGetSaAttribute(
let sa_type_s: Result<_, _>;

unsafe { sa_type_s = CStr::from_ptr(sa_type).to_str() }
SCLogInfo!("{:#?}", sa_type_s);
SCLogDebug!("{:#?}", sa_type_s);

if let Ok(sa) = sa_type_s {
if tx.ike_version == 1 {
Expand Down
Loading
Loading