Detect dynamic register keywords 4683 v1

[catenacyber]

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

Describe changes:

• detect: helper to have pure rust keywords
• make keywords registration dynamic
• detect/snmp: move keywords to rust
• convert unit test DetectSNMPCommunityTest to a SV test.
• snmp.pdu_type use a generic uint32 for detection, allowing >2 and such

SV_BRANCH=OISF/suricata-verify#1804

Continuation of #9871 after merge of #10819

After the merge of loggers, pure rust plugins will need pure rust keywords.
The plan is to do this for all rust app-layers, now only done for SNMP, which has both integers and buffers as keywords.

[suricata-qa]

WARNING:

field	baseline	test	%
	build_asan

Pipeline 20365

[suricata-qa]

WARNING:

field	baseline	test	%
	build_asan

Pipeline 20366

[suricata-qa]

WARNING:

field	baseline	test	%
	build_asan

Pipeline 20367

[victorjulien]

Set to draft to avoid the force pushes monopolizing qalab runtime.

[jasonish]

naming: no_mangle and extern C should follow C naming conventions.

[jasonish]

As this isn't pub, and just a function pointer, you could drop the no_mangle and use Rust conventions. Meaning you could drop the prefixes in the function name as well, as the visibility is just this module.

[catenacyber]

What does the no_mangle do ?
I thought I needed it for the ABI

[jasonish]

No mangle just keeps the name as-is in the symbol table so linkers can find it. If you just point to it using a function pointer its not needed and best let Rust do its name mangling.

The extern is for the ABI.

[catenacyber]

Thanks Jason, makes sense

[suricata-qa]

WARNING:

field	baseline	test	%
	build_asan

Pipeline 20385

[catenacyber]

So, problem statement.
I want to have C functions that are only called by rust code.
(Workaround would be to add usages in C)
Linker is not happy about it as it optimizes, and removes unused code, just to say afterwards that there are undefined reference

I tried in code :

• attribute used
• attribute externally_visible

in compilation :

• Did not manage to use -Wl,--start-group with autotool Makefile.am (mixing of ldflags and ldadd)
• Double libc.a librust.a libc.a librust.a, so that each definition comes after a use (also just libc.a librust.a libc.a)
• Tried to mix both archives into a single one, but then this single archive gets optimized and main.c ends up having unreferenced stuff

By the way, I do not understand how unused ConfGetDouble gets linked in the end in suricata

[jasonish]

So, problem statement. I want to have C functions that are only called by rust code. (Workaround would be to add usages in C) Linker is not happy about it as it optimizes, and removes unused code, just to say afterwards that there are undefined reference

I tried in code :

* attribute used

* attribute externally_visible

in compilation :

* Did not manage to use `-Wl,--start-group` with autotool Makefile.am (mixing of ldflags and ldadd)

* Double libc.a librust.a libc.a librust.a, so that each definition comes after a use (also just libc.a librust.a libc.a)

* Tried to mix both archives into a single one, but then this single archive gets optimized and main.c ends up having unreferenced stuff

By the way, I do not understand how unused ConfGetDouble gets linked in the end in suricata

I don't think the linker is removing the code, -rdynamic is supposed to make sure that doesn't happen. Its just the link only looks at libraries listed after to resolve patterns. So when we do C library first, then rust, it can't find it.

So portable doing the start-group type stuff is probably the way to resolve this.

[jasonish]

One thought... Can you move those functions over to Rust, even if very C like, then just call more primitives on the C that are for sure used?

[jasonish]

Did not manage to use -Wl,--start-group with autotool Makefile.am (mixing of ldflags and ldadd)

If I recall correctly, libtool will re-order all the compilation args in such away that this doesn't work unfortunately.

[catenacyber]

One thought... Can you move those functions over to Rust, even if very C like, then just call more primitives on the C that are for sure used?

Good idea

[catenacyber]

One thought... Can you move those functions over to Rust, even if very C like, then just call more primitives on the C that are for sure used?

Good idea

But does not seem possible for DetectHelperKeywordRegister at least...

[catenacyber]

I don't think the linker is removing the code, -rdynamic is supposed to make sure that doesn't happen. Its just the link only looks at libraries listed after to resolve patterns. So when we do C library first, then rust, it can't find it.

So portable doing the start-group type stuff is probably the way to resolve this.

Yes, but that does not look so portable...

[catenacyber]

Continued in #10992

[jasonish]

I don't think the linker is removing the code, -rdynamic is supposed to make sure that doesn't happen. Its just the link only looks at libraries listed after to resolve patterns. So when we do C library first, then rust, it can't find it.

So portable doing the start-group type stuff is probably the way to resolve this.

Yes, but that does not look so portable...

Possibly. It is required for plugins, but we only test that on Linux. I know they're broken on Windows. Possibly Mac as well.

[jasonish]

One thought... Can you move those functions over to Rust, even if very C like, then just call more primitives on the C that are for sure used?

Good idea

But does not seem possible for DetectHelperKeywordRegister at least...

Function point in the context object?

[catenacyber]

But does not seem possible for DetectHelperKeywordRegister at least...

Function point in the context object?

Possible, but I am not a big fan of this context object... I see that for frames, you have set up empty stubs for cfg test :-)

[jasonish]

But does not seem possible for DetectHelperKeywordRegister at least...

Function point in the context object?

Possible, but I am not a big fan of this context object... I see that for frames, you have set up empty stubs for cfg test :-)

Is anyone?

I've prototyped re-working the build to this order:

• generate headers from rust code
• build C code into library (no main)
• build rust code into library that also contains the C library code, so a single library
• build suricata application from single library

It works, and makes the C code available to Rust unit tests. Essentially eliminating the need for the context object. Doesn't work with ASAN -- builds fail. So a bit unfortunate on that front.

[catenacyber]

Looks nice.

How does it fail with ASAN ?

[jasonish]

How does it fail with ASAN ?

At link time.. Scenario is:

• build C objects files with asan
• build rust library
• can build suricata application
• cannot build rust unit tests, they're essentially an application

Built the Rust "test" apps require manual linking with asan, however when manually linking with asan it must be the first lib linked, and I can't make it the first with Rust flags and what not. Tried using the Rust -Z sanitize stuff, but no luck there either.

[catenacyber]

In any case, I think that the functionality of having C functions only called by rust could be useful.

But if it gets too painful for this PR, I will resort to use them in C also...
But it looks not far in next version...
MacOS build is failing, but I think I found a "bug" in the build system for fuzz targets

[jasonish]

In any case, I think that the functionality of having C functions only called by rust could be useful.

But if it gets too painful for this PR, I will resort to use them in C also... But it looks not far in next version... MacOS build is failing, but I think I found a "bug" in the build system for fuzz targets

if -lsuricata_c -lsuricata_rust -lsuricata_c does allow it to work, its a reasonable work-around as long as we're stuck with libtool. I'll look at getting rid of libtool after we remove the option of bundled/unbundled libhtp.