aiden
/
suricata
mirror of https://github.com/OISF/suricata
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

dns: provide events for recoverable parse errors

Browse Source 
Add events for the following resource name parsing issues:

- name truncated as its too long
- maximum number of labels reached
- infinite loop

Currently these events are only registered when encountered, but
recoverable. That is where we are able to return some of the name,
usually in a truncated state.

As name parsing has many code paths, we pass in a pointer to a flag
field that can be updated by the name parser, this is done in
addition to the flags being set on a specific name as when logging we
want to designate which fields are truncated, etc. But for alerts, we
just care that something happened during the parse. It also reduces
errors as it won't be forgotten to check for the flags and set the
event if some new parser is written that also parses names.

Ticket: #7280
pull/12272/head
Jason Ish 2 years ago committed by Victor Julien
parent 37f4c52b22
commit 19cf0f8133
3 changed files with 142 additions and 61 deletions
Show Stats Download Patch File Download Diff File

9
rules/dns-events.rules
Unescape Escape View File

@ -8,3 +8,12 @@ alert dns any any -> any any (msg:"SURICATA DNS Not a response"; flow:to_client;
# Z flag (reserved) not 0
alert dns any any -> any any (msg:"SURICATA DNS Z flag set"; app-layer-event:dns.z_flag_set; classtype:protocol-command-decode; sid:2240006; rev:2;)
alert dns any any -> any any (msg:"SURICATA DNS Invalid opcode"; app-layer-event:dns.invalid_opcode; classtype:protocol-command-decode; sid:2240007; rev:1;)
# A resource name was too long (over 1025 chars)
alert dns any any -> any any (msg:"SURICATA DNS Name too long"; app-layer-event:dns.name_too_long; classtype:protocol-command-decode; sid:224008; rev:1;)
# An infinite loop was found while decoding a DNS resource name.
alert dns any any -> any any (msg:"SURICATA DNS Infinite loop"; app-layer-event:dns.infinite_loop; classtype:protocol-command-decode; sid:224009; rev:1;)
# Suricata's maximum number of DNS name labels was reached while parsing a resource name.
alert dns any any -> any any (msg:"SURICATA DNS Too many labels"; app-layer-event:dns.too_many_labels; classtype:protocol-command-decode; sid:224010; rev:1;)

38
rust/src/dns/dns.rs
Unescape Escape View File

@ -129,6 +129,12 @@ pub enum DNSEvent {
NotResponse,
ZFlagSet,
InvalidOpcode,
/// A DNS resource name was exessively long and was truncated.
NameTooLong,
/// An infinite loop was found while parsing a name.
InfiniteLoop,
/// Too many labels were found.
TooManyLabels,
}
#[derive(Debug, PartialEq, Eq)]
@ -383,7 +389,7 @@ pub(crate) fn dns_parse_request(input: &[u8]) -> Result<DNSTransaction, DNSParse
};
match parser::dns_parse_body(body, input, header) {
Ok((_, request)) => {
Ok((_, (request, parse_flags))) => {
if request.header.flags & 0x8000 != 0 {
SCLogDebug!("DNS message is not a request");
return Err(DNSParseError::NotRequest);
@ -399,10 +405,23 @@ pub(crate) fn dns_parse_request(input: &[u8]) -> Result<DNSTransaction, DNSParse
SCLogDebug!("Z-flag set on DNS request");
tx.set_event(DNSEvent::ZFlagSet);
}
if opcode >= 7 {
tx.set_event(DNSEvent::InvalidOpcode);
}
if parse_flags.contains(DNSNameFlags::TRUNCATED) {
tx.set_event(DNSEvent::NameTooLong);
}
if parse_flags.contains(DNSNameFlags::INFINITE_LOOP) {
tx.set_event(DNSEvent::InfiniteLoop);
}
if parse_flags.contains(DNSNameFlags::LABEL_LIMIT) {
tx.set_event(DNSEvent::TooManyLabels);
}
return Ok(tx);
}
Err(Err::Incomplete(_)) => {
@ -426,7 +445,7 @@ pub(crate) fn dns_parse_response(input: &[u8]) -> Result<DNSTransaction, DNSPars
};
match parser::dns_parse_body(body, input, header) {
Ok((_, response)) => {
Ok((_, (response, parse_flags))) => {
SCLogDebug!("Response header flags: {}", response.header.flags);
let z_flag = response.header.flags & 0x0040 != 0;
let opcode = ((response.header.flags >> 11) & 0xf) as u8;
@ -444,10 +463,23 @@ pub(crate) fn dns_parse_response(input: &[u8]) -> Result<DNSTransaction, DNSPars
SCLogDebug!("Z-flag set on DNS response");
tx.set_event(DNSEvent::ZFlagSet);
}
if opcode >= 7 {
tx.set_event(DNSEvent::InvalidOpcode);
}
if parse_flags.contains(DNSNameFlags::TRUNCATED) {
tx.set_event(DNSEvent::NameTooLong);
}
if parse_flags.contains(DNSNameFlags::INFINITE_LOOP) {
tx.set_event(DNSEvent::InfiniteLoop);
}
if parse_flags.contains(DNSNameFlags::LABEL_LIMIT) {
tx.set_event(DNSEvent::TooManyLabels);
}
return Ok(tx);
}
Err(Err::Incomplete(_)) => {
@ -778,7 +810,7 @@ fn probe(input: &[u8], dlen: usize) -> (bool, bool, bool) {
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),
Ok((_, (request, _flags))) => probe_header_validity(&request.header, dlen),
Err(Err::Incomplete(_)) => (false, false, true),
Err(_) => (false, false, false),
},

156
rust/src/dns/parser.rs
Unescape Escape View File

@ -60,7 +60,9 @@ static MAX_NAME_LEN: usize = 1025;
/// Parameters:
/// start: the start of the name
/// message: the complete message that start is a part of with the DNS header
fn dns_parse_name<'b>(start: &'b [u8], message: &'b [u8]) -> IResult<&'b [u8], DNSName> {
fn dns_parse_name<'b>(
start: &'b [u8], message: &'b [u8], parse_flags: &mut DNSNameFlags,
) -> IResult<&'b [u8], DNSName> {
let mut pos = start;
let mut pivot = start;
let mut name: Vec<u8> = Vec::with_capacity(32);
@ -145,6 +147,8 @@ fn dns_parse_name<'b>(start: &'b [u8], message: &'b [u8]) -> IResult<&'b [u8], D
}
}
parse_flags.insert(flags);
// If we followed a pointer we return the position after the first
// pointer followed. Is there a better way to see if these slices
// diverged from each other? A straight up comparison would
@ -167,7 +171,7 @@ fn dns_parse_name<'b>(start: &'b [u8], message: &'b [u8]) -> IResult<&'b [u8], D
/// multi-string TXT entry as a single quote string, similar to the
/// output of dig. Something to consider for a future version.
fn dns_parse_answer<'a>(
slice: &'a [u8], message: &'a [u8], count: usize,
slice: &'a [u8], message: &'a [u8], count: usize, flags: &mut DNSNameFlags,
) -> IResult<&'a [u8], Vec<DNSAnswerEntry>> {
let mut answers = Vec::new();
let mut input = slice;
@ -180,8 +184,10 @@ fn dns_parse_answer<'a>(
data: &'a [u8],
}
fn subparser<'a>(i: &'a [u8], message: &'a [u8]) -> IResult<&'a [u8], Answer<'a>> {
let (i, name) = dns_parse_name(i, message)?;
fn subparser<'a>(
i: &'a [u8], message: &'a [u8], flags: &mut DNSNameFlags,
) -> IResult<&'a [u8], Answer<'a>> {
let (i, name) = dns_parse_name(i, message, flags)?;
let (i, rrtype) = be_u16(i)?;
let (i, rrclass) = be_u16(i)?;
let (i, ttl) = be_u32(i)?;
@ -197,7 +203,7 @@ fn dns_parse_answer<'a>(
}
for _ in 0..count {
match subparser(input, message) {
match subparser(input, message, flags) {
Ok((rem, val)) => {
let n = match val.rrtype {
DNS_RECORD_TYPE_TXT => {
@ -227,8 +233,11 @@ 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)))(val.data);
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 {
@ -261,9 +270,11 @@ fn dns_parse_answer<'a>(
/// Arguments are suitable for using with call!:
///
/// call!(complete_dns_message_buffer)
fn dns_parse_query<'a>(input: &'a [u8], message: &'a [u8]) -> IResult<&'a [u8], DNSQueryEntry> {
fn dns_parse_query<'a>(
input: &'a [u8], message: &'a [u8], flags: &mut DNSNameFlags,
) -> IResult<&'a [u8], DNSQueryEntry> {
let i = input;
let (i, name) = dns_parse_name(i, message)?;
let (i, name) = dns_parse_name(i, message, flags)?;
let (i, rrtype) = be_u16(i)?;
let (i, rrclass) = be_u16(i)?;
Ok((
@ -284,22 +295,30 @@ fn dns_parse_rdata_aaaa(input: &[u8]) -> IResult<&[u8], DNSRData> {
rest(input).map(|(input, data)| (input, DNSRData::AAAA(data.to_vec())))
}
fn dns_parse_rdata_cname<'a>(input: &'a [u8], message: &'a [u8]) -> IResult<&'a [u8], DNSRData> {
dns_parse_name(input, message).map(|(input, name)| (input, DNSRData::CNAME(name)))
fn dns_parse_rdata_cname<'a>(
input: &'a [u8], message: &'a [u8], flags: &mut DNSNameFlags,
) -> IResult<&'a [u8], DNSRData> {
dns_parse_name(input, message, flags).map(|(input, name)| (input, DNSRData::CNAME(name)))
}
fn dns_parse_rdata_ns<'a>(input: &'a [u8], message: &'a [u8]) -> IResult<&'a [u8], DNSRData> {
dns_parse_name(input, message).map(|(input, name)| (input, DNSRData::NS(name)))
fn dns_parse_rdata_ns<'a>(
input: &'a [u8], message: &'a [u8], flags: &mut DNSNameFlags,
) -> IResult<&'a [u8], DNSRData> {
dns_parse_name(input, message, flags).map(|(input, name)| (input, DNSRData::NS(name)))
}
fn dns_parse_rdata_ptr<'a>(input: &'a [u8], message: &'a [u8]) -> IResult<&'a [u8], DNSRData> {
dns_parse_name(input, message).map(|(input, name)| (input, DNSRData::PTR(name)))
fn dns_parse_rdata_ptr<'a>(
input: &'a [u8], message: &'a [u8], flags: &mut DNSNameFlags,
) -> IResult<&'a [u8], DNSRData> {
dns_parse_name(input, message, flags).map(|(input, name)| (input, DNSRData::PTR(name)))
}
fn dns_parse_rdata_soa<'a>(input: &'a [u8], message: &'a [u8]) -> IResult<&'a [u8], DNSRData> {
fn dns_parse_rdata_soa<'a>(
input: &'a [u8], message: &'a [u8], flags: &mut DNSNameFlags,
) -> IResult<&'a [u8], DNSRData> {
let i = input;
let (i, mname) = dns_parse_name(i, message)?;
let (i, rname) = dns_parse_name(i, message)?;
let (i, mname) = dns_parse_name(i, message, flags)?;
let (i, rname) = dns_parse_name(i, message, flags)?;
let (i, serial) = be_u32(i)?;
let (i, refresh) = be_u32(i)?;
let (i, retry) = be_u32(i)?;
@ -319,20 +338,24 @@ fn dns_parse_rdata_soa<'a>(input: &'a [u8], message: &'a [u8]) -> IResult<&'a [u
))
}
fn dns_parse_rdata_mx<'a>(input: &'a [u8], message: &'a [u8]) -> IResult<&'a [u8], DNSRData> {
fn dns_parse_rdata_mx<'a>(
input: &'a [u8], message: &'a [u8], flags: &mut DNSNameFlags,
) -> IResult<&'a [u8], DNSRData> {
// For MX we skip over the preference field before
// parsing out the name.
let (i, _) = be_u16(input)?;
let (i, name) = dns_parse_name(i, message)?;
let (i, name) = dns_parse_name(i, message, flags)?;
Ok((i, DNSRData::MX(name)))
}
fn dns_parse_rdata_srv<'a>(input: &'a [u8], message: &'a [u8]) -> IResult<&'a [u8], DNSRData> {
fn dns_parse_rdata_srv<'a>(
input: &'a [u8], message: &'a [u8], flags: &mut DNSNameFlags,
) -> IResult<&'a [u8], DNSRData> {
let i = input;
let (i, priority) = be_u16(i)?;
let (i, weight) = be_u16(i)?;
let (i, port) = be_u16(i)?;
let (i, target) = dns_parse_name(i, message)?;
let (i, target) = dns_parse_name(i, message, flags)?;
Ok((
i,
DNSRData::SRV(DNSRDataSRV {
@ -389,20 +412,20 @@ fn dns_parse_rdata_unknown(input: &[u8]) -> IResult<&[u8], DNSRData> {
}
fn dns_parse_rdata<'a>(
input: &'a [u8], message: &'a [u8], rrtype: u16,
input: &'a [u8], message: &'a [u8], rrtype: u16, flags: &mut DNSNameFlags,
) -> IResult<&'a [u8], DNSRData> {
match rrtype {
DNS_RECORD_TYPE_A => dns_parse_rdata_a(input),
DNS_RECORD_TYPE_AAAA => dns_parse_rdata_aaaa(input),
DNS_RECORD_TYPE_CNAME => dns_parse_rdata_cname(input, message),
DNS_RECORD_TYPE_PTR => dns_parse_rdata_ptr(input, message),
DNS_RECORD_TYPE_SOA => dns_parse_rdata_soa(input, message),
DNS_RECORD_TYPE_MX => dns_parse_rdata_mx(input, message),
DNS_RECORD_TYPE_NS => dns_parse_rdata_ns(input, message),
DNS_RECORD_TYPE_CNAME => dns_parse_rdata_cname(input, message, flags),
DNS_RECORD_TYPE_PTR => dns_parse_rdata_ptr(input, message, flags),
DNS_RECORD_TYPE_SOA => dns_parse_rdata_soa(input, message, flags),
DNS_RECORD_TYPE_MX => dns_parse_rdata_mx(input, message, flags),
DNS_RECORD_TYPE_NS => dns_parse_rdata_ns(input, message, flags),
DNS_RECORD_TYPE_TXT => dns_parse_rdata_txt(input),
DNS_RECORD_TYPE_NULL => dns_parse_rdata_null(input),
DNS_RECORD_TYPE_SSHFP => dns_parse_rdata_sshfp(input),
DNS_RECORD_TYPE_SRV => dns_parse_rdata_srv(input, message),
DNS_RECORD_TYPE_SRV => dns_parse_rdata_srv(input, message, flags),
DNS_RECORD_TYPE_OPT => dns_parse_rdata_opt(input),
_ => dns_parse_rdata_unknown(input),
}
@ -431,20 +454,27 @@ pub fn dns_parse_header(i: &[u8]) -> IResult<&[u8], DNSHeader> {
pub fn dns_parse_body<'a>(
i: &'a [u8], message: &'a [u8], header: DNSHeader,
) -> IResult<&'a [u8], DNSMessage> {
let (i, queries) = count(|b| dns_parse_query(b, message), header.questions as usize)(i)?;
let (i, answers) = dns_parse_answer(i, message, header.answer_rr as usize)?;
let (i, authorities) = dns_parse_answer(i, message, header.authority_rr as usize)?;
let (i, additionals) = dns_parse_answer(i, message, header.additional_rr as usize)?;
) -> IResult<&'a [u8], (DNSMessage, DNSNameFlags)> {
let mut flags = DNSNameFlags::default();
let (i, queries) = count(
|b| dns_parse_query(b, message, &mut flags),
header.questions as usize,
)(i)?;
let (i, answers) = dns_parse_answer(i, message, header.answer_rr as usize, &mut flags)?;
let (i, authorities) = dns_parse_answer(i, message, header.authority_rr as usize, &mut flags)?;
let (i, additionals) = dns_parse_answer(i, message, header.additional_rr as usize, &mut flags)?;
Ok((
i,
DNSMessage {
header,
queries,
answers,
authorities,
additionals,
},
(
DNSMessage {
header,
queries,
answers,
authorities,
additionals,
},
flags,
),
))
}
@ -464,7 +494,8 @@ mod tests {
0x03, 0x63, 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, /* .com.... */
];
let expected_remainder: &[u8] = &[0x00, 0x01, 0x00];
let (remainder, name) = dns_parse_name(buf, buf).unwrap();
let mut flags = DNSNameFlags::default();
let (remainder, name) = dns_parse_name(buf, buf, &mut flags).unwrap();
assert_eq!("client-cf.dropbox.com".as_bytes(), &name.value[..]);
assert_eq!(remainder, expected_remainder);
}
@ -498,7 +529,8 @@ mod tests {
// The name at offset 54 is the complete name.
let start1 = &buf[54..];
let res1 = dns_parse_name(start1, message);
let mut flags = DNSNameFlags::default();
let res1 = dns_parse_name(start1, message, &mut flags);
assert_eq!(
res1,
Ok((
@ -513,7 +545,8 @@ mod tests {
// The second name starts at offset 80, but is just a pointer
// to the first.
let start2 = &buf[80..];
let res2 = dns_parse_name(start2, message);
let mut flags = DNSNameFlags::default();
let res2 = dns_parse_name(start2, message, &mut flags);
assert_eq!(
res2,
Ok((
@ -528,7 +561,8 @@ mod tests {
// The third name starts at offset 94, but is a pointer to a
// portion of the first.
let start3 = &buf[94..];
let res3 = dns_parse_name(start3, message);
let mut flags = DNSNameFlags::default();
let res3 = dns_parse_name(start3, message, &mut flags);
assert_eq!(
res3,
Ok((
@ -543,7 +577,8 @@ mod tests {
// The fourth name starts at offset 110, but is a pointer to a
// portion of the first.
let start4 = &buf[110..];
let res4 = dns_parse_name(start4, message);
let mut flags = DNSNameFlags::default();
let res4 = dns_parse_name(start4, message, &mut flags);
assert_eq!(
res4,
Ok((
@ -584,7 +619,8 @@ mod tests {
// packet).
let start: &[u8] = &buf[100..];
let res = dns_parse_name(start, message);
let mut flags = DNSNameFlags::default();
let res = dns_parse_name(start, message, &mut flags);
assert_eq!(
res,
Ok((
@ -612,7 +648,7 @@ mod tests {
let (body, header) = dns_parse_header(pkt).unwrap();
let res = dns_parse_body(body, pkt, header);
let (rem, request) = res.unwrap();
let (rem, (request, _flags)) = res.unwrap();
// The request should be fully parsed.
assert!(rem.is_empty());
@ -674,7 +710,7 @@ mod tests {
let (body, header) = dns_parse_header(pkt).unwrap();
let res = dns_parse_body(body, pkt, header);
let (rem, request) = res.unwrap();
let (rem, (request, _flags)) = res.unwrap();
assert!(rem.is_empty());
assert_eq!(
@ -726,7 +762,7 @@ mod tests {
}
/// Parse a DNS response.
fn dns_parse_response(message: &[u8]) -> IResult<&[u8], DNSMessage> {
fn dns_parse_response(message: &[u8]) -> IResult<&[u8], (DNSMessage, DNSNameFlags)> {
let i = message;
let (i, header) = dns_parse_header(i)?;
dns_parse_body(i, message, header)
@ -751,7 +787,7 @@ mod tests {
0x00, 0x04, 0xc0, 0x00, 0x4e, 0x19, /* ....N. */
];
let (rem, response) = dns_parse_response(pkt).unwrap();
let (rem, (response, _flags)) = dns_parse_response(pkt).unwrap();
// The response should be full parsed.
assert_eq!(rem.len(), 0);
@ -839,7 +875,7 @@ mod tests {
0x00, 0x00, 0x00, 0x00, /* .... */
];
let (rem, response) = dns_parse_response(pkt).unwrap();
let (rem, (response, _flags)) = dns_parse_response(pkt).unwrap();
// The response should be fully parsed.
assert!(rem.is_empty());
@ -916,7 +952,7 @@ mod tests {
0x44, 0x03, 0xc5, 0xe9, 0x01, /* D.... */
];
let (rem, response) = dns_parse_response(pkt).unwrap();
let (rem, (response, _flags)) = dns_parse_response(pkt).unwrap();
// The response should be fully parsed.
assert_eq!(rem.len(), 0);
@ -1017,7 +1053,7 @@ mod tests {
0x67, 0x6c, 0x65, 0x03, 0x63, 0x6f, 0x6d, 0x00,
];
let (rem, response) = dns_parse_response(pkt).unwrap();
let (rem, (response, _flags)) = dns_parse_response(pkt).unwrap();
// The data should be fully parsed.
assert_eq!(rem.len(), 0);
@ -1060,7 +1096,8 @@ mod tests {
}
}
let (rem, name) = dns_parse_name(&buf, &buf).unwrap();
let mut flags = DNSNameFlags::default();
let (rem, name) = dns_parse_name(&buf, &buf, &mut flags).unwrap();
assert_eq!(name.value.len(), MAX_NAME_LEN);
assert!(name.flags.contains(DNSNameFlags::TRUNCATED));
assert!(rem.is_empty());
@ -1077,7 +1114,8 @@ mod tests {
// This should fail as we've hit the segment limit without a
// pointer, we'd need to keep parsing more segments to figure
// out where the next data point lies.
assert!(dns_parse_name(&buf, &buf).is_err());
let mut flags = DNSNameFlags::default();
assert!(dns_parse_name(&buf, &buf, &mut flags).is_err());
}
#[test]
@ -1097,7 +1135,8 @@ mod tests {
buf.push(0b1100_0000);
buf.push(0b000_0000);
let (_rem, name) = dns_parse_name(&buf[4..], &buf).unwrap();
let mut flags = DNSNameFlags::default();
let (_rem, name) = dns_parse_name(&buf[4..], &buf, &mut flags).unwrap();
assert_eq!(name.value.len(), 255);
assert!(name.flags.contains(DNSNameFlags::TRUNCATED));
}
@ -1107,6 +1146,7 @@ mod tests {
let mut buf = vec![];
buf.push(0b1100_0000);
buf.push(0b0000_0000);
assert!(dns_parse_name(&buf, &buf).is_err());
let mut flags = DNSNameFlags::default();
assert!(dns_parse_name(&buf, &buf, &mut flags).is_err());
}
}

Write Preview
Loading…
Cancel
Save