Bug #8165
openFTP data is not blocked with drop rule and filemd5 match
Description
Running suricata in IPS mode with a ftp-data drop rule to match on MD5. The event is alerted and with a drop rule, logging claims it is dropped but the FTP data packet gets through and the ftp client gets the file. The next TCP acks are blocked.
The EICAR file is a small file so it is only 1 ftp data packet. I don't know if more packets to trigger the md5 match would affect the behaviour.
See suricata-verify/tests/smtp-attachment-md5/ for same md5 and similar rule used but with ftp-data or ip protocol instead. I haven't tried other hash methods.
Tested on 8.0.1, 7.0.8, 7.0.6, 7.0.3. Also 4.0.6 but that was a heavily modified version.
Attached packet captures are of the client side.
Files
TW Updated by Thomas Winter 8 months ago
Forgot to say, the same file over http and smtp is properly blocked where the data packet is dropped.
TW Updated by Thomas Winter 8 months ago
A potential problem is FTPDataStateGetTxFiles and FTPDataGetAlstateProgress do
if (direction == ftpdata_state->direction)
I believe this should be
if (direction & ftpdata_state->direction)
as DetectFileInspectGeneric gives direction as (STREAM_TOSERVER|STREAM_TOCLIENT).
Making this change hasn't helped the test case.
PA Updated by Philippe Antoine 4 days ago
- Status changed from New to Feedback
- Label Needs Suricata-Verify test added
Could you provide a reproducer as a suricata-verify test ?
PA Updated by Philippe Antoine 4 days ago
- Status changed from Feedback to Triaged
- Assignee set to OISF Dev
but the FTP data packet gets through and the ftp client gets the file
Actually, we would need the live setup from @jasonish to test this
TW Updated by Thomas Winter 4 days ago
I have pushed a suricata-verify here https://github.com/thomasjwinter/suricata-verify/commit/63e66f10469fe52d8ddb0018e349ad4fca785fce
The problem appears to be that the eof is not triggered on the ftp data packet but on the tcp fin/ack. The eof is what signals file close for the md5 check. Potentially the size could be tracked to determine eof earlier.
PA Updated by Philippe Antoine 3 days ago
Thomas Winter wrote in #note-5:
I have pushed a suricata-verify here https://github.com/thomasjwinter/suricata-verify/commit/63e66f10469fe52d8ddb0018e349ad4fca785fce
Cool, can you do a PR with it ? and label it "requires fix"