Skip to content

Comments

Dns over http2 5773 v10#11196

Closed
catenacyber wants to merge 10 commits intoOISF:masterfrom
catenacyber:dns-over-http2-5773-v10
Closed

Dns over http2 5773 v10#11196
catenacyber wants to merge 10 commits intoOISF:masterfrom
catenacyber:dns-over-http2-5773-v10

Conversation

@catenacyber
Copy link
Contributor

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

Describe changes:

  • analyze DNS over HTTP2

SV_BRANCH=OISF/suricata-verify#1734

Draft to get feedback about approach...

#10984 with needed rebase

@jasonish would you like to already take in the first 2 commits (json schema and remove unneeded mut) ?

TODO :

Functionnaly, in terms of output :

  • The flow will have doh2 as app_proto (and http2 as app_proto_orig)
  • There are doh2 events that have both http2 and dns fields. dns logging is done like alerts, not like dns events...
  • DNS and HTTP and HTTP2 signatures work on DOH2
  • Signatures can be DOH2 specific. These signatures can combine http, http2 and dns keywords.
  • DNS Frames do not work on DoH2

Memory management

  • a HTTP2 tx can own 2 DNS tx (one to client, one to server)
  • a HTTP2 tx stores the streamed DNS response content until it is complete before parsing it (limiting to U16_MAX bytes)

API

  • There is a new DOH2 app-layer protocol which resorts to HTTP2 for most of the things
  • HTTP2 parsing discovering DOH2 changes the app_proto to DOH2, doh2 can be enabled or disabled in suricata.yaml config
  • There are more alproto "comparison" functions
  • Every DNS keyword needs to check the flow alproto to change the transaction
  • Every DNS/HTTP keyword is automatically registered for DOH2 (hacking DetectAppLayerMpmRegister2...

@suricata-qa
Copy link

ERROR:

ERROR: QA failed on ASAN_TLPR1_cfg.

Pipeline 20889

by making tx parsing and creation more easily available,
without needing a dns state.

Dns event NotResponse is now set on the right tx, and not the one
before.

Also debug log for Z-flag on request says "request" instead of
"response"

Also rustfmt dns.rs
Now a flow alproto can be changed by a call to AppLayerParserParse
when HTTP2 forces the flow to turn into DOH2.
Ticket: 5773

Handles both directions the same way for data if content type is
application/dns-message
@catenacyber catenacyber force-pushed the dns-over-http2-5773-v10 branch from 0cbd128 to decd292 Compare June 1, 2024 12:08
@codecov
Copy link

codecov bot commented Jun 1, 2024

Codecov Report

Attention: Patch coverage is 91.37931% with 30 lines in your changes are missing coverage. Please review.

Project coverage is 82.97%. Comparing base (10a367b) to head (decd292).

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #11196    +/-   ##
========================================
  Coverage   82.96%   82.97%            
========================================
  Files         942      942            
  Lines      250569   250781   +212     
========================================
+ Hits       207876   208074   +198     
- Misses      42693    42707    +14     
Flag Coverage Δ
fuzzcorpus 61.29% <69.82%> (-0.01%) ⬇️
livemode 18.69% <6.32%> (-0.02%) ⬇️
pcap 44.59% <76.14%> (+0.09%) ⬆️
suricata-verify 61.64% <83.04%> (+0.02%) ⬆️
unittests 60.61% <33.90%> (-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 20897

@jasonish
Copy link
Member

jasonish commented Jun 3, 2024

@jasonish would you like to already take in the first 2 commits (json schema and remove unneeded mut) ?

Yes.

@catenacyber
Copy link
Contributor Author

jasonish would you like to already take in the first 2 commits (json schema and remove unneeded mut) ?

Yes.

#11226

@catenacyber
Copy link
Contributor Author

Rebased in #11242

@catenacyber catenacyber closed this Jun 5, 2024
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.

3 participants