Skip to content

Detect tcp noupdatetotx 6299 v9 #10145

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.

Sign up for GitHub

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

Closed
catenacyber wants to merge 4 commits into from
+109 −104
Closed

Detect tcp noupdatetotx 6299 v9 #10145

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ea12eec
detect: avoids case of useless detection on txs
catenacyber Sep 7, 2023
d8dcc8e
detect: merge sorted lists instead of qsort
catenacyber Sep 18, 2023
e461b80
detect: do not store state without flags
catenacyber Sep 18, 2023
94deb28
mqtt: fix logic when setting event
catenacyber Oct 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/detect.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,7 @@ static bool DetectRunTxInspectRule(ThreadVars *tv,
} else if ((inspect_flags & DE_STATE_FLAG_FULL_INSPECT) == 0 && mpm_in_progress) {
TRACE_SID_TXS(s->id, tx, "no need to store no-match sig, "
"mpm will revisit it");
} else {
} else if (inspect_flags != 0) {
Copy link
Member

@victorjulien victorjulien Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we store more than just the flags, we also store file_no_match. Can we have a case where we'd need that stored still?

Copy link
Contributor Author

@catenacyber catenacyber Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good point from your knowledge and wisdom. Will look that through

Copy link
Contributor Author

@catenacyber catenacyber Jan 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note, feels weird to see file_no_match as u16 when it is only 0 or 1 up to its use in StoreFileNoMatchCnt as += file_no_match;

Copy link
Member

@victorjulien victorjulien Jan 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal of the logic, but I wouldn't be surprised if it is broken, is to stop tracking the files if all sigs that need it definitively failed to match. So it should increment this for each unique sig that fails to match.

Copy link
Contributor Author

@catenacyber catenacyber Jan 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal of the logic, but I wouldn't be surprised if it is broken, is to stop tracking the files if all sigs that need it definitively failed to match. So it should increment this for each unique sig that fails to match.

That is what I understood from reading the code

TRACE_SID_TXS(s->id, tx, "storing state: flags %08x", inspect_flags);
DetectRunStoreStateTx(scratch->sgh, f, tx->tx_ptr, tx->tx_id, s,
inspect_flags, flow_flags, file_no_match);
Expand Down