add test for extra tls alert #2080
Conversation
inashivb
added
the
requires suricata fix
| # Test Description | ||
|
|
||
| This test shows that Suricata generates an additional alert for TLS | ||
| for the given PCAP which shouldn't be there. |
There was a problem hiding this comment.
For 9901033 as per test.yaml: alert.signature_id: 9901033
There was a problem hiding this comment.
At which packet does it happen ? The pseudo flush ?
There was a problem hiding this comment.
At which packet does it happen ? The pseudo flush ?
The extra alert happens at pseudo flush. Yes. The test filter that indicates that i test.yaml is:
- filter:
count: 0
match:
event_type: alert
not-has-key: pcap_cntNote that the event generated w the pseudo pkt misses pcap_cnt key and also has pkt_src: stream (flow timeout) (so subtest 1 in the test also helps making sure of that)
There was a problem hiding this comment.
My thoughts are that it should not fire as-is. Its somewhat non-sensical, at least its being logged relative to a packet (start of flow) where it should not be. Its only because of information found in the future, and ends up in the pseudo-packet that we end up alerting. Or at least its confusing.
As we're only pushing this pseudo-packet through to wrap up transactions, my thought was to skip detection on non-transaction data, in particular packet rules per the discussion here: OISF/suricata#11862. But maybe this is expected behavior and there is no issue. I think we're still discussing the behavior and if this is correct or not.
There was a problem hiding this comment.
at least its being logged relative to a packet (start of flow)
How so with pkt_src: stream (flow timeout) ?
information found in the future
Do we time travel ?
As we're only pushing this pseudo-packet through to wrap up transactions,
Beware there are timeouts pseudo packets, but other one likes HTTP1->HTTP2 upgrade...
IIRC, there was one ticket where we needed the pseudo packet to match on the content of the last SSH packet before going encrypted cf https://redmine.openinfosecfoundation.org/issues/6578 (and one similar for frames)
There was a problem hiding this comment.
Do we time travel ?
Apparently we do :)
In the log we have an alert at 2024-09-09T12:15:56.542526-0600, followed by an alert from 2024-09-09T12:15:55.863332-0600.
That second timestamp matches the first packet in the pcap, however hits on the rule:
alert tcp any any -> $EXTERNAL_NET 443 (flow: to_server; flowbits:isset, tls_error; \
sid:09901033; rev:1; msg:"Allow TLS error handling (outgoing packet)"; )
and is logged with flowbits, even detected as tls, however we don't know any of that at 2024-09-09T12:15:55.863332-0600, unless we time travel to the future :)
Kidding aside, that's why this alert looks so out of place.
There was a problem hiding this comment.
How so with pkt_src: stream (flow timeout) ?
Flow timeout also seems odd, as it didn't timeout. Its cleanly closed.
There was a problem hiding this comment.
So, there is a bug in the timestamp of timeout packets, right ?
Flow timeout also seems odd, as it didn't timeout. It's cleanly closed.
I think we just time out cleanly closed flows after sometime checking there is no session reuse...
|
Ticket created: https://redmine.openinfosecfoundation.org/issues/7318 |
Ticket
If your pull request is related to a Suricata ticket, please provide
the full URL to the ticket here so this pull request can monitor
changes to the ticket status:
Redmine ticket: https://redmine.openinfosecfoundation.org/issues/7318