dns: new keywords: dns.answer.name, dns.query.name - v5

doc/userguide/devguide/extending/app-layer/transactions.rst

@@ -68,7 +68,7 @@ Rule Matching
 
 Transaction progress is also used for certain keywords to know what is the minimum state before we can expect a match: until that, Suricata won't even try to look for the patterns.
 
-As seen in ``DetectAppLayerMpmRegister2`` that has ``int progress`` as parameter, and ``DetectAppLayerInspectEngineRegister2``, which expects ``int tx_min_progress``, for instance. In the code snippet,
+As seen in ``DetectAppLayerMpmRegister`` that has ``int progress`` as parameter, and ``DetectAppLayerInspectEngineRegister``, which expects ``int tx_min_progress``, for instance. In the code snippet,
 ``HTTP2StateDataClient``, ``HTTP2StateDataServer`` and ``0`` are the values passed to the functions - in the last
 example, for ``FTPDATA``,
 the existence of a transaction implies that a file is being transferred. Hence the ``0`` value.
@@ -80,18 +80,18 @@ the existence of a transaction implies that a file is being transferred. Hence t
     {
         .
         .
-        DetectAppLayerMpmRegister2("file_data", SIG_FLAG_TOSERVER, 2,
+        DetectAppLayerMpmRegister("file_data", SIG_FLAG_TOSERVER, 2,
                 PrefilterMpmFiledataRegister, NULL,
                 ALPROTO_HTTP2, HTTP2StateDataClient);
-        DetectAppLayerMpmRegister2("file_data", SIG_FLAG_TOCLIENT, 2,
+        DetectAppLayerMpmRegister("file_data", SIG_FLAG_TOCLIENT, 2,
                 PrefilterMpmFiledataRegister, NULL,
                 ALPROTO_HTTP2, HTTP2StateDataServer);
         .
         .
-        DetectAppLayerInspectEngineRegister2("file_data",
+        DetectAppLayerInspectEngineRegister("file_data",
             ALPROTO_HTTP2, SIG_FLAG_TOCLIENT, HTTP2StateDataServer,
             DetectEngineInspectFiledata, NULL);
-        DetectAppLayerInspectEngineRegister2(
+        DetectAppLayerInspectEngineRegister(
                 "file_data", ALPROTO_FTPDATA, SIG_FLAG_TOSERVER, 0, DetectEngineInspectFiledata, NULL);
         .
         .

doc/userguide/rules/dns-keywords.rst

@@ -1,10 +1,27 @@
 DNS Keywords
 ============
 
-There are some more content modifiers (If you are unfamiliar with
-content modifiers, please visit the page :doc:`payload-keywords` These
-ones make sure the signature checks a specific part of the
-network-traffic.
+Suricata supports sticky buffers as well as keywords for efficiently
+matching on specific fields in DNS messages.
+
+Note that sticky buffers are expected to be followed by one or more
+:doc:`payload-keywords`.
+
+dns.answer.name
+---------------
+
+``dns.answer.name`` is a sticky buffer that is used to look at the
+name field in DNS answer resource records.
+
+``dns.answer.name`` will look at both requests and responses, so
+``flow`` is recommended to confine to a specific direction.
+
+The buffer being matched on contains the complete re-assembled
+resource name, for example "www.suricata.io".
+
+``dns.answer.name`` supports :doc:`multi-buffer-matching`.
+
+``dns.answer.name`` was introduced in Suricata 8.0.0.
 
 dns.opcode
 ----------
@@ -32,20 +49,26 @@ Match on DNS requests where the **opcode** is NOT 0::
 dns.query
 ---------
 
-With **dns.query** the DNS request queries are inspected. The dns.query
-keyword works a bit different from the normal content modifiers. When
-used in a rule all contents following it are affected by it.  Example:
+``dns.query`` is a sticky buffer that is used to inspect DNS query
+names in DNS request messages. Example::
 
-  alert dns any any -> any any (msg:"Test dns.query option";
-  dns.query; content:"google"; nocase; sid:1;)
+  alert dns any any -> any any (msg:"Test dns.query option"; dns.query; content:"google"; nocase; sid:1;)
+
+Being a sticky buffer, payload keywords such as content are to be used after ``dns.query``:
 
 .. image:: dns-keywords/dns_query.png
 
-The **dns.query** keyword affects all following contents, until pkt_data
-is used or it reaches the end of the rule.
+The ``dns.query`` keyword affects all following contents, until
+pkt_data is used or it reaches the end of the rule.
 
 .. note:: **dns.query** is equivalent to the older **dns_query**.
 
+.. note:: **dns.query** will only match on DNS request messages, to
+          also match on DNS response message, see
+          `dns.query.name`_.
+
+``dns.query.name`` supports :doc:`multi-buffer-matching`.
+
 Normalized Buffer
 ~~~~~~~~~~~~~~~~~
 
@@ -68,7 +91,19 @@ DNS query on the wire (snippet)::
 
     mail.google.com
 
-Multiple Buffer Matching
-~~~~~~~~~~~~~~~~~~~~~~~~
+dns.query.name
+---------------
+
+``dns.query.name`` is a sticky buffer that is used to look at the name
+field in DNS query (question) resource records. It is nearly identical
+to ``dns.query`` but supports both DNS requests and responses.
+
+``dns.query.name`` will look at both requests and responses, so
+``flow`` is recommended to confine to a specific direction.
+
+The buffer being matched on contains the complete re-assembled
+resource name, for example "www.suricata.io".
+
+``dns.query.name`` supports :doc:`multi-buffer-matching`.
 
-``dns.query`` supports multiple buffer matching, see :doc:`multi-buffer-matching`.
+``dns.query.name`` was introduced in Suricata 8.0.0.

rust/rustfmt.toml

@@ -1,4 +1,4 @@
 # Rust format configuration file. If empty, then this is a message that
 # we expect the default formatting rules to be used.
 
-fn_args_layout = "compressed"
+fn_params_layout = "compressed"

rust/src/dns/detect.rs

@@ -156,44 +156,36 @@ mod test {
 
     #[test]
     fn test_match_opcode() {
-        assert!(
-            match_opcode(
-                &DetectDnsOpcode {
-                    negate: false,
-                    opcode: 0,
-                },
-                0b0000_0000_0000_0000,
-            )
-        );
+        assert!(match_opcode(
+            &DetectDnsOpcode {
+                negate: false,
+                opcode: 0,
+            },
+            0b0000_0000_0000_0000,
+        ));
 
-        assert!(
-            !match_opcode(
-                &DetectDnsOpcode {
-                    negate: true,
-                    opcode: 0,
-                },
-                0b0000_0000_0000_0000,
-            )
-        );
+        assert!(!match_opcode(
+            &DetectDnsOpcode {
+                negate: true,
+                opcode: 0,
+            },
+            0b0000_0000_0000_0000,
+        ));
 
-        assert!(
-            match_opcode(
-                &DetectDnsOpcode {
-                    negate: false,
-                    opcode: 4,
-                },
-                0b0010_0000_0000_0000,
-            )
-        );
+        assert!(match_opcode(
+            &DetectDnsOpcode {
+                negate: false,
+                opcode: 4,
+            },
+            0b0010_0000_0000_0000,
+        ));
 
-        assert!(
-            !match_opcode(
-                &DetectDnsOpcode {
-                    negate: true,
-                    opcode: 4,
-                },
-                0b0010_0000_0000_0000,
-            )
-        );
+        assert!(!match_opcode(
+            &DetectDnsOpcode {
+                negate: true,
+                opcode: 4,
+            },
+            0b0010_0000_0000_0000,
+        ));
     }
 }

rust/src/dns/dns.rs

@@ -221,13 +221,7 @@ pub struct DNSAnswerEntry {
 }
 
 #[derive(Debug)]
-pub struct DNSRequest {
-    pub header: DNSHeader,
-    pub queries: Vec<DNSQueryEntry>,
-}
-
-#[derive(Debug)]
-pub struct DNSResponse {
+pub struct DNSMessage {
     pub header: DNSHeader,
     pub queries: Vec<DNSQueryEntry>,
     pub answers: Vec<DNSAnswerEntry>,
@@ -237,8 +231,8 @@ pub struct DNSResponse {
 #[derive(Debug, Default)]
 pub struct DNSTransaction {
     pub id: u64,
-    pub request: Option<DNSRequest>,
-    pub response: Option<DNSResponse>,
+    pub request: Option<DNSMessage>,
+    pub response: Option<DNSMessage>,
     pub tx_data: AppLayerTxData,
 }
 
@@ -250,10 +244,10 @@ impl Transaction for DNSTransaction {
 
 impl DNSTransaction {
     pub fn new(direction: Direction) -> Self {
-	Self {
-	    tx_data: AppLayerTxData::for_direction(direction),
+        Self {
+            tx_data: AppLayerTxData::for_direction(direction),
             ..Default::default()
-	}
+        }
     }
 
     /// Get the DNS transactions ID (not the internal tracking ID).
@@ -402,7 +396,7 @@ impl DNSState {
             return !is_tcp;
         };
 
-        match parser::dns_parse_request_body(body, input, header) {
+        match parser::dns_parse_body(body, input, header) {
             Ok((_, request)) => {
                 if request.header.flags & 0x8000 != 0 {
                     SCLogDebug!("DNS message is not a request");
@@ -474,7 +468,7 @@ impl DNSState {
             return !is_tcp;
         };
 
-        match parser::dns_parse_response_body(body, input, header) {
+        match parser::dns_parse_body(body, input, header) {
             Ok((_, response)) => {
                 SCLogDebug!("Response header flags: {}", response.header.flags);
 
@@ -702,14 +696,9 @@ fn probe(input: &[u8], dlen: usize) -> (bool, bool, bool) {
         }
     }
 
-    match parser::dns_parse_request(input) {
-        Ok((_, request)) => {
-            return probe_header_validity(&request.header, dlen);
-        }
-        Err(Err::Incomplete(_)) => match parser::dns_parse_header(input) {
-            Ok((_, header)) => {
-                return probe_header_validity(&header, dlen);
-            }
+    match parser::dns_parse_header(input) {
+        Ok((body, header)) => match parser::dns_parse_body(body, input, header) {
+            Ok((_, request)) => probe_header_validity(&request.header, dlen),
             Err(Err::Incomplete(_)) => (false, false, true),
             Err(_) => (false, false, false),
         },
@@ -864,21 +853,54 @@ pub unsafe extern "C" fn rs_dns_state_get_tx_data(
 
 export_state_data_get!(rs_dns_get_state_data, DNSState);
 
+/// Get the DNS query name at index i.
 #[no_mangle]
-pub unsafe extern "C" fn rs_dns_tx_get_query_name(
-    tx: &mut DNSTransaction, i: u32, buf: *mut *const u8, len: *mut u32,
-) -> u8 {
-    if let Some(request) = &tx.request {
-        if (i as usize) < request.queries.len() {
-            let query = &request.queries[i as usize];
+pub unsafe extern "C" fn SCDnsTxGetQueryName(
+    tx: &mut DNSTransaction, to_client: bool, i: u32, buf: *mut *const u8, len: *mut u32,
+) -> bool {
+    let queries = if to_client {
+        tx.response.as_ref().map(|response| &response.queries)
+    } else {
+        tx.request.as_ref().map(|request| &request.queries)
+    };
+    let index = i as usize;
+
+    if let Some(queries) = queries {
+        if let Some(query) = queries.get(index) {
             if !query.name.is_empty() {
-                *len = query.name.len() as u32;
                 *buf = query.name.as_ptr();
-                return 1;
+                *len = query.name.len() as u32;
+                return true;
             }
         }
     }
-    return 0;
+
+    false
+}
+
+/// Get the DNS response answer name and index i.
+#[no_mangle]
+pub unsafe extern "C" fn SCDnsTxGetAnswerName(
+    tx: &mut DNSTransaction, to_client: bool, i: u32, buf: *mut *const u8, len: *mut u32,
+) -> bool {
+    let answers = if to_client {
+        tx.response.as_ref().map(|response| &response.answers)
+    } else {
+        tx.request.as_ref().map(|request| &request.answers)
+    };
+    let index = i as usize;
+
+    if let Some(answers) = answers {
+        if let Some(answer) = answers.get(index) {
+            if !answer.name.is_empty() {
+                *buf = answer.name.as_ptr();
+                *len = answer.name.len() as u32;
+                return true;
+            }
+        }
+    }
+
+    false
 }
 
 /// Get the DNS transaction ID of a transaction.

rust/src/dns/log.rs

@@ -476,7 +476,7 @@ fn dns_log_json_answer_detail(answer: &DNSAnswerEntry) -> Result<JsonBuilder, Js
 }
 
 fn dns_log_json_answer(
-    js: &mut JsonBuilder, response: &DNSResponse, flags: u64,
+    js: &mut JsonBuilder, response: &DNSMessage, flags: u64,
 ) -> Result<(), JsonError> {
     let header = &response.header;
 
@@ -524,7 +524,8 @@ fn dns_log_json_answer(
                 match &answer.data {
                     DNSRData::A(addr) | DNSRData::AAAA(addr) => {
                         if !answer_types.contains_key(&type_string) {
-                            answer_types.insert(type_string.to_string(), JsonBuilder::try_new_array()?);
+                            answer_types
+                                .insert(type_string.to_string(), JsonBuilder::try_new_array()?);
                         }
                         if let Some(a) = answer_types.get_mut(&type_string) {
                             a.append_string(&dns_print_addr(addr))?;
@@ -537,31 +538,35 @@ fn dns_log_json_answer(
                     | DNSRData::NULL(bytes)
                     | DNSRData::PTR(bytes) => {
                         if !answer_types.contains_key(&type_string) {
-                            answer_types.insert(type_string.to_string(), JsonBuilder::try_new_array()?);
+                            answer_types
+                                .insert(type_string.to_string(), JsonBuilder::try_new_array()?);
                         }
                         if let Some(a) = answer_types.get_mut(&type_string) {
                             a.append_string_from_bytes(bytes)?;
                         }
                     }
                     DNSRData::SOA(soa) => {
                         if !answer_types.contains_key(&type_string) {
-                            answer_types.insert(type_string.to_string(), JsonBuilder::try_new_array()?);
+                            answer_types
+                                .insert(type_string.to_string(), JsonBuilder::try_new_array()?);
                         }
                         if let Some(a) = answer_types.get_mut(&type_string) {
                             a.append_object(&dns_log_soa(soa)?)?;
                         }
                     }
                     DNSRData::SSHFP(sshfp) => {
                         if !answer_types.contains_key(&type_string) {
-                            answer_types.insert(type_string.to_string(), JsonBuilder::try_new_array()?);
+                            answer_types
+                                .insert(type_string.to_string(), JsonBuilder::try_new_array()?);
                         }
                         if let Some(a) = answer_types.get_mut(&type_string) {
                             a.append_object(&dns_log_sshfp(sshfp)?)?;
                         }
                     }
                     DNSRData::SRV(srv) => {
                         if !answer_types.contains_key(&type_string) {
-                            answer_types.insert(type_string.to_string(), JsonBuilder::try_new_array()?);
+                            answer_types
+                                .insert(type_string.to_string(), JsonBuilder::try_new_array()?);
                         }
                         if let Some(a) = answer_types.get_mut(&type_string) {
                             a.append_object(&dns_log_srv(srv)?)?;