dns: add dns.rcode keyword v7 #10249
Conversation
Feature OISF#6621 It matches the rcode field in DNS It's an unsigned integer match valid ranges = [0-23] Does not support prefilter Supports flow in both directions
| } else if let Some(response) = &tx.response { | ||
| response.header.flags | ||
| } else { | ||
| return 0; | ||
| }; |
There was a problem hiding this comment.
When I ran the tests mentioned in the description of the PR they all came back successful. This is the a major change I made from the last PR. Please let me know if this is correct or if it is causing the check failures?
There was a problem hiding this comment.
The test that is failing is requires https://github.com/OISF/suricata/actions/runs/7655557469/job/20861980818#step:19:85
As you pointed out in a chat, I think that the failures are due to the fact that your SV PR has tests that are outdated, since your Suri PR is current to Suri master.
My guess is that if you rebase and submit a new SV PR, ci checks will go green again :)
There was a problem hiding this comment.
To answer your question directly: overall, this looks correct to me :)
| 4u8, | ||
| )); | ||
|
|
||
| assert!(!detect_match_uint( | ||
| &DetectUintData { | ||
| mode: DetectUintMode::DetectUintModeNe, | ||
| arg1: 4, | ||
| arg2: 0, | ||
| }, | ||
| 4u8, |
There was a problem hiding this comment.
I also made the change to 4u8 which is different from the previous PR. I'm just highlighting the changes so it's easier to troubleshoot the reason for check failures.
| } else if let Some(response) = &tx.response { | ||
| response.header.flags | ||
| } else { | ||
| return 0; | ||
| }; |
There was a problem hiding this comment.
To answer your question directly: overall, this looks correct to me :)
| return 0; | ||
| }; | ||
|
|
||
| let rcode = (header_flags & 0xf) as u8; |
There was a problem hiding this comment.
As Philippe asked herehttps://github.com//pull/10233/files/a122126f2541f3e09c703de68444f39683e7848c#r1464948794 - could this be 23, in the end, as we know from the specifications that it can?
There was a problem hiding this comment.
I'm not really sure if I understand what you mean.
There was a problem hiding this comment.
Although this needed to be addressed, I introduced some noise with my question.
But we've discussed this in our weekly call, conclusion was that:
- commit message should be updated
- documentation should indicate that current version covers the RFC specification for rcodes up to 15, and share redmine ticket link as well as related RFC.
|
Followed by #10438 |
Feature #6621
Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/6621
Previous PR: #10233
Describe changes:
DetectUintData<u8>instead of simple u8doc/userguide/rules/dns-keywords.rstfile.SV_BRANCH=OISF/suricata-verify#1578