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.

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

dns: add missing dns keywords to schema.json v6 #10460

Closed
wants to merge 1 commit into from

Conversation

hadiqaalamdar
Copy link
Contributor

Feature #5642

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

Previous PR: #10432

Describe changes:

  • made recommended changes from the last PR

SV_BRANCH=OISF/suricata-verify#1588

Found and added missing dns fields in schema.json after manual code review.
Added description to these newly added dns fields.
Feature OISF#5642
@@ -999,6 +999,10 @@
"description": "DNS opcode as an integer",
"type": "integer"
},
"tc": {
"description": "A 1-bit subfield for truncated response that specifies if the length of the message exceeds the allowed length",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like others' opinions on this but I think this looks very RFC definition and not very Suricata specific like what does this field in Suricata logs represent. Like DNS truncated response field as a boolean..
Note that this is not a change requested. Would like your and other opinions on this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think there is a difference for this field between RFC and Suricata...

If I want to be nitpicky, I would say 1-bit subfield is redundant

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is your question towards description style? If so, I understand what you say.
Have we ever discussed what we'd like these descriptions to look like? Any style guide or something?

Copy link
Member

@jasonish jasonish Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be pretty simple, perhaps along the lines of:
DNS truncation flag.

We shouldn't actually say that this means the DNS truncation message was truncated. Its easy to set this value to true or false whether the actual DNS message was truncated or not. So this should just describe what that header value was set to.

@hadiqaalamdar
Copy link
Contributor Author

New PR: #10476

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.

5 participants