File hashing 4279 v4 #5929
Closed
File hashing 4279 v4 #5929
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Files are truncated when they exceed stream reassembly depth. However, for some purposes, a hash of the prefix of the file is still useful, as long as we know the size of this prefix. We therefore change the logging logic to output the hash in the case of file truncation.
Allow capping number of bytes of files used for file hash. This is to allow for consistency and avoid relying on whatever the stream reassembly limit happens to be. If 0 or not present, the original behaviour of hashing up to the stream reassembly limit remains. If the value is greater than the stream reassembly limit, the stream reassembly limit takes precedence.
rtkjweeks
force-pushed
the
file-hashing-4279-v4
branch
2 times, most recently
from
5ef4d2e
to
eba5b5c
Compare
Codecov Report
@@ Coverage Diff @@
## master #5929 +/- ##
==========================================
- Coverage 76.69% 76.68% -0.01%
==========================================
Files 604 604
Lines 187657 187837 +180
==========================================
+ Hits 143926 144046 +120
- Misses 43731 43791 +60
Flags with carried forward coverage won't be shown. Click here to find out more. |
rtkjweeks
force-pushed
the
file-hashing-4279-v4
branch
from
eba5b5c
to
5142b09
Compare
jasonish
reviewed
Sep 3, 2021
Comment on lines
+222
to
+231
| if !bytes_hashed.is_null() { | ||
| if g_hash_byte_limit > 0 { | ||
| let bytes_remaining = match g_hash_byte_limit > *bytes_hashed { | ||
| true => g_hash_byte_limit - *bytes_hashed, | ||
| false => 0, | ||
| }; | ||
| if len > bytes_remaining { | ||
| len = bytes_remaining; | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't like the length logic being put into a low level hashing function which is not file-store specific. I think this accounting should be moved out of here.
jasonish
requested changes
Sep 3, 2021
|
Ping @rtkjweeks, any plans to continue on this? |
victorjulien
added a commit
to victorjulien/suricata
that referenced
this pull request
Mar 23, 2023
If urilen induced depth was set, later DetectContentPropagateLimits() would apply a wrong depth setting, leading to a false negative in some cases. Bug: OISF#5929.
victorjulien
added a commit
to victorjulien/suricata
that referenced
this pull request
Mar 23, 2023
If urilen induced depth was set, later DetectContentPropagateLimits() would apply a wrong depth setting, leading to a false negative in some cases. Bug: OISF#5929.
victorjulien
added a commit
to victorjulien/suricata
that referenced
this pull request
Mar 23, 2023
If urilen induced depth was set, later DetectContentPropagateLimits() would apply a wrong depth setting, leading to a false negative in some cases. Bug: OISF#5929.
victorjulien
added a commit
to victorjulien/suricata
that referenced
this pull request
Mar 24, 2023
If urilen induced depth was set, later DetectContentPropagateLimits() would apply a wrong depth setting, leading to a false negative in some cases. Bug: OISF#5929.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make sure these boxes are signed before submitting your Pull Request -- thank you.
Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/4279
Previous PR:
#5882
Describe changes:
suricata-verify-pr: 430
#suricata-verify-repo:
#suricata-verify-branch:
#suricata-update-pr:
#suricata-update-repo:
#suricata-update-branch:
#libhtp-pr:
#libhtp-repo:
#libhtp-branch: