Dns rust keywords 7529 v3 #12535
Dns rust keywords 7529 v3 #12535
Conversation
Ticket: 7529 Ticket: 3725 Adds url for dns.opcode on the way Move dns.query unit tests to SV on the way
Ticket: 6723
Ticket: 6723
catenacyber
requested review from
jasonish,
jufajardini and
victorjulien
as code owners
So that its contents can be reused when translating unit tests to SV tests
catenacyber
force-pushed
the
dns-rust-keywords-7529-v3
branch
from
b350c6d to
4011693
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12535 +/- ##
==========================================
+ Coverage 80.68% 80.70% +0.01%
==========================================
Files 925 919 -6
Lines 258914 258458 -456
==========================================
- Hits 208914 208581 -333
+ Misses 50000 49877 -123
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: QA ran without warnings. Pipeline 24655 |
| let kw = SCSigTableElmt { | ||
| name: b"dns.query.name\0".as_ptr() as *const libc::c_char, | ||
| desc: b"DNS query name sticky buffer\0".as_ptr() as *const libc::c_char, | ||
| url: b"/rules/dns-keywords.html#dns-query-name\0".as_ptr() as *const libc::c_char, | ||
| Setup: dns_detect_query_name_setup, | ||
| flags: SIGMATCH_NOOPT | SIGMATCH_INFO_STICKY_BUFFER, | ||
| AppLayerTxMatch: None, | ||
| Free: None, | ||
| }; | ||
| let _g_dns_query_name_kw_id = DetectHelperKeywordRegister(&kw); |
There was a problem hiding this comment.
Have you looked at making registering keywords more of a Rust experience? Rather than this C like code from Rust? By that I mean registration functions where you can use Rust strings, and hide the as_ptr() and stuff away.
There was a problem hiding this comment.
Good idea, will do
| G_DNS_QUERY_NAME_BUFFER_ID = DetectHelperMultiBufferMpmRegister( | ||
| b"dns.query.name\0".as_ptr() as *const libc::c_char, | ||
| b"dns query name\0".as_ptr() as *const libc::c_char, | ||
| ALPROTO_DNS, | ||
| true, | ||
| /* Register in both directions as the query is usually echoed back | ||
| in the response. */ | ||
| true, | ||
| dns_query_name_get_data_wrapper, | ||
| ); |
There was a problem hiding this comment.
This results in:
DetectAppLayerMultiRegister(name, alproto, SIG_FLAG_TOSERVER, 0, GetData, 2, 0);
However, on the C side:
DetectAppLayerMultiRegister(keyword, ALPROTO_DNS, SIG_FLAG_TOSERVER, 0, GetBuffer, 2, 1);
I haven't looked into it, but is min-progress not needed? Is 1 equivalent to 0? Is 2 equivalent to 1?
Its not a 1:1 conversion.
There was a problem hiding this comment.
but is min-progress not needed?
Not really for DNS : you get your unidirectional transaction as soon as you parse it.
So every DNS transaction always has progress 1, so it is like the whole concept of progress is irrelevant for DNS transactions.
But this may be indeed a pitfall in other cases...
There was a problem hiding this comment.
Is 1 equivalent to 0?
yes here
2 is some default priority...
Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/7529
https://redmine.openinfosecfoundation.org/issues/3725
Describe changes:
UTHBuildPacketRealto produce more real packets#11932 next protocol
#12496 with SV test replacing the unit tests
SV_BRANCH=OISF/suricata-verify#2275