Conversation
Moving it to be able to use it globally later. Ticket: OISF#7888
This will be used to code the version of the log to use so backward compatibility can be achieved: upgrading Suricata without changing the configuration should not trigger major changes in the log format.
The app_proto is logged in a small subset (fileinfo, flow, frame, netflow) in Suricata when it is an interesting information for all events type as it includes detection information and protocol transition. This patch updates the code to log app_proto in all events if there is a Flow available. It is making use of EveAddAppProto function to get interesting information such as original application protocol or difference between server and client side. Backward compatibility is preserved as app_proto information will only be logged when the eve-log.version is greater or equal to 2. Ticket: 7888
Log the version of EVE used in the event.
regit
force-pushed
the
app_proto_logging-v2
branch
from
548c28a to
48ed06d
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14760 +/- ##
=======================================
Coverage 82.15% 82.15%
=======================================
Files 1003 1003
Lines 263674 263708 +34
=======================================
+ Hits 216611 216649 +38
+ Misses 47063 47059 -4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
| #metadata: no | ||
| # Include suricata version. Default no. | ||
| #suricata-version: yes | ||
| version: 2 |
There was a problem hiding this comment.
I guess this needs some documentation
There was a problem hiding this comment.
And Maybe a ticket on its own
| #define JSON_ADDR_LEN 46 | ||
| #define JSON_PROTO_LEN 16 | ||
|
|
||
| #define EVE_MAX_VERSION 2 |
There was a problem hiding this comment.
Please comment what these mean...
|
I see the value in the SV tests examples, but sometimes, this feels uselessly verbose like "app_proto": "mqtt",
"event_type": "mqtt",Could we have a better logic to only add info when it brings something ? |
|
If I understand things correctly this PR adds a global record version and enables the app-logging by default for that. I guess I disagree with both approaches. We have some record level versioning (using Wrt enabling more by default, I don't think we should be doing this. I think we need some though about different verbosity profiles, as generating too many and too large records is one of the issues many users run into. |
victorjulien
left a comment
victorjulien
left a comment
There was a problem hiding this comment.
Needs some more thought on how to carefully expand logging, w/o blowing up records even further by default.
3 participants
Make sure these boxes are checked accordingly before submitting your Pull Request -- thank you.
Contribution style:
https://docs.suricata.io/en/latest/devguide/contributing/contribution-process.html
Our Contribution agreements:
https://suricata.io/about/contribution-agreement/ (note: this is only required once)
Changes (if applicable):
(including schema descriptions)
https://redmine.openinfosecfoundation.org/projects/suricata/issues
Link to ticket: https://redmine.openinfosecfoundation.org/issues/7888
Describe changes:
Provide values to any of the below to override the defaults.
SV_BRANCH=OISF/suricata-verify#2903