Skip to content

Comments

[draft] dns: dns.response.answer.name sticky buffer - v1#9686

Closed
jasonish wants to merge 5 commits intoOISF:masterfrom
jasonish:dns-response/v1
Closed

[draft] dns: dns.response.answer.name sticky buffer - v1#9686
jasonish wants to merge 5 commits intoOISF:masterfrom
jasonish:dns-response/v1

Conversation

@jasonish
Copy link
Member

@jasonish jasonish commented Oct 24, 2023

This implements the sticky buffer dns.response.answer.name to match on names on DNS response answers.

The idea is to fill this out with more keywords like:

  • dns.response.answer.data.a
  • dns.response.answer.data.txt
  • dns.response.answer.data.sshfp.fingerprint
    and so on.

But before I continue on I want to make sure this is the good example of a stick buffer, with prefiltering and multi-buffer support, as it looks like our template is out of date. After this is given an OK, I'll apply the changes to the template.

Then I'd also like to isolate the common patterns of the sticky buffers and abstract the boilerplate away.

SV_BRANCH=OISF/suricata-verify#1444

The old DetectAppLayerMpmRegister has not been around since 4.1.x.
Rename the v2 of this function to a versionless function as there is no
documentation referring to what the 2 means.
Rename DetectAppLayerInspectEngine2 to DetectAppLayerInspectEngine as
there is no other variant of this function, and the versioning with
lack of supporting documentation can lead to confusion.
@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

Merging #9686 (06b877d) into master (2fe2d82) will decrease coverage by 0.04%.
The diff coverage is 85.99%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9686      +/-   ##
==========================================
- Coverage   82.39%   82.36%   -0.04%     
==========================================
  Files         968      969       +1     
  Lines      274337   274326      -11     
==========================================
- Hits       226047   225937     -110     
- Misses      48290    48389      +99     
Flag Coverage Δ
fuzzcorpus 64.62% <85.99%> (-0.07%) ⬇️
suricata-verify 60.85% <85.60%> (-0.07%) ⬇️
unittests 62.83% <85.60%> (-0.03%) ⬇️

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

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 16305

@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.memcap_pressure_max 88 97 110.23%

Pipeline 16306

@jasonish jasonish force-pushed the dns-response/v1 branch 2 times, most recently from 5eaf857 to 9335903 Compare October 25, 2023 05:53
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 16307

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 16309

@jasonish jasonish force-pushed the dns-response/v1 branch 3 times, most recently from 037f8fc to 783f80d Compare October 25, 2023 22:56
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 16314

@jasonish jasonish changed the title [draft] dns: dns response keywords - v1 [draft] dns: dns.response.answer.name sticky buffer - v1 Oct 26, 2023
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 16320

@victorjulien
Copy link
Member

Think it makes sense. Not finding the commit separation entirely logical, I would just squash them all together (the keyword commits, not talking about the API rename)

@jasonish
Copy link
Member Author

Think it makes sense. Not finding the commit separation entirely logical, I would just squash them all together (the keyword commits, not talking about the API rename)

Yeah, its more for review of the individual items that were applied over and above the current template for sticky buffers.

@catenacyber
Copy link
Contributor

Continued in #9813 right ?

@jasonish jasonish deleted the dns-response/v1 branch December 14, 2023 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants