dns: parse multiple txt segments into an array

A DNS TXT answer record can actually be made of up multiple TXT
entries in a single record. Suricata currently expands these into
multiple TXT records, however that is not very representative of the
actualy DNS message.

Instead, if a TXT record contains multiple labels, parse them into an
array.

We still expand multiple TXT segements into multiple TXT records at
logging time for compatibility, but this will allow something like
MDNS to log more accurately to the protocol.

rust/src/dns/dns.rs

@@ -243,8 +243,8 @@ pub enum DNSRData {
     PTR(DNSName),
     MX(DNSName),
     NS(DNSName),
-    // RData is text
-    TXT(Vec<u8>),
+    // TXT records are an array of TXT entries
+    TXT(Vec<Vec<u8>>),
     NULL(Vec<u8>),
     // RData has several fields
     SOA(DNSRDataSOA),

rust/src/dns/log.rs

@@ -385,7 +385,11 @@ fn dns_log_srv(srv: &DNSRDataSRV) -> Result<JsonBuilder, JsonError> {
     return Ok(js);
 }
 
-fn dns_log_json_answer_detail(answer: &DNSAnswerEntry) -> Result<JsonBuilder, JsonError> {
+/// Log a single DNS answer entry.
+///
+/// For items that may be array, such as TXT records, i will designate
+/// which entry to log.
+fn dns_log_json_answer_detail(answer: &DNSAnswerEntry, i: usize) -> Result<JsonBuilder, JsonError> {
     let mut jsa = JsonBuilder::try_new_object()?;
 
     jsa.set_string_from_bytes("rrname", &answer.name.value)?;
@@ -405,7 +409,14 @@ fn dns_log_json_answer_detail(answer: &DNSAnswerEntry) -> Result<JsonBuilder, Js
                 jsa.set_bool("rdata_truncated", true)?;
             }
         }
-        DNSRData::TXT(bytes) | DNSRData::NULL(bytes) => {
+        DNSRData::TXT(txt) => {
+            if let Some(txt) = txt.get(i) {
+                jsa.set_string_from_bytes("rdata", txt)?;
+            } else {
+                debug_validate_fail!("txt entry does not exist");
+            }
+        }
+        DNSRData::NULL(bytes) => {
             jsa.set_string_from_bytes("rdata", bytes)?;
         }
         DNSRData::SOA(soa) => {
@@ -502,7 +513,18 @@ fn dns_log_json_answer(
                             a.append_string_from_bytes(&name.value)?;
                         }
                     }
-                    DNSRData::TXT(bytes) | DNSRData::NULL(bytes) => {
+                    DNSRData::TXT(txt_strings) => {
+                        if !answer_types.contains_key(&type_string) {
+                            answer_types
+                                .insert(type_string.to_string(), JsonBuilder::try_new_array()?);
+                        }
+                        if let Some(a) = answer_types.get_mut(&type_string) {
+                            for txt in txt_strings {
+                                a.append_string_from_bytes(txt)?;
+                            }
+                        }
+                    }
+                    DNSRData::NULL(bytes) => {
                         if !answer_types.contains_key(&type_string) {
                             answer_types
                                 .insert(type_string.to_string(), JsonBuilder::try_new_array()?);
@@ -543,7 +565,16 @@ fn dns_log_json_answer(
             }
 
             if flags & LOG_FORMAT_DETAILED != 0 {
-                js_answers.append_object(&dns_log_json_answer_detail(answer)?)?;
+                match &answer.data {
+                    DNSRData::TXT(asdf) => {
+                        for i in 0..asdf.len() {
+                            js_answers.append_object(&dns_log_json_answer_detail(answer, i)?)?;
+                        }
+                    }
+                    _ => {
+                        js_answers.append_object(&dns_log_json_answer_detail(answer, 0)?)?;
+                    }
+                }
             }
         }
 
@@ -566,8 +597,18 @@ fn dns_log_json_answer(
     if !response.authorities.is_empty() {
         js.open_array("authorities")?;
         for auth in &response.authorities {
-            let auth_detail = dns_log_json_answer_detail(auth)?;
-            js.append_object(&auth_detail)?;
+            match &auth.data {
+                DNSRData::TXT(txt) => {
+                    for i in 0..txt.len() {
+                        let auth_detail = dns_log_json_answer_detail(auth, i)?;
+                        js.append_object(&auth_detail)?;
+                    }
+                }
+                _ => {
+                    let auth_detail = dns_log_json_answer_detail(auth, 0)?;
+                    js.append_object(&auth_detail)?;
+                }
+            }
         }
         js.close()?;
     }
@@ -584,8 +625,18 @@ fn dns_log_json_answer(
                 js.open_array("additionals")?;
                 is_js_open = true;
             }
-            let add_detail = dns_log_json_answer_detail(add)?;
-            js.append_object(&add_detail)?;
+            match &add.data {
+                DNSRData::TXT(txt) => {
+                    for i in 0..txt.len() {
+                        let add_detail = dns_log_json_answer_detail(add, i)?;
+                        js.append_object(&add_detail)?;
+                    }
+                }
+                _ => {
+                    let add_detail = dns_log_json_answer_detail(add, 0)?;
+                    js.append_object(&add_detail)?;
+                }
+            }
         }
         if is_js_open {
             js.close()?;
@@ -630,7 +681,18 @@ fn dns_log_json_answers(
                             a.append_string_from_bytes(&name.value)?;
                         }
                     }
-                    DNSRData::TXT(bytes) | DNSRData::NULL(bytes) => {
+                    DNSRData::TXT(txt) => {
+                        if !answer_types.contains_key(&type_string) {
+                            answer_types
+                                .insert(type_string.to_string(), JsonBuilder::try_new_array()?);
+                        }
+                        if let Some(a) = answer_types.get_mut(&type_string) {
+                            for txt in txt {
+                                a.append_string_from_bytes(txt)?;
+                            }
+                        }
+                    }
+                    DNSRData::NULL(bytes) => {
                         if !answer_types.contains_key(&type_string) {
                             answer_types
                                 .insert(type_string.to_string(), JsonBuilder::try_new_array()?);
@@ -671,7 +733,16 @@ fn dns_log_json_answers(
             }
 
             if flags & LOG_FORMAT_DETAILED != 0 {
-                js_answers.append_object(&dns_log_json_answer_detail(answer)?)?;
+                match &answer.data {
+                    DNSRData::TXT(txt) => {
+                        for i in 0..txt.len() {
+                            js_answers.append_object(&dns_log_json_answer_detail(answer, i)?)?;
+                        }
+                    }
+                    _ => {
+                        js_answers.append_object(&dns_log_json_answer_detail(answer, 0)?)?;
+                    }
+                }
             }
         }
 
@@ -812,8 +883,18 @@ fn log_json(tx: &DNSTransaction, flags: u64, jb: &mut JsonBuilder) -> Result<(),
     if !message.authorities.is_empty() {
         jb.open_array("authorities")?;
         for auth in &message.authorities {
-            let auth_detail = dns_log_json_answer_detail(auth)?;
-            jb.append_object(&auth_detail)?;
+            match &auth.data {
+                DNSRData::TXT(txt) => {
+                    for i in 0..txt.len() {
+                        let auth_detail = dns_log_json_answer_detail(auth, i)?;
+                        jb.append_object(&auth_detail)?;
+                    }
+                }
+                _ => {
+                    let auth_detail = dns_log_json_answer_detail(auth, 0)?;
+                    jb.append_object(&auth_detail)?;
+                }
+            }
         }
         jb.close()?;
     }
@@ -830,8 +911,18 @@ fn log_json(tx: &DNSTransaction, flags: u64, jb: &mut JsonBuilder) -> Result<(),
                 jb.open_array("additionals")?;
                 is_jb_open = true;
             }
-            let add_detail = dns_log_json_answer_detail(add)?;
-            jb.append_object(&add_detail)?;
+            match &add.data {
+                DNSRData::TXT(txt) => {
+                    for i in 0..txt.len() {
+                        let add_detail = dns_log_json_answer_detail(add, i)?;
+                        jb.append_object(&add_detail)?;
+                    }
+                }
+                _ => {
+                    let add_detail = dns_log_json_answer_detail(add, 0)?;
+                    jb.append_object(&add_detail)?;
+                }
+            }
         }
         if is_jb_open {
             jb.close()?;

rust/src/dns/lua.rs

@@ -162,8 +162,19 @@ pub extern "C" fn SCDnsLuaGetAnswerTable(clua: &mut CLuaState, tx: &mut DNSTrans
                         lua.settable(-3);
                     }
                 }
-                DNSRData::TXT(ref bytes)
-                | DNSRData::NULL(ref bytes)
+                DNSRData::TXT(ref txt) => {
+                    if !txt.is_empty() {
+                        lua.pushstring("addr");
+                        let combined = txt
+                            .iter()
+                            .map(|s| String::from_utf8_lossy(s))
+                            .collect::<Vec<_>>()
+                            .join(" ");
+                        lua.pushstring(&combined);
+                        lua.settable(-3);
+                    }
+                }
+                DNSRData::NULL(ref bytes)
                 | DNSRData::Unknown(ref bytes) => {
                     if !bytes.is_empty() {
                         lua.pushstring("addr");

rust/src/dns/parser.rs

@@ -17,11 +17,11 @@
 
 //! Nom parsers for DNS.
 
-use crate::dns::dns::*;
 use crate::detect::EnumString;
-use nom7::combinator::{complete, rest};
+use crate::dns::dns::*;
+use nom7::combinator::rest;
 use nom7::error::ErrorKind;
-use nom7::multi::{count, length_data, many_m_n};
+use nom7::multi::{count, length_data};
 use nom7::number::streaming::{be_u16, be_u32, be_u8};
 use nom7::{error_position, Err, IResult};
 
@@ -206,18 +206,6 @@ fn dns_parse_answer<'a>(
     for _ in 0..count {
         match subparser(input, message, flags) {
             Ok((rem, val)) => {
-                let n = if val.rrtype == DNSRecordType::TXT as u16 {
-                    // For TXT records we need to run the parser
-                    // multiple times. Set n high, to the maximum
-                    // value based on a max txt side of 65535, but
-                    // taking into considering that strings need
-                    // to be quoted, so half that.
-                    32767
-                } else {
-                    // For all other types we only want to run the
-                    // parser once, so set n to 1.
-                    1
-                };
                 // edge case for additional section of type=OPT
                 // with empty data (data length = 0x0000)
                 if val.data.is_empty() && val.rrtype == DNSRecordType::OPT as u16 {
@@ -231,27 +219,14 @@ fn dns_parse_answer<'a>(
                     input = rem;
                     continue;
                 }
-                let result: IResult<&'a [u8], Vec<DNSRData>> = many_m_n(
-                    1,
-                    n,
-                    complete(|b| dns_parse_rdata(b, message, val.rrtype, flags)),
-                )(val.data);
-                match result {
-                    Ok((_, rdatas)) => {
-                        for rdata in rdatas {
-                            answers.push(DNSAnswerEntry {
-                                name: val.name.clone(),
-                                rrtype: val.rrtype,
-                                rrclass: val.rrclass,
-                                ttl: val.ttl,
-                                data: rdata,
-                            });
-                        }
-                    }
-                    Err(e) => {
-                        return Err(e);
-                    }
-                }
+                let (_, rdata) = dns_parse_rdata(val.data, message, val.rrtype, flags)?;
+                answers.push(DNSAnswerEntry {
+                    name: val.name.clone(),
+                    rrtype: val.rrtype,
+                    rrclass: val.rrclass,
+                    ttl: val.ttl,
+                    data: rdata,
+                });
                 input = rem;
             }
             Err(e) => {
@@ -366,8 +341,16 @@ fn dns_parse_rdata_srv<'a>(
 }
 
 fn dns_parse_rdata_txt(input: &[u8]) -> IResult<&[u8], DNSRData> {
-    let (i, txt) = length_data(be_u8)(input)?;
-    Ok((i, DNSRData::TXT(txt.to_vec())))
+    let mut txt_strings = Vec::new();
+    let mut i = input;
+
+    while !i.is_empty() {
+        let (j, txt) = length_data(be_u8)(i)?;
+        txt_strings.push(txt.to_vec());
+        i = j;
+    }
+
+    Ok((i, DNSRData::TXT(txt_strings)))
 }
 
 fn dns_parse_rdata_null(input: &[u8]) -> IResult<&[u8], DNSRData> {
@@ -474,13 +457,14 @@ pub fn dns_parse_body<'a>(
     let mut invalid_additionals = false;
     let mut additionals = Vec::new();
     if !invalid_authorities {
-        let additionals_parsed = dns_parse_answer(i_next, message, header.additional_rr as usize, &mut flags);
+        let additionals_parsed =
+            dns_parse_answer(i_next, message, header.additional_rr as usize, &mut flags);
         if let Ok((i, additionals_ok)) = additionals_parsed {
-                additionals = additionals_ok;
-                i_next = i;
-            } else {
-                invalid_additionals = true;
-            }
+            additionals = additionals_ok;
+            i_next = i;
+        } else {
+            invalid_additionals = true;
+        }
     }
     Ok((
         i_next,