Skip to content

Batch backports 6.0.x/1.1: Backports of 5550, 5604 #8975

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jlucovsky wants to merge 6 commits into from
Closed

Batch backports 6.0.x/1.1: Backports of 5550, 5604 #8975

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9cac5a7
stream: update no-flow checks
victorjulien Apr 25, 2023
cecea2e
counters: make tcp stats independent of flow, ssn
victorjulien Apr 25, 2023
3ec1aaf
dns: mark test buffers with rustfmt::skip
jasonish Dec 21, 2022
3566ec0
dns: parse and alert on invalid opcodes
jasonish Dec 21, 2022
c2ec63e
dns: validate header on every incoming message
jasonish Dec 21, 2022
01c7161
dns: split header and body parsing
jasonish Dec 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rules/dns-events.rules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;)
43 changes: 43 additions & 0 deletions rust/src/applayer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,49 @@ use crate::filecontainer::FileContainer;
use crate::applayer;
use std::os::raw::{c_void,c_char,c_int};

#[repr(C)]
pub struct StreamSlice {
input: *const u8,
input_len: u32,
/// STREAM_* flags
flags: u8,
offset: u64,
}

impl StreamSlice {

/// Create a StreamSlice from a Rust slice. Useful in unit tests.
pub fn from_slice(slice: &[u8], flags: u8, offset: u64) -> Self {
Self {
input: slice.as_ptr() as *const u8,
input_len: slice.len() as u32,
flags,
offset
}
}

pub fn is_gap(&self) -> bool {
self.input.is_null() && self.input_len > 0
}
pub fn gap_size(&self) -> u32 {
self.input_len
}
pub fn as_slice(&self) -> &[u8] {
unsafe { std::slice::from_raw_parts(self.input, self.input_len as usize) }
}
pub fn is_empty(&self) -> bool {
self.input_len == 0
}
pub fn len(&self) -> u32 {
self.input_len
}
pub fn offset_from(&self, slice: &[u8]) -> u32 {
self.len() - slice.len() as u32
}
pub fn flags(&self) -> u8 {
self.flags
}
}
#[repr(C)]
#[derive(Debug,PartialEq)]
pub struct AppLayerTxConfig {
Expand Down
119 changes: 81 additions & 38 deletions rust/src/dns/dns.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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",
}
}

Expand All @@ -155,6 +157,7 @@ impl DNSEvent {
1 => Some(DNSEvent::NotRequest),
2 => Some(DNSEvent::NotResponse),
4 => Some(DNSEvent::ZFlagSet),
5 => Some(DNSEvent::InvalidOpcode),
_ => None,
}
}
Expand All @@ -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
}
}
Expand Down Expand Up @@ -496,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 {
Expand All @@ -506,6 +520,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);
Expand All @@ -517,6 +532,10 @@ impl DNSState {
self.set_event(DNSEvent::ZFlagSet);
}

if opcode >= 7 {
self.set_event(DNSEvent::InvalidOpcode);
}

return true;
}
Err(nom::Err::Incomplete(_)) => {
Expand All @@ -534,7 +553,21 @@ impl DNSState {
}
}

pub fn parse_response(&mut self, input: &[u8]) -> bool {
fn parse_request_udp(&mut self, stream_slice: StreamSlice) -> bool {
let input = stream_slice.as_slice();
Comment thread
jlucovsky marked this conversation as resolved.
self.parse_request(input, false)
}

fn parse_response_udp(&mut self, stream_slice: StreamSlice) -> bool {
let input = stream_slice.as_slice();
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)) => {

Expand All @@ -546,6 +579,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 {
Expand All @@ -562,6 +596,10 @@ impl DNSState {
self.set_event(DNSEvent::ZFlagSet);
}

if opcode >= 7 {
self.set_event(DNSEvent::InvalidOpcode);
}

return true;
}
Err(nom::Err::Incomplete(_)) => {
Expand Down Expand Up @@ -606,8 +644,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 {
Expand Down Expand Up @@ -653,8 +691,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 {
Expand Down Expand Up @@ -692,21 +730,19 @@ 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
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);
}
Expand Down Expand Up @@ -803,11 +839,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(StreamSlice::from_slice(buf, STREAM_TOSERVER, 0));
AppLayerResult::ok()
}

#[no_mangle]
Expand All @@ -821,11 +854,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(StreamSlice::from_slice(buf, STREAM_TOCLIENT, 0));
AppLayerResult::ok()
}

/// C binding parse a DNS request. Returns 1 on success, -1 on failure.
Expand Down Expand Up @@ -1200,6 +1230,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. */
Expand Down Expand Up @@ -1235,6 +1266,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. */
Expand Down Expand Up @@ -1271,6 +1303,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. */
Expand Down Expand Up @@ -1314,6 +1347,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. */
Expand Down Expand Up @@ -1359,6 +1393,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,
Expand All @@ -1373,12 +1408,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,
Expand All @@ -1392,12 +1428,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,
Expand All @@ -1411,14 +1448,15 @@ 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.
//
// 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,
Expand All @@ -1434,14 +1472,15 @@ 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.
//
// 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,
Expand All @@ -1457,12 +1496,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,
Expand Down Expand Up @@ -1557,11 +1597,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,
Expand All @@ -1574,6 +1616,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,
Expand Down
Loading