From: Sascha Steinbiss Date: Fri, 9 Jun 2023 09:13:35 +0000 (+0200) Subject: rfb: never return error on unknown traffic X-Git-Tag: suricata-6.0.14~62 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8f1e08b28e149c22e1c85e1b0f77b434c8e7ef79;p=thirdparty%2Fsuricata.git rfb: never return error on unknown traffic We only try to parse a small subset of what is possible in RFB. Currently we only understand some standard auth schemes and stop parsing when the server-client handshake is complete. Since in IPS mode returning an error from the parser causes drops that are likely uncalled for, we do not want to return errors when we simply do not understand what happens in the traffic. This addresses Redmine #5912. Bug: #5915. (cherry picked from commit 1f8a5874fbc6816a7aeb59ba668ebd2bf7c206ed) --- diff --git a/rules/Makefile.am b/rules/Makefile.am index e4d5635c51..5711679836 100644 --- a/rules/Makefile.am +++ b/rules/Makefile.am @@ -15,6 +15,7 @@ modbus-events.rules \ mqtt-events.rules \ nfs-events.rules \ ntp-events.rules \ +rfb-events.rules \ smb-events.rules \ smtp-events.rules \ ssh-events.rules \ diff --git a/rules/rfb-events.rules b/rules/rfb-events.rules new file mode 100644 index 0000000000..08bc493f42 --- /dev/null +++ b/rules/rfb-events.rules @@ -0,0 +1,10 @@ +# RFB app-layer event rules. +# +# These SIDs fall in the 2233000+ range. See: +# http://doc.emergingthreats.net/bin/view/Main/SidAllocation and +# https://redmine.openinfosecfoundation.org/projects/suricata/wiki/AppLayer + +alert rfb any any -> any any (msg:"SURICATA RFB Malformed or unknown message"; app-layer-event:rfb.malformed_message; classtype:protocol-command-decode; sid:2233000; rev:1;) +alert rfb any any -> any any (msg:"SURICATA RFB Unimplemented security type"; app-layer-event:rfb.unimplemented_security_type; classtype:protocol-command-decode; sid:2233001; rev:1;) +alert rfb any any -> any any (msg:"SURICATA RFB Unknown security result"; app-layer-event:rfb.unknown_security_result; classtype:protocol-command-decode; sid:2233002; rev:1;) +alert rfb any any -> any any (msg:"SURICATA RFB Unexpected State in Parser"; app-layer-event:rfb.confused_state; classtype:protocol-command-decode; sid:2233003; rev:1;) diff --git a/rust/src/rfb/detect.rs b/rust/src/rfb/detect.rs index d30014a139..da876457ae 100644 --- a/rust/src/rfb/detect.rs +++ b/rust/src/rfb/detect.rs @@ -22,9 +22,7 @@ use std::ptr; #[no_mangle] pub unsafe extern "C" fn rs_rfb_tx_get_name( - tx: &mut RFBTransaction, - buffer: *mut *const u8, - buffer_len: *mut u32, + tx: &mut RFBTransaction, buffer: *mut *const u8, buffer_len: *mut u32, ) -> u8 { if let Some(ref r) = tx.tc_server_init { let p = &r.name; @@ -42,10 +40,7 @@ pub unsafe extern "C" fn rs_rfb_tx_get_name( } #[no_mangle] -pub unsafe extern "C" fn rs_rfb_tx_get_sectype( - tx: &mut RFBTransaction, - sectype: *mut u32, -) -> u8 { +pub unsafe extern "C" fn rs_rfb_tx_get_sectype(tx: &mut RFBTransaction, sectype: *mut u32) -> u8 { if let Some(ref r) = tx.chosen_security_type { *sectype = *r; return 1; @@ -58,8 +53,7 @@ pub unsafe extern "C" fn rs_rfb_tx_get_sectype( #[no_mangle] pub unsafe extern "C" fn rs_rfb_tx_get_secresult( - tx: &mut RFBTransaction, - secresult: *mut u32, + tx: &mut RFBTransaction, secresult: *mut u32, ) -> u8 { if let Some(ref r) = tx.tc_security_result { *secresult = r.status; @@ -67,4 +61,4 @@ pub unsafe extern "C" fn rs_rfb_tx_get_secresult( } return 0; -} \ No newline at end of file +} diff --git a/rust/src/rfb/logger.rs b/rust/src/rfb/logger.rs index c990b3dbb6..5f8be358cd 100644 --- a/rust/src/rfb/logger.rs +++ b/rust/src/rfb/logger.rs @@ -17,10 +17,10 @@ // Author: Frank Honza -use std; -use std::fmt::Write; use super::rfb::{RFBState, RFBTransaction}; use crate::jsonbuilder::{JsonBuilder, JsonError}; +use std; +use std::fmt::Write; fn log_rfb(tx: &RFBTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> { js.open_object("rfb")?; @@ -63,15 +63,17 @@ fn log_rfb(tx: &RFBTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> { } js.close()?; } - _ => () + _ => (), } if let Some(security_result) = &tx.tc_security_result { let _ = match security_result.status { 0 => js.set_string("security_result", "OK")?, 1 => js.set_string("security-result", "FAIL")?, 2 => js.set_string("security_result", "TOOMANY")?, - _ => js.set_string("security_result", - &format!("UNKNOWN ({})", security_result.status))?, + _ => js.set_string( + "security_result", + &format!("UNKNOWN ({})", security_result.status), + )?, }; } js.close()?; // Close authentication. @@ -91,15 +93,27 @@ fn log_rfb(tx: &RFBTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> { js.set_string_from_bytes("name", &tc_server_init.name)?; js.open_object("pixel_format")?; - js.set_uint("bits_per_pixel", tc_server_init.pixel_format.bits_per_pixel as u64)?; + js.set_uint( + "bits_per_pixel", + tc_server_init.pixel_format.bits_per_pixel as u64, + )?; js.set_uint("depth", tc_server_init.pixel_format.depth as u64)?; - js.set_bool("big_endian", tc_server_init.pixel_format.big_endian_flag != 0)?; - js.set_bool("true_color", tc_server_init.pixel_format.true_colour_flag != 0)?; + js.set_bool( + "big_endian", + tc_server_init.pixel_format.big_endian_flag != 0, + )?; + js.set_bool( + "true_color", + tc_server_init.pixel_format.true_colour_flag != 0, + )?; js.set_uint("red_max", tc_server_init.pixel_format.red_max as u64)?; js.set_uint("green_max", tc_server_init.pixel_format.green_max as u64)?; js.set_uint("blue_max", tc_server_init.pixel_format.blue_max as u64)?; js.set_uint("red_shift", tc_server_init.pixel_format.red_shift as u64)?; - js.set_uint("green_shift", tc_server_init.pixel_format.green_shift as u64)?; + js.set_uint( + "green_shift", + tc_server_init.pixel_format.green_shift as u64, + )?; js.set_uint("blue_shift", tc_server_init.pixel_format.blue_shift as u64)?; js.close()?; @@ -112,9 +126,9 @@ fn log_rfb(tx: &RFBTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> { } #[no_mangle] -pub extern "C" fn rs_rfb_logger_log(_state: &mut RFBState, - tx: *mut std::os::raw::c_void, - js: &mut JsonBuilder) -> bool { +pub extern "C" fn rs_rfb_logger_log( + _state: &mut RFBState, tx: *mut std::os::raw::c_void, js: &mut JsonBuilder, +) -> bool { let tx = cast_pointer!(tx, RFBTransaction); log_rfb(tx, js).is_ok() } diff --git a/rust/src/rfb/mod.rs b/rust/src/rfb/mod.rs index 4bde7935cb..68d37ec8a0 100644 --- a/rust/src/rfb/mod.rs +++ b/rust/src/rfb/mod.rs @@ -20,4 +20,4 @@ pub mod detect; pub mod logger; pub mod parser; -pub mod rfb; \ No newline at end of file +pub mod rfb; diff --git a/rust/src/rfb/parser.rs b/rust/src/rfb/parser.rs index 9ec311eb3b..37c6d3d16e 100644 --- a/rust/src/rfb/parser.rs +++ b/rust/src/rfb/parser.rs @@ -17,10 +17,11 @@ // Author: Frank Honza -use std::fmt; -use nom::*; use nom::number::streaming::*; +use nom::*; +use std::fmt; +#[derive(Debug, PartialEq)] pub enum RFBGlobalState { TCServerProtocolVersion, TCSupportedSecurityTypes, @@ -33,7 +34,7 @@ pub enum RFBGlobalState { TSVncResponse, TCSecurityResult, TSClientInit, - Message + Skip, } impl fmt::Display for RFBGlobalState { @@ -50,43 +51,43 @@ impl fmt::Display for RFBGlobalState { RFBGlobalState::TCSecurityResult => write!(f, "TCSecurityResult"), RFBGlobalState::TCServerSecurityType => write!(f, "TCServerSecurityType"), RFBGlobalState::TSClientInit => write!(f, "TSClientInit"), - RFBGlobalState::Message => write!(f, "Message") + RFBGlobalState::Skip => write!(f, "Skip"), } } } pub struct ProtocolVersion { pub major: String, - pub minor: String + pub minor: String, } pub struct SupportedSecurityTypes { pub number_of_types: u8, - pub types: Vec + pub types: Vec, } pub struct SecurityTypeSelection { - pub security_type: u8 + pub security_type: u8, } pub struct ServerSecurityType { - pub security_type: u32 + pub security_type: u32, } pub struct SecurityResult { - pub status: u32 + pub status: u32, } pub struct FailureReason { - pub reason_string: String + pub reason_string: String, } pub struct VncAuth { - pub secret: Vec + pub secret: Vec, } pub struct ClientInit { - pub shared: u8 + pub shared: u8, } pub struct PixelFormat { @@ -107,7 +108,7 @@ pub struct ServerInit { pub height: u16, pub pixel_format: PixelFormat, pub name_length: u32, - pub name: Vec + pub name: Vec, } named!(pub parse_protocol_version, @@ -257,8 +258,8 @@ named!(pub parse_server_init, #[cfg(test)] mod tests { - use nom::*; use super::*; + use nom::*; /// Simple test of some valid data. #[test] @@ -277,8 +278,7 @@ mod tests { Err(Err::Incomplete(_)) => { panic!("Result should not have been incomplete."); } - Err(Err::Error(err)) | - Err(Err::Failure(err)) => { + Err(Err::Error(err)) | Err(Err::Failure(err)) => { panic!("Result should not be an error: {:?}.", err); } } @@ -287,13 +287,10 @@ mod tests { #[test] fn test_parse_server_init() { let buf = [ - 0x05, 0x00, 0x03, 0x20, 0x20, 0x18, 0x00, 0x01, - 0x00, 0xff, 0x00, 0xff, 0x00, 0xff, 0x10, 0x08, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1e, - 0x61, 0x6e, 0x65, 0x61, 0x67, 0x6c, 0x65, 0x73, - 0x40, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x68, 0x6f, - 0x73, 0x74, 0x2e, 0x6c, 0x6f, 0x63, 0x61, 0x6c, - 0x64, 0x6f, 0x6d, 0x61, 0x69, 0x6e + 0x05, 0x00, 0x03, 0x20, 0x20, 0x18, 0x00, 0x01, 0x00, 0xff, 0x00, 0xff, 0x00, 0xff, + 0x10, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1e, 0x61, 0x6e, 0x65, 0x61, + 0x67, 0x6c, 0x65, 0x73, 0x40, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x68, 0x6f, 0x73, 0x74, + 0x2e, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x64, 0x6f, 0x6d, 0x61, 0x69, 0x6e, ]; let result = parse_server_init(&buf); @@ -310,11 +307,9 @@ mod tests { Err(Err::Incomplete(_)) => { panic!("Result should not have been incomplete."); } - Err(Err::Error(err)) | - Err(Err::Failure(err)) => { + Err(Err::Error(err)) | Err(Err::Failure(err)) => { panic!("Result should not be an error: {:?}.", err); } } } - } diff --git a/rust/src/rfb/rfb.rs b/rust/src/rfb/rfb.rs index e167eb66d8..1ad79370dc 100644 --- a/rust/src/rfb/rfb.rs +++ b/rust/src/rfb/rfb.rs @@ -18,17 +18,37 @@ // Author: Frank Honza // Sascha Steinbiss -use std; -use std::ffi::CString; -use std::mem::transmute; -use crate::core::{self, ALPROTO_UNKNOWN, AppProto, Flow, IPPROTO_TCP}; +use super::parser; use crate::applayer; use crate::applayer::*; +use crate::core::{self, AppProto, Flow, ALPROTO_UNKNOWN, IPPROTO_TCP}; use nom; -use super::parser; +use std; +use std::ffi::{CStr, CString}; +use std::mem::transmute; static mut ALPROTO_RFB: AppProto = ALPROTO_UNKNOWN; +#[derive(FromPrimitive, Debug)] +pub enum RFBEvent { + UnimplementedSecurityType = 0, + UnknownSecurityResult, + MalformedMessage, + ConfusedState, +} + +impl RFBEvent { + fn from_i32(value: i32) -> Option { + match value { + 0 => Some(RFBEvent::UnimplementedSecurityType), + 1 => Some(RFBEvent::UnknownSecurityResult), + 2 => Some(RFBEvent::MalformedMessage), + 3 => Some(RFBEvent::ConfusedState), + _ => None, + } + } +} + pub struct RFBTransaction { tx_id: u64, pub complete: bool, @@ -84,6 +104,10 @@ impl RFBTransaction { core::sc_detect_engine_state_free(state); } } + + fn set_event(&mut self, event: RFBEvent) { + core::sc_app_layer_decoder_events_set_event_raw(&mut self.events, event as u8); + } } impl Drop for RFBTransaction { @@ -95,7 +119,7 @@ impl Drop for RFBTransaction { pub struct RFBState { tx_id: u64, transactions: Vec, - state: parser::RFBGlobalState + state: parser::RFBGlobalState, } impl RFBState { @@ -103,7 +127,7 @@ impl RFBState { Self { tx_id: 0, transactions: Vec::new(), - state: parser::RFBGlobalState::TCServerProtocolVersion + state: parser::RFBGlobalState::TCServerProtocolVersion, } } @@ -180,13 +204,19 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.ts_client_protocol_version = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!( + "no transaction set at protocol selection stage" + ); } } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { + // We even failed to parse the protocol version. return AppLayerResult::err(); } } @@ -201,21 +231,41 @@ impl RFBState { match chosen_security_type { 2 => self.state = parser::RFBGlobalState::TCVncChallenge, 1 => self.state = parser::RFBGlobalState::TSClientInit, - _ => return AppLayerResult::err(), + _ => { + if let Some(current_transaction) = self.get_current_tx() { + current_transaction + .set_event(RFBEvent::UnimplementedSecurityType); + } + // We have just have seen a security type we don't know about. + // This is not bad per se, it might just mean this is a + // proprietary one not in the spec. + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); + } } if let Some(current_transaction) = self.get_current_tx() { current_transaction.ts_security_type_selection = Some(request); current_transaction.chosen_security_type = Some(chosen_security_type as u32); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at security type stage"); } } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // We failed to parse the security type. + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -230,14 +280,22 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.ts_vnc_response = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at security result stage"); } } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -252,28 +310,45 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.ts_client_init = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at client init stage"); } } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // We failed to parse the client init. + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } - parser::RFBGlobalState::Message => { - //todo implement RFB messages, for now we stop here - return AppLayerResult::err(); - } - parser::RFBGlobalState::TCServerProtocolVersion => { - SCLogDebug!("Reversed traffic, expected response."); - return AppLayerResult::err(); + parser::RFBGlobalState::Skip => { + // End of parseable handshake reached, skip rest of traffic + return AppLayerResult::ok(); } _ => { - SCLogDebug!("Invalid state for request {}", self.state); - current = b""; + // We have gotten out of sync with the expected state flow. + // This could happen since we use a global state (i.e. that + // is used for both directions), but if traffic can not be + // parsed as expected elsewhere, we might not have advanced + // a state for one direction but received data in the + // "unexpected" direction, causing the parser to end up + // here. Let's stop trying to parse the traffic but still + // accept it. + SCLogDebug!("Invalid state for request: {}", self.state); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::ConfusedState); + } + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -287,7 +362,11 @@ impl RFBState { let mut current = input; let mut consumed = 0; - SCLogDebug!("response_state {}, response_len {}", self.state, input.len()); + SCLogDebug!( + "response_state {}, response_len {}", + self.state, + input.len() + ); loop { if current.len() == 0 { return AppLayerResult::ok(); @@ -306,13 +385,17 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_server_protocol_version = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set but we just set one"); } } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { + // We even failed to parse the protocol version. return AppLayerResult::err(); } } @@ -324,8 +407,14 @@ impl RFBState { current = rem; SCLogDebug!( - "supported_security_types: {}, types: {}", request.number_of_types, - request.types.iter().map(ToString::to_string).map(|v| v + " ").collect::() + "supported_security_types: {}, types: {}", + request.number_of_types, + request + .types + .iter() + .map(ToString::to_string) + .map(|v| v + " ") + .collect::() ); self.state = parser::RFBGlobalState::TSSecurityTypeSelection; @@ -336,14 +425,22 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_supported_security_types = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at security type stage"); } } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -361,23 +458,44 @@ impl RFBState { 1 => self.state = parser::RFBGlobalState::TSClientInit, 2 => self.state = parser::RFBGlobalState::TCVncChallenge, _ => { - // TODO Event unknown security type - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction + .set_event(RFBEvent::UnimplementedSecurityType); + } else { + debug_validate_fail!( + "no transaction set at security type stage" + ); + } + // We have just have seen a security type we don't know about. + // This is not bad per se, it might just mean this is a + // proprietary one not in the spec. + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_server_security_type = Some(request); - current_transaction.chosen_security_type = Some(chosen_security_type); + current_transaction.chosen_security_type = + Some(chosen_security_type); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at security type stage"); } } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -392,14 +510,22 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_vnc_challenge = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at auth stage"); } } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -415,19 +541,34 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_security_result = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!( + "no transaction set at security result stage" + ); } } else if request.status == 1 { self.state = parser::RFBGlobalState::TCFailureReason; } else { - // TODO: Event: unknown security result value + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::UnknownSecurityResult); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -437,15 +578,23 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_failure_reason = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at failure reason stage"); } - return AppLayerResult::err(); + return AppLayerResult::ok(); } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -455,40 +604,58 @@ impl RFBState { consumed += current.len() - rem.len(); current = rem; - self.state = parser::RFBGlobalState::Message; + self.state = parser::RFBGlobalState::Skip; if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_server_init = Some(request); // connection initialization is complete and parsed current_transaction.complete = true; } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at server init stage"); } } Err(nom::Err::Incomplete(_)) => { - return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32); + return AppLayerResult::incomplete( + consumed as u32, + (current.len() + 1) as u32, + ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } - parser::RFBGlobalState::Message => { + parser::RFBGlobalState::Skip => { //todo implement RFB messages, for now we stop here - return AppLayerResult::err(); + return AppLayerResult::ok(); } _ => { - SCLogDebug!("Invalid state for response"); - return AppLayerResult::err(); + // We have gotten out of sync with the expected state flow. + // This could happen since we use a global state (i.e. that + // is used for both directions), but if traffic can not be + // parsed as expected elsewhere, we might not have advanced + // a state for one direction but received data in the + // "unexpected" direction, causing the parser to end up + // here. Let's stop trying to parse the traffic but still + // accept it. + SCLogDebug!("Invalid state for response: {}", self.state); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::ConfusedState); + } + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } } fn tx_iterator( - &mut self, - min_tx_id: u64, - state: &mut u64, + &mut self, min_tx_id: u64, state: &mut u64, ) -> Option<(&RFBTransaction, u64, bool)> { let mut index = *state as usize; let len = self.transactions.len(); @@ -509,17 +676,13 @@ impl RFBState { // C exports. -export_tx_get_detect_state!( - rs_rfb_tx_get_detect_state, - RFBTransaction -); -export_tx_set_detect_state!( - rs_rfb_tx_set_detect_state, - RFBTransaction -); +export_tx_get_detect_state!(rs_rfb_tx_get_detect_state, RFBTransaction); +export_tx_set_detect_state!(rs_rfb_tx_set_detect_state, RFBTransaction); #[no_mangle] -pub extern "C" fn rs_rfb_state_new(_orig_state: *mut std::os::raw::c_void, _orig_proto: AppProto) -> *mut std::os::raw::c_void { +pub extern "C" fn rs_rfb_state_new( + _orig_state: *mut std::os::raw::c_void, _orig_proto: AppProto, +) -> *mut std::os::raw::c_void { let state = RFBState::new(); let boxed = Box::new(state); return unsafe { transmute(boxed) }; @@ -532,23 +695,15 @@ pub extern "C" fn rs_rfb_state_free(state: *mut std::os::raw::c_void) { } #[no_mangle] -pub extern "C" fn rs_rfb_state_tx_free( - state: *mut std::os::raw::c_void, - tx_id: u64, -) { +pub extern "C" fn rs_rfb_state_tx_free(state: *mut std::os::raw::c_void, tx_id: u64) { let state = cast_pointer!(state, RFBState); state.free_tx(tx_id); } #[no_mangle] pub extern "C" fn rs_rfb_parse_request( - _flow: *const Flow, - state: *mut std::os::raw::c_void, - _pstate: *mut std::os::raw::c_void, - input: *const u8, - input_len: u32, - _data: *const std::os::raw::c_void, - _flags: u8, + _flow: *const Flow, state: *mut std::os::raw::c_void, _pstate: *mut std::os::raw::c_void, + input: *const u8, input_len: u32, _data: *const std::os::raw::c_void, _flags: u8, ) -> AppLayerResult { let state = cast_pointer!(state, RFBState); let buf = build_slice!(input, input_len as usize); @@ -557,13 +712,8 @@ pub extern "C" fn rs_rfb_parse_request( #[no_mangle] pub extern "C" fn rs_rfb_parse_response( - _flow: *const Flow, - state: *mut std::os::raw::c_void, - _pstate: *mut std::os::raw::c_void, - input: *const u8, - input_len: u32, - _data: *const std::os::raw::c_void, - _flags: u8, + _flow: *const Flow, state: *mut std::os::raw::c_void, _pstate: *mut std::os::raw::c_void, + input: *const u8, input_len: u32, _data: *const std::os::raw::c_void, _flags: u8, ) -> AppLayerResult { let state = cast_pointer!(state, RFBState); let buf = build_slice!(input, input_len as usize); @@ -572,8 +722,7 @@ pub extern "C" fn rs_rfb_parse_response( #[no_mangle] pub extern "C" fn rs_rfb_state_get_tx( - state: *mut std::os::raw::c_void, - tx_id: u64, + state: *mut std::os::raw::c_void, tx_id: u64, ) -> *mut std::os::raw::c_void { let state = cast_pointer!(state, RFBState); match state.get_tx(tx_id) { @@ -587,25 +736,20 @@ pub extern "C" fn rs_rfb_state_get_tx( } #[no_mangle] -pub extern "C" fn rs_rfb_state_get_tx_count( - state: *mut std::os::raw::c_void, -) -> u64 { +pub extern "C" fn rs_rfb_state_get_tx_count(state: *mut std::os::raw::c_void) -> u64 { let state = cast_pointer!(state, RFBState); return state.tx_id; } #[no_mangle] -pub extern "C" fn rs_rfb_state_progress_completion_status( - _direction: u8, -) -> std::os::raw::c_int { +pub extern "C" fn rs_rfb_state_progress_completion_status(_direction: u8) -> std::os::raw::c_int { // This parser uses 1 to signal transaction completion status. return 1; } #[no_mangle] pub extern "C" fn rs_rfb_tx_get_alstate_progress( - tx: *mut std::os::raw::c_void, - _direction: u8, + tx: *mut std::os::raw::c_void, _direction: u8, ) -> std::os::raw::c_int { let tx = cast_pointer!(tx, RFBTransaction); if tx.complete { @@ -616,7 +760,7 @@ pub extern "C" fn rs_rfb_tx_get_alstate_progress( #[no_mangle] pub extern "C" fn rs_rfb_state_get_events( - tx: *mut std::os::raw::c_void + tx: *mut std::os::raw::c_void, ) -> *mut core::AppLayerDecoderEvents { let tx = cast_pointer!(tx, RFBTransaction); return tx.events; @@ -624,38 +768,64 @@ pub extern "C" fn rs_rfb_state_get_events( #[no_mangle] pub extern "C" fn rs_rfb_state_get_event_info( - _event_name: *const std::os::raw::c_char, - _event_id: *mut std::os::raw::c_int, - _event_type: *mut core::AppLayerEventType, + event_name: *const std::os::raw::c_char, event_id: *mut std::os::raw::c_int, + event_type: *mut core::AppLayerEventType, ) -> std::os::raw::c_int { - return -1; + if event_name == std::ptr::null() { + return -1; + } + let c_event_name: &CStr = unsafe { CStr::from_ptr(event_name) }; + let event = match c_event_name.to_str() { + Ok(s) => { + match s { + "unimplemented_security_type" => RFBEvent::UnimplementedSecurityType as i32, + "unknown_security_result" => RFBEvent::UnknownSecurityResult as i32, + "malformed_message" => RFBEvent::MalformedMessage as i32, + "confused_state" => RFBEvent::ConfusedState as i32, + _ => -1, // unknown event + } + } + Err(_) => -1, // UTF-8 conversion failed + }; + unsafe { + *event_type = core::APP_LAYER_EVENT_TYPE_TRANSACTION; + *event_id = event as std::os::raw::c_int; + }; + 0 } #[no_mangle] -pub extern "C" fn rs_rfb_state_get_event_info_by_id(_event_id: std::os::raw::c_int, - _event_name: *mut *const std::os::raw::c_char, - _event_type: *mut core::AppLayerEventType +pub extern "C" fn rs_rfb_state_get_event_info_by_id( + event_id: std::os::raw::c_int, event_name: *mut *const std::os::raw::c_char, + event_type: *mut core::AppLayerEventType, ) -> i8 { - return -1; + if let Some(e) = RFBEvent::from_i32(event_id as i32) { + let estr = match e { + RFBEvent::UnimplementedSecurityType => "unimplemented_security_type\0", + RFBEvent::UnknownSecurityResult => "unknown_security_result\0", + RFBEvent::MalformedMessage => "malformed_message\0", + RFBEvent::ConfusedState => "confused_state\0", + }; + unsafe { + *event_name = estr.as_ptr() as *const std::os::raw::c_char; + *event_type = core::APP_LAYER_EVENT_TYPE_TRANSACTION; + }; + 0 + } else { + -1 + } } + #[no_mangle] pub extern "C" fn rs_rfb_state_get_tx_iterator( - _ipproto: u8, - _alproto: AppProto, - state: *mut std::os::raw::c_void, - min_tx_id: u64, - _max_tx_id: u64, - istate: &mut u64, + _ipproto: u8, _alproto: AppProto, state: *mut std::os::raw::c_void, min_tx_id: u64, + _max_tx_id: u64, istate: &mut u64, ) -> applayer::AppLayerGetTxIterTuple { let state = cast_pointer!(state, RFBState); match state.tx_iterator(min_tx_id, istate) { Some((tx, out_tx_id, has_next)) => { let c_tx = unsafe { transmute(tx) }; - let ires = applayer::AppLayerGetTxIterTuple::with_values( - c_tx, - out_tx_id, - has_next, - ); + let ires = applayer::AppLayerGetTxIterTuple::with_values(c_tx, out_tx_id, has_next); return ires; } None => { @@ -693,7 +863,7 @@ pub unsafe extern "C" fn rs_rfb_register_parser() { set_de_state: rs_rfb_tx_set_detect_state, get_events: Some(rs_rfb_state_get_events), get_eventinfo: Some(rs_rfb_state_get_event_info), - get_eventinfo_byid : Some(rs_rfb_state_get_event_info_by_id), + get_eventinfo_byid: Some(rs_rfb_state_get_event_info_by_id), localstorage_new: None, localstorage_free: None, get_files: None, @@ -706,18 +876,10 @@ pub unsafe extern "C" fn rs_rfb_register_parser() { let ip_proto_str = CString::new("tcp").unwrap(); - if AppLayerProtoDetectConfProtoDetectionEnabled( - ip_proto_str.as_ptr(), - parser.name, - ) != 0 - { + if AppLayerProtoDetectConfProtoDetectionEnabled(ip_proto_str.as_ptr(), parser.name) != 0 { let alproto = AppLayerRegisterProtocolDetection(&parser, 1); ALPROTO_RFB = alproto; - if AppLayerParserConfParserEnabled( - ip_proto_str.as_ptr(), - parser.name, - ) != 0 - { + if AppLayerParserConfParserEnabled(ip_proto_str.as_ptr(), parser.name) != 0 { let _ = AppLayerRegisterParser(&parser, alproto); } SCLogDebug!("Rust rfb parser registered.");