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

Next/20210831/v12 #6323

Closed
wants to merge 71 commits into from
Closed

Conversation

victorjulien
Copy link
Member

#6319 with minor fixes
#6318
#6316
#6315
#6275
#5999
#5645
#5524
#5514
#5154

suricata-verify-pr: 526

victorjulien and others added 30 commits August 31, 2021 14:46
Make sure most common branch is handled first to assist branch
prediction.

Macros still play a small role to please our 'action' cocci check.
Dump all patterns to `patterns.json`, with the pattern, a total count (`cnt`),
count of how many times this pattern is the mpm (`mpm`) and some of the flags.

Patterns are listed per buffer. So payload, http_uri, etc.
Add 'inspect_type' "packet" and "tx" for the two record types. Add more metadata
when available.
In the flow worker timeout path it is assumed that we can hand off
flows to the recycler after processing, implying that `Flow::use_cnt` is 0.
However, there was a case where this assumption was incorrect.

When during flow timeout handling the last processed data would trigger a
protocol upgrade, two additional pseudo packets would be created that were
then pushed all the way through the engine packet paths. This would mean
they both took a flow reference and would hold that until after the flow
was handed off to the recycler. Thread safety implementation would make
sure this didn't lead to crashes.

This patch handles this case returning these packets to the pool from
the timeout handling.
Don't increment the flow timeout counter for flows that are not
really timeout (as use_cnt is non zero). And also don't take into
account bypassed flows in the counter for flow timeout in use.
Main point is to document it is interacting with the capture
layer.
As the FlowBypassedTimeout function is interacting with the capture
method it is possible that the return changes between the call that
did trigger the timeout and the actual state (ie if packets arrive
in between the two calls). So we should not use the call to
FlowBypassedTimeout in the assert.
It differentiates memory error than regular ones.
jasonish and others added 22 commits August 31, 2021 22:10
using set_info_level == SMB2_FILE_DISPOSITION_INFO
Replace dead link

Dataset on ll.mit.edu returns 404. Link updated with a search result of more datasets.
Add a bundle.sh script to bundle the requirements of libhtp
and suricata-update. This uses a Python like requirements.txt
file to specify the URL to download for libhtp and suricata-update.
If one of the ppp peers sends a packet with an acknowledge flag,
the ppp payload will be empty and DecodePPP will return TM_ECODE_FAILED.
To handle this case, the packet_length field in the GRE extended header (https://tools.ietf.org/html/rfc2637#section-4.1) is used.
DecodeGRE no longer tries to parse PPP payload if packet_length is zero.
Bug: OISF#3703.

Don't log files too soon.
To make sure a transaction is not evicted before all file logging is complete.
Storing too early can lead to files being considered TRUNCATED if the
TCP state is not yet CLOSED when logging is triggered. This has been
observed with FTP-DATA and might also be an issue with simple HTTP.
Avoid evicting a tx before the filedata logger has decided it is
done.
After a file has been closed (CLOSE, COMMIT command or EOF/SYNC part of
READ/WRITE data block) mark it as such so that new file commands on that
file do not reuse the transaction.

When a file transfer is completed it will be flagged as such and not be
found anymore by the NFSState::get_file_tx_by_handle() method. This forces
a new transaction to be created.
@victorjulien victorjulien requested review from jasonish, norg and a team as code owners September 1, 2021 06:08
@codecov
Copy link

codecov bot commented Sep 1, 2021

Codecov Report

Merging #6323 (d72f1d2) into master (7551247) will decrease coverage by 0.06%.
The diff coverage is 70.68%.

@@            Coverage Diff             @@
##           master    #6323      +/-   ##
==========================================
- Coverage   76.95%   76.88%   -0.07%     
==========================================
  Files         611      612       +1     
  Lines      185955   186347     +392     
==========================================
+ Hits       143102   143281     +179     
- Misses      42853    43066     +213     
Flag Coverage Δ
fuzzcorpus 52.61% <33.27%> (-0.25%) ⬇️
suricata-verify 51.19% <66.60%> (+0.07%) ⬆️
unittests 63.04% <35.22%> (-0.08%) ⬇️

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

@victorjulien victorjulien mentioned this pull request Sep 1, 2021
@victorjulien
Copy link
Member Author

replaced by #6324

@victorjulien victorjulien deleted the next/20210831/v12 branch November 19, 2021 05:29
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.

6 participants