Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SDP: implements sticky buffers v3 #12327

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

SDP: implements sticky buffers v3 #12327

wants to merge 21 commits into from

Conversation

glongo
Copy link
Contributor

@glongo glongo commented Dec 30, 2024 •
edited
Loading

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

Contribution style:

Our Contribution agreements:

Changes (if applicable):

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

Describe changes:

  • Rebase
  • Sticky buffers for SDP have been implemented.
    • The empty check within sticky buffer for optional fields has been removed
  • sdp.media.bandwidth and sdp.media.attribute sticky buffers have not been implemented yet because the media field is a vector of MediaDescription, and both the bandwidth and attribute fields are vectors as well.
    I believe the multi-buffer API cannot handle such a situation.
    If I'm not mistaken, a simple solution could be to "stringify" these fields like this:
"media_description": {
    "bandwidth": "AS:64,AS:65,AS:66",
    "attribute": "sendonly,recvonly"
}
  • Media's encryption key is logged now.
  • Multiple time and repeat_time fields are now parsed correctly

Personal considerations:

  • The sdp.media.* sticky buffers were initially meant to be named sdp.media_descriptions.*, but I realized the names were too long. I wonder if I should rename the media_description field to media for consistency.
  • I don't like the name sdp.media.media; I would prefer to rename it to sdp.media.name, or just sdp.media, and adjust the logged field name if necessary.
  • CI will probably fail due to the changes made to the way the time and repeat_time fields are logged.

SV_BRANCH=OISF/suricata-verify#2210

glongo added 21 commits December 30, 2024 16:30
@glongo
sdp: log media's encryption key
aa0cea8 
The encryption key subfield of the media description field is not
logged when it should be.

Ticket OISF#7305
@glongo
sdp: stringify structured fields
0d75a96 
The current parser implementations take a field, such as connection data, and
split it into subfields for a specific structure (e.g., struct ConnectionData).
However, following this approach requires several sticky buffers to match the
whole field, which can make a rule a bit verbose and doesn't offer any advantage
for matching specific parts of a field.

With this patch, a single line is still split into pieces if it makes sense for
parsing purposes, but these pieces are then reassembled into a single string.
This way, only one sticky buffer is needed to match the entire field.

Ticket OISF#7291
@glongo
sdp: parse time and repeat_time multiple times
f4ca1ee 
As defined in RFC4566, the time and repeat_time fields can be present
multiple times but they are currently parsed only once.

Ticket OISF#7325
@glongo
sdp: add sdp.session_name sticky buffer
e906c37 
This adds a sticky buffer to match the "Session name" field in both
requests and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.session_info sticky buffer
b3c719b 
This adds a sticky buffer to match the "Session information" field in
both requests and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.origin sticky buffer
f5e6238 
This adds a sticky buffer to match the "Origin" field in both requests
and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.uri sticky buffer
f8507c1 
This adds a sticky buffer to match the "Uri" field in both requests and
responses.

Ticket OISF#7291
@glongo
sdp: add sdp.email sticky buffer
3020a5f 
This adds a sticky buffer to match the "Email" field in both requests
and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.phone_number sticky buffer
fe6e664 
This adds a sticky buffer to match the "Phone number" field in both
requests and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.connection_data sticky buffer
60f5d14 
This adds a sticky buffer to match the "Connection data" field in both
requests and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.bandwidth sticky buffer
9a53b06 
This adds a sticky (multi) buffer to match the "Bandwidth" field in both
requests and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.time sticky buffer
3103c72 
This adds a sticky buffer to match the "Time" field in both requests and
responses.

Ticket OISF#7291
@glongo
sdp: add sdp.repeat_time sticky buffer
a8791ee 
This adds a sticky buffer to match the "Repeat time" field in both
requests and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.timezone sticky buffer
6787f67 
This adds a sticky bufffer to match the "Timezone" field in both
requests and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.encryption_key sticky buffer
eb6d9fc 
This adds a sticky buffer to match the "Encryption key" field in both
requests and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.attribute sticky buffer
4d46351 
This adds a sticky (multi) buffer to match the "Attribute" field in both
requests and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.media.media sticky buffer
17ed01b 
This adds a sticky (multi) buffer to match the "Media" subfield of the
"Media description" field in both requests and responses.

Ticket OISF#7291
@glongo
sdp: add sdp.media.media_info sticky buffer
bb595d3 
This adds a stick (multi) buffer to match the "Session information"
subfield of the "Media description" field in both requests and
responses.

Ticket OISF#7291
@glongo
sdp: add sdp.media.connection_data sticky buffer
457b5ab 
This adds a sticky (multi) buffer to match the "Connection data"
subfield of the "Media description" field in both requests and
responses.

Ticket OISF#7291
@glongo
sdp: add sdp.media.encryption_key sticky buffer
985ec98 
This adds a sticky (multi) buffer to match the "Encryption key" subfield
of the "Media description" field in both requests and responses.

Ticket OISF#7291
@glongo
doc: add sdp sticky buffers
0be952b
@glongo glongo mentioned this pull request Dec 30, 2024
Open
@codecov Codecov
Copy link

codecov bot commented Dec 30, 2024

Codecov Report

Attention: Patch coverage is 92.09756% with 81 lines in your changes missing coverage. Please review.

Project coverage is 83.23%. Comparing base (6f937c7) to head (0be952b).

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #12327      +/-   ##
==========================================
- Coverage   83.26%   83.23%   -0.03%     
==========================================
  Files         912      913       +1     
  Lines      257643   258551     +908     
==========================================
+ Hits       214521   215215     +694     
- Misses      43122    43336     +214
Flag Coverage Δ
fuzzcorpus 60.95% <35.95%> (-0.20%) ⬇️
livemode 19.48% <31.43%> (+0.08%) ⬆️
pcap 44.38% <36.14%> (-0.05%) ⬇️
suricata-verify 63.05% <91.74%> (+0.19%) ⬆️
unittests 59.09% <34.73%> (-0.11%) ⬇️

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonish jasonish Awaiting requested review from jasonish jasonish is a code owner

@jufajardini jufajardini Awaiting requested review from jufajardini jufajardini is a code owner

@victorjulien victorjulien Awaiting requested review from victorjulien victorjulien is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@glongo