Skip to content

Change suricata.dns schema to match current DNS structure #1919

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
Merged
merged 4 commits into from
Oct 21, 2021
Merged

Change suricata.dns schema to match current DNS structure #1919

merged 4 commits into from
Oct 21, 2021

Conversation

satta
Copy link
Contributor

@satta satta commented Oct 21, 2021 •
edited
Loading

📔 Description

This PR updates the suricata.dns schema to match the currently used EVE-JSON structure output by recent Suricata versions.

📝 Checklist

  • All user-facing changes have changelog entries.

🎯 Review Instructions

Can be tested by importing EVE-JSON with some more elaborate DNS events, and checking whether they are reproduced correctly in the exported JSON.

@dominiklohmann
Copy link
Member

Thanks! I'll take it from here and push a commit that fixes the crash on export and adds an additional integration test for verification.

@dominiklohmann dominiklohmann added bug Incorrect behavior enhancement ✨ labels Oct 21, 2021
@dominiklohmann dominiklohmann self-assigned this Oct 21, 2021
@satta
Copy link
Contributor Author

satta commented Oct 21, 2021

Thanks!

@mavam
Copy link
Member

mavam commented Oct 21, 2021

The schema is dependent on how you configure the DNS output in the suricata.yaml file, right?

(Once we have the sum/union types, we can support this.)

@satta
Copy link
Contributor Author

satta commented Oct 21, 2021 •
edited
Loading

The schema as proposed should cover the maximum of what output is configurable in Suricata (I read the new DNS Rust logging code). That is, detailed answers and grouped activated, and all supported record types logged.

I do assume though that no one is using DNSv1 logging any more. That has not been the default since Suricata 5.0 (2019).

@dominiklohmann
Add an integration test for suricata.dns.answers
90faef9 
The data contains a record inside a list, which is not well supported
and was not covered by any integration tests yet. This ensures we can
at least import and export it.
@dominiklohmann dominiklohmann changed the title change schema to match current DNS structure Change suricata.dns schema to match current DNS structure Oct 21, 2021
@dominiklohmann
Copy link
Member

I've pushed an integration test. The crash mentioned in #1916 I cannot reproduce on this branch, and #1888 fixes all issues with alias types categorically by making them transparent for developers. So I think once CI is happy here we can go ahead and merge.

@dominiklohmann dominiklohmann linked an issue Oct 21, 2021 that may be closed by this pull request
Suricata DNS schema scope is too limited #1916
Closed
@dominiklohmann
Update integration test baseline
f266f32 
The new fields in the suricata.dns type mean that other integration test
baselines had to be updated as well.
@dominiklohmann dominiklohmann merged commit 46cfb4a into tenzir:master Oct 21, 2021
@satta satta deleted the suricata-schema branch October 25, 2021 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dominiklohmann dominiklohmann dominiklohmann approved these changes

Assignees

@dominiklohmann dominiklohmann

Labels
bug Incorrect behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Suricata DNS schema scope is too limited
3 participants
@satta @dominiklohmann @mavam