RFC/Preview: Filestore stream-depth fixes v2 #3265
Closed
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
added 4 commits
March 5, 2018 09:24
When a new tcp session is created, the reassemby_depth value is set by stream_config.reassembly_depth but it could happen that this value is updated later. For instance, when filestore keyword is used the function TcpSessionSetReassemblyDepth is called to update it. StreamTcpReassembleCheckDepth doesn't use the value updated, which is represented by ssn->reassembly_depth, but use the old one represented by stream_config.reassembly_depth. As a result, reassembly_depth value pointed by ssn is not considered when a stream is reassembled, and a stream can be truncated because the wrong reassembly_depth value is used. To fix this, TcpSessionSetReassemblyDepth now checks on ssn->reassembly_depth instead of stream_config.reassembly_depth. Ticket OISF#2264
When an app-layer parser is enabled, it could set its own stream_depth value calling the API AppLayerParserSetStreamDepth. Then, the function AppLayerParserPostStreamSetup will replace the stream_depth value already set with stream_config.reassembly_depth. To avoid overwriting, in AppLayerParserSetStreamDepth API a flag will be set internally to specify that a value is already set.
If a signature contains the filestore keyword, a file is truncated if its size is greater than response-body-limit. When file-store.stream-depth is set, it should be considered when a file is stored but in that case is ignored and the files stored are truncated.
norg
reviewed
Mar 5, 2018
| @@ -63,6 +63,15 @@ of the filename. For example, if the SHA256 hex string of an extracted | |||
| file starts with "f9bc6d..." the file we be placed in the directory | |||
| `filestore/f9`. | |||
|
|
|||
| The size of a file that can be stored depends on ``file-store.stream-depth``, | |||
| if this value is reached a file can be truncated and not stored completely. | |||
There was a problem hiding this comment.
f this value is reached , a file can be truncated and might not be stored completely
|
new PR: #3282 |
silentcreek
added a commit
to silentcreek/suricata
that referenced
this pull request
Feb 3, 2020
Using the run-as configuration option with the nflog capture method results in the following error during the startup of suricata: [ERRCODE: SC_ERR_NFLOG_BIND(248)] - nflog_bind_pf() for AF_INET failed This is because SCDropMainThreadCaps does not have any capabilities defined for the nflog runmode (unlike other runmodes). Therefore, apply the same capabilities to the nflog runmode that are already defined for the nfqueue runmode. This has been confirmed to allow suricata start and drop its privileges in the nflog runmode. Fixes redmine issue OISF#3265. Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>
2 tasks
victorjulien
pushed a commit
to victorjulien/suricata
that referenced
this pull request
Feb 10, 2020
Using the run-as configuration option with the nflog capture method results in the following error during the startup of suricata: [ERRCODE: SC_ERR_NFLOG_BIND(248)] - nflog_bind_pf() for AF_INET failed This is because SCDropMainThreadCaps does not have any capabilities defined for the nflog runmode (unlike other runmodes). Therefore, apply the same capabilities to the nflog runmode that are already defined for the nfqueue runmode. This has been confirmed to allow suricata start and drop its privileges in the nflog runmode. Fixes redmine issue OISF#3265. Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>
victorjulien
pushed a commit
to victorjulien/suricata
that referenced
this pull request
Feb 11, 2020
Using the run-as configuration option with the nflog capture method results in the following error during the startup of suricata: [ERRCODE: SC_ERR_NFLOG_BIND(248)] - nflog_bind_pf() for AF_INET failed This is because SCDropMainThreadCaps does not have any capabilities defined for the nflog runmode (unlike other runmodes). Therefore, apply the same capabilities to the nflog runmode that are already defined for the nfqueue runmode. This has been confirmed to allow suricata start and drop its privileges in the nflog runmode. Fixes redmine issue OISF#3265. Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de> (cherry picked from commit 1262ecb)
victorjulien
pushed a commit
to victorjulien/suricata
that referenced
this pull request
Feb 12, 2020
Using the run-as configuration option with the nflog capture method results in the following error during the startup of suricata: [ERRCODE: SC_ERR_NFLOG_BIND(248)] - nflog_bind_pf() for AF_INET failed This is because SCDropMainThreadCaps does not have any capabilities defined for the nflog runmode (unlike other runmodes). Therefore, apply the same capabilities to the nflog runmode that are already defined for the nfqueue runmode. This has been confirmed to allow suricata start and drop its privileges in the nflog runmode. Fixes redmine issue OISF#3265. Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de> (cherry picked from commit 1262ecb)
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/2264
Describe changes:
In the previous patchset, when a TX was flagged by file-store, at some point
the request or response body reassembled reaches its respective body limit,
so the chunk's len calculated by the line below is zero, which is cleary wrong:
This happens because a TX is flagged but then the stream depth value is not used
to determine the length of a chunk.
To calculate the correct length, when a TX is not flagged the body limit is used,
in the other case, when a TX is flagged, the stream depth value set is used.
While testing it, I have noticed that in case of file storing a file in TOSERVER direction,
if a file will be truncated its state will be closed, as you can see in the example below:
The file size is around ~150kb, and here even the size is less the state is set to CLOSED.
Attached files contains all the cases tested: stream_depth_tests.txt
PRScript output (if applicable):