From cef0ff4b030ef061aa0ca2a5b234214345380161 Mon Sep 17 00:00:00 2001 From: Philip Akekudaga Date: Mon, 21 Sep 2026 10:08:52 -0400 Subject: [PATCH 1/7] rust/derive: allow enum_string_style on u16 enums The EnumStringU8 and EnumStringU32 derives both register the suricata attribute, but EnumStringU16 does not, so a u16 enumeration cannot select a string style even though stringenum.rs handles every width the same way. Register the attribute for u16 as well. Ticket: #8131. --- rust/derive/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/derive/src/lib.rs b/rust/derive/src/lib.rs index 6b6c1b4e01..af1ba46025 100644 --- a/rust/derive/src/lib.rs +++ b/rust/derive/src/lib.rs @@ -64,7 +64,7 @@ pub fn derive_enum_string_u8(input: TokenStream) -> TokenStream { stringenum::derive_enum_string::(input, "u8") } -#[proc_macro_derive(EnumStringU16, attributes(name))] +#[proc_macro_derive(EnumStringU16, attributes(name, suricata))] pub fn derive_enum_string_u16(input: TokenStream) -> TokenStream { stringenum::derive_enum_string::(input, "u16") } From bde116fa891ba9b8c244c38ea1a91eaabe47597e Mon Sep 17 00:00:00 2001 From: Philip Akekudaga Date: Wed, 14 Oct 2026 14:42:39 -0400 Subject: [PATCH 2/7] rust/derive: add PascalCase enum string style A PascalCase style logs the variant name as it is written, for enumerations whose names are logged that way by another library, such as the modbus function codes logged by sawp. Parsing stays case insensitive. Ticket: #8131. --- rust/derive/src/stringenum.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/rust/derive/src/stringenum.rs b/rust/derive/src/stringenum.rs index 78f986faac..3687d447e7 100644 --- a/rust/derive/src/stringenum.rs +++ b/rust/derive/src/stringenum.rs @@ -48,9 +48,13 @@ The enum can have a suricata attribute, which is a list of named values - enum_string_style can be: - UPPERCASE : uppercase is used for logging and parsing - LOG_UPPERCASE : uppercase is only used for logging, snake_case for parsing + - PascalCase : the variant name is used as is for logging, parsing is case insensitive With UPPERCASE ExampleEnum::BestValueEver.to_str() is "BESTVALUEEVER" (without underscores) + +With PascalCase +ExampleEnum::BestValueEver.to_str() is "BestValueEver" */ extern crate proc_macro; @@ -108,6 +112,11 @@ where names_upper.push(v.ident.to_string().to_ascii_uppercase()); names_lower.push(v.ident.to_string().to_ascii_lowercase()); } + "PascalCase" => { + names.push(v.ident.to_string()); + names_upper.push(v.ident.to_string().to_ascii_uppercase()); + names_lower.push(v.ident.to_string().to_ascii_lowercase()); + } "LOG_UPPERCASE" => { names.push(v.ident.to_string().to_ascii_uppercase()); let fname = transform_name(&v.ident.to_string()); From d9dcc225d05cb1771719c5d21c51f8ede162d8ec Mon Sep 17 00:00:00 2001 From: Philip Akekudaga Date: Fri, 31 Jul 2026 14:41:40 -0400 Subject: [PATCH 3/7] modbus: add MBAP header keywords Add uint keywords for the Modbus MBAP header values already present in the EVE log: modbus.unit_id, modbus.transaction_id and modbus.protocol_id. They follow the ENIP keyword pattern: uint matching with ranges and negation, inspecting the request in to-server direction and the response in to-client direction. Ticket: #8131. --- rust/src/modbus/detect.rs | 205 +++++++++++++++++++++++++++++++++++++- 1 file changed, 202 insertions(+), 3 deletions(-) diff --git a/rust/src/modbus/detect.rs b/rust/src/modbus/detect.rs index 203442f65f..3177397fa7 100644 --- a/rust/src/modbus/detect.rs +++ b/rust/src/modbus/detect.rs @@ -15,9 +15,14 @@ * 02110-1301, USA. */ -use super::modbus::ModbusTransaction; -use super::modbus::ALPROTO_MODBUS; -use crate::core::STREAM_TOSERVER; +use super::modbus::{ModbusTransaction, ALPROTO_MODBUS}; +use crate::core::{STREAM_TOCLIENT, STREAM_TOSERVER}; +use crate::detect::uint::{ + DetectUintData, SCDetectU16Free, SCDetectU16Match, SCDetectU16Parse, SCDetectU8Free, + SCDetectU8Match, SCDetectU8Parse, +}; +use crate::detect::{SIGMATCH_INFO_UINT16, SIGMATCH_INFO_UINT8}; +use crate::direction::Direction; use lazy_static::lazy_static; use regex::Regex; use sawp_modbus::{AccessType, CodeCategory, Data, Flags, FunctionCode, Message}; @@ -498,6 +503,152 @@ unsafe extern "C" fn modbus_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { std::mem::drop(Box::from_raw(ctx)); } +static mut G_MODBUS_UNIT_ID_KW_ID: u16 = 0; +static mut G_MODBUS_UNIT_ID_BUFFER_ID: c_int = 0; +static mut G_MODBUS_TRANSACTION_ID_KW_ID: u16 = 0; +static mut G_MODBUS_TRANSACTION_ID_BUFFER_ID: c_int = 0; +static mut G_MODBUS_PROTOCOL_ID_KW_ID: u16 = 0; +static mut G_MODBUS_PROTOCOL_ID_BUFFER_ID: c_int = 0; + +/// Get the message matching the direction of the packet under inspection: +/// the request for to-server, the response for to-client. +fn tx_get_message(tx: &ModbusTransaction, direction: u8) -> Option<&Message> { + let direction: Direction = direction.into(); + if direction == Direction::ToServer { + return tx.request.as_ref(); + } + return tx.response.as_ref(); +} + +unsafe extern "C" fn unit_id_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = SCDetectU8Parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_UNIT_ID_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_UNIT_ID_BUFFER_ID, + ) + .is_null() + { + unit_id_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn unit_id_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(msg) = tx_get_message(tx, flags) { + return SCDetectU8Match(msg.unit_id, ctx); + } + return 0; +} + +unsafe extern "C" fn unit_id_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU8Free(ctx); +} + +unsafe extern "C" fn transaction_id_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = SCDetectU16Parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_TRANSACTION_ID_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_TRANSACTION_ID_BUFFER_ID, + ) + .is_null() + { + transaction_id_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn transaction_id_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(msg) = tx_get_message(tx, flags) { + return SCDetectU16Match(msg.transaction_id, ctx); + } + return 0; +} + +unsafe extern "C" fn transaction_id_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU16Free(ctx); +} + +unsafe extern "C" fn protocol_id_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = SCDetectU16Parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_PROTOCOL_ID_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_PROTOCOL_ID_BUFFER_ID, + ) + .is_null() + { + protocol_id_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn protocol_id_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(msg) = tx_get_message(tx, flags) { + return SCDetectU16Match(msg.protocol_id, ctx); + } + return 0; +} + +unsafe extern "C" fn protocol_id_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU16Free(ctx); +} + #[no_mangle] pub unsafe extern "C" fn SCDetectModbusRegister() { let kw = SCSigTableAppLiteElmt { @@ -516,6 +667,54 @@ pub unsafe extern "C" fn SCDetectModbusRegister() { STREAM_TOSERVER, 0, ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.unit_id\0".as_ptr() as *const c_char, + desc: b"match on Modbus unit identifier\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-unit-id\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(unit_id_match), + Setup: Some(unit_id_setup), + Free: Some(unit_id_free), + flags: SIGMATCH_INFO_UINT8, + }; + G_MODBUS_UNIT_ID_KW_ID = SCDetectHelperKeywordRegister(&kw); + G_MODBUS_UNIT_ID_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.unit_id\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.transaction_id\0".as_ptr() as *const c_char, + desc: b"match on Modbus transaction identifier\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-transaction-id\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(transaction_id_match), + Setup: Some(transaction_id_setup), + Free: Some(transaction_id_free), + flags: SIGMATCH_INFO_UINT16, + }; + G_MODBUS_TRANSACTION_ID_KW_ID = SCDetectHelperKeywordRegister(&kw); + G_MODBUS_TRANSACTION_ID_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.transaction_id\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.protocol_id\0".as_ptr() as *const c_char, + desc: b"match on Modbus protocol identifier\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-protocol-id\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(protocol_id_match), + Setup: Some(protocol_id_setup), + Free: Some(protocol_id_free), + flags: SIGMATCH_INFO_UINT16, + }; + G_MODBUS_PROTOCOL_ID_KW_ID = SCDetectHelperKeywordRegister(&kw); + G_MODBUS_PROTOCOL_ID_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.protocol_id\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); } #[cfg(test)] From d990620d2adc55858bb5b51ac475a1ff27aa322e Mon Sep 17 00:00:00 2001 From: Philip Akekudaga Date: Mon, 21 Sep 2026 10:16:28 -0400 Subject: [PATCH 4/7] modbus: add function code keywords Add uint keywords for the Modbus function-level values already present in the EVE log: modbus.function (function code), modbus.subfunction (diagnostic subfunction code) and modbus.exception_code (exception code in error responses). They match numerically, with ranges and negation, and also accept the code names that the EVE log prints in function_code, diagnostic.code and exception.code, so a name read from a log can be used as is in a rule. Names are matched case insensitively and can be negated. The catch-all variants of the sawp enumerations are deliberately left out of the names: sawp assigns them to any value it cannot decode, but their discriminants collide with real codes, so accepting them by name would match something other than what the name says. Error responses carry the request function code with its highest bit set, while the logged name is derived from the code without that bit. A name therefore does not match an error response; the numeric form does. Ticket: #8131. --- rust/src/modbus/detect.rs | 492 +++++++++++++++++++++++++++++++++++++- 1 file changed, 487 insertions(+), 5 deletions(-) diff --git a/rust/src/modbus/detect.rs b/rust/src/modbus/detect.rs index 3177397fa7..170a90d291 100644 --- a/rust/src/modbus/detect.rs +++ b/rust/src/modbus/detect.rs @@ -18,11 +18,14 @@ use super::modbus::{ModbusTransaction, ALPROTO_MODBUS}; use crate::core::{STREAM_TOCLIENT, STREAM_TOSERVER}; use crate::detect::uint::{ - DetectUintData, SCDetectU16Free, SCDetectU16Match, SCDetectU16Parse, SCDetectU8Free, - SCDetectU8Match, SCDetectU8Parse, + detect_parse_uint_enum, DetectUintData, SCDetectU16Free, SCDetectU16Match, SCDetectU16Parse, + SCDetectU8Free, SCDetectU8Match, SCDetectU8Parse, +}; +use crate::detect::{ + EnumString, SIGMATCH_INFO_ENUM_UINT, SIGMATCH_INFO_UINT16, SIGMATCH_INFO_UINT8, }; -use crate::detect::{SIGMATCH_INFO_UINT16, SIGMATCH_INFO_UINT8}; use crate::direction::Direction; +use crate::jsonbuilder::JsonBuilder; use lazy_static::lazy_static; use regex::Regex; use sawp_modbus::{AccessType, CodeCategory, Data, Flags, FunctionCode, Message}; @@ -32,8 +35,9 @@ use std::os::raw::{c_char, c_int, c_void}; use std::str::FromStr; use suricata_sys::sys::{ AppProto, DetectEngineCtx, DetectEngineThreadCtx, Flow, SCDetectHelperBufferProgressRegister, - SCDetectHelperKeywordRegister, SCDetectSignatureSetAppProto, SCSigMatchAppendSMToList, - SCSigTableAppLiteElmt, SigMatchCtx, Signature, + SCDetectHelperKeywordJsonInfoRegister, SCDetectHelperKeywordRegister, + SCDetectSignatureSetAppProto, SCJsonBuilder, SCSigMatchAppendSMToList, SCSigTableAppLiteElmt, + SigMatchCtx, Signature, }; lazy_static! { @@ -509,6 +513,12 @@ static mut G_MODBUS_TRANSACTION_ID_KW_ID: u16 = 0; static mut G_MODBUS_TRANSACTION_ID_BUFFER_ID: c_int = 0; static mut G_MODBUS_PROTOCOL_ID_KW_ID: u16 = 0; static mut G_MODBUS_PROTOCOL_ID_BUFFER_ID: c_int = 0; +static mut G_MODBUS_FUNCTION_KW_ID: u16 = 0; +static mut G_MODBUS_FUNCTION_BUFFER_ID: c_int = 0; +static mut G_MODBUS_SUBFUNCTION_KW_ID: u16 = 0; +static mut G_MODBUS_SUBFUNCTION_BUFFER_ID: c_int = 0; +static mut G_MODBUS_EXCEPTION_CODE_KW_ID: u16 = 0; +static mut G_MODBUS_EXCEPTION_CODE_BUFFER_ID: c_int = 0; /// Get the message matching the direction of the packet under inspection: /// the request for to-server, the response for to-client. @@ -649,6 +659,282 @@ unsafe extern "C" fn protocol_id_free(_de: *mut DetectEngineCtx, ctx: *mut c_voi SCDetectU16Free(ctx); } +/// Modbus function codes, mirroring sawp_modbus::FunctionCode without its +/// Unknown variant, whose value collides with function code 44 +#[repr(u8)] +#[derive(EnumStringU8)] +#[suricata(enum_string_style = "PascalCase")] +pub enum ModbusFunctionCode { + RdCoils = 0x01, + RdDiscreteInputs = 0x02, + RdHoldRegs = 0x03, + RdInputRegs = 0x04, + WrSingleCoil = 0x05, + WrSingleReg = 0x06, + RdExcStatus = 0x07, + Diagnostic = 0x08, + Program484 = 0x09, + Poll484 = 0x0a, + GetCommEventCtr = 0x0b, + GetCommEventLog = 0x0c, + ProgramController = 0x0d, + PollController = 0x0e, + WrMultCoils = 0x0f, + WrMultRegs = 0x10, + ReportServerID = 0x11, + Program884 = 0x12, + ResetCommLink = 0x13, + RdFileRec = 0x14, + WrFileRec = 0x15, + MaskWrReg = 0x16, + RdWrMultRegs = 0x17, + RdFIFOQueue = 0x18, + MEI = 0x2b, +} + +/// Modbus diagnostic subfunction codes, mirroring +/// sawp_modbus::DiagnosticSubfunction without its Reserved variant, whose +/// value collides with subfunction 22 +#[repr(u16)] +#[derive(EnumStringU16)] +#[suricata(enum_string_style = "PascalCase")] +pub enum ModbusDiagnosticSubfunction { + RetQueryData = 0x00, + RestartCommOpt = 0x01, + RetDiagReg = 0x02, + ChangeInputDelimiter = 0x03, + ForceListenOnlyMode = 0x04, + ClearCtrDiagReg = 0x0a, + RetBusMsgCount = 0x0b, + RetBusCommErrCount = 0x0c, + RetBusExcErrCount = 0x0d, + RetServerMsgCount = 0x0e, + RetServerNoRespCount = 0x0f, + RetServerNAKCount = 0x10, + RetServerBusyCount = 0x11, + RetBusCharOverrunCount = 0x12, + RetOverrunErrCount = 0x13, + ClearOverrunCounterFlag = 0x14, + GetClearPlusStats = 0x15, +} + +/// Modbus exception codes, mirroring sawp_modbus::ExceptionCode without its +/// Unknown variant, whose value collides with exception code 12 +#[repr(u8)] +#[derive(EnumStringU8)] +#[suricata(enum_string_style = "PascalCase")] +pub enum ModbusExceptionCode { + IllegalFunction = 0x01, + IllegalDataAddr = 0x02, + IllegalDataValue = 0x03, + ServerDeviceFail = 0x04, + Ack = 0x05, + ServerDeviceBusy = 0x06, + NegAck = 0x07, + MemParityErr = 0x08, + GatewayPathUnavailable = 0x0a, + GatewayTargetFailToResp = 0x0b, +} + +unsafe fn function_parse(raw: *const c_char) -> *mut DetectUintData { + let raw: &CStr = CStr::from_ptr(raw); //unsafe + if let Ok(s) = raw.to_str() { + if let Some(ctx) = detect_parse_uint_enum::(s) { + let boxed = Box::new(ctx); + return Box::into_raw(boxed) as *mut _; + } + } + return std::ptr::null_mut(); +} + +unsafe fn subfunction_parse(raw: *const c_char) -> *mut DetectUintData { + let raw: &CStr = CStr::from_ptr(raw); //unsafe + if let Ok(s) = raw.to_str() { + if let Some(ctx) = detect_parse_uint_enum::(s) { + let boxed = Box::new(ctx); + return Box::into_raw(boxed) as *mut _; + } + } + return std::ptr::null_mut(); +} + +unsafe fn exception_code_parse(raw: *const c_char) -> *mut DetectUintData { + let raw: &CStr = CStr::from_ptr(raw); //unsafe + if let Ok(s) = raw.to_str() { + if let Some(ctx) = detect_parse_uint_enum::(s) { + let boxed = Box::new(ctx); + return Box::into_raw(boxed) as *mut _; + } + } + return std::ptr::null_mut(); +} + +fn tx_get_function(tx: &ModbusTransaction, direction: u8) -> Option { + return tx_get_message(tx, direction).map(|msg| msg.function.raw); +} + +fn tx_get_subfunction(tx: &ModbusTransaction, direction: u8) -> Option { + if let Some(msg) = tx_get_message(tx, direction) { + if let Data::Diagnostic { func, .. } = &msg.data { + return Some(func.raw); + } + } + return None; +} + +fn tx_get_exception_code(tx: &ModbusTransaction, direction: u8) -> Option { + if let Some(msg) = tx_get_message(tx, direction) { + if let Data::Exception(exc) = &msg.data { + return Some(exc.raw); + } + } + return None; +} + +unsafe extern "C" fn function_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = function_parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_FUNCTION_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_FUNCTION_BUFFER_ID, + ) + .is_null() + { + function_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn function_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(val) = tx_get_function(tx, flags) { + return SCDetectU8Match(val, ctx); + } + return 0; +} + +unsafe extern "C" fn function_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU8Free(ctx); +} + +unsafe extern "C" fn function_list_values(jsb: *mut SCJsonBuilder) { + let jsb = cast_pointer!(jsb, JsonBuilder); + let _ = ModbusFunctionCode::list_values(jsb); +} + +unsafe extern "C" fn subfunction_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = subfunction_parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_SUBFUNCTION_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_SUBFUNCTION_BUFFER_ID, + ) + .is_null() + { + subfunction_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn subfunction_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(val) = tx_get_subfunction(tx, flags) { + return SCDetectU16Match(val, ctx); + } + return 0; +} + +unsafe extern "C" fn subfunction_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU16Free(ctx); +} + +unsafe extern "C" fn subfunction_list_values(jsb: *mut SCJsonBuilder) { + let jsb = cast_pointer!(jsb, JsonBuilder); + let _ = ModbusDiagnosticSubfunction::list_values(jsb); +} + +unsafe extern "C" fn exception_code_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = exception_code_parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_EXCEPTION_CODE_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_EXCEPTION_CODE_BUFFER_ID, + ) + .is_null() + { + exception_code_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn exception_code_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(val) = tx_get_exception_code(tx, flags) { + return SCDetectU8Match(val, ctx); + } + return 0; +} + +unsafe extern "C" fn exception_code_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU8Free(ctx); +} + +unsafe extern "C" fn exception_code_list_values(jsb: *mut SCJsonBuilder) { + let jsb = cast_pointer!(jsb, JsonBuilder); + let _ = ModbusExceptionCode::list_values(jsb); +} + #[no_mangle] pub unsafe extern "C" fn SCDetectModbusRegister() { let kw = SCSigTableAppLiteElmt { @@ -715,6 +1001,63 @@ pub unsafe extern "C" fn SCDetectModbusRegister() { STREAM_TOSERVER | STREAM_TOCLIENT, 0, ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.function\0".as_ptr() as *const c_char, + desc: b"match on Modbus function code\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-function\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(function_match), + Setup: Some(function_setup), + Free: Some(function_free), + flags: SIGMATCH_INFO_UINT8 | SIGMATCH_INFO_ENUM_UINT, + }; + G_MODBUS_FUNCTION_KW_ID = SCDetectHelperKeywordRegister(&kw); + SCDetectHelperKeywordJsonInfoRegister(G_MODBUS_FUNCTION_KW_ID, Some(function_list_values)); + G_MODBUS_FUNCTION_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.function\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.subfunction\0".as_ptr() as *const c_char, + desc: b"match on Modbus diagnostic subfunction code\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-subfunction\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(subfunction_match), + Setup: Some(subfunction_setup), + Free: Some(subfunction_free), + flags: SIGMATCH_INFO_UINT16 | SIGMATCH_INFO_ENUM_UINT, + }; + G_MODBUS_SUBFUNCTION_KW_ID = SCDetectHelperKeywordRegister(&kw); + SCDetectHelperKeywordJsonInfoRegister( + G_MODBUS_SUBFUNCTION_KW_ID, + Some(subfunction_list_values), + ); + G_MODBUS_SUBFUNCTION_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.subfunction\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.exception_code\0".as_ptr() as *const c_char, + desc: b"match on Modbus exception code in error responses\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-exception-code\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(exception_code_match), + Setup: Some(exception_code_setup), + Free: Some(exception_code_free), + flags: SIGMATCH_INFO_UINT8 | SIGMATCH_INFO_ENUM_UINT, + }; + G_MODBUS_EXCEPTION_CODE_KW_ID = SCDetectHelperKeywordRegister(&kw); + SCDetectHelperKeywordJsonInfoRegister( + G_MODBUS_EXCEPTION_CODE_KW_ID, + Some(exception_code_list_values), + ); + G_MODBUS_EXCEPTION_CODE_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.exception_code\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); } #[cfg(test)] @@ -722,7 +1065,146 @@ mod test { use super::super::modbus::ModbusState; use super::*; use crate::applayer::*; + use crate::detect::uint::DetectUintMode; use sawp::parser::Direction; + use sawp_modbus::{DiagnosticSubfunction, ExceptionCode}; + + #[test] + fn test_parse_function_enum() { + let ctx = detect_parse_uint_enum::("8").unwrap(); + assert_eq!(ctx.arg1, 8); + let ctx = detect_parse_uint_enum::("Diagnostic").unwrap(); + assert_eq!(ctx.arg1, 8); + assert_eq!(ctx.mode, DetectUintMode::DetectUintModeEqual); + let ctx = detect_parse_uint_enum::("RdCoils").unwrap(); + assert_eq!(ctx.arg1, 1); + // names are case insensitive + let ctx = detect_parse_uint_enum::("rdcoils").unwrap(); + assert_eq!(ctx.arg1, 1); + let ctx = detect_parse_uint_enum::("RDCOILS").unwrap(); + assert_eq!(ctx.arg1, 1); + let ctx = detect_parse_uint_enum::("MEI").unwrap(); + assert_eq!(ctx.arg1, 0x2b); + // negation + let ctx = detect_parse_uint_enum::("!Diagnostic").unwrap(); + assert_eq!(ctx.arg1, 8); + assert_eq!(ctx.mode, DetectUintMode::DetectUintModeNe); + // the sawp catch-all variant is not a name we accept + assert!(detect_parse_uint_enum::("Unknown").is_none()); + assert!(detect_parse_uint_enum::("").is_none()); + assert!(detect_parse_uint_enum::("NoSuchFunction").is_none()); + assert!(detect_parse_uint_enum::("256").is_none()); + } + + #[test] + fn test_parse_subfunction_enum() { + let ctx = detect_parse_uint_enum::("4").unwrap(); + assert_eq!(ctx.arg1, 4); + let ctx = detect_parse_uint_enum::("ForceListenOnlyMode") + .unwrap(); + assert_eq!(ctx.arg1, 4); + let ctx = detect_parse_uint_enum::("forcelistenonlymode") + .unwrap(); + assert_eq!(ctx.arg1, 4); + let ctx = + detect_parse_uint_enum::("RetQueryData").unwrap(); + assert_eq!(ctx.arg1, 0); + let ctx = detect_parse_uint_enum::("GetClearPlusStats") + .unwrap(); + assert_eq!(ctx.arg1, 0x15); + let ctx = + detect_parse_uint_enum::("!RetDiagReg").unwrap(); + assert_eq!(ctx.arg1, 2); + assert_eq!(ctx.mode, DetectUintMode::DetectUintModeNe); + // the sawp catch-all variant is not a name we accept + assert!(detect_parse_uint_enum::("Reserved").is_none()); + assert!(detect_parse_uint_enum::("").is_none()); + } + + #[test] + fn test_parse_exception_code_enum() { + let ctx = detect_parse_uint_enum::("11").unwrap(); + assert_eq!(ctx.arg1, 11); + let ctx = + detect_parse_uint_enum::("GatewayTargetFailToResp").unwrap(); + assert_eq!(ctx.arg1, 0x0b); + let ctx = + detect_parse_uint_enum::("gatewaytargetfailtoresp").unwrap(); + assert_eq!(ctx.arg1, 0x0b); + let ctx = detect_parse_uint_enum::("IllegalFunction").unwrap(); + assert_eq!(ctx.arg1, 1); + let ctx = detect_parse_uint_enum::("!Ack").unwrap(); + assert_eq!(ctx.arg1, 5); + assert_eq!(ctx.mode, DetectUintMode::DetectUintModeNe); + // the sawp catch-all variant is not a name we accept + assert!(detect_parse_uint_enum::("Unknown").is_none()); + assert!(detect_parse_uint_enum::("").is_none()); + } + + // every name logged by sawp parses back to its value, except the catch-all + #[test] + fn test_function_names_roundtrip() { + for v in 0..=u8::MAX { + let code = FunctionCode::from_raw(v); + let name = code.to_string(); + let ctx = detect_parse_uint_enum::(&name); + if code == FunctionCode::Unknown { + assert!(ctx.is_none(), "{} must not parse", name); + assert!(ModbusFunctionCode::from_u(v).is_none()); + } else { + let ctx = ctx.unwrap_or_else(|| panic!("{} ({}) does not parse", name, v)); + assert_eq!(ctx.mode, DetectUintMode::DetectUintModeEqual); + assert_eq!(ctx.arg1, v); + assert_eq!( + ModbusFunctionCode::from_u(v).map(|c| c.to_str()), + Some(&name[..]) + ); + } + } + } + + #[test] + fn test_subfunction_names_roundtrip() { + for v in 0..=u16::MAX { + let code = + DiagnosticSubfunction::try_from(v).unwrap_or(DiagnosticSubfunction::Reserved); + let name = code.to_string(); + let ctx = detect_parse_uint_enum::(&name); + if code == DiagnosticSubfunction::Reserved { + assert!(ctx.is_none(), "{} must not parse", name); + assert!(ModbusDiagnosticSubfunction::from_u(v).is_none()); + } else { + let ctx = ctx.unwrap_or_else(|| panic!("{} ({}) does not parse", name, v)); + assert_eq!(ctx.mode, DetectUintMode::DetectUintModeEqual); + assert_eq!(ctx.arg1, v); + assert_eq!( + ModbusDiagnosticSubfunction::from_u(v).map(|c| c.to_str()), + Some(&name[..]) + ); + } + } + } + + #[test] + fn test_exception_code_names_roundtrip() { + for v in 0..=u8::MAX { + let code = ExceptionCode::try_from(v).unwrap_or(ExceptionCode::Unknown); + let name = code.to_string(); + let ctx = detect_parse_uint_enum::(&name); + if code == ExceptionCode::Unknown { + assert!(ctx.is_none(), "{} must not parse", name); + assert!(ModbusExceptionCode::from_u(v).is_none()); + } else { + let ctx = ctx.unwrap_or_else(|| panic!("{} ({}) does not parse", name, v)); + assert_eq!(ctx.mode, DetectUintMode::DetectUintModeEqual); + assert_eq!(ctx.arg1, v); + assert_eq!( + ModbusExceptionCode::from_u(v).map(|c| c.to_str()), + Some(&name[..]) + ); + } + } + } #[test] fn test_parse() { From b11dc3084afeb2b45bc3278a4593766794c8491d Mon Sep 17 00:00:00 2001 From: Philip Akekudaga Date: Fri, 31 Jul 2026 14:41:40 -0400 Subject: [PATCH 5/7] modbus: add read/write keywords Add uint keywords for the Modbus data-access values already present in the EVE log: modbus.read.address, modbus.read.quantity, modbus.write.address, modbus.write.quantity and modbus.write.value. Read keywords match read request headers. Write keywords also match the echoed values in write responses, following what is logged. Multiple-write responses echo a quantity where single-write responses echo a value, so modbus.write.quantity matches that echo and modbus.write.value only matches single writes. Ticket: #8131. --- rust/src/modbus/detect.rs | 389 +++++++++++++++++++++++++++++++++++++- 1 file changed, 388 insertions(+), 1 deletion(-) diff --git a/rust/src/modbus/detect.rs b/rust/src/modbus/detect.rs index 170a90d291..f2299f9c22 100644 --- a/rust/src/modbus/detect.rs +++ b/rust/src/modbus/detect.rs @@ -28,7 +28,7 @@ use crate::direction::Direction; use crate::jsonbuilder::JsonBuilder; use lazy_static::lazy_static; use regex::Regex; -use sawp_modbus::{AccessType, CodeCategory, Data, Flags, FunctionCode, Message}; +use sawp_modbus::{AccessType, CodeCategory, Data, Flags, FunctionCode, Message, Read, Write}; use std::ffi::CStr; use std::ops::{Range, RangeInclusive}; use std::os::raw::{c_char, c_int, c_void}; @@ -519,6 +519,16 @@ static mut G_MODBUS_SUBFUNCTION_KW_ID: u16 = 0; static mut G_MODBUS_SUBFUNCTION_BUFFER_ID: c_int = 0; static mut G_MODBUS_EXCEPTION_CODE_KW_ID: u16 = 0; static mut G_MODBUS_EXCEPTION_CODE_BUFFER_ID: c_int = 0; +static mut G_MODBUS_READ_ADDRESS_KW_ID: u16 = 0; +static mut G_MODBUS_READ_ADDRESS_BUFFER_ID: c_int = 0; +static mut G_MODBUS_READ_QUANTITY_KW_ID: u16 = 0; +static mut G_MODBUS_READ_QUANTITY_BUFFER_ID: c_int = 0; +static mut G_MODBUS_WRITE_ADDRESS_KW_ID: u16 = 0; +static mut G_MODBUS_WRITE_ADDRESS_BUFFER_ID: c_int = 0; +static mut G_MODBUS_WRITE_QUANTITY_KW_ID: u16 = 0; +static mut G_MODBUS_WRITE_QUANTITY_BUFFER_ID: c_int = 0; +static mut G_MODBUS_WRITE_VALUE_KW_ID: u16 = 0; +static mut G_MODBUS_WRITE_VALUE_BUFFER_ID: c_int = 0; /// Get the message matching the direction of the packet under inspection: /// the request for to-server, the response for to-client. @@ -935,6 +945,303 @@ unsafe extern "C" fn exception_code_list_values(jsb: *mut SCJsonBuilder) { let _ = ModbusExceptionCode::list_values(jsb); } +fn tx_get_read(tx: &ModbusTransaction, direction: u8) -> Option<&Read> { + if let Some(msg) = tx_get_message(tx, direction) { + match &msg.data { + Data::Read(read) | Data::ReadWrite { read, .. } => return Some(read), + _ => {} + } + } + return None; +} + +fn tx_get_write(tx: &ModbusTransaction, direction: u8) -> Option<&Write> { + if let Some(msg) = tx_get_message(tx, direction) { + match &msg.data { + Data::Write(write) | Data::ReadWrite { write, .. } => return Some(write), + _ => {} + } + } + return None; +} + +fn tx_get_read_address(tx: &ModbusTransaction, direction: u8) -> Option { + if let Some(Read::Request { address, .. }) = tx_get_read(tx, direction) { + return Some(*address); + } + return None; +} + +fn tx_get_read_quantity(tx: &ModbusTransaction, direction: u8) -> Option { + if let Some(Read::Request { quantity, .. }) = tx_get_read(tx, direction) { + return Some(*quantity); + } + return None; +} + +fn tx_get_write_address(tx: &ModbusTransaction, direction: u8) -> Option { + if let Some(write) = tx_get_write(tx, direction) { + match write { + Write::MultReq { address, .. } + | Write::Mask { address, .. } + | Write::Other { address, .. } => { + return Some(*address); + } + } + } + return None; +} + +fn tx_get_write_quantity(tx: &ModbusTransaction, direction: u8) -> Option { + if let Some(msg) = tx_get_message(tx, direction) { + match &msg.data { + Data::Write(Write::MultReq { quantity, .. }) + | Data::ReadWrite { + write: Write::MultReq { quantity, .. }, + .. + } => { + return Some(*quantity); + } + // multiple-write responses echo the quantity in Write::Other + Data::Write(Write::Other { data, .. }) + if msg.access_type.contains(AccessType::MULTIPLE) => + { + return Some(*data); + } + _ => {} + } + } + return None; +} + +fn tx_get_write_value(tx: &ModbusTransaction, direction: u8) -> Option { + if let Some(msg) = tx_get_message(tx, direction) { + if let Data::Write(Write::Other { data, .. }) = &msg.data { + // multiple-write responses echo the quantity in Write::Other, + // only match the value of single writes + if msg.access_type.contains(AccessType::SINGLE) { + return Some(*data); + } + } + } + return None; +} + +unsafe extern "C" fn read_address_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = SCDetectU16Parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_READ_ADDRESS_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_READ_ADDRESS_BUFFER_ID, + ) + .is_null() + { + read_address_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn read_address_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(val) = tx_get_read_address(tx, flags) { + return SCDetectU16Match(val, ctx); + } + return 0; +} + +unsafe extern "C" fn read_address_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU16Free(ctx); +} + +unsafe extern "C" fn read_quantity_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = SCDetectU16Parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_READ_QUANTITY_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_READ_QUANTITY_BUFFER_ID, + ) + .is_null() + { + read_quantity_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn read_quantity_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(val) = tx_get_read_quantity(tx, flags) { + return SCDetectU16Match(val, ctx); + } + return 0; +} + +unsafe extern "C" fn read_quantity_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU16Free(ctx); +} + +unsafe extern "C" fn write_address_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = SCDetectU16Parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_WRITE_ADDRESS_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_WRITE_ADDRESS_BUFFER_ID, + ) + .is_null() + { + write_address_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn write_address_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(val) = tx_get_write_address(tx, flags) { + return SCDetectU16Match(val, ctx); + } + return 0; +} + +unsafe extern "C" fn write_address_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU16Free(ctx); +} + +unsafe extern "C" fn write_quantity_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = SCDetectU16Parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_WRITE_QUANTITY_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_WRITE_QUANTITY_BUFFER_ID, + ) + .is_null() + { + write_quantity_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn write_quantity_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(val) = tx_get_write_quantity(tx, flags) { + return SCDetectU16Match(val, ctx); + } + return 0; +} + +unsafe extern "C" fn write_quantity_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU16Free(ctx); +} + +unsafe extern "C" fn write_value_setup( + de: *mut DetectEngineCtx, s: *mut Signature, raw: *const c_char, +) -> c_int { + if SCDetectSignatureSetAppProto(s, ALPROTO_MODBUS) != 0 { + return -1; + } + let ctx = SCDetectU16Parse(raw) as *mut c_void; + if ctx.is_null() { + return -1; + } + if SCSigMatchAppendSMToList( + de, + s, + G_MODBUS_WRITE_VALUE_KW_ID, + ctx as *mut SigMatchCtx, + G_MODBUS_WRITE_VALUE_BUFFER_ID, + ) + .is_null() + { + write_value_free(std::ptr::null_mut(), ctx); + return -1; + } + return 0; +} + +unsafe extern "C" fn write_value_match( + _de: *mut DetectEngineThreadCtx, _f: *mut Flow, flags: u8, _state: *mut c_void, + tx: *mut c_void, _sig: *const Signature, ctx: *const SigMatchCtx, +) -> c_int { + let tx = cast_pointer!(tx, ModbusTransaction); + let ctx = cast_pointer!(ctx, DetectUintData); + if let Some(val) = tx_get_write_value(tx, flags) { + return SCDetectU16Match(val, ctx); + } + return 0; +} + +unsafe extern "C" fn write_value_free(_de: *mut DetectEngineCtx, ctx: *mut c_void) { + // Just unbox... + let ctx = cast_pointer!(ctx, DetectUintData); + SCDetectU16Free(ctx); +} + #[no_mangle] pub unsafe extern "C" fn SCDetectModbusRegister() { let kw = SCSigTableAppLiteElmt { @@ -1058,6 +1365,86 @@ pub unsafe extern "C" fn SCDetectModbusRegister() { STREAM_TOSERVER | STREAM_TOCLIENT, 0, ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.read.address\0".as_ptr() as *const c_char, + desc: b"match on address of Modbus read requests\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-read-address\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(read_address_match), + Setup: Some(read_address_setup), + Free: Some(read_address_free), + flags: SIGMATCH_INFO_UINT16, + }; + G_MODBUS_READ_ADDRESS_KW_ID = SCDetectHelperKeywordRegister(&kw); + G_MODBUS_READ_ADDRESS_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.read.address\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.read.quantity\0".as_ptr() as *const c_char, + desc: b"match on quantity of Modbus read requests\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-read-quantity\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(read_quantity_match), + Setup: Some(read_quantity_setup), + Free: Some(read_quantity_free), + flags: SIGMATCH_INFO_UINT16, + }; + G_MODBUS_READ_QUANTITY_KW_ID = SCDetectHelperKeywordRegister(&kw); + G_MODBUS_READ_QUANTITY_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.read.quantity\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.write.address\0".as_ptr() as *const c_char, + desc: b"match on address of Modbus write accesses\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-write-address\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(write_address_match), + Setup: Some(write_address_setup), + Free: Some(write_address_free), + flags: SIGMATCH_INFO_UINT16, + }; + G_MODBUS_WRITE_ADDRESS_KW_ID = SCDetectHelperKeywordRegister(&kw); + G_MODBUS_WRITE_ADDRESS_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.write.address\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.write.quantity\0".as_ptr() as *const c_char, + desc: b"match on quantity of Modbus multiple-write requests\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-write-quantity\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(write_quantity_match), + Setup: Some(write_quantity_setup), + Free: Some(write_quantity_free), + flags: SIGMATCH_INFO_UINT16, + }; + G_MODBUS_WRITE_QUANTITY_KW_ID = SCDetectHelperKeywordRegister(&kw); + G_MODBUS_WRITE_QUANTITY_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.write.quantity\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); + let kw = SCSigTableAppLiteElmt { + name: b"modbus.write.value\0".as_ptr() as *const c_char, + desc: b"match on value of Modbus single-write accesses\0".as_ptr() as *const c_char, + url: b"/rules/modbus-keyword.html#modbus-write-value\0".as_ptr() as *const c_char, + AppLayerTxMatch: Some(write_value_match), + Setup: Some(write_value_setup), + Free: Some(write_value_free), + flags: SIGMATCH_INFO_UINT16, + }; + G_MODBUS_WRITE_VALUE_KW_ID = SCDetectHelperKeywordRegister(&kw); + G_MODBUS_WRITE_VALUE_BUFFER_ID = SCDetectHelperBufferProgressRegister( + b"modbus.write.value\0".as_ptr() as *const c_char, + ALPROTO_MODBUS, + STREAM_TOSERVER | STREAM_TOCLIENT, + 0, + ); } #[cfg(test)] From 1777b58fa905c7154c066ec363b47df93b641eb9 Mon Sep 17 00:00:00 2001 From: Philip Akekudaga Date: Mon, 21 Sep 2026 11:13:11 -0400 Subject: [PATCH 6/7] doc/userguide: document modbus keywords Document modbus.unit_id, modbus.transaction_id, modbus.protocol_id, modbus.function, modbus.subfunction, modbus.exception_code, modbus.read.address, modbus.read.quantity, modbus.write.address, modbus.write.quantity and modbus.write.value, naming the EVE field each one is taken from and how error responses and write response echoes are matched. List the names accepted by modbus.function, modbus.subfunction and modbus.exception_code. Say that the names logged for codes that have none are not accepted, as they stand for more than one value, and that a name does not match the error response of a request, since the logged name drops the error bit that the matched value carries. Note how the address keywords differ from the 1-based addressing of the modbus keyword in the sections of the two keywords concerned, rather than once for the whole set. Ticket: #8131. --- doc/userguide/rules/modbus-keyword.rst | 287 +++++++++++++++++++++++++ 1 file changed, 287 insertions(+) diff --git a/doc/userguide/rules/modbus-keyword.rst b/doc/userguide/rules/modbus-keyword.rst index a485c71a19..18200b1a1a 100644 --- a/doc/userguide/rules/modbus-keyword.rst +++ b/doc/userguide/rules/modbus-keyword.rst @@ -129,3 +129,290 @@ V1.0b, the MODBUS slave device addresses on serial line are assigned from 1 to Paper and presentation (in french) on Modbus support are available : http://www.ssi.gouv.fr/agence/publication/detection-dintrusion-dans-les-systemes-industriels-suricata-et-le-cas-modbus/ + +Per-value keywords +------------------ + +The following keywords match on single values of a Modbus transaction, +taken from the same fields the EVE ``modbus`` event logs. They all use +:ref:`unsigned integers ` and support ranges and +negation. In to-server direction they match on the request; in to-client +direction they match on the response, for the values a response carries. +Read requests are only matched in to-server direction, as a read response +carries data rather than an address and a quantity. + +modbus.unit_id +~~~~~~~~~~~~~~ + +Match on the Modbus unit identifier, the address of a slave device +connected on a sub-network behind a bridge or gateway. + +modbus.unit_id uses an :ref:`unsigned 8-bits integer `. + +Examples:: + + modbus.unit_id:10; + modbus.unit_id:>247; + +modbus.transaction_id +~~~~~~~~~~~~~~~~~~~~~ + +Match on the Modbus/TCP transaction identifier used to pair requests +and responses. + +modbus.transaction_id uses an +:ref:`unsigned 16-bits integer `. + +Example:: + + modbus.transaction_id:1; + +modbus.protocol_id +~~~~~~~~~~~~~~~~~~ + +Match on the Modbus/TCP protocol identifier. The specification requires +this field to be 0, so a non-zero value is an anomaly. + +modbus.protocol_id uses an +:ref:`unsigned 16-bits integer `. + +Example:: + + modbus.protocol_id:!0; + +modbus.function +~~~~~~~~~~~~~~~ + +Match on the Modbus function code, as it is logged in ``function_raw``. +It can be specified as the integer value or as the function code name +logged in ``function_code``. Names are case insensitive and can be +negated with ``!``. + +.. note:: + + Error responses carry the request function code with the highest bit + set (e.g. 136 for a failed function 8), so ``modbus.function:>127;`` + matches any error response. ``function_code`` is logged without that + bit, so a name matches a request, and a response that is not an error, + but never an error response. Match those by value, with + ``modbus.function:>127;`` or the raised code. + + ``Unknown`` is logged in ``function_code`` for every code that has no + name, so it does not identify one value and is not accepted here. The + codes it covers are still matched by value. + +modbus.function uses an :ref:`unsigned 8-bits integer `. + +Syntax:: + + modbus.function:[op] + modbus.function:[!] + +See ``suricata --list-keywords=json | jq '."modbus.function".enum_values'`` + +Where name is one of: + +* RdCoils +* RdDiscreteInputs +* RdHoldRegs +* RdInputRegs +* WrSingleCoil +* WrSingleReg +* RdExcStatus +* Diagnostic +* Program484 +* Poll484 +* GetCommEventCtr +* GetCommEventLog +* ProgramController +* PollController +* WrMultCoils +* WrMultRegs +* ReportServerID +* Program884 +* ResetCommLink +* RdFileRec +* WrFileRec +* MaskWrReg +* RdWrMultRegs +* RdFIFOQueue +* MEI + +Examples:: + + modbus.function:8; + modbus.function:Diagnostic; # same as above + modbus.function:!RdCoils; + modbus.function:>127; + +modbus.subfunction +~~~~~~~~~~~~~~~~~~ + +Match on the subfunction code of Modbus Diagnostics (function 8) +messages. It can be specified as the integer value or as the +subfunction name logged in ``diagnostic.code``. Names are case +insensitive and can be negated with ``!``. + +.. note:: + + ``Reserved`` is logged in ``diagnostic.code`` for every subfunction + that has no name, so it does not identify one value and is not + accepted here. Those subfunctions are still matched by value. + +modbus.subfunction uses an +:ref:`unsigned 16-bits integer `. + +Syntax:: + + modbus.subfunction:[op] + modbus.subfunction:[!] + +See ``suricata --list-keywords=json | jq '."modbus.subfunction".enum_values'`` + +Where name is one of: + +* RetQueryData +* RestartCommOpt +* RetDiagReg +* ChangeInputDelimiter +* ForceListenOnlyMode +* ClearCtrDiagReg +* RetBusMsgCount +* RetBusCommErrCount +* RetBusExcErrCount +* RetServerMsgCount +* RetServerNoRespCount +* RetServerNAKCount +* RetServerBusyCount +* RetBusCharOverrunCount +* RetOverrunErrCount +* ClearOverrunCounterFlag +* GetClearPlusStats + +Examples:: + + modbus.function:8; modbus.subfunction:4; + modbus.function:Diagnostic; modbus.subfunction:ForceListenOnlyMode; + +modbus.exception_code +~~~~~~~~~~~~~~~~~~~~~ + +Match on the exception code of Modbus error responses. It can be +specified as the integer value or as the exception name logged in +``exception.code``. Names are case insensitive and can be negated with +``!``. + +.. note:: + + ``Unknown`` is logged in ``exception.code`` for every exception code + that has no name, so it does not identify one value and is not + accepted here. Those codes are still matched by value. + +modbus.exception_code uses an +:ref:`unsigned 8-bits integer `. + +Syntax:: + + modbus.exception_code:[op] + modbus.exception_code:[!] + +See ``suricata --list-keywords=json | jq '."modbus.exception_code".enum_values'`` + +Where name is one of: + +* IllegalFunction +* IllegalDataAddr +* IllegalDataValue +* ServerDeviceFail +* Ack +* ServerDeviceBusy +* NegAck +* MemParityErr +* GatewayPathUnavailable +* GatewayTargetFailToResp + +Examples:: + + modbus.exception_code:11; + modbus.exception_code:GatewayTargetFailToResp; # same as above + +modbus.read.address +~~~~~~~~~~~~~~~~~~~ + +Match on the starting address of Modbus read requests. + +Unlike the ``address`` option of the ``modbus`` keyword above, this +matches the raw protocol value as it is logged, without the +1 offset, +and matches only the starting address of an access rather than the +whole accessed range. + +modbus.read.address uses an +:ref:`unsigned 16-bits integer `. + +Example:: + + modbus.read.address:1000; + +modbus.read.quantity +~~~~~~~~~~~~~~~~~~~~ + +Match on the quantity of items in Modbus read requests. + +modbus.read.quantity uses an +:ref:`unsigned 16-bits integer `. + +Example:: + + modbus.read.quantity:>1000; + +modbus.write.address +~~~~~~~~~~~~~~~~~~~~ + +Match on the starting address of Modbus write accesses. In to-client +direction this matches the address echoed in write responses. + +Unlike the ``address`` option of the ``modbus`` keyword above, this +matches the raw protocol value as it is logged, without the +1 offset, +and matches only the starting address of an access rather than the +whole accessed range. + +modbus.write.address uses an +:ref:`unsigned 16-bits integer `. + +Example:: + + modbus.write.address:500; + +modbus.write.quantity +~~~~~~~~~~~~~~~~~~~~~ + +Match on the quantity of items in Modbus multiple-write requests, as it +is logged in ``write.quantity``. In to-client direction this matches the +quantity echoed in multiple-write responses, which the protocol carries +in the value field and the log reports as ``write.data``. + +modbus.write.quantity uses an +:ref:`unsigned 16-bits integer `. + +Example:: + + modbus.write.quantity:>100; + +modbus.write.value +~~~~~~~~~~~~~~~~~~ + +Match on the value of Modbus single-write accesses, as it is logged in +``write.data``. In to-client direction this matches the value echoed in +single-write responses. + +.. note:: + + Multiple-write responses are not matched, as they echo a quantity + rather than a value (see ``modbus.write.quantity``). + +modbus.write.value uses an +:ref:`unsigned 16-bits integer `. + +Example:: + + modbus.write.address:500; modbus.write.value:>200; From c6a3c766853052c641a4f21a0c72b7bb4ce0fd9c Mon Sep 17 00:00:00 2001 From: Philip Akekudaga Date: Fri, 31 Jul 2026 14:41:40 -0400 Subject: [PATCH 7/7] schema: map new modbus keywords Map the new modbus per-value keywords to the EVE fields they match. Also add the write.quantity, write.and_mask and write.or_mask properties and allow string data in multiple-write requests, matching what the modbus logger already emits, so that alerts embedding such transactions validate against the schema. Ticket: #8131. --- etc/schema.json | 132 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 119 insertions(+), 13 deletions(-) diff --git a/etc/schema.json b/etc/schema.json index 4cff77cefb..63a377c02f 100644 --- a/etc/schema.json +++ b/etc/schema.json @@ -4110,7 +4110,12 @@ "type": "string" }, "raw": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.subfunction" + ] + } } } }, @@ -4121,7 +4126,12 @@ "type": "string" }, "function_raw": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.function" + ] + } }, "mei": { "type": "object", @@ -4139,35 +4149,84 @@ } }, "protocol_id": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.protocol_id" + ] + } }, "read": { "type": "object", "additionalProperties": false, "properties": { "address": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.read.address" + ] + } }, "quantity": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.read.quantity" + ] + } } } }, "transaction_id": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.transaction_id" + ] + } }, "unit_id": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.unit_id" + ] + } }, "write": { "type": "object", "additionalProperties": false, "properties": { "address": { + "type": "integer", + "suricata": { + "keywords": [ + "modbus.write.address" + ] + } + }, + "and_mask": { "type": "integer" }, "data": { + "type": ["string", "number"], + "suricata": { + "keywords": [ + "modbus.write.value" + ] + } + }, + "or_mask": { "type": "integer" + }, + "quantity": { + "type": "integer", + "suricata": { + "keywords": [ + "modbus.write.quantity" + ] + } } } } @@ -4197,7 +4256,12 @@ "type": "string" }, "raw": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.subfunction" + ] + } } } }, @@ -4212,7 +4276,12 @@ "type": "string" }, "raw": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.exception_code" + ] + } } } }, @@ -4220,10 +4289,20 @@ "type": "string" }, "function_raw": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.function" + ] + } }, "protocol_id": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.protocol_id" + ] + } }, "read": { "type": "object", @@ -4235,19 +4314,46 @@ } }, "transaction_id": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.transaction_id" + ] + } }, "unit_id": { - "type": "integer" + "type": "integer", + "suricata": { + "keywords": [ + "modbus.unit_id" + ] + } }, "write": { "type": "object", "additionalProperties": false, "properties": { "address": { + "type": "integer", + "suricata": { + "keywords": [ + "modbus.write.address" + ] + } + }, + "and_mask": { "type": "integer" }, "data": { + "type": "integer", + "suricata": { + "keywords": [ + "modbus.write.value", + "modbus.write.quantity" + ] + } + }, + "or_mask": { "type": "integer" } }