test: add ftp too many tx comment test#3037
Conversation
alinse-pltzr
force-pushed
the
ftp-max-tx-comment-and-rule
branch
2 times, most recently
from
84eae96 to
3d606d8
Compare
|
I do not understand the purpose of this test |
The test should contain a pcap that creates many txs (you can add a suricata.yaml to lower the max-tx limit) and the test should also use the rule put in the suricata PR to check that it triggers |
alinse-pltzr
force-pushed
the
ftp-max-tx-comment-and-rule
branch
from
3d606d8 to
780a264
Compare
ok, thanks for the clarity :) I adjusted the test accordingly. |
alinse-pltzr
force-pushed
the
ftp-max-tx-comment-and-rule
branch
from
780a264 to
24d9191
Compare
| @@ -0,0 +1,9 @@ | |||
| args: | |||
There was a problem hiding this comment.
We must add a:
requires:
min-version: 9
To ensure that the CI checks pass for versions less than 9, so we could have the test merged.
alinse-pltzr
force-pushed
the
ftp-max-tx-comment-and-rule
branch
from
24d9191 to
90d96bb
Compare
|
|
||
| app-layer: | ||
| ftp: | ||
| max-tx: 2 |
There was a problem hiding this comment.
Why should this pcap create more than 3 txs ?
There was a problem hiding this comment.
I ve been asuming that a pair of a request and a response are one transaction, because of this part of the documentation: https://docs.suricata.io/en/latest/devguide/extending/app-layer/transactions.html#general-concepts
counting the pairs of requests and responses in my pcap, there are more than 3 tx in my understanding.
what did I get wrong, and whats the correct defintion of a tx?
There was a problem hiding this comment.
max-tx is about the maximum live transactions.
Once we see the response (and have run detection and logging on it), the transaction gets freed and does not longer count as a live translation
There was a problem hiding this comment.
ok thanks, this makes sense, i ll generate a pcap with open live transactions
|
Is this PR superseded by #3041 ? cc @jlucovsky |
alinse-pltzr
force-pushed
the
ftp-max-tx-comment-and-rule
branch
from
90d96bb to
d2ccc02
Compare
Yes |
|
So closing in favor of #3041 |
4 participants
changes
test for: OISF/suricata#15213
test if suricata.yaml loads properly
Im not quite sure what to test exactly, since the addded rule uses an event, which doesn't exist yet (see: https://redmine.openinfosecfoundation.org/issues/8489)