rust/sip: register parser for tcp v7 #9483

glongo · 2023-09-13T14:15:41Z

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

I have read the contributing guide lines at https://docs.suricata.io/en/latest/devguide/codebase/contributing/contribution-process.html
I have signed the Open Information Security Foundation contribution agreement at https://suricata.io/about/contribution-agreement/
I have updated the user guide (in doc/userguide/) to reflect the changes made (if applicable)

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

Describe changes:

Rebase
Commits squashed
Added comment with reference to RFC3261

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/1386
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

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. 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

codecov · 2023-09-13T14:44:35Z

Codecov Report

Merging #9483 (d9c9fa3) into master (2b57179) will decrease coverage by 0.03%.
The diff coverage is 80.41%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9483      +/-   ##
==========================================
- Coverage   82.18%   82.16%   -0.03%     
==========================================
  Files         968      968              
  Lines      274199   274336     +137     
==========================================
+ Hits       225363   225406      +43     
- Misses      48836    48930      +94

Flag	Coverage Δ
fuzzcorpus	`64.05% <39.17%> (-0.05%)`	⬇️
suricata-verify	`60.88% <80.41%> (-0.03%)`	⬇️
unittests	`62.85% <7.73%> (-0.04%)`	⬇️

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

catenacyber · 2023-09-13T14:56:04Z

suricata.yaml.in

@@ -50,6 +50,7 @@ vars:
    GENEVE_PORTS: 6081
    VXLAN_PORTS: 4789
    TEREDO_PORTS: 3544
+    SIP_PORTS: "[5060, 5061]"


This is meant to be used in signatures, right ?

git grep SIP_PORTS does not yield anything else

yes, that's right.

catenacyber · 2023-09-13T14:58:49Z

rust/src/sip/sip.rs

+        if AppLayerParserStateIssetFlag(pstate, APP_LAYER_PARSER_EOF_TS) > 0 {
+            return AppLayerResult::ok();
+        } else {
+            return AppLayerResult::err();


How can we end up here ? stream_slice.is_empty() && AppLayerParserStateIssetFlag(pstate, APP_LAYER_PARSER_EOF_TS) == 0

I'm not really sure it's needed actually, but I thought that it would be good to check if eof is reached.

catenacyber · 2023-09-13T15:00:08Z

Maybe commit message rust/sip: register parser for sip can be rust/sip: register parser for tcp

catenacyber · 2023-09-13T15:00:57Z

Where should we document this change @jufajardini ? upgrade section ?

glongo · 2023-09-13T15:03:23Z

Maybe commit message rust/sip: register parser for sip can be rust/sip: register parser for tcp

It's meant to be tcp indeed, but I don't know why I wrote sip :)

rust/src/sip/sip.rs

jufajardini · 2023-11-08T11:06:42Z

Where should we document this change @jufajardini ? upgrade section ?

(Sorry for the belated answer, I obviously missed this.)

I think that:

upgrade section
suricata.yaml (for the ports config portion)
maybe something related to output, to indicate at least the sip_tcp and sip_udp bit?

catenacyber

Thanks Giuseppe. Still need some nit and a bit of doc

victorjulien · 2023-11-19T06:16:30Z

Since 8 development is now started, we can get this in when you're ready.

glongo · 2023-11-24T10:58:26Z

Replaced by #9880

glongo added 5 commits September 13, 2023 11:11

rust/sip: rustfmt sip module

7401589

sip/parser: accept valid chars

7653bf7

Accepts valid characters as defined in RFC3261.

rust/sip: register parser for sip

7a00157

This patch lets the parser to work over tcp protocol, taking care of handling data before calling the request/response parsers. Ticket OISF#3351.

rust/sip: add direction to transaction

395d7e9

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

suricata.yaml: define SIP_PORTS

d9c9fa3

glongo requested review from jasonish, victorjulien and a team as code owners September 13, 2023 14:15

glongo mentioned this pull request Sep 13, 2023

sip: add tests for sip over tcp v2 OISF/suricata-verify#1386

Closed

catenacyber mentioned this pull request Sep 13, 2023

rust/sip: register parser for tcp v6 #9387

Closed

3 tasks

catenacyber reviewed Sep 13, 2023

View reviewed changes

rust/src/sip/sip.rs Show resolved Hide resolved

catenacyber requested changes Nov 8, 2023

View reviewed changes

glongo closed this Nov 24, 2023

glongo deleted the dev-sip-tcp-v7 branch February 28, 2024 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rust/sip: register parser for tcp v7 #9483

rust/sip: register parser for tcp v7 #9483

glongo commented Sep 13, 2023

codecov bot commented Sep 13, 2023

catenacyber Sep 13, 2023

glongo Sep 13, 2023

catenacyber Sep 13, 2023

glongo Sep 13, 2023

catenacyber commented Sep 13, 2023 •

edited

Loading

catenacyber commented Sep 13, 2023

glongo commented Sep 13, 2023

jufajardini commented Nov 8, 2023

catenacyber left a comment

victorjulien commented Nov 19, 2023

glongo commented Nov 24, 2023

rust/sip: register parser for tcp v7 #9483

rust/sip: register parser for tcp v7 #9483

Conversation

glongo commented Sep 13, 2023

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

codecov bot commented Sep 13, 2023

Codecov Report

catenacyber Sep 13, 2023

Choose a reason for hiding this comment

glongo Sep 13, 2023

Choose a reason for hiding this comment

catenacyber Sep 13, 2023

Choose a reason for hiding this comment

glongo Sep 13, 2023

Choose a reason for hiding this comment

catenacyber commented Sep 13, 2023 • edited Loading

catenacyber commented Sep 13, 2023

glongo commented Sep 13, 2023

jufajardini commented Nov 8, 2023

catenacyber left a comment

Choose a reason for hiding this comment

victorjulien commented Nov 19, 2023

glongo commented Nov 24, 2023

catenacyber commented Sep 13, 2023 •

edited

Loading