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

(7.0.x backport) Applayer truncate 7044/v3 #11898

Open
wants to merge 3 commits into
base: main-7.0.x
Choose a base branch
from

Conversation

inashivb
Copy link
Member

@inashivb inashivb commented Oct 8, 2024

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

Previous PR: #11891

Changes since v2:

Note: These were not clean cherry-picks and are different from master so didn't use -x

jasonish and others added 3 commits October 8, 2024 12:58
With the latest brew changes, a virtualenv is required to install
pyyaml.

(cherry picked from commit 2b16369)
as its functionality is already covered by the generic code.
This leaves APP_LAYER_PARSER_TRUNC_TC and APP_LAYER_PARSER_TRUNC_TS
to avoid introducing any breaking changes.
FlowGetDisruptionFlags sets STREAM_DEPTH flag in case the respective
stream depth was reached. This flag tells that whether all the open files
should be truncated or not.

Bug 7044
truncate fn is only active and used by dcerpc and smb parsers. In case
stream depth is reached for any side, truncate fn is supposed to set the
tx entity (request/response) in the same direction as complete so the
other side is not forever waiting for data.

However, whether the stream depth is reached is already checked by
AppLayerParserGetStateProgress fn which is called by:
- DetectTx
- DetectEngineInspectBufferGeneric
- AppLayerParserSetTransactionInspectId
- OutputTxLog
- AppLayerParserTransactionsCleanup

and, in such a case, StateGetProgressCompletionStatus is returned for
the respective direction. This fn following efc9a7a, always returns 1
as long as the direction is valid meaning that the progress for the
current direction is marked complete. So, there is no need for the additional
callback to mark the entities as done in case of depth or a gap.

Remove all calls to the truncate fn but leave the registration fn intact
to avoid introducing any breaking API changes.

Bug 7044
@inashivb inashivb changed the title Applayer truncate 7044/v3 (7.0.x backport) Applayer truncate 7044/v3 Oct 8, 2024
@inashivb inashivb marked this pull request as ready for review October 8, 2024 08:31
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 23040

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