-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 #12441
Conversation
and use generic logger callback prototype with later cast
Ticket: 7151 Ticket: 7152 Ticket: 7154
Ticket: 7149 Ticket: 7150 Ticket: 7153
pub const APP_LAYER_EVENT_TYPE_TRANSACTION : AppLayerEventType = AppLayerEventType::APP_LAYER_EVENT_TYPE_TRANSACTION; | ||
|
||
pub const SIGMATCH_NOOPT: u16 = detect::SIGMATCH_NOOPT; | ||
pub const SIGMATCH_INFO_STICKY_BUFFER: u16 = detect::SIGMATCH_INFO_STICKY_BUFFER; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like most of this stuff belongs in Suricata proper? I don't think its specific to plugin right? Its just needed by plugins, but its not specific to plugins, which is probably what this crate should be for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss in the main part...
Maybe not a crate then. We have plugin.rs which is only for use by plugins as well. Crate does seem like a better idea, but probably not if it depends on the suricata crate. I think we need to focus on:
As this will then be locked down for 8. I haven't put much focus on Rust plugins yet, so don't have much comments on it yet. |
For plugin support, we should provide what is the PR previous version of suricata.rs : So, do you see a good way ? |
We could cbindgen a C header file, then bindgen it again to rust to get this rust header-ish file, and have the Suricata-plugin crate define some other methods (like the jsonbuilder rusty feel-like even if we use the C API) |
But the plugin already requires the suricata crate right? So maybe the extra crate shouldn't be used. It was just a thought - maybe not a good one. My redis output plugin has some stuff that should be in Suricata proper as well. I'd like to rebase that soon which will give me a better feel for what is going on. |
Wrong :-p |
Any reason why not? Thats how I'd expect plugins to get access to data structures, SCLog stuff, and so no. At least thats how existing capture and output plugins do it. |
This PR brings over the custom bindings my redis output has been carrying around for a while: #12446. |
I do not want my plugin to import and compile 200 dependencies from Suricata to know the value of IPPROTO_TCP = 6 |
Thanks |
But for an example, I don't think providing all that stuff in suricata.rs is ideal as most of it already exists in Suricata, just lacks the Rust bindings. For example, the logging - we already have that exposed in Rust. So what I think you're doing is more likely an edge case, not the normal case so probably not ideal for a an example plugin. I think the Rust way here would be to break up the Rust part of Suricata into multiple crates, so you can import stuff at a more fine grained level. |
ERROR: ERROR: QA failed on build_asan. Pipeline 24315 |
I totally agree. I am working on another POC using cbindgen + bindgen to make a crate Suricata-plugin that exports only a fraction of what Suricata has already available from rust.
I am not sure I follow you. But logging is a good example : |
Next in #12448 |
Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/4102 with all 6 subtickets
Describe changes:
@jufajardini what do you think about the doc ?
@jasonish as you created the tickets, is this what you expected ?
Draft: last commit is WIP for a Suricata-plugin crate.
But it recompiles every dependency of Suricata even if I just need some constants !!!
@jasonish do you see a better way ?