Skip to content
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.

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

Applayer plugin 5053 v3.11 #12110

Closed
wants to merge 16 commits into from

Conversation

catenacyber
Copy link
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/5053

Describe changes:

  • get ready to use dynamic number of app-layer protos (also work with static constant) in some places
  • preventive fix of macro with parenthesis cc @jufajardini

#12066 with review taken into account

  • making ALPROTO_FAILED constant=1
  • more verbose commit messages
  • get rid of a limit or 32 app-layer protocols for protocol detection with probing parsers.

This limit was luckily not a real problem for now.
The bug is setting probing parser done too soon. This may happen if the first packet does not have a full PDU...

  • ALPROTO_TEMPLATE=32 is not really meaningful
  • ALPROTO_RDP probes only one byte ! (which may be a problem in itself)
  • ALPROTO_BITTORRENT_DHT and ALPROTO_POP3 do not probe
  • ALPROTO_HTTP2 has probing but only on one side, the other side being a pattern. So we can technically get a different behavior but it is in an edge case that is not a real usage of HTTP2 : with a flow having HTTP2 where we only see server talking to client if the first packet has not enough data for a HTTP2 header.

Example plugin at https://github.com/catenacyber/suricata-zabbix

instead of a global variable.

For easier initialization with dynamic number of protocols
for expectation_proto

Ticket: 5053
so that we can use safely EXCEPTION_POLICY_MAX*sizeof(x)
Ticket: 5053

delay after initialization so that StringToAppProto works
As too many cases are found when splitting tcp payload
As it is also used for HTTP/1
Remove it only for TCP and keep it for UDP.
Because some alprotos will remain static and defined as a constant,
such as ALPROTO_UNKNOWN=0, or ALPROTO_FAILED.

The regular already used protocols keep for now their static
identifier such as ALPROTO_SNMP, but this could be made more
dynamic in a later commit.

ALPROTO_FAILED was used in comparison and these needed to change to use
either ALPROTO_MAX or use standard function AppProtoIsValid
Ticket: 5053

The names are now dynamically registered at runtime.
The AppProto alproto enum identifiers are still static for now.

This is the final step before app-layer plugins.
@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW1_stats_chk
.app_layer.error.tls.parser 1152 1203 104.43%
SURI_TLPR1_stats_chk
.app_layer.tx.ftp 95372 102178 107.14%
.ftp.memuse 2878 10638 369.63%

Pipeline 23302

@catenacyber catenacyber marked this pull request as draft November 12, 2024 16:48
@catenacyber
Copy link
Contributor Author

Draft for unclean git history

@catenacyber catenacyber reopened this Nov 12, 2024
There was an implicit limit of 32 app-layer protocols
used by probing parsers through a mask, meaning that
Suricata should not support more than 32 app-layer protocols
in total.

This limit is relaxed to each flow not being able to
run more than 32 probing parsers, meaning that for each source
and destination port combination, the sum of registered
probing parsers should not exceed 32, even if there are more
than 32 in total.
@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW1_stats_chk
.app_layer.error.tls.parser 1152 1203 104.43%
SURI_TLPR1_stats_chk
.app_layer.tx.ftp 95372 102160 107.12%
.ftp.memuse 2878 10638 369.63%

Pipeline 23311

@catenacyber
Copy link
Contributor Author

Next in #12116

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.

2 participants