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

lib: callback on tcp data #6247

Closed
wants to merge 1 commit into from
Closed

Conversation

catenacyber
Copy link
Contributor

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

Describe changes:

  • Adds a callback on tcp data (ie AppLayerHandleTCPData) to be used in libsuricata

Built with export CFLAGS=-DLIBSURICATA_BUILD=1
Ngrep POC is available at https://github.com/catenacyber/ngrep-libsuricata

The other alternative I see to have some ngrep over libsuricata, benefiting from suricata TCP reassembly, is to have a big architectural change in the tcp reassembly to have sequentially tcp reassembly, then app-layer processing on data supplied by tcp reassembly,
ie to call AppLayerHandleTCPData (or StreamTcpReassembleAppLayer) directly from FlowWorker after FlowWorkerStreamTCPUpdate, instead of having AppLayerHandleTCPData getting called in a 9-deep functions stack
This alternative is complex, and should also take into account the fact that suricata TCP reassembly depends on app-layer processing (sic !) for the case with a request like GOT / HTTP/1.1 (not GET) and response like HTTP/1.1 200 OK where protocol detection recognizes HTTP1 in the response (and nothing in the request), but then wants to parse the HTTP1-ish request before parsing the response. cf AppLayerTest05

letting the callback decide if suricata proceeds further
@catenacyber catenacyber requested a review from jasonish July 2, 2021 09:44
@codecov
Copy link

codecov bot commented Jul 2, 2021

Codecov Report

Merging #6247 (a5c7b82) into master (b8499de) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #6247      +/-   ##
==========================================
- Coverage   76.94%   76.94%   -0.01%     
==========================================
  Files         611      611              
  Lines      186146   186146              
==========================================
- Hits       143236   143227       -9     
- Misses      42910    42919       +9     
Flag Coverage Δ
fuzzcorpus 52.84% <100.00%> (+0.01%) ⬆️
suricata-verify 51.12% <100.00%> (-0.04%) ⬇️
unittests 63.08% <100.00%> (ø)

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

@jasonish
Copy link
Member

jasonish commented Jul 2, 2021

Not too keen on the compile time option. In the bigger picture of a libsuricata, Suricata itself is the main consumer so I think we need to avoid scenarios where:

  • Only using libsuricata for Suricata itself, compile as normal.
  • Using libsuricata for something else, add these compile time options as well.

@suricata-qa
Copy link

field test baseline % diff
tlpr1_stats_chk
.tcp.insert_list_fail 219 255 86.0%

@catenacyber
Copy link
Contributor Author

Continued in #6252

@catenacyber catenacyber closed this Jul 5, 2021
victorjulien added a commit to victorjulien/suricata that referenced this pull request Aug 3, 2023
Fixes multi-tenant multi-loader crashes.

Bug: OISF#6247.
victorjulien added a commit to victorjulien/suricata that referenced this pull request Aug 3, 2023
Fixes multi-tenant multi-loader crashes.

Bug: OISF#6247.
victorjulien added a commit to victorjulien/suricata that referenced this pull request Aug 8, 2023
Fixes multi-tenant multi-loader crashes.

Bug: OISF#6247.
victorjulien added a commit to victorjulien/suricata that referenced this pull request Aug 8, 2023
Switch to DetectParseRegex and use a local pcre2_match_data to
avoid concurrency issues.

Bug: OISF#6247.
victorjulien added a commit to victorjulien/suricata that referenced this pull request Aug 11, 2023
Fixes multi-tenant multi-loader crashes.

Bug: OISF#6247.
victorjulien added a commit to victorjulien/suricata that referenced this pull request Aug 11, 2023
Switch to DetectParseRegex and use a local pcre2_match_data to
avoid concurrency issues.

Bug: OISF#6247.
jlucovsky pushed a commit to jlucovsky/suricata that referenced this pull request Aug 14, 2023
Fixes multi-tenant multi-loader crashes.

Bug: OISF#6247.
jlucovsky pushed a commit to jlucovsky/suricata that referenced this pull request Aug 14, 2023
Switch to DetectParseRegex and use a local pcre2_match_data to
avoid concurrency issues.

Bug: OISF#6247.
yatink pushed a commit to yatink/suricata that referenced this pull request Aug 19, 2023
Fixes multi-tenant multi-loader crashes.

Bug: OISF#6247.
yatink pushed a commit to yatink/suricata that referenced this pull request Aug 19, 2023
Switch to DetectParseRegex and use a local pcre2_match_data to
avoid concurrency issues.

Bug: OISF#6247.
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