detect/sip: add sticky buffers to match headers v4 #11330

glongo · 2024-06-20T04:52:17Z

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/contributing/contribution-process.html
I have signed the Open Information Security Foundation contribution agreement at
https://suricata.io/about/contribution-agreement/ (note: this is only required once)
I have updated the user guide (in doc/userguide/) to reflect the changes made (if applicable)
I have created a ticket at
https://redmine.openinfosecfoundation.org/projects/suricata/issues
(if applicable)

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

Describe changes:

Store multiple values for a single header
Header name in compact form is extended into long form

SV_BRANCH=OISF/suricata-verify#1926

A stub file has been added to implement the sticky buffers for SIP headers. Ticket OISF#6374

According to RFC 3261, a single header can be repeated one or more times, and its name can also be specified using the 'compact form.' This patch updates the hashmap used for storing headers to accommodate multiple values instead of just one. Additionally, if a header name is defined in the compact form, it is expanded into its long form (i.e., the standard name). This conversion simplifies the logic for matching a given header and ensures 1:1 parity with keywords. Ticket OISF#6374

To match on response SIP headers, those headers must be stored. Ticket OISF#6374

Ticket OISF#6374

codecov · 2024-06-20T05:13:57Z

Codecov Report

Attention: Patch coverage is 95.83333% with 6 lines in your changes missing coverage. Please review.

Project coverage is 82.41%. Comparing base (6256391) to head (5334c3f).
Report is 115 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11330      +/-   ##
==========================================
- Coverage   82.41%   82.41%   -0.01%     
==========================================
  Files         934      942       +8     
  Lines      247239   247376     +137     
==========================================
+ Hits       203773   203875     +102     
- Misses      43466    43501      +35

Flag	Coverage Δ
fuzzcorpus	`60.22% <57.04%> (-0.02%)`	⬇️
livemode	`18.77% <41.54%> (+0.02%)`	⬆️
pcap	`43.75% <57.04%> (-0.04%)`	⬇️
suricata-verify	`61.40% <95.77%> (+0.04%)`	⬆️
unittests	`59.31% <57.63%> (+<0.01%)`	⬆️

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

catenacyber · 2024-06-20T09:57:19Z

Thanks Giuseppe, quick remark : you can now have this in pure rust
cf #11316 for instance

glongo · 2024-06-20T12:00:10Z

Thanks Giuseppe, quick remark : you can now have this in pure rust cf #11316 for instance

I saw your commit about rust keywords merged but I have a question:
Can multi-buffer keywords already be written in Rust?

catenacyber · 2024-06-20T12:17:04Z

Thanks Giuseppe, quick remark : you can now have this in pure rust cf #11316 for instance

I saw your commit about rust keywords merged but I have a question: Can multi-buffer keywords already be written in Rust?

I am doing it in #11316, this requires 7266b69

catenacyber

Thanks for the work :-)

This PR needs a rebase

CI : 🟢
Code : checking now
Commits segmentation : I would squash some together
Commit messages : ok for me
Git ID set : looks fine for me
CLA : you already contributed
Doc update : looks fine
Redmine ticket : ok
Rustfmt : ok
Tests : SV ok
Dependencies added: none

catenacyber · 2024-07-13T19:53:26Z

src/detect-sip-via.c

+
+void RegisterSipHeadersVia(void)
+{
+    DetectSipHeadersRegisterStub();


You can do them in rust now

Am I wrong, or has your PR for multibuffer keywords in rust not been merged yet?

It has been merged today :-)

catenacyber · 2024-07-13T19:55:55Z

rust/src/sip/parser.rs

@@ -97,6 +97,22 @@ fn is_header_value(b: u8) -> bool {
    is_alphanumeric(b) || is_token_char(b) || b"\"#$&(),/;:<=>?@[]{}()^|~\\\t\n\r ".contains(&b)
 }

+fn expand_header_name(h: &str) -> &str {
+    match h {
+        "i" => "Call-ID",


Can we detect if some header uses the short or long form ?

Why should we detect that?

To get the most expressivity.
I do no say we should do that, I just asked if we can.

I guess this deserves some mention in the docs., like

This keyword matches on the From field that can be present in SIP headers. It matches both short and regular form, and these forms cannot be distinguished by this keyword.

I'm not sure I understand what you mean. If a header uses the short form, it is 'expanded' ( converted to the long form) to enable matching in both cases with its related sticky buffer.
If this is what you are referring to, it has already been implemented.
If not, I'm unclear on what you mean by 'detecting which form is used by a header.'

I meant 2 things :

If a header uses the short form, it is 'expanded' ( converted to the long form) to enable matching in both cases with its related sticky buffer.

This should be in the docs

And the doc should also mention that there is only the normalized form, not the raw one (as we can have for http uri for instance where there are 2 keywords)

I meant 2 things :

If a header uses the short form, it is 'expanded' ( converted to the long form) to enable matching in both cases with its related sticky buffer.

1. This should be in the docs 2. And the doc should also mention that there is only the normalized form, not the raw one (as we can have for http uri for instance where there are 2 keywords)

+1

jufajardini

I only have a nit comment for the copyright year change in sip/detect.rs, but do agree with Philippe's comment that we should mention in the docs about the headers being normalized :)

jufajardini · 2024-07-18T19:40:59Z

rust/src/sip/detect.rs

@@ -1,4 +1,4 @@
-/* Copyright (C) 2019 Open Information Security Foundation
+/* Copyright (C) 2024 Open Information Security Foundation


nit: when updating copyright years, we add the current year beside the initial one:

Suggested change

/* Copyright (C) 2024 Open Information Security Foundation

/* Copyright (C) 2019-2024 Open Information Security Foundation

glongo · 2024-08-10T14:53:08Z

Replaced with #11620

glongo added 12 commits June 19, 2024 21:21

detect/sip: add stub file for headers keywords

f73c43d

A stub file has been added to implement the sticky buffers for SIP headers. Ticket OISF#6374

rust/sip: store response headers

8a0fa35

To match on response SIP headers, those headers must be stored. Ticket OISF#6374

rust/sip: match on headers map

65236a2

Ticket OISF#6374

detect/sip: add sip.from sticky buffer

b56eec5

Ticket OISF#6374

detect/sip: add sip.to sticky buffer

00ef079

Ticket OISF#6374

detect/sip: add sip.via sticky buffer

a16ec14

Ticket OISF#6374

detect/sip: add sip.user_agent sticky buffer

4d96d0c

Ticket OISF#6374

detect/sip: add sip.content_type sticky buffer

025dcaa

Ticket OISF#6374

detect/sip: add sip.content_length sticky buffer

a7923d9

Ticket OISF#6374

detect/sip: register headers sticky buffers

9368204

Ticket OISF#6374

doc: add new sip keywords

5334c3f

glongo requested review from jasonish, jufajardini and victorjulien as code owners June 20, 2024 04:52

glongo mentioned this pull request Jun 20, 2024

detect/sip: add sticky buffers to match headers v3 #11004

Closed

3 tasks

catenacyber added the needs rebase Needs rebase to master label Jul 13, 2024

catenacyber requested changes Jul 13, 2024

View reviewed changes

catenacyber reviewed Jul 13, 2024

View reviewed changes

jufajardini requested changes Jul 18, 2024

View reviewed changes

glongo closed this Aug 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

detect/sip: add sticky buffers to match headers v4 #11330

detect/sip: add sticky buffers to match headers v4 #11330

glongo commented Jun 20, 2024

codecov bot commented Jun 20, 2024 •

edited

Loading

catenacyber commented Jun 20, 2024

glongo commented Jun 20, 2024

catenacyber commented Jun 20, 2024

catenacyber left a comment

catenacyber Jul 13, 2024

glongo Jul 15, 2024

catenacyber Jul 16, 2024

catenacyber Jul 13, 2024

glongo Jul 14, 2024

catenacyber Jul 15, 2024

glongo Jul 15, 2024

catenacyber Jul 15, 2024

jufajardini Jul 31, 2024

jufajardini left a comment

jufajardini Jul 18, 2024

glongo commented Aug 10, 2024

		@@ -1,4 +1,4 @@
		/* Copyright (C) 2019 Open Information Security Foundation
		/* Copyright (C) 2024 Open Information Security Foundation

	/* Copyright (C) 2024 Open Information Security Foundation
	/* Copyright (C) 2019-2024 Open Information Security Foundation

detect/sip: add sticky buffers to match headers v4 #11330

detect/sip: add sticky buffers to match headers v4 #11330

Conversation

glongo commented Jun 20, 2024

codecov bot commented Jun 20, 2024 • edited Loading

Codecov Report

catenacyber commented Jun 20, 2024

glongo commented Jun 20, 2024

catenacyber commented Jun 20, 2024

catenacyber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jufajardini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glongo commented Aug 10, 2024

codecov bot commented Jun 20, 2024 •

edited

Loading