ftp: fixups for firewall mode - v1 - #15656
Conversation
In FTP, the first transaction is a banner from the server. Skip TS inspection on this transaction. Allows firewall mode to get past this transaction and onto the next. Ticket: OISF#8662
The ftpdata_command would never match a RETR in the to_server direction, or a STOR in the to_client direction, this effectively makes the ftpdata_command applicable to passive mode only. Instead, remove the directionality on the ftpdata_command. Its "meta" information passed from the control channel and isn't really directional. I think the idea was to abstract away active/passive from the rule writer, but unfortunately doesn't allow the ftpdata_command keyword to work in the more strict firewll mode. Ticket: OISF#8663
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #15656 +/- ##
==========================================
- Coverage 82.90% 82.90% -0.01%
==========================================
Files 1006 1006
Lines 273648 273644 -4
==========================================
- Hits 226869 226865 -4
Misses 46779 46779
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Information: QA ran without warnings. Pipeline = 32061 |
|
Maybe I'm missing something, but what is (potentially) controversial about the ftpdata_command direction change? |
It was probably added for a purpose and now I'm removing it. Is there a reason it was designed this way? |
I don't see an explanation for it git, so I think it's just not tested / developed against active mode. Do you remember @regit ? |
|
I don't really remember the reason behind that. I tried to remember something that could be a risk linked to that but nothing rings a bell. |
|
Merged in #15663, thanks! |
4 participants
Ticket: https://redmine.openinfosecfoundation.org/issues/8663
Ticket: https://redmine.openinfosecfoundation.org/issues/8662
SV_BRANCH=OISF/suricata-verify#3167
First round of fixups for FTP in firewall mode.
First, allow the the first, to-client only transaction to pass so we can move
onto other transactions.
Second, remove the directionality of the ftpdata_command keyword. In firewall
mode, the request hooks never evaluation in passive mode, and the response
hooks never evaluate in active mode. And this data is information on the state
anyways, not really directional so I made the choice to just remove direction.
To be discussed here if not ideal.