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.13 #12118

Closed
wants to merge 17 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

#12116 with clean history and in-tree plugin testing (without logging enabled)

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.
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 102159 107.12%
.ftp.memuse 2878 10638 369.63%

Pipeline 23315

@victorjulien victorjulien added the needs rebase Needs rebase to master label Nov 14, 2024
edition = "2021"

[lib]
crate-type = ["cdylib"]
Copy link
Member

Choose a reason for hiding this comment

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

I think this is correct for using the C ABI? @chifflier?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, during the talk I thought dylib was used.
cdylib is the correct thing to do to have a stable API (in C), so it's all good :)

Copy link
Contributor

@chifflier chifflier Nov 15, 2024

Choose a reason for hiding this comment

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

Note: you might want to use something like bindgen to generate bindings automatically. Also, it may be good to use a structure with a api_version field (as first field) for the registration, so that updating the protocol will be easier in the future

Copy link
Member

Choose a reason for hiding this comment

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

@chifflier this may interest you #12062

@catenacyber
Copy link
Contributor Author

Rebased in #12127

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase Needs rebase to master
Development

Successfully merging this pull request may close these issues.

5 participants