]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
rfb: never return error on unknown traffic
authorSascha Steinbiss <satta@debian.org>
Fri, 9 Jun 2023 09:13:35 +0000 (11:13 +0200)
committerSascha Steinbiss <satta@debian.org>
Fri, 14 Jul 2023 08:47:46 +0000 (10:47 +0200)
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)

rules/Makefile.am
rules/rfb-events.rules [new file with mode: 0644]
rust/src/rfb/detect.rs
rust/src/rfb/logger.rs
rust/src/rfb/mod.rs
rust/src/rfb/parser.rs
rust/src/rfb/rfb.rs

index e4d5635c51ae00271458945cfe9fedb78f24db5b..57116798368f290587e810a38ec3e9ea6e2f7443 100644 (file)
@@ -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 (file)
index 0000000..08bc493
--- /dev/null
@@ -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;)
index d30014a13999b7a4fa4a73a47d00108862a9a428..da876457ae54dfe4b848416a37c98a2e95b8bf7f 100644 (file)
@@ -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
+}
index c990b3dbb647d8917d2fc223402206bee2c83e5d..5f8be358cdd07f6e8dc18d2989ab9793058bf553 100644 (file)
 
 // Author: Frank Honza <frank.honza@dcso.de>
 
-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()
 }
index 4bde7935cb7145c4c6b7ee276aa804dc5c8be07d..68d37ec8a0eb1d222153e4d8faabf4b65379719d 100644 (file)
@@ -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;
index 9ec311eb3b4f6041c04b9d21320fbd38985da1f3..37c6d3d16eb6050761d9b0864afffbc06b308bea 100644 (file)
 
 // Author: Frank Honza <frank.honza@dcso.de>
 
-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<u8>
+    pub types: Vec<u8>,
 }
 
 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<u8>
+    pub secret: Vec<u8>,
 }
 
 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<u8>
+    pub name: Vec<u8>,
 }
 
 named!(pub parse_protocol_version<ProtocolVersion>,
@@ -257,8 +258,8 @@ named!(pub parse_server_init<ServerInit>,
 
 #[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);
             }
         }
     }
-
 }
index e167eb66d865f50814b3f9bfec8ee49257e20224..1ad79370dc92ea522ed331d84d23b1c9f6507cd4 100644 (file)
 // Author: Frank Honza <frank.honza@dcso.de>
 //         Sascha Steinbiss <sascha.steinbiss@dcso.de>
 
-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<RFBEvent> {
+        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<RFBTransaction>,
-    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::<String>()
+                                "supported_security_types: {}, types: {}",
+                                request.number_of_types,
+                                request
+                                    .types
+                                    .iter()
+                                    .map(ToString::to_string)
+                                    .map(|v| v + " ")
+                                    .collect::<String>()
                             );
 
                             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.");