Skip to content

Batch backports 6.0.x/1.1: Backports of 5550, 5604#8985

Closed
jlucovsky wants to merge 6 commits into
OISF:master-6.0.xfrom
jlucovsky:batch-backports-6.0.x/1.2
Closed

Batch backports 6.0.x/1.1: Backports of 5550, 5604#8985
jlucovsky wants to merge 6 commits into
OISF:master-6.0.xfrom
jlucovsky:batch-backports-6.0.x/1.2

Conversation

@jlucovsky
Copy link
Copy Markdown
Contributor

@jlucovsky jlucovsky commented Jun 7, 2023

Continuation of #8975

Link to redmine ticket:

Describe changes:

  • Backport of commits for 5604, 5550.

Updates:

  • Abandon StreamSlice changes in dns.rs per review comment.

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.

SV_REPO=
SV_BRANCH=
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

victorjulien and others added 6 commits June 6, 2023 08:15
@victorjulien @jlucovsky
stream: update no-flow checks
9cac5a7 
(cherry picked from commit 0360cb6)
@victorjulien @jlucovsky
counters: make tcp stats independent of flow, ssn
cecea2e 
Counters depended on availability of flow and tcp session, meaning
that 2 memcaps could affect the counters.

Bug: OISF#5017.
(cherry picked from commit 36f6e05)
@jasonish @jlucovsky
dns: mark test buffers with rustfmt::skip
3ec1aaf 
(cherry picked from commit 39d2524)
@jasonish @jlucovsky
dns: parse and alert on invalid opcodes
3566ec0 
Accept DNS messages with an invalid opcode that are otherwise
valid. Such DNS message will create a parser event.

This is a change of behavior, previously an invalid opcode would cause
the DNS message to not be detected or parsed as DNS.

Issue: OISF#5444
(cherry picked from commit c98c49d)
@jasonish @jlucovsky
dns: validate header on every incoming message
3a682ec 
As UDP streams getting probed, a stream that does not appear to be DNS
at first, may have a single packet that does look close enough to DNS
to be picked up as DNS causing every subsequent packet to result in a
parser error.

To mitigate this, probe every incoming DNS message header for validity
before continuing onto the body.  If the header doesn't validate as
DNS, just ignore the packet so no parse error is registered.

(cherry picked from commit 595700a)
@jasonish @jlucovsky
dns: split header and body parsing
268e4e6 
As part of extra header validation, split out DNS body parsing to
avoid the overhead of parsing the header twice.

(cherry picked from commit d720ead)
@jlucovsky jlucovsky requested review from a team, jasonish and victorjulien as code owners June 7, 2023 13:39
@jlucovsky jlucovsky mentioned this pull request Jun 7, 2023
Closed
jasonish
jasonish reviewed Jun 7, 2023
View reviewed changes
Comment thread rust/src/applayer.rs
@jasonish
Copy link
Copy Markdown
Member

jasonish commented Jun 7, 2023

Otherwise DNS appears to be OK. The SV test dns/dns-invalid-opcode should get updated to be not so 7.0 specific I guess.. It currently also looks for fields that don't exist in 6.0.

@suricata-qa
Copy link
Copy Markdown

suricata-qa commented Jun 7, 2023

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.tcp.synack 6809615 6047299 88.81%
.tcp.rst 4872947 4600901 94.42%

Pipeline 14297

@ct0br0
Copy link
Copy Markdown

ct0br0 commented Jun 7, 2023

Have a QA rebase ready for when this merges

@victorjulien victorjulien mentioned this pull request Jun 8, 2023
Merged
@victorjulien
Copy link
Copy Markdown
Member

victorjulien commented Jun 9, 2023

Merged in #8995, thanks!

@jlucovsky jlucovsky deleted the batch-backports-6.0.x/1.2 branch June 9, 2023 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasonish jasonish jasonish left review comments

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

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jlucovsky @jasonish @suricata-qa @ct0br0 @victorjulien