Skip to content

Comments

Output alert applayer v20.1#10300

Closed
catenacyber wants to merge 3 commits intoOISF:masterfrom
catenacyber:output-alert-applayer-v20.1
Closed

Output alert applayer v20.1#10300
catenacyber wants to merge 3 commits intoOISF:masterfrom
catenacyber:output-alert-applayer-v20.1

Conversation

@catenacyber
Copy link
Contributor

Link to redmine tickets:
https://redmine.openinfosecfoundation.org/issues/3827
Preliminary work for https://redmine.openinfosecfoundation.org/issues/5053

Describe changes:

  • output: unify boilerplate code (trying to become a lines-of-code neutral contributor to Suricata ;-) and rising the percentage of rust files )
  • output/dns: do not add empty app-layer metadata
  • dnp3: restrict function scope to one file

There is one behavior change for SSH switching from LOG_DIR_PACKET to LOG_DIR_FLOW
What do you think about it ?
Should there be 2 functions for JsonGenericLogger ? like JsonGenericLoggerDirPacket and JsonGenericLoggerDirFlow ? So that we do not have any behavioral changes
Or should we choose to unify the behavior between the protocols ? And if so, should we choose LOG_DIR_PACKET or LOG_DIR_FLOW ?

#10166 rebased

SV_BRANCH=pr/1490

OISF/suricata-verify#1490

@catenacyber catenacyber requested review from a team and victorjulien as code owners February 2, 2024 12:19
@victorjulien victorjulien self-assigned this Feb 2, 2024
@jasonish
Copy link
Member

jasonish commented Feb 2, 2024

There is one behavior change for SSH switching from LOG_DIR_PACKET to LOG_DIR_FLOW

Can you make a ticket for the SSH direction change? As this could be a breaking change on its own.

@catenacyber
Copy link
Contributor Author

Can you make a ticket for the SSH direction change? As this could be a breaking change on its own.

Yes, but if it is the right thing to do :

We can

  • keep the current behavior
  • have all protocols follow LOG_DIR_PACKET
  • have all protocols follow LOG_DIR_FLOW

Protocols with LOG_DIR_PACKET : BitTorrent, ike, krb5, nfs, quic, rdp, sip, snap, tftp
Protocols with LOG_DIR_FLOW : dcerpc, dnp3, dns, ftp, http, modbus, mqtt, pgsql, rfb, smb, smtp, ssh, tls

@jasonish
Copy link
Member

jasonish commented Feb 2, 2024

Can you make a ticket for the SSH direction change? As this could be a breaking change on its own.

Yes, but if it is the right thing to do :

We can

* keep the current behavior

* have all protocols follow LOG_DIR_PACKET

* have all protocols follow LOG_DIR_FLOW

Protocols with LOG_DIR_PACKET : BitTorrent, ike, krb5, nfs, quic, rdp, sip, snap, tftp Protocols with LOG_DIR_FLOW : dcerpc, dnp3, dns, ftp, http, modbus, mqtt, pgsql, rfb, smb, smtp, ssh, tls

Also have to consider those that log requests separate from responses. This is probably where packet direction makes sense. Currently LOG_DIR_FLOW is used for DNS, but it probably shouldn't be. But should still continue to be used for TLS, and SSH for instance.

@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW1_stats_chk
.flow.spare 999831 1100151 110.03%
SURI_TLPR1_stats_chk
.memcap_pressure 57 48 84.21%
.memcap_pressure_max 62 69 111.29%

Pipeline 18073

@catenacyber
Copy link
Contributor Author

Thank you Jason,

Continued in #10319

@catenacyber catenacyber closed this Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants