-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
protocol-change: sets event in case of failure #7296
Conversation
Protocol change can fail if one protocol change is already occuring.
Codecov Report
@@ Coverage Diff @@
## master #7296 +/- ##
==========================================
- Coverage 75.82% 75.77% -0.05%
==========================================
Files 656 656
Lines 190051 190057 +6
==========================================
- Hits 144102 144020 -82
- Misses 45949 46037 +88
Flags with carried forward coverage won't be shown. Click here to find out more. |
Victor, you requested this event, so assigning this draft to you ;-) |
Information: ERROR: QA failed on tlpw1_files_sha256.
Pipeline 7251 |
@@ -87,4 +87,6 @@ alert http any any -> any any (msg:"SURICATA HTTP invalid Range header value"; f | |||
|
|||
alert http any any -> any any (msg:"SURICATA HTTP file name too long"; flow:established; app-layer-event:http.file_name_too_long; flowint:http.anomaly.count,+,1; classtype:protocol-command-decode; sid:2221052; rev:1;) | |||
|
|||
# next sid 2221053 | |||
alert http any any -> any any (msg:"SURICATA HTTP failed protocol change"; flow:established; app-layer-event:http.failed_protocol_change; flowint:http.anomaly.count,+,1; classtype:protocol-command-decode; sid:2221053; rev:1;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any case where this isn't about a HTTPS upgrade?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP1->HTTP2
@@ -30,4 +30,5 @@ alert smtp any any -> any any (msg:"SURICATA SMTP Mime boundary length exceeded" | |||
alert smtp any any -> any any (msg:"SURICATA SMTP duplicate fields"; flow:established,to_server; app-layer-event:smtp.duplicate_fields; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220018; rev:1;) | |||
alert smtp any any -> any any (msg:"SURICATA SMTP unparsable content"; flow:established,to_server; app-layer-event:smtp.unparsable_content; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220019; rev:1;) | |||
alert smtp any any -> any any (msg:"SURICATA SMTP filename truncated"; flow:established,to_server; app-layer-event:smtp.mime_long_filename; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220020; rev:1;) | |||
# next sid 2220021 | |||
alert smtp any any -> any any (msg:"SURICATA SMTP failed protocol change"; flow:established; app-layer-event:smtp.failed_protocol_change; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220021; rev:1;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is always about STARTTLS, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess so
Are there pcaps to test this scenario? If not can you craft them? |
Replaced by #7745 with S-V test linked ;-) |
So as to avoid undefined behavior with a 0-sized variable length array Ticket: OISF#7296
So as to avoid undefined behavior with a 0-sized variable length array Ticket: OISF#7296
So as to avoid undefined behavior with a 0-sized variable length array Ticket: OISF#7296
Link to redmine ticket:
None
Describe changes:
Another way to do this would be to set a packet's app-layer event, but
AppLayerRequestProtocolChange
does not have access to the currentPacket
and its callerFTPParseResponse
does not have it neither...So, this PR implements different events for the different app-layer protocols.
FTP and PGSQL do not have events yet, and are not part of this...