dns: validate header on every incoming message

As UDP streams getting probed, a stream that does not appear to be DNS
at first, may have a single packet that does look close enough to DNS
to be picked up as DNS causing every subsequent packet to result in a
parser error.

To mitigate this, probe every incoming DNS message header for validity
before continuing onto the body.  If the header doesn't validate as
DNS, just ignore the packet so no parse error is registered.

(cherry picked from commit 595700ab7e9dc9d12d46cf4d6833a86840decdf9)

diff --git a/rust/src/dns/dns.rs b/rust/src/dns/dns.rs
index 9403a26620dd019a084bdaa5f3ef15a5eb73ebd9..f2ffc6d704a704cbb17b1c4673e12ab9db212505 100644 (file)
--- a/rust/src/dns/dns.rs
+++ b/rust/src/dns/dns.rs
@@ -500,7 +500,17 @@ impl DNSState {
                                                         event as u8);
     }
 
-    pub fn parse_request(&mut self, input: &[u8]) -> bool {
+    fn validate_header(&self, input: &[u8]) -> bool {
+        parser::dns_parse_header(input)
+            .map(|(_, header)| probe_header_validity(header, input.len()).0)
+            .unwrap_or(false)
+    }
+
+    fn parse_request(&mut self, input: &[u8], is_tcp: bool) -> bool {
+        if !self.validate_header(input) {
+            return !is_tcp;
+        }
+
         match parser::dns_parse_request(input) {
             Ok((_, request)) => {
                 if request.header.flags & 0x8000 != 0 {
@@ -543,7 +553,19 @@ impl DNSState {
         }
     }
 
-    pub fn parse_response(&mut self, input: &[u8]) -> bool {
+    fn parse_request_udp(&mut self, input: &[u8]) -> bool {
+        self.parse_request(input, false)
+    }
+
+    fn parse_response_udp(&mut self, input: &[u8]) -> bool {
+        self.parse_response(input, false)
+    }
+
+    pub fn parse_response(&mut self, input: &[u8], is_tcp: bool) -> bool {
+        if !self.validate_header(input) {
+            return !is_tcp;
+        }
+
         match parser::dns_parse_response(input) {
             Ok((_, response)) => {
 
@@ -620,8 +642,8 @@ impl DNSState {
             SCLogDebug!("[request] Have {} bytes, need {} to parse",
                         cur_i.len(), size + 2);
             if size > 0 && cur_i.len() >= size + 2 {
-                let msg = &cur_i[0..(size + 2)];
-                if self.parse_request(&msg[2..]) {
+                let msg = &cur_i[2..(size + 2)];
+                if self.parse_request(msg, true) {
                     cur_i = &cur_i[(size + 2)..];
                     consumed += size  + 2;
                 } else {
@@ -667,8 +689,8 @@ impl DNSState {
             SCLogDebug!("[response] Have {} bytes, need {} to parse",
                         cur_i.len(), size + 2);
             if size > 0 && cur_i.len() >= size + 2 {
-                let msg = &cur_i[0..(size + 2)];
-                if self.parse_response(&msg[2..]) {
+                let msg = &cur_i[2..(size + 2)];
+                if self.parse_response(msg, true) {
                     cur_i = &cur_i[(size + 2)..];
                     consumed += size + 2;
                 } else {
@@ -706,16 +728,19 @@ impl DNSState {
 const DNS_HEADER_SIZE: usize = 12;
 
 fn probe_header_validity(header: DNSHeader, rlen: usize) -> (bool, bool, bool) {
-    if 2 * (header.additional_rr as usize
-        + header.answer_rr as usize
-        + header.authority_rr as usize
-        + header.questions as usize)
-        + DNS_HEADER_SIZE
-        > rlen
-    {
-        //not enough data for such a DNS record
+    let min_msg_size = 2
+        * (header.additional_rr as usize
+            + header.answer_rr as usize
+            + header.authority_rr as usize
+            + header.questions as usize)
+        + DNS_HEADER_SIZE;
+
+    if min_msg_size > rlen {
+        // Not enough data for records defined in the header, or
+        // impossibly large.
         return (false, false, false);
     }
+
     let is_request = header.flags & 0x8000 == 0;
     return (true, is_request, false);
 }
@@ -812,11 +837,8 @@ pub extern "C" fn rs_dns_parse_request(_flow: *const core::Flow,
                                        -> AppLayerResult {
     let state = cast_pointer!(state, DNSState);
     let buf = unsafe{std::slice::from_raw_parts(input, input_len as usize)};
-    if state.parse_request(buf) {
-        AppLayerResult::ok()
-    } else {
-        AppLayerResult::err()
-    }
+    state.parse_request_udp(buf);
+    AppLayerResult::ok()
 }
 
 #[no_mangle]
@@ -830,11 +852,8 @@ pub extern "C" fn rs_dns_parse_response(_flow: *const core::Flow,
                                         -> AppLayerResult {
     let state = cast_pointer!(state, DNSState);
     let buf = unsafe{std::slice::from_raw_parts(input, input_len as usize)};
-    if state.parse_response(buf) {
-        AppLayerResult::ok()
-    } else {
-        AppLayerResult::err()
-    }
+    state.parse_response_udp(buf);
+    AppLayerResult::ok()
 }
 
 /// C binding parse a DNS request. Returns 1 on success, -1 on failure.
@@ -1387,7 +1406,7 @@ mod tests {
             0x80,
         ];
         let mut state = DNSState::new();
-        assert!(state.parse_response(buf));
+        assert!(state.parse_response(buf, false));
     }
 
     // Port of the C RustDNSUDPParserTest02 unit test.
@@ -1407,7 +1426,7 @@ mod tests {
             0x10,0x00,0x02,0xC0,0x85,0x00,0x00,0x29,0x05,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
         ];
         let mut state = DNSState::new();
-        assert!(state.parse_response(buf));
+        assert!(state.parse_response(buf, false));
     }
 
     // Port of the C RustDNSUDPParserTest03 unit test.
@@ -1427,7 +1446,7 @@ mod tests {
             0x29,0x05,0x00,0x00,0x00,0x00,0x00,0x00,0x00
         ];
         let mut state = DNSState::new();
-        assert!(state.parse_response(buf));
+        assert!(state.parse_response(buf, false));
     }
 
     // Port of the C RustDNSUDPParserTest04 unit test.
@@ -1451,7 +1470,7 @@ mod tests {
             0x6b,0x00,0x01,0x00,0x01,0x00,0x09,0x3a,0x80,0x00,0x04,0x0a,0x1e,0x1c,0x5f
         ];
         let mut state = DNSState::new();
-        assert!(state.parse_response(buf));
+        assert!(state.parse_response(buf, false));
     }
 
     // Port of the C RustDNSUDPParserTest05 unit test.
@@ -1475,7 +1494,7 @@ mod tests {
             0x6b,0x00,0x01,0x00,0x01,0x00,0x09,0x3a,0x80,0x00,0x04,0x0a,0x1e,0x1c,0x5f
         ];
         let mut state = DNSState::new();
-        assert!(!state.parse_response(buf));
+        assert!(!state.parse_response(buf, false));
     }
 
     // Port of the C RustDNSTCPParserTestMultiRecord unit test.