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 example and doc #12416

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

@jasonish as you created the tickets, is this what you expected ?

@catenacyber catenacyber requested review from jasonish, jufajardini and a team as code owners January 17, 2025 11:22
@catenacyber catenacyber force-pushed the applayer-plugin-5053-v5 branch from fb6bf6d to 5c864b6 Compare January 17, 2025 11:29
@suricata-qa
Copy link

ERROR:

ERROR: QA failed on build_asan.

Pipeline 24249

@@ -0,0 +1,547 @@
// This file is kind of the include required by API
Copy link
Member

Choose a reason for hiding this comment

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

this looks way to broad to me, even containing lots of things we define directly in rust. So why duplicate that? In general, we need to get to a point there this file is not needed at all.

Copy link
Member

Choose a reason for hiding this comment

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

I see another project here, making Rust plugins a first class citizen. All this "support" code should land in Suricata proper.

I have also needed to fill in some missing bits for an output plugin like this, just haven't brought them back into Suricata. But we probably need some "suricata_plugin" module or crate, that provides this support code to Rust, at least for the plugin specific stuff. Anything that's just filling in missing Suricata API should be in the main suricata rust package.

But it probably brings in more C into Rust, and maybe bindgen is worth another look instead of doing it by hand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

containing lots of things we define directly in rust.

Yes

So why duplicate that?

Because my rust plugin needs to know the value of IPPROTO_TCP
I would like to have some #include <libsuricata.h> but that does not work in rust

@jasonish do you have ideas ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But we probably need some "suricata_plugin" module or crate, that provides this support code to Rust, at least for the plugin specific stuff

I will try something...

But even after that, Victor, we will still want some code for JsonBuilder to wrap around the C API to feel like usual...

Ticket: 7151
Ticket: 7152
Ticket: 7154
Ticket: 7149
Ticket: 7150
Ticket: 7153
@catenacyber catenacyber force-pushed the applayer-plugin-5053-v5 branch from 5c864b6 to eec4e3e Compare January 17, 2025 16:05
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

Attention: Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.

Project coverage is 80.66%. Comparing base (8f6795d) to head (eec4e3e).
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12416      +/-   ##
==========================================
+ Coverage   80.63%   80.66%   +0.03%     
==========================================
  Files         918      919       +1     
  Lines      258696   259064     +368     
==========================================
+ Hits       208598   208973     +375     
+ Misses      50098    50091       -7     
Flag Coverage Δ
fuzzcorpus 56.90% <89.47%> (+0.09%) ⬆️
livemode 19.35% <5.26%> (-0.05%) ⬇️
pcap 44.37% <50.00%> (+0.07%) ⬆️
suricata-verify 63.28% <94.44%> (+0.04%) ⬆️
unittests 58.45% <34.21%> (-0.07%) ⬇️

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

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24262

@catenacyber
Copy link
Contributor Author

Something next in #12441

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