ike: log attributes as objects - v2 #13927
Conversation
IKE attributes are an array of TLV style objects, this means there can
be duplicate types seen on the wire. However, Suricata logs these as a
mapping with the type as the key. This can result in the JSON
containing duplicate keys.
To address this, log the attributes as an array of objects, allow
duplicates to exist, for example:
"client": {
"proposals": [
{
"sa_life_duration": "Unknown",
"sa_life_duration_raw": 86400,
}
}
}
is now logged as:
"client": {
"proposals": [
{"key": "sa_life_duration", "value": "Unknown", "raw": 86400}
]
}
Also adds `"version": 2` to each IKE record to note the change of
format from previous versions.
Ticket: OISF#7902
Update EVE documentation script to handle union types like:
"type": ["string", "number"]
| "type": "string" | ||
| }, | ||
| "sa_life_type_raw": { | ||
| "version": { |
There was a problem hiding this comment.
Could be a little confusing with the 2 versions below.
There was a problem hiding this comment.
think we discussed this before for other records.
suricata_record_version:2
srv:2
suricata:{
version:2
}
other ideas?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Could it happen that we have both
record versionandschema 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.
There was a problem hiding this comment.
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13927 +/- ##
==========================================
- Coverage 83.81% 83.81% -0.01%
==========================================
Files 1011 1011
Lines 275479 275474 -5
==========================================
- Hits 230905 230882 -23
- Misses 44574 44592 +18
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Information: QA ran without warnings. Pipeline = 27759 |
|
Replaced by #13948 |
Ticket: https://redmine.openinfosecfoundation.org/issues/7902
SV_BRANCH=OISF/suricata-verify#2665
Also:
Previous PR: #13907
Changes: