Detect tcp noupdatetotx 6299 v10#10160
Conversation
When a TCP flow packet has not led to app-layer updates, it is useless to run DetectRunTx, as there cannot be new matches. This happens for instance, when one side sends in a row multiple packets which are not acked (and thus not parsed in IDS mode). Doing so requires to move up the call to AppLayerParserSetTransactionInspectId so that it is run the same times DetectRunTx is run, and not in the case where the transaction was not updated. Ticket: 6299
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #10160 +/- ##
==========================================
- Coverage 82.19% 82.07% -0.12%
==========================================
Files 974 974
Lines 271825 271812 -13
==========================================
- Hits 223416 223092 -324
- Misses 48409 48720 +311
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: QA ran without warnings. Pipeline 17508 |
catenacyber
force-pushed
the
detect-tcp-noupdatetotx-6299-v10
branch
from
e965e4c to
54a42e0
Compare
Ticket: OISF#6299 Simply because it is faster (just linear). This is for merging match_array into tx_candidates
If flags are zero, there is nothing to store and remember. Stored signatures will be reused on a later packet, and qsorted (which may be expensive), with newer matches candidates. Avoiding to store, leads to avoid the call to qsort.
Especially sets transactions to complete when we get a response without having seen the request, so that the transactions end up getting cleaned (instead of living/leaking in the state). Also try to set the event on the relevant transaction, instead of creating a new transaction just for the purpose of having the event. Ticket: OISF#6299
catenacyber
force-pushed
the
detect-tcp-noupdatetotx-6299-v10
branch
from
54a42e0 to
47e9c1c
Compare
|
Information: QA ran without warnings. Pipeline 17514 |
| tx.tx_ptr, tx.tx_id, s->id, id); | ||
| } | ||
| } | ||
| do_sort = (array_idx > x); // sort if match added anything |
There was a problem hiding this comment.
Why do we need to sort these ?
Asking because below there is
do_sort |= (old && old != array_idx);
which means the sorting only takes place if there were already match candidates and new ones were added from stored.
But stored rules do not look ordered to me on every case :
If I have 4 rules
Rule 1 and 3 get stored on first packet (because partial match on first buffers, need to be confirmed, like http request line, waiting for http body)
Rule 2 and 4 get stored on first packet (because partial match on their first buffers, need to be confirmed, like http header, waiting for http body)
Then stored will have 1,3,2,4 on third packet with http body, and do_sort will be false because old==0
Thinking about this in the context of #10231
There was a problem hiding this comment.
Why do we need to sort these ?
How can I make a SV test fail if they are not sorted in the end ?
|
Merged in #10277, thanks! |
3 participants
Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/6299
Describe changes:
#10145 with
DetectRunStoreStateTxid = s->numand nots->id