]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
dns: parse multiple txt segments into an array
authorJason Ish <jason.ish@oisf.net>
Tue, 27 May 2025 17:27:23 +0000 (11:27 -0600)
committerVictor Julien <victor@inliniac.net>
Thu, 29 May 2025 08:59:16 +0000 (10:59 +0200)
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
rust/src/dns/log.rs
rust/src/dns/lua.rs
rust/src/dns/parser.rs

index 3c2d2539e9ae4e5eb3efc77c64000dcf96c54e80..579ae7a4e11d2e27a9d9f2e7f7ea95a48e5f2bfb 100644 (file)
@@ -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),
index 447dda1b23f2dfdcd1f7c7d5ee40ffb67bff3c4d..f3e14596946606f03831426bde98ba223caf332f 100644 (file)
@@ -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()?;
index 240d55428ea7f4dcf93598f0f125e394ee9ae1c4..802a66d38e9bafdcabd7a970dbf552e3cb5a0150 100644 (file)
@@ -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");
index ee2366137f230f0f13f34b8f798bf7c13a1490ff..a3e2a88df79eb1ff934fcbecf02b6a764d86117e 100644 (file)
 
 //! 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,