rust/sip: register parser for tcp v5 #9321
Conversation
Accepts valid characters as defined in RFC3261.
This patch lets the parser to work over tcp protocol, taking care of handling data before calling the request/response parsers.
This patch permits to set a direction when a new transaction is created in order to avoid 'signature shadowing' as reported by Eric Leblond in commit 5aaf507
This permits to log sip messages over tcp.
|
editted PR description to use new SV PR picking style, as I wasn't sure the old style still works. Re-triggered build tests, too. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #9321 +/- ##
==========================================
- Coverage 82.41% 82.41% -0.01%
==========================================
Files 968 968
Lines 274055 274198 +143
==========================================
+ Hits 225874 225971 +97
- Misses 48181 48227 +46
Flags with carried forward coverage won't be shown. Click here to find out more. |
Yeah, that style doesn't work anymore. |
| if let Ok((_, resp_line)) = sip_take_line(input) { | ||
| tx.response_line = resp_line; | ||
| } |
There was a problem hiding this comment.
I know this isn't new, just a refactor. But should we be creating an empty transaction of sip_take_line fails?
There was a problem hiding this comment.
If sip_parse_request is succeeded, sip_take_line shouldn't fail because the whole request is parsed before.
| @@ -50,7 +50,7 @@ vars: | |||
| GENEVE_PORTS: 6081 | |||
| VXLAN_PORTS: 4789 | |||
| TEREDO_PORTS: 3544 | |||
|
|
|||
| SIP_PORTS: "[5060, 5061]" | |||
There was a problem hiding this comment.
Empty line is lost here, can you please add back.
| pub unsafe extern "C" fn rs_sip_probing_parser_tcp_ts( | ||
| _flow: *const Flow, | ||
| _direction: u8, | ||
| input: *const u8, | ||
| input_len: u32, | ||
| _rdir: *mut u8, | ||
| ) -> AppProto { |
There was a problem hiding this comment.
This isn't our formatting for new rust code. Would you be willing to rustfmt the whole SIP module, commit, then apply your changes letting rustfmt do its thing? I think the module is small enough to do a one-shot rustfmt on it, then apply your changes.
|
Replaced by #9387 |
Make sure these boxes are signed before submitting your Pull Request -- thank you.
Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/3351
Previous PR: #8897
Describe changes:
suricata-verify-pr: 1341
SV_BRANCH=OISF/suricata-verify#1341