Skip to content

Comments

rust/sip: register parser for tcp v14#10098

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

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

Conversation

@glongo
Copy link
Contributor

@glongo glongo commented Dec 27, 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:

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/1538
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 27, 2023

Codecov Report

Attention: Patch coverage is 90.83665% with 23 lines in your changes are missing coverage. Please review.

Project coverage is 82.11%. Comparing base (5cc872f) to head (4d6c05f).
Report is 183 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10098      +/-   ##
==========================================
- Coverage   82.19%   82.11%   -0.09%     
==========================================
  Files         975      975              
  Lines      271940   272109     +169     
==========================================
- Hits       223523   223429      -94     
- Misses      48417    48680     +263     
Flag Coverage Δ
fuzzcorpus 62.99% <67.23%> (+0.07%) ⬆️
suricata-verify 61.13% <91.06%> (-0.29%) ⬇️
unittests 62.83% <31.47%> (-0.01%) ⬇️

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

@glongo
Copy link
Contributor Author

glongo commented Feb 15, 2024

Is this PR ready to be merged?

@jufajardini
Copy link
Contributor

Is this PR ready to be merged?

The documentation additions look good to me.

Unfortunately, this PR needs to be rebased, now.

I saw that there was a thread about where the frames should be registered, considering there seemed to be some disagreement there, I'll add a label decision required here, to bring attention to that.

@victorjulien victorjulien added the needs rebase Needs rebase to main label Feb 27, 2024
@victorjulien
Copy link
Member

This implements general parser improvements that should probably be backported to 7, can you create a ticket for it?

@glongo
Copy link
Contributor Author

glongo commented Feb 27, 2024

Replaced by #10513

@glongo glongo closed this Feb 27, 2024
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

Development

Successfully merging this pull request may close these issues.

3 participants