Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions doc/userguide/rules/dns-keywords.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,15 @@ dns.opcode

This keyword matches on the **opcode** found in the DNS header flags.

dns.opcode uses an :ref:`unsigned 8-bit integer <rules-integer-keywords>`.

Syntax
~~~~~~

::

dns.opcode:[!]<number>
dns.opcode:[!]<number1>-<number2>

Examples
~~~~~~~~
Expand All @@ -46,6 +49,14 @@ Match on DNS requests where the **opcode** is NOT 0::

dns.opcode:!0;

Match on DNS requests where the **opcode** is between 7 and 15, exclusively:

dns.opcode:7-15;

Match on DNS requests where the **opcode** is not between 7 and 15:

dns.opcode:!7-15;

dns.query
---------

Expand Down
2 changes: 1 addition & 1 deletion rust/src/detect/uint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub enum DetectUintMode {
DetectUintModeNegBitmask,
}

#[derive(Debug)]
#[derive(Debug, PartialEq)]
#[repr(C)]
pub struct DetectUintData<T> {
pub arg1: T,
Expand Down
167 changes: 52 additions & 115 deletions rust/src/dns/detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,49 +16,15 @@
*/

use super::dns::DNSTransaction;
use crate::core::*;
use std::ffi::CStr;
use std::os::raw::{c_char, c_void};

#[derive(Debug, PartialEq, Eq)]
pub struct DetectDnsOpcode {
negate: bool,
opcode: u8,
}

/// Parse a DNS opcode argument returning the code and if it is to be
/// negated or not.
///
/// For now only an indication that an error occurred is returned, not
/// the details of the error.
fn parse_opcode(opcode: &str) -> Result<DetectDnsOpcode, ()> {
let mut negated = false;
for (i, c) in opcode.chars().enumerate() {
match c {
' ' | '\t' => {
continue;
}
'!' => {
negated = true;
}
_ => {
let code: u8 = opcode[i..].parse().map_err(|_| ())?;
return Ok(DetectDnsOpcode {
negate: negated,
opcode: code,
});
}
}
}
Err(())
}
use crate::core::Direction;
use crate::detect::uint::{detect_match_uint, DetectUintData};

/// Perform the DNS opcode match.
///
/// 1 will be returned on match, otherwise 0 will be returned.
#[no_mangle]
pub extern "C" fn rs_dns_opcode_match(
tx: &mut DNSTransaction, detect: &mut DetectDnsOpcode, flags: u8,
tx: &mut DNSTransaction, detect: &mut DetectUintData<u8>, flags: u8,
) -> u8 {
let header_flags = if flags & Direction::ToServer as u8 != 0 {
if let Some(request) = &tx.request {
Expand All @@ -76,116 +42,87 @@ pub extern "C" fn rs_dns_opcode_match(
// Not to server or to client??
return 0;
};
let opcode = ((header_flags >> 11) & 0xf) as u8;

match_opcode(detect, header_flags).into()
}

fn match_opcode(detect: &DetectDnsOpcode, flags: u16) -> bool {
let opcode = ((flags >> 11) & 0xf) as u8;
if detect.negate {
detect.opcode != opcode
} else {
detect.opcode == opcode
}
}

#[no_mangle]
pub unsafe extern "C" fn rs_detect_dns_opcode_parse(carg: *const c_char) -> *mut c_void {
if carg.is_null() {
return std::ptr::null_mut();
}
let arg = match CStr::from_ptr(carg).to_str() {
Ok(arg) => arg,
_ => {
return std::ptr::null_mut();
}
};

match parse_opcode(arg) {
Ok(detect) => Box::into_raw(Box::new(detect)) as *mut _,
Err(_) => std::ptr::null_mut(),
}
}

#[no_mangle]
pub unsafe extern "C" fn rs_dns_detect_opcode_free(ptr: *mut c_void) {
if !ptr.is_null() {
std::mem::drop(Box::from_raw(ptr as *mut DetectDnsOpcode));
if detect_match_uint(detect, opcode) {
return 1;
}
return 0;
}

#[cfg(test)]
mod test {
use super::*;
use crate::detect::uint::{detect_parse_uint, DetectUintMode};

#[test]
fn parse_opcode_good() {
assert_eq!(
parse_opcode("1"),
Ok(DetectDnsOpcode {
negate: false,
opcode: 1
})
);
assert_eq!(
parse_opcode("123"),
Ok(DetectDnsOpcode {
negate: false,
opcode: 123
})
detect_parse_uint::<u8>("1").unwrap().1,
DetectUintData {
mode: DetectUintMode::DetectUintModeEqual,
arg1: 1,
arg2: 0,
}
);
assert_eq!(
parse_opcode("!123"),
Ok(DetectDnsOpcode {
negate: true,
opcode: 123
})
detect_parse_uint::<u8>("123").unwrap().1,
DetectUintData {
mode: DetectUintMode::DetectUintModeEqual,
arg1: 123,
arg2: 0,
}
);
assert_eq!(
parse_opcode("!123"),
Ok(DetectDnsOpcode {
negate: true,
opcode: 123
})
detect_parse_uint::<u8>("!123").unwrap().1,
DetectUintData {
mode: DetectUintMode::DetectUintModeNe,
arg1: 123,
arg2: 0,
}
);
assert_eq!(parse_opcode(""), Err(()));
assert_eq!(parse_opcode("!"), Err(()));
assert_eq!(parse_opcode("! "), Err(()));
assert_eq!(parse_opcode("!asdf"), Err(()));
assert!(detect_parse_uint::<u8>("").is_err());
assert!(detect_parse_uint::<u8>("!").is_err());
assert!(detect_parse_uint::<u8>("! ").is_err());
assert!(detect_parse_uint::<u8>("!asdf").is_err());
}

#[test]
fn test_match_opcode() {
assert!(match_opcode(
&DetectDnsOpcode {
negate: false,
opcode: 0,
assert!(detect_match_uint(
&DetectUintData {
mode: DetectUintMode::DetectUintModeEqual,
arg1: 0,
arg2: 0,
},
0b0000_0000_0000_0000,
));

assert!(!match_opcode(
&DetectDnsOpcode {
negate: true,
opcode: 0,
assert!(!detect_match_uint(
&DetectUintData {
mode: DetectUintMode::DetectUintModeNe,
arg1: 0,
arg2: 0,
},
0b0000_0000_0000_0000,
));

assert!(match_opcode(
&DetectDnsOpcode {
negate: false,
opcode: 4,
assert!(detect_match_uint(
&DetectUintData {
mode: DetectUintMode::DetectUintModeEqual,
arg1: 4,
arg2: 0,
},
0b0010_0000_0000_0000,
((0b0010_0000_0000_0000 >> 11) & 0xf) as u8,
));

assert!(!match_opcode(
&DetectDnsOpcode {
negate: true,
opcode: 4,
assert!(!detect_match_uint(
&DetectUintData {
mode: DetectUintMode::DetectUintModeNe,
arg1: 4,
arg2: 0,
},
0b0010_0000_0000_0000,
((0b0010_0000_0000_0000 >> 11) & 0xf) as u8,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have a couple of tests for range and negative range too if they're allowed for opcode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why ?

There are already tests for generic DetectUintData for range and negative range.
Feels to me like duplicating...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok sure. I just saw the other modes in the test here and thought it was missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just translated the existing unit tests, right ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep. thought that since this PR enabled range acceptance, why was that mode missing in tests when there were others. I mean this is cool with me if there are tests already. Thanks :)

));
}
}
5 changes: 3 additions & 2 deletions src/detect-dns-opcode.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "detect-parse.h"
#include "detect-engine.h"
#include "detect-engine-uint.h"
#include "detect-dns-opcode.h"
#include "rust.h"

Expand All @@ -35,7 +36,7 @@ static int DetectDnsOpcodeSetup(DetectEngineCtx *de_ctx, Signature *s,
return -1;
}

void *detect = rs_detect_dns_opcode_parse(str);
void *detect = DetectU8Parse(str);
if (detect == NULL) {
SCLogError("failed to parse dns.opcode: %s", str);
return -1;
Expand All @@ -57,7 +58,7 @@ static void DetectDnsOpcodeFree(DetectEngineCtx *de_ctx, void *ptr)
{
SCEnter();
if (ptr != NULL) {
rs_dns_detect_opcode_free(ptr);
rs_detect_u8_free(ptr);
}
SCReturn;
}
Expand Down