diff --git a/configure.ac b/configure.ac index 6920047abb6f..83acfbc8f927 100644 --- a/configure.ac +++ b/configure.ac @@ -268,7 +268,7 @@ CFLAGS="${CFLAGS} -DOS_WIN32" WINDOWS_PATH="yes" AC_DEFINE([HAVE_NON_POSIX_MKDIR], [1], [mkdir is not POSIX compliant: single arg]) - RUST_LDADD=" -lws2_32 -liphlpapi -lwbemuuid -lOle32 -lOleAut32 -lUuid -luserenv -lshell32 -ladvapi32 -lgcc_eh -lbcrypt" + RUST_LDADD=" -lws2_32 -liphlpapi -lwbemuuid -lOle32 -lOleAut32 -lUuid -luserenv -lshell32 -ladvapi32 -lgcc_eh -lbcrypt -lntdll" TRY_WPCAP="yes" ;; *-*-cygwin) diff --git a/rules/dns-events.rules b/rules/dns-events.rules index 0e34dae139d0..d4c02b5c2f78 100644 --- a/rules/dns-events.rules +++ b/rules/dns-events.rules @@ -7,3 +7,4 @@ alert dns any any -> any any (msg:"SURICATA DNS Not a request"; flow:to_server; alert dns any any -> any any (msg:"SURICATA DNS Not a response"; flow:to_client; app-layer-event:dns.not_a_response; classtype:protocol-command-decode; sid:2240005; rev:2;) # 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;) diff --git a/rust/src/dns/dns.rs b/rust/src/dns/dns.rs index ace6d0a39468..460c97def1e1 100644 --- a/rust/src/dns/dns.rs +++ b/rust/src/dns/dns.rs @@ -29,7 +29,7 @@ use crate::core::STREAM_TOSERVER; use crate::core::{self, AppProto, ALPROTO_UNKNOWN, IPPROTO_UDP, IPPROTO_TCP}; use crate::dns::parser; -use nom::IResult; +use nom::{Err, IResult}; use nom::number::streaming::be_u16; /// DNS record types. @@ -133,10 +133,11 @@ static mut ALPROTO_DNS: AppProto = ALPROTO_UNKNOWN; #[repr(u32)] pub enum DNSEvent { - MalformedData = 0, - NotRequest = 1, - NotResponse = 2, - ZFlagSet = 3, + MalformedData, + NotRequest, + NotResponse, + ZFlagSet, + InvalidOpcode, } impl DNSEvent { @@ -146,6 +147,7 @@ impl DNSEvent { DNSEvent::NotRequest => "NOT_A_REQUEST\0", DNSEvent::NotResponse => "NOT_A_RESPONSE\0", DNSEvent::ZFlagSet => "Z_FLAG_SET\0", + DNSEvent::InvalidOpcode => "INVALID_OPCODE\0", } } @@ -155,6 +157,7 @@ impl DNSEvent { 1 => Some(DNSEvent::NotRequest), 2 => Some(DNSEvent::NotResponse), 4 => Some(DNSEvent::ZFlagSet), + 5 => Some(DNSEvent::InvalidOpcode), _ => None, } } @@ -165,6 +168,7 @@ impl DNSEvent { "not_a_request" => Some(DNSEvent::NotRequest), "not_a_response" => Some(DNSEvent::NotRequest), "z_flag_set" => Some(DNSEvent::ZFlagSet), + "invalid_opcode" => Some(DNSEvent::InvalidOpcode), _ => None } } @@ -496,8 +500,23 @@ impl DNSState { event as u8); } - pub fn parse_request(&mut self, input: &[u8]) -> bool { - match parser::dns_parse_request(input) { + fn validate_header<'a>(&self, input: &'a [u8]) -> Option<(&'a [u8], DNSHeader)> { + if let Ok((body, header)) = parser::dns_parse_header(input) { + if probe_header_validity(&header, input.len()).0 { + return Some((body, header)); + } + } + None + } + + fn parse_request(&mut self, input: &[u8], is_tcp: bool) -> bool { + let (body, header) = if let Some((body, header)) = self.validate_header(input) { + (body, header) + } else { + return !is_tcp; + }; + + match parser::dns_parse_request_body(body, input, header) { Ok((_, request)) => { if request.header.flags & 0x8000 != 0 { SCLogDebug!("DNS message is not a request"); @@ -506,6 +525,7 @@ impl DNSState { } let z_flag = request.header.flags & 0x0040 != 0; + let opcode = ((request.header.flags >> 11) & 0xf) as u8; let mut tx = self.new_tx(); tx.request = Some(request); @@ -517,6 +537,10 @@ impl DNSState { self.set_event(DNSEvent::ZFlagSet); } + if opcode >= 7 { + self.set_event(DNSEvent::InvalidOpcode); + } + return true; } Err(nom::Err::Incomplete(_)) => { @@ -534,8 +558,22 @@ impl DNSState { } } - pub fn parse_response(&mut self, input: &[u8]) -> bool { - match parser::dns_parse_response(input) { + 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 { + let (body, header) = if let Some((body, header)) = self.validate_header(input) { + (body, header) + } else { + return !is_tcp; + }; + + match parser::dns_parse_response_body(body, input, header) { Ok((_, response)) => { SCLogDebug!("Response header flags: {}", response.header.flags); @@ -546,6 +584,7 @@ impl DNSState { } let z_flag = response.header.flags & 0x0040 != 0; + let opcode = ((response.header.flags >> 11) & 0xf) as u8; let mut tx = self.new_tx(); if let Some(ref mut config) = &mut self.config { @@ -562,6 +601,10 @@ impl DNSState { self.set_event(DNSEvent::ZFlagSet); } + if opcode >= 7 { + self.set_event(DNSEvent::InvalidOpcode); + } + return true; } Err(nom::Err::Incomplete(_)) => { @@ -606,8 +649,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 { @@ -653,8 +696,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 { @@ -691,22 +734,20 @@ impl DNSState { const DNS_HEADER_SIZE: usize = 12; -fn probe_header_validity(header: DNSHeader, rlen: usize) -> (bool, bool, bool) { - let opcode = ((header.flags >> 11) & 0xf) as u8; - if opcode >= 7 { - //unassigned opcode - return (false, false, false); - } - 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 +fn probe_header_validity(header: &DNSHeader, rlen: usize) -> (bool, bool, bool) { + 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); } @@ -722,7 +763,7 @@ fn probe(input: &[u8], dlen: usize) -> (bool, bool, bool) { // parse a complete message, so perform header validation only. if input.len() < dlen { if let Ok((_, header)) = parser::dns_parse_header(input) { - return probe_header_validity(header, dlen); + return probe_header_validity(&header, dlen); } else { return (false, false, false); } @@ -730,17 +771,15 @@ 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(nom::Err::Incomplete(_)) => { - match parser::dns_parse_header(input) { - Ok((_, header)) => { - return probe_header_validity(header, dlen); - } - Err(nom::Err::Incomplete(_)) => (false, false, true), - Err(_) => (false, false, false), - } + return probe_header_validity(&request.header, dlen); } + Err(Err::Incomplete(_)) => match parser::dns_parse_header(input) { + Ok((_, header)) => { + return probe_header_validity(&header, dlen); + } + Err(Err::Incomplete(_)) => (false, false, true), + Err(_) => (false, false, false), + }, Err(_) => (false, false, false), } } @@ -803,11 +842,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] @@ -821,11 +857,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. @@ -1200,6 +1233,7 @@ mod tests { fn test_dns_parse_request_tcp_valid() { // A UDP DNS request with the DNS payload starting at byte 42. // From pcap: https://github.com/jasonish/suricata-verify/blob/7cc0e1bd0a5249b52e6e87d82d57c0b6aaf75fce/dns-udp-dig-a-www-suricata-ids-org/dig-a-www.suricata-ids.org.pcap + #[rustfmt::skip] let buf: &[u8] = &[ 0x00, 0x15, 0x17, 0x0d, 0x06, 0xf7, 0xd8, 0xcb, /* ........ */ 0x8a, 0xed, 0xa1, 0x46, 0x08, 0x00, 0x45, 0x00, /* ...F..E. */ @@ -1235,6 +1269,7 @@ mod tests { fn test_dns_parse_request_tcp_short_payload() { // A UDP DNS request with the DNS payload starting at byte 42. // From pcap: https://github.com/jasonish/suricata-verify/blob/7cc0e1bd0a5249b52e6e87d82d57c0b6aaf75fce/dns-udp-dig-a-www-suricata-ids-org/dig-a-www.suricata-ids.org.pcap + #[rustfmt::skip] let buf: &[u8] = &[ 0x00, 0x15, 0x17, 0x0d, 0x06, 0xf7, 0xd8, 0xcb, /* ........ */ 0x8a, 0xed, 0xa1, 0x46, 0x08, 0x00, 0x45, 0x00, /* ...F..E. */ @@ -1271,6 +1306,7 @@ mod tests { fn test_dns_parse_response_tcp_valid() { // A UDP DNS response with the DNS payload starting at byte 42. // From pcap: https://github.com/jasonish/suricata-verify/blob/7cc0e1bd0a5249b52e6e87d82d57c0b6aaf75fce/dns-udp-dig-a-www-suricata-ids-org/dig-a-www.suricata-ids.org.pcap + #[rustfmt::skip] let buf: &[u8] = &[ 0xd8, 0xcb, 0x8a, 0xed, 0xa1, 0x46, 0x00, 0x15, /* .....F.. */ 0x17, 0x0d, 0x06, 0xf7, 0x08, 0x00, 0x45, 0x00, /* ......E. */ @@ -1314,6 +1350,7 @@ mod tests { fn test_dns_parse_response_tcp_short_payload() { // A UDP DNS response with the DNS payload starting at byte 42. // From pcap: https://github.com/jasonish/suricata-verify/blob/7cc0e1bd0a5249b52e6e87d82d57c0b6aaf75fce/dns-udp-dig-a-www-suricata-ids-org/dig-a-www.suricata-ids.org.pcap + #[rustfmt::skip] let buf: &[u8] = &[ 0xd8, 0xcb, 0x8a, 0xed, 0xa1, 0x46, 0x00, 0x15, /* .....F.. */ 0x17, 0x0d, 0x06, 0xf7, 0x08, 0x00, 0x45, 0x00, /* ......E. */ @@ -1359,6 +1396,7 @@ mod tests { * TTL: 86400 * serial 20130422 refresh 28800 retry 7200 exp 604800 min ttl 86400 * ns, hostmaster */ + #[rustfmt::skip] let buf: &[u8] = &[ 0x00, 0x3c, 0x85, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x0b, 0x61, 0x62, 0x63, @@ -1373,12 +1411,13 @@ 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. #[test] fn test_dns_udp_parser_test_02() { + #[rustfmt::skip] let buf: &[u8] = &[ 0x6D,0x08,0x84,0x80,0x00,0x01,0x00,0x08,0x00,0x00,0x00,0x01,0x03,0x57,0x57,0x57, 0x04,0x54,0x54,0x54,0x54,0x03,0x56,0x56,0x56,0x03,0x63,0x6F,0x6D,0x02,0x79,0x79, @@ -1392,12 +1431,13 @@ 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. #[test] fn test_dns_udp_parser_test_03() { + #[rustfmt::skip] let buf: &[u8] = &[ 0x6F,0xB4,0x84,0x80,0x00,0x01,0x00,0x02,0x00,0x02,0x00,0x03,0x03,0x57,0x57,0x77, 0x0B,0x56,0x56,0x56,0x56,0x56,0x56,0x56,0x56,0x56,0x56,0x56,0x03,0x55,0x55,0x55, @@ -1411,7 +1451,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. @@ -1419,6 +1459,7 @@ mod tests { // Test the TXT records in an answer. #[test] fn test_dns_udp_parser_test_04() { + #[rustfmt::skip] let buf: &[u8] = &[ 0xc2,0x2f,0x81,0x80,0x00,0x01,0x00,0x01,0x00,0x01,0x00,0x01,0x0a,0x41,0x41,0x41, 0x41,0x41,0x4f,0x31,0x6b,0x51,0x41,0x05,0x3d,0x61,0x75,0x74,0x68,0x03,0x73,0x72, @@ -1434,7 +1475,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. @@ -1442,6 +1483,7 @@ mod tests { // Test TXT records in answer with a bad length. #[test] fn test_dns_udp_parser_test_05() { + #[rustfmt::skip] let buf: &[u8] = &[ 0xc2,0x2f,0x81,0x80,0x00,0x01,0x00,0x01,0x00,0x01,0x00,0x01,0x0a,0x41,0x41,0x41, 0x41,0x41,0x4f,0x31,0x6b,0x51,0x41,0x05,0x3d,0x61,0x75,0x74,0x68,0x03,0x73,0x72, @@ -1457,12 +1499,13 @@ 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. #[test] fn test_dns_tcp_parser_multi_record() { + #[rustfmt::skip] let buf: &[u8] = &[ 0x00, 0x1e, 0x00, 0x00, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x30, @@ -1557,11 +1600,13 @@ mod tests { #[test] fn test_dns_tcp_parser_split_payload() { /* incomplete payload */ + #[rustfmt::skip] let buf1: &[u8] = &[ 0x00, 0x1c, 0x10, 0x32, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 ]; /* complete payload plus the start of a new payload */ + #[rustfmt::skip] let buf2: &[u8] = &[ 0x00, 0x1c, 0x10, 0x32, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -1574,6 +1619,7 @@ mod tests { ]; /* and the complete payload again with no trailing data. */ + #[rustfmt::skip] let buf3: &[u8] = &[ 0x00, 0x1c, 0x10, 0x32, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, diff --git a/rust/src/dns/log.rs b/rust/src/dns/log.rs index bba983873ed5..27abab245afc 100644 --- a/rust/src/dns/log.rs +++ b/rust/src/dns/log.rs @@ -488,10 +488,12 @@ fn dns_log_json_answer(js: &mut JsonBuilder, response: &DNSResponse, flags: u64) js.set_bool("z", true)?; } - for query in &response.queries { + let opcode = ((header.flags >> 11) & 0xf) as u8; + js.set_uint("opcode", opcode as u64)?; + + if let Some(query) = response.queries.first() { js.set_string_from_bytes("rrname", &query.name)?; js.set_string("rrtype", &dns_rrtype_string(query.rrtype))?; - break; } js.set_string("rcode", &dns_rcode_string(header.flags))?; @@ -602,6 +604,8 @@ fn dns_log_query(tx: &mut DNSTransaction, if request.header.flags & 0x0040 != 0 { jb.set_bool("z", true)?; } + let opcode = ((request.header.flags >> 11) & 0xf) as u8; + jb.set_uint("opcode", opcode as u64)?; return Ok(true); } } diff --git a/rust/src/dns/parser.rs b/rust/src/dns/parser.rs index d25e0ad23310..dd16c8f49b55 100644 --- a/rust/src/dns/parser.rs +++ b/rust/src/dns/parser.rs @@ -20,6 +20,7 @@ use nom::IResult; use nom::combinator::rest; use nom::error::ErrorKind; +use nom::multi::count; use nom::number::streaming::{be_u8, be_u16, be_u32}; use nom; use crate::dns::dns::*; @@ -50,10 +51,8 @@ named!(pub dns_parse_header, /// /// Parameters: /// start: the start of the name -/// message: the complete message that start is a part of -pub fn dns_parse_name<'a, 'b>(start: &'b [u8], - message: &'b [u8]) - -> IResult<&'b [u8], Vec> { +/// message: the complete message that start is a part of with the DNS header +pub fn dns_parse_name<'b>(start: &'b [u8], message: &'b [u8]) -> IResult<&'b [u8], Vec> { let mut pos = start; let mut pivot = start; let mut name: Vec = Vec::with_capacity(32); @@ -197,26 +196,27 @@ fn dns_parse_answer<'a>(slice: &'a [u8], message: &'a [u8], count: usize) /// Parse a DNS response. -pub fn dns_parse_response<'a>(slice: &'a [u8]) - -> IResult<&[u8], DNSResponse> { - do_parse!( - slice, - header: dns_parse_header - >> queries: count!( - call!(dns_parse_query, slice), header.questions as usize) - >> answers: call!( - dns_parse_answer, slice, header.answer_rr as usize) - >> authorities: call!( - dns_parse_answer, slice, header.authority_rr as usize) - >> ( - DNSResponse{ - header: header, - queries: queries, - answers: answers, - authorities: authorities, - } - ) - ) +pub fn dns_parse_response(message: &[u8]) -> IResult<&[u8], DNSResponse> { + let i = message; + let (i, header) = dns_parse_header(i)?; + dns_parse_response_body(i, message, header) +} + +pub fn dns_parse_response_body<'a>( + i: &'a [u8], message: &'a [u8], header: DNSHeader, +) -> IResult<&'a [u8], DNSResponse> { + 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)?; + Ok(( + i, + DNSResponse { + header, + queries, + answers, + authorities, + }, + )) } /// Parse a single DNS query. @@ -344,19 +344,18 @@ pub fn dns_parse_rdata<'a>(input: &'a [u8], message: &'a [u8], rrtype: u16) } /// Parse a DNS request. -pub fn dns_parse_request<'a>(input: &'a [u8]) -> IResult<&[u8], DNSRequest> { - do_parse!( - input, - header: dns_parse_header >> - queries: count!(call!(dns_parse_query, input), - header.questions as usize) >> - ( - DNSRequest{ - header: header, - queries: queries, - } - ) - ) +pub fn dns_parse_request(input: &[u8]) -> IResult<&[u8], DNSRequest> { + let i = input; + let (i, header) = dns_parse_header(i)?; + dns_parse_request_body(i, input, header) +} + +pub fn dns_parse_request_body<'a>( + input: &'a [u8], message: &'a [u8], header: DNSHeader, +) -> IResult<&'a [u8], DNSRequest> { + let i = input; + let (i, queries) = count(|b| dns_parse_query(b, message), header.questions as usize)(i)?; + Ok((i, DNSRequest { header, queries })) } #[cfg(test)] diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c index caee50ba7497..fee52a3e61bb 100644 --- a/src/app-layer-ftp.c +++ b/src/app-layer-ftp.c @@ -353,14 +353,15 @@ static void FTPTransactionFree(FTPTransaction *tx) FTPFree(tx, sizeof(*tx)); } -static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state) +static int FTPGetLineForDirection( + FtpState *state, FtpLineState *line_state, bool *current_line_truncated) { void *ptmp; if (line_state->current_line_lf_seen == 1) { /* we have seen the lf for the previous line. Clear the parser * details to parse new line */ line_state->current_line_lf_seen = 0; - state->current_line_truncated = false; + *current_line_truncated = false; if (line_state->current_line_db == 1) { line_state->current_line_db = 0; FTPFree(line_state->db, line_state->db_len); @@ -387,7 +388,7 @@ static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state) int32_t input_len = state->input_len; if ((uint32_t)input_len > ftp_max_line_len) { input_len = ftp_max_line_len; - state->current_line_truncated = true; + *current_line_truncated = true; } line_state->db = FTPMalloc(input_len); if (line_state->db == NULL) { @@ -396,12 +397,12 @@ static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state) line_state->current_line_db = 1; memcpy(line_state->db, state->input, input_len); line_state->db_len = input_len; - } else if (!state->current_line_truncated) { + } else if (!*current_line_truncated) { int32_t input_len = state->input_len; if (line_state->db_len + input_len > ftp_max_line_len) { input_len = ftp_max_line_len - line_state->db_len; DEBUG_VALIDATE_BUG_ON(input_len < 0); - state->current_line_truncated = true; + *current_line_truncated = true; } if (input_len > 0) { ptmp = FTPRealloc( @@ -427,12 +428,12 @@ static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state) line_state->current_line_lf_seen = 1; if (line_state->current_line_db == 1) { - if (!state->current_line_truncated) { + if (!*current_line_truncated) { int32_t input_len = lf_idx + 1 - state->input; if (line_state->db_len + input_len > ftp_max_line_len) { input_len = ftp_max_line_len - line_state->db_len; DEBUG_VALIDATE_BUG_ON(input_len < 0); - state->current_line_truncated = true; + *current_line_truncated = true; } if (input_len > 0) { ptmp = FTPRealloc( @@ -465,17 +466,18 @@ static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state) state->current_line = state->input; if (lf_idx - state->input > ftp_max_line_len) { state->current_line_len = ftp_max_line_len; - state->current_line_truncated = true; + *current_line_truncated = true; } else { state->current_line_len = lf_idx - state->input; } - if (state->input != lf_idx && - *(lf_idx - 1) == 0x0D) { - state->current_line_len--; - state->current_line_delimiter_len = 2; - } else { - state->current_line_delimiter_len = 1; + if (!*current_line_truncated) { + if (state->input != lf_idx && *(lf_idx - 1) == 0x0D) { + state->current_line_len--; + state->current_line_delimiter_len = 2; + } else { + state->current_line_delimiter_len = 1; + } } } @@ -497,9 +499,11 @@ static int FTPGetLine(FtpState *state) /* toserver */ if (state->direction == 0) - return FTPGetLineForDirection(state, &state->line_state[0]); + return FTPGetLineForDirection( + state, &state->line_state[0], &state->current_line_truncated_ts); else - return FTPGetLineForDirection(state, &state->line_state[1]); + return FTPGetLineForDirection( + state, &state->line_state[1], &state->current_line_truncated_tc); } /** @@ -631,7 +635,7 @@ static AppLayerResult FTPParseRequest(Flow *f, void *ftp_state, tx->command_descriptor = cmd_descriptor; tx->request_length = CopyCommandLine(&tx->request, state->current_line, state->current_line_len); - tx->request_truncated = state->current_line_truncated; + tx->request_truncated = state->current_line_truncated_ts; /* change direction (default to server) so expectation will handle * the correct message when expectation will match. @@ -856,7 +860,7 @@ static AppLayerResult FTPParseResponse(Flow *f, void *ftp_state, AppLayerParserS FTPString *response = FTPStringAlloc(); if (likely(response)) { response->len = CopyCommandLine(&response->str, state->current_line, state->current_line_len); - response->truncated = state->current_line_truncated; + response->truncated = state->current_line_truncated_tc; TAILQ_INSERT_TAIL(&tx->response_list, response, next); } } diff --git a/src/app-layer-ftp.h b/src/app-layer-ftp.h index af975bd14d50..716cee7e921d 100644 --- a/src/app-layer-ftp.h +++ b/src/app-layer-ftp.h @@ -177,7 +177,8 @@ typedef struct FtpState_ { /** length of the line in current_line. Doesn't include the delimiter */ uint32_t current_line_len; uint8_t current_line_delimiter_len; - bool current_line_truncated; + bool current_line_truncated_ts; + bool current_line_truncated_tc; /* 0 for toserver, 1 for toclient */ FtpLineState line_state[2]; diff --git a/src/decode-tcp.c b/src/decode-tcp.c index 84d7595cffdd..fee390fbb4a9 100644 --- a/src/decode-tcp.c +++ b/src/decode-tcp.c @@ -259,6 +259,15 @@ int DecodeTCP(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, return TM_ECODE_FAILED; } + /* update counters */ + if ((p->tcph->th_flags & (TH_SYN | TH_ACK)) == (TH_SYN | TH_ACK)) { + StatsIncr(tv, dtv->counter_tcp_synack); + } else if (p->tcph->th_flags & (TH_SYN)) { + StatsIncr(tv, dtv->counter_tcp_syn); + } + if (p->tcph->th_flags & (TH_RST)) { + StatsIncr(tv, dtv->counter_tcp_rst); + } #ifdef DEBUG SCLogDebug("TCP sp: %" PRIu32 " -> dp: %" PRIu32 " - HLEN: %" PRIu32 " LEN: %" PRIu32 " %s%s%s%s%s%s", GET_TCP_SRC_PORT(p), GET_TCP_DST_PORT(p), TCP_GET_HLEN(p), len, diff --git a/src/decode.c b/src/decode.c index 243dce422c07..574f91451a0f 100644 --- a/src/decode.c +++ b/src/decode.c @@ -537,6 +537,11 @@ void DecodeRegisterPerfCounters(DecodeThreadVars *dtv, ThreadVars *tv) dtv->counter_null = StatsRegisterCounter("decoder.null", tv); dtv->counter_sll = StatsRegisterCounter("decoder.sll", tv); dtv->counter_tcp = StatsRegisterCounter("decoder.tcp", tv); + + dtv->counter_tcp_syn = StatsRegisterCounter("tcp.syn", tv); + dtv->counter_tcp_synack = StatsRegisterCounter("tcp.synack", tv); + dtv->counter_tcp_rst = StatsRegisterCounter("tcp.rst", tv); + dtv->counter_udp = StatsRegisterCounter("decoder.udp", tv); dtv->counter_sctp = StatsRegisterCounter("decoder.sctp", tv); dtv->counter_icmpv4 = StatsRegisterCounter("decoder.icmpv4", tv); diff --git a/src/decode.h b/src/decode.h index ded72485787b..3c160c220ae7 100644 --- a/src/decode.h +++ b/src/decode.h @@ -677,6 +677,9 @@ typedef struct DecodeThreadVars_ uint16_t counter_ipv4; uint16_t counter_ipv6; uint16_t counter_tcp; + uint16_t counter_tcp_syn; + uint16_t counter_tcp_synack; + uint16_t counter_tcp_rst; uint16_t counter_udp; uint16_t counter_icmpv4; uint16_t counter_icmpv6; diff --git a/src/stream-tcp.c b/src/stream-tcp.c index 2a921b31df1a..dc6d8f1e20cb 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -5085,16 +5085,6 @@ int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt, } } - /* update counters */ - if ((p->tcph->th_flags & (TH_SYN|TH_ACK)) == (TH_SYN|TH_ACK)) { - StatsIncr(tv, stt->counter_tcp_synack); - } else if (p->tcph->th_flags & (TH_SYN)) { - StatsIncr(tv, stt->counter_tcp_syn); - } - if (p->tcph->th_flags & (TH_RST)) { - StatsIncr(tv, stt->counter_tcp_rst); - } - /* broken TCP http://ask.wireshark.org/questions/3183/acknowledgment-number-broken-tcp-the-acknowledge-field-is-nonzero-while-the-ack-flag-is-not-set */ if (!(p->tcph->th_flags & TH_ACK) && TCP_GET_ACK(p) != 0) { StreamTcpSetEvent(p, STREAM_PKT_BROKEN_ACK); @@ -5476,6 +5466,11 @@ int TcpSessionPacketSsnReuse(const Packet *p, const Flow *f, const void *tcp_ssn TmEcode StreamTcp (ThreadVars *tv, Packet *p, void *data, PacketQueueNoLock *pq) { + DEBUG_VALIDATE_BUG_ON(p->flow == NULL); + if (unlikely(p->flow == NULL)) { + return TM_ECODE_OK; + } + StreamTcpThread *stt = (StreamTcpThread *)data; SCLogDebug("p->pcap_cnt %"PRIu64, p->pcap_cnt); @@ -5487,11 +5482,6 @@ TmEcode StreamTcp (ThreadVars *tv, Packet *p, void *data, PacketQueueNoLock *pq) return TM_ECODE_OK; } - if (p->flow == NULL) { - StatsIncr(tv, stt->counter_tcp_no_flow); - return TM_ECODE_OK; - } - HandleThreadId(tv, p, stt); /* only TCP packets with a flow from here */ @@ -5531,10 +5521,6 @@ TmEcode StreamTcpThreadInit(ThreadVars *tv, void *initdata, void **data) stt->counter_tcp_pseudo = StatsRegisterCounter("tcp.pseudo", tv); stt->counter_tcp_pseudo_failed = StatsRegisterCounter("tcp.pseudo_failed", tv); stt->counter_tcp_invalid_checksum = StatsRegisterCounter("tcp.invalid_checksum", tv); - stt->counter_tcp_no_flow = StatsRegisterCounter("tcp.no_flow", tv); - stt->counter_tcp_syn = StatsRegisterCounter("tcp.syn", tv); - stt->counter_tcp_synack = StatsRegisterCounter("tcp.synack", tv); - stt->counter_tcp_rst = StatsRegisterCounter("tcp.rst", tv); stt->counter_tcp_midstream_pickups = StatsRegisterCounter("tcp.midstream_pickups", tv); stt->counter_tcp_wrong_thread = StatsRegisterCounter("tcp.pkt_on_wrong_thread", tv); diff --git a/src/stream-tcp.h b/src/stream-tcp.h index 48b32a4fb6e3..d071d95ec391 100644 --- a/src/stream-tcp.h +++ b/src/stream-tcp.h @@ -91,16 +91,8 @@ typedef struct StreamTcpThread_ { uint16_t counter_tcp_pseudo_failed; /** packets rejected because their csum is invalid */ uint16_t counter_tcp_invalid_checksum; - /** TCP packets with no associated flow */ - uint16_t counter_tcp_no_flow; /** sessions reused */ uint16_t counter_tcp_reused_ssn; - /** syn pkts */ - uint16_t counter_tcp_syn; - /** syn/ack pkts */ - uint16_t counter_tcp_synack; - /** rst pkts */ - uint16_t counter_tcp_rst; /** midstream pickups */ uint16_t counter_tcp_midstream_pickups; /** wrong thread */