Skip to content

Comments

rust/sip: register parser for tcp v11#9967

Closed
glongo wants to merge 8 commits intoOISF:masterfrom
glongo:dev-sip-tcp-v11
Closed

rust/sip: register parser for tcp v11#9967
glongo wants to merge 8 commits intoOISF:masterfrom
glongo:dev-sip-tcp-v11

Conversation

@glongo
Copy link
Contributor

@glongo glongo commented Dec 5, 2023

Make sure these boxes are signed before submitting your Pull Request -- thank you.

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/3351

Describe changes:

  • Remove OPTIONS from pattern matching

Provide values to any of the below to override the defaults.

To use a pull request use a branch name like pr/N where N is the
pull request number.

Alternatively, SV_BRANCH may also be a link to an
OISF/suricata-verify pull-request.

SV_REPO=
SV_BRANCH=pr/1510
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

Accepts valid characters as defined in RFC3261.
The `is_version_char` function incorrectly allowed characters that are not
part of the valid SIP version "SIP/2.0".

For instance, 'HTTP/1.1' was mistakenly accepted as a valid SIP version,
although it's not.

This commit fixes the issue by updating the condition to strictly
check for the correct version string.
This patch lets the parser to work over tcp protocol, taking care of handling
data before calling the request/response parsers.

Ticket OISF#3351.
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 detect the SIP protocol using pattern matching instead of
probing parser.

Since it is no longer used, the respective probing functions have been removed.
@codecov
Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Merging #9967 (13468a1) into master (13cc493) will increase coverage by 0.00%.
Report is 23 commits behind head on master.
The diff coverage is 90.45%.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #9967    +/-   ##
========================================
  Coverage   82.39%   82.40%            
========================================
  Files         970      970            
  Lines      271359   271518   +159     
========================================
+ Hits       223586   223743   +157     
- Misses      47773    47775     +2     
Flag Coverage Δ
fuzzcorpus 64.44% <69.77%> (+<0.01%) ⬆️
suricata-verify 61.31% <90.66%> (+<0.01%) ⬆️
unittests 62.87% <32.78%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@catenacyber catenacyber added the needs rebase Needs rebase to main label Dec 6, 2023
@catenacyber
Copy link
Contributor

Let's wait for QA feedback

@glongo
Copy link
Contributor Author

glongo commented Dec 12, 2023

Any updates on this?

@catenacyber
Copy link
Contributor

@ct0br0 did QA miss this PR ?

@glongo at one point, this will still need a rebase to solve the conflict about doc/userguide/upgrade.rst

@ct0br0
Copy link

ct0br0 commented Dec 12, 2023

If a rebase is needed, the run is skipped. Guessing this is a continuation of another PR, so there may have been overlap in the qa-tag and when this was made too. LMK when it's rebased and I'll take a look at the pipeline.

@glongo
Copy link
Contributor Author

glongo commented Dec 12, 2023

New PR: #10037

@glongo glongo closed this Dec 12, 2023
@glongo glongo deleted the dev-sip-tcp-v11 branch February 28, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs rebase Needs rebase to main waiting for qa

Development

Successfully merging this pull request may close these issues.

4 participants