Skip to content

Detect dynamic register keywords 4683 v6#11291

Closed
catenacyber wants to merge 5 commits into
OISF:masterfrom
catenacyber:detect-dynamic-register-keywords-4683-v6
Closed

Detect dynamic register keywords 4683 v6#11291
catenacyber wants to merge 5 commits into
OISF:masterfrom
catenacyber:detect-dynamic-register-keywords-4683-v6

Conversation

@catenacyber
Copy link
Copy Markdown
Contributor

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
  • snmp.pdu_type use a generic uint32 for detection, allowing >2 and such
  • detect/dhcp: move keywords to rust
  • detect/websocket: move keywords to rust
  • detect/enip: move keywords to rust

Continuation of #9871 after merge of #10819

After the merge of loggers, pure rust plugins need pure rust keywords.
The plan is to do this for all rust app-layers, now only done for 4 protocols, which has both integers and buffers as keywords.
This can come in later PRs
Then, there will be the final PR/commits for app-layer plugins

#11068 rebased with enip after merge of #11218 for build fixes

detect: make number of keywords dynamic

Ticket: 4683
Ticket: 4863

On the way, convert unit test DetectSNMPCommunityTest to a SV test.

And also, make snmp.pdu_type use a generic uint32 for detection,
allowing operators, instead of just equality.
@catenacyber catenacyber force-pushed the detect-dynamic-register-keywords-4683-v6 branch from c4fe3dd to 9048cff Compare June 11, 2024 13:50
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 52.45580% with 726 lines in your changes missing coverage. Please review.

Project coverage is 78.99%. Comparing base (f0dbfe8) to head (9048cff).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11291      +/-   ##
==========================================
- Coverage   82.45%   78.99%   -3.47%     
==========================================
  Files         961      934      -27     
  Lines      251710   251970     +260     
==========================================
- Hits       207552   199043    -8509     
- Misses      44158    52927    +8769     
Flag Coverage Δ
fuzzcorpus 60.28% <52.12%> (-0.03%) ⬇️
livemode 18.77% <39.29%> (+0.08%) ⬆️
pcap 43.77% <39.48%> (-0.03%) ⬇️
suricata-verify ?
unittests 59.91% <39.16%> (-0.05%) ⬇️

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

@suricata-qa
Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline 21059

Copy link
Copy Markdown
Member

@victorjulien victorjulien left a comment

Choose a reason for hiding this comment

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

Nice work, lets get this merged.

One follow up request: I don't like the now dynamic DETECT_TBLSIZE_IDX and friends as all uppercase. That we normally reserve for macros/defines.

@victorjulien victorjulien added this to the 8.0 milestone Jun 15, 2024
@victorjulien
Copy link
Copy Markdown
Member

Merged in #11309, thanks!

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