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 v8 #12455

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/4102 with all 6 subtickets

Describe changes:

  • add app-layer plugin example with template protocol
  • document app-layer plugins

@jufajardini what do you think about the doc ?

Jason told me this fits the tickets

#12448 using suricata as a crate by the plugin

Draft:

Notice that it is a really bad idea to use an empty enum as FFI type

  • still need to split the suricata crate into 2 with a suricata-sys crate that has 0 dependendy and is the one used by the plugin

cc @jasonish

and use generic logger callback prototype with later cast

and do some other small modifications so that the plugin
has less diff
make should rerun cbindgen if cbindgen.toml is modified
Just use a regular compile time rust export, instead of having
a runtime definition through the SuricataContext structure
so that it can be used by plugins

Avoid export by cbindgen as this constant is also defined in C
so that it can be used by plugins that want to call SCLogError
and such
so that they can be used by rust plugins
when compiling without unit tests
detect-transform-base64.c:47:9: warning: macro is not used [-Wunused-macros]
   47 | #define DETECT_TRANSFORM_FROM_BASE64_MODE_DEFAULT (uint8_t) Base64ModeRFC4648
JsonBuilder struct should not be made public because it is not
repr(C), and thus cannot be used as such by plugins,
because rust compiler does nots guarantee struct layout if we
do not use a fixed representation.

Also define a generic standard prototype for EveJsonSimpleTxLogFunc
and enfore it without cast for every function, each function
taking care of casting to its specific transaction type.
Ticket: 7151
Ticket: 7152
Ticket: 7154
Ticket: 7149
Ticket: 7150
Ticket: 7153
@suricata-qa
Copy link

ERROR:

ERROR: QA failed on build_asan.

Pipeline 24329

Comment on lines +107 to +109
// JsonBuilder should not be made public outside the crate as it is not repr(C)
#[derive(Debug, Clone)]
pub struct JsonBuilder {
pub(crate) struct JsonBuilder {
Copy link
Member

Choose a reason for hiding this comment

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

Might not be the only case case for Suricata as a library. Could have a Rust crate pulling Suricata for utility reasons and need access.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, but how do you propose to avoid the pitfall that some plugin use suricata crate JsonBuilder instead of using the suricata runtime one ? Just rely on docs ?

Copy link
Member

Choose a reason for hiding this comment

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

Probably through documentation. I'm not too keen on artificially limiting the usefulness of the library to protect users of just one use case.

@catenacyber catenacyber marked this pull request as draft January 23, 2025 15:24
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 91.55556% with 19 lines in your changes missing coverage. Please review.

Project coverage is 80.66%. Comparing base (95e8427) to head (9249686).
Report is 13 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12455      +/-   ##
==========================================
+ Coverage   80.63%   80.66%   +0.03%     
==========================================
  Files         920      920              
  Lines      258704   259125     +421     
==========================================
+ Hits       208595   209033     +438     
+ Misses      50109    50092      -17     
Flag Coverage Δ
fuzzcorpus 56.94% <73.30%> (+0.12%) ⬆️
livemode 19.34% <11.76%> (-0.05%) ⬇️
pcap 44.36% <64.25%> (+0.02%) ⬆️
suricata-verify 63.31% <91.44%> (+0.05%) ⬆️
unittests 58.43% <17.56%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@catenacyber
Copy link
Contributor Author

Replaced by #12465

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.

3 participants