Merge pull request #324 from zeek/topic/jsiwek/gh-320

Improve RFB (VNC) protocol parsing

src/analyzer/protocol/rfb/RFB.cc

@@ -15,6 +15,7 @@ RFB_Analyzer::RFB_Analyzer(Connection* c)
 	{
 	interp = new binpac::RFB::RFB_Conn(this);
 	had_gap = false;
+	invalid = false;
 	}
 
 RFB_Analyzer::~RFB_Analyzer()
@@ -49,6 +50,15 @@ void RFB_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
 		// deliver data to the other side if the script layer can handle this.
 		return;
 
+	if ( invalid )
+		return;
+
+	if ( interp->saw_handshake() && ! orig )
+		// Don't try parsing server data after the handshake
+		// (it's not completely implemented and contains mostly
+		// uninteresting pixel data).
+		return;
+
 	try
 		{
 		interp->NewData(orig, data, data + len);
@@ -56,6 +66,7 @@ void RFB_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
 	catch ( const binpac::Exception& e )
 		{
 		ProtocolViolation(fmt("Binpac exception: %s", e.c_msg()));
+		invalid = true;
 		}
 	}
 

src/analyzer/protocol/rfb/RFB.h

@@ -35,6 +35,7 @@ protected:
 	binpac::RFB::RFB_Conn* interp;
 
 	bool had_gap;
+	bool invalid;
 
 };
 

src/analyzer/protocol/rfb/rfb-analyzer.pac

@@ -30,7 +30,7 @@ refine flow RFB_Flow += {
 		return true;
 		%}
 
-	function proc_security_types(msg: RFBSecurityTypes) : bool
+	function proc_security_types(msg: RFBSecurityType) : bool
 		%{
 		if ( rfb_authentication_type )
 			BifEvent::generate_rfb_authentication_type(connection()->bro_analyzer(), connection()->bro_analyzer()->Conn(), ${msg.sectype});
@@ -47,7 +47,15 @@ refine flow RFB_Flow += {
 	function proc_handle_server_params(msg:RFBServerInit) : bool
 		%{
 		if ( rfb_server_parameters )
-			BifEvent::generate_rfb_server_parameters(connection()->bro_analyzer(), connection()->bro_analyzer()->Conn(), bytestring_to_val(${msg.name}), ${msg.width}, ${msg.height});
+			{
+			auto vec_ptr = ${msg.name};
+			auto name_ptr = &((*vec_ptr)[0]);
+			BifEvent::generate_rfb_server_parameters(
+			    connection()->bro_analyzer(), connection()->bro_analyzer()->Conn(),
+			    new StringVal(${msg.name}->size(), (const char*)name_ptr),
+			    ${msg.width},
+			    ${msg.height});
+			}
 		return true;
 		%}
 
@@ -61,27 +69,66 @@ refine flow RFB_Flow += {
 
 refine connection RFB_Conn += {
 	%member{
-		enum states {
+		enum ServerState {
-			AWAITING_SERVER_BANNER = 0,
+			SERVER_VERSION            =  0,
-			AWAITING_CLIENT_BANNER = 1,
+			SERVER_AUTH_TYPE          =  1,
-			AWAITING_SERVER_AUTH_TYPES = 2,
+			SERVER_AUTH_TYPE37        =  2,
-			AWAITING_SERVER_CHALLENGE = 3,
+			SERVER_AUTH_FAILURE       =  3,
-			AWAITING_CLIENT_RESPONSE = 4,
+			SERVER_AUTH_VNC_CHALLENGE =  4,
-			AWAITING_SERVER_AUTH_RESULT = 5,
+			SERVER_AUTH_ARD_CHALLENGE =  5,
-			AWAITING_CLIENT_SHARE_FLAG = 6,
+			SERVER_AUTH_RESULT        =  6,
-			AWAITING_SERVER_PARAMS = 7,
+			SERVER_INIT               =  7,
-			AWAITING_CLIENT_AUTH_METHOD = 8,
+			SERVER_MESSAGE_TYPE       =  8,
-			AWAITING_SERVER_ARD_CHALLENGE = 9,
+			SERVER_MESSAGE            =  9,
-			AWAITING_CLIENT_ARD_RESPONSE = 10,
+			SERVER_WAIT               = 99,
-			AWAITING_SERVER_AUTH_TYPES37 = 11,
+			SERVER_INVALID            =100,
-			AWAITING_CLIENT_AUTH_TYPE_SELECTED37 = 12,
-			RFB_MESSAGE = 13
 		};
+
+		enum ClientState {
+			CLIENT_VERSION           =  0,
+			CLIENT_AUTH_SELECTION    =  1,
+			CLIENT_AUTH_VNC_RESPONSE =  2,
+			CLIENT_AUTH_ARD_RESPONSE =  3,
+			CLIENT_INIT              =  4,
+			CLIENT_MESSAGE_TYPE      =  5,
+			CLIENT_MESSAGE           =  6,
+			CLIENT_WAIT              = 99,
+			CLIENT_INVALID           =100,
+		};
+
+		int version = 0;
+		uint8 client_state = CLIENT_VERSION;
+		uint8 server_state = SERVER_VERSION;
+		uint16 ard_key_length = 0;
+		uint8 next_client_msg = 0;
+		uint8 next_server_msg = 0;
+		uint8 bytes_per_pixel = 0;
+		bool saw_full_handshake = false;
+	%}
+
+	function saw_handshake() : bool
+		%{
+		return saw_full_handshake;
+		%}
+
+	function get_ard_key_length() : uint16
+		%{
+		return ard_key_length;
 		%}
 
 	function get_state(client: bool) : int
 		%{
-		return state;
+		return client ? client_state : server_state;
+		%}
+
+	function get_next_msg_type(client: bool) : uint8
+		%{
+		return client ? next_client_msg : next_server_msg;
+		%}
+
+	function get_bytes_per_pixel() : uint8
+		%{
+		return bytes_per_pixel;
 		%}
 
 	function handle_banners(client: bool, msg: RFBProtocolVersion) : bool
@@ -94,115 +141,234 @@ refine connection RFB_Conn += {
 
 			// Apple specifies minor version "889" but talks v37
 			if ( minor_version >= 7 )
-				state = AWAITING_SERVER_AUTH_TYPES37;
-			else
-				state = AWAITING_SERVER_AUTH_TYPES;
-			}
-			else
-				state = AWAITING_CLIENT_BANNER;
-
-		return true;
-		%}
-
-	function handle_ard_challenge() : bool
-		%{
-		state = AWAITING_CLIENT_ARD_RESPONSE;
-		return true;
-		%}
-
-	function handle_ard_response() : bool
-		%{
-		state = AWAITING_SERVER_AUTH_RESULT;
-		return true;
-		%}
-
-	function handle_auth_request() : bool
-		%{
-		state = AWAITING_CLIENT_RESPONSE;
-		return true;
-		%}
-
-	function handle_auth_response() : bool
-		%{
-		state = AWAITING_SERVER_AUTH_RESULT;
-		return true;
-		%}
-
-	function handle_security_result(msg: RFBSecurityResult) : bool
-		%{
-		if ( ${msg.result} == 0 )
 				{
-			state = AWAITING_CLIENT_SHARE_FLAG;
+				server_state = SERVER_AUTH_TYPE37;
+				client_state = CLIENT_AUTH_SELECTION;
 				}
+			else
+				{
+				server_state = SERVER_AUTH_TYPE;
+				client_state = CLIENT_WAIT;
+				}
+
+			}
+		else
+			server_state = SERVER_WAIT;
+
 		return true;
 		%}
 
-	function handle_client_init(msg: RFBClientInit) : bool
+	function handle_security_type(msg: RFBSecurityType): bool
-		%{
-		state = AWAITING_SERVER_PARAMS;
-		return true;
-		%}
-
-	function handle_server_init(msg: RFBServerInit) : bool
-		%{
-		state = RFB_MESSAGE;
-		return true;
-		%}
-
-	function handle_security_types(msg: RFBSecurityTypes): bool
 		%{
 		if ( msg->sectype() == 0 )
-			{ // No auth
+			{
-			state = AWAITING_CLIENT_SHARE_FLAG;
+			// Invalid / failure.
-			return true;
+			server_state = SERVER_AUTH_FAILURE;
+			client_state = CLIENT_INIT;
+			}
+		else if ( msg->sectype() == 1 )
+			{
+			// No auth.
+			server_state = SERVER_INIT;
+			client_state = CLIENT_INIT;
+			}
+		else if ( msg->sectype() == 2 )
+			{
+			// VNC auth.
+			server_state = SERVER_AUTH_VNC_CHALLENGE;
+			client_state = CLIENT_AUTH_VNC_RESPONSE;
+			}
+		else
+			{
+			// Shouldn't be a possible.
+			bro_analyzer()->ProtocolViolation(fmt("invalid RFB security type %u", msg->sectype()));
 			}
 
-		if ( msg->sectype() == 2 )
+		return true;
-			{ // VNC
+		%}
-			if ( ${msg.possible_challenge}.length() == 16 )
+
-				// Challenge was already sent with this message
+	function handle_fail_reason_string(msg: RFBFailReasonString): bool
-				state = AWAITING_CLIENT_RESPONSE;
+		%{
-			else
+		// Connection failed, server should close, but maybe see if it
-				state = AWAITING_SERVER_CHALLENGE;
+		// proceeds anyway.
-			}
+		server_state = SERVER_INIT;
 		return true;
 		%}
 
 	function handle_security_types37(msg: RFBSecurityTypes37): bool
 		%{
 		if ( ${msg.count} == 0 )
-			{ // No auth
+			{
-			state = AWAITING_CLIENT_SHARE_FLAG;
+			server_state = SERVER_AUTH_FAILURE;
-			return true;
 			}
-		state = AWAITING_CLIENT_AUTH_TYPE_SELECTED37;
+		else
+			{
+			server_state = SERVER_WAIT;
+			}
+
 		return true;
 		%}
 
 	function handle_auth_type_selected(msg: RFBAuthTypeSelected): bool
 		%{
-		if ( ${msg.type} == 30 )
-			{ // Apple Remote Desktop
-				state = AWAITING_SERVER_ARD_CHALLENGE;
-				return true;
-			}
-
 		if ( ${msg.type} == 1 )
 			{
 			if ( version > 7 )
-					state = AWAITING_SERVER_AUTH_RESULT;
+				server_state = SERVER_AUTH_RESULT;
 			else
-					state = AWAITING_CLIENT_SHARE_FLAG;
+				server_state = SERVER_INIT;
+
+			client_state = CLIENT_INIT;
+			}
+		else if ( ${msg.type} == 2 )
+			{
+			server_state = SERVER_AUTH_VNC_CHALLENGE;
+			client_state = CLIENT_AUTH_VNC_RESPONSE;
+			}
+		else if ( ${msg.type} == 30 )
+			{
+			// Apple Remote Desktop
+			server_state = SERVER_AUTH_ARD_CHALLENGE;
+			//client_state = CLIENT_AUTH_ARD_RESPONSE;
+			// need to wait for the key length to be set by server
+			client_state = CLIENT_WAIT;
 			}
 		else
-			state = AWAITING_SERVER_CHALLENGE;
+			{
+			bro_analyzer()->ProtocolViolation(fmt("unknown RFB auth selection: %u", ${msg.type}));
+			}
 
 		return true;
 		%}
 
-	%member{
+	function handle_ard_challenge(msg: RFBSecurityARDChallenge) : bool
-		uint8 state = AWAITING_SERVER_BANNER;
+		%{
-		int version = 0;
+		ard_key_length = ${msg.key_length};
+		server_state = SERVER_AUTH_RESULT;
+		client_state = CLIENT_AUTH_ARD_RESPONSE;
+		return true;
+		%}
+
+	function handle_ard_response() : bool
+		%{
+		client_state = CLIENT_INIT;
+		return true;
+		%}
+
+	function handle_auth_request() : bool
+		%{
+		server_state = SERVER_AUTH_RESULT;
+		client_state = CLIENT_AUTH_VNC_RESPONSE;
+		return true;
+		%}
+
+	function handle_auth_response() : bool
+		%{
+		client_state = CLIENT_INIT;
+		return true;
+		%}
+
+	function handle_security_result(msg: RFBSecurityResult) : bool
+		%{
+		if ( ${msg.result} == 0 )
+			// OK
+			server_state = SERVER_INIT;
+		else if ( ${msg.result} == 1 )
+			// Failed
+			server_state = SERVER_AUTH_FAILURE;
+		else
+			bro_analyzer()->ProtocolViolation(fmt("invalid RFB auth result: %u", ${msg.result}));
+
+		return true;
+		%}
+
+	function handle_client_init(msg: RFBClientInit) : bool
+		%{
+		client_state = CLIENT_MESSAGE_TYPE;
+		return true;
+		%}
+
+	function handle_server_init(msg: RFBServerInit) : bool
+		%{
+		auto bits_per_pixel = (*${msg.pixel_format})[0];
+		bytes_per_pixel = bits_per_pixel / 8;
+		server_state = SERVER_MESSAGE_TYPE;
+		saw_full_handshake = true;
+		return true;
+		%}
+
+	function handle_wait_data(client: bool) : bool
+		%{
+		if ( client )
+			client_state = CLIENT_INVALID;
+		else
+			server_state = SERVER_INVALID;
+
+		return true;
+		%}
+
+	function handle_invalid_data(client: bool) : bool
+		%{
+		throw binpac::Exception(fmt("invalid data from RFB %s", client ? "client" : "server"));
+		return true;
+		%}
+
+	function handle_client_message_type(type: uint8) : bool
+		%{
+		next_client_msg = type;
+		client_state = CLIENT_MESSAGE;
+		return true;
+		%}
+
+	function handle_client_message(type: uint8) : bool
+		%{
+		client_state = CLIENT_MESSAGE_TYPE;
+		return true;
+		%}
+
+	function handle_server_message_type(type: uint8) : bool
+		%{
+		next_server_msg = type;
+		server_state = SERVER_MESSAGE;
+		return true;
+		%}
+
+	function handle_server_message(type: uint8) : bool
+		%{
+		server_state = SERVER_MESSAGE_TYPE;
+		return true;
+		%}
+
+	function handle_client_set_pixel_format(msg: ClientSetPixelFormat) : bool
+		%{
+		auto bits_per_pixel = (*${msg.pixel_format})[0];
+		bytes_per_pixel = bits_per_pixel / 8;
+		return true;
+		%}
+
+	function handle_client_set_encodings(msg: ClientSetEncodings) : bool
+		%{
+		return true;
+		%}
+
+	function handle_client_framebuffer_update_request(msg: ClientFramebufferUpdateRequest) : bool
+		%{
+		return true;
+		%}
+
+	function handle_client_key_event(msg: ClientKeyEvent) : bool
+		%{
+		return true;
+		%}
+
+	function handle_client_pointer_event(msg: ClientPointerEvent) : bool
+		%{
+		return true;
+		%}
+
+	function handle_client_cut_text(msg: ClientCutText) : bool
+		%{
+		return true;
 		%}
 };
 

src/analyzer/protocol/rfb/rfb-protocol.pac

@@ -1,20 +1,35 @@
-enum states {
+enum ServerState {
-			AWAITING_SERVER_BANNER = 0,
+	SERVER_VERSION            =  0,
-			AWAITING_CLIENT_BANNER = 1,
+	SERVER_AUTH_TYPE          =  1,
-			AWAITING_SERVER_AUTH_TYPES = 2,
+	SERVER_AUTH_TYPE37        =  2,
-			AWAITING_SERVER_CHALLENGE = 3,
+	SERVER_AUTH_FAILURE       =  3,
-			AWAITING_CLIENT_RESPONSE = 4,
+	SERVER_AUTH_VNC_CHALLENGE =  4,
-			AWAITING_SERVER_AUTH_RESULT = 5,
+	SERVER_AUTH_ARD_CHALLENGE =  5,
-			AWAITING_CLIENT_SHARE_FLAG = 6,
+	SERVER_AUTH_RESULT        =  6,
-			AWAITING_SERVER_PARAMS = 7,
+	SERVER_INIT               =  7,
-			AWAITING_CLIENT_AUTH_METHOD = 8,
+	SERVER_MESSAGE_TYPE       =  8,
-			AWAITING_SERVER_ARD_CHALLENGE = 9,
+	SERVER_MESSAGE            =  9,
-			AWAITING_CLIENT_ARD_RESPONSE = 10,
+	SERVER_WAIT               = 99,
-			AWAITING_SERVER_AUTH_TYPES37 = 11,
+	SERVER_INVALID            =100,
-			AWAITING_CLIENT_AUTH_TYPE_SELECTED37 = 12,
-			RFB_MESSAGE = 13
 };
 
+enum ClientState {
+	CLIENT_VERSION           =  0,
+	CLIENT_AUTH_SELECTION    =  1,
+	CLIENT_AUTH_VNC_RESPONSE =  2,
+	CLIENT_AUTH_ARD_RESPONSE =  3,
+	CLIENT_INIT              =  4,
+	CLIENT_MESSAGE_TYPE      =  5,
+	CLIENT_MESSAGE           =  6,
+	CLIENT_WAIT              = 99,
+	CLIENT_INVALID           =100,
+};
+
+# The protocol specifies some 32-bit variable-length data fields with the
+# length derived from packet data.
+# This value enforces sane length values to help prevent excessive buffering.
+let MAX_DATA_LENGTH: uint32 = 65536;
+
 type RFBProtocolVersion (client: bool) = record {
 	header: "RFB ";
 	major_ver: bytestring &length=3;
@@ -24,117 +39,317 @@ type RFBProtocolVersion (client: bool) = record {
 } &let {
 	proc: bool = $context.connection.handle_banners(client, this);
 	proc2: bool = $context.flow.proc_rfb_version(client, major_ver, minor_ver);
-}
+} &length=12;
 
-type RFBSecurityTypes = record {
+type RFBFailReasonString = record {
-	sectype: uint32;
+	len: uint32 &enforce(len < MAX_DATA_LENGTH);
-	possible_challenge: bytestring &restofdata;
+	str: bytestring &length=len;
 } &let {
-	proc: bool = $context.connection.handle_security_types(this);
+	proc: bool = $context.connection.handle_fail_reason_string(this);
+} &length=(4 + len);
+
+type RFBSecurityType = record {
+	sectype: uint32;
+} &let {
+	proc: bool = $context.connection.handle_security_type(this);
 	proc2: bool = $context.flow.proc_security_types(this);
-};
+} &length=4;
 
 type RFBSecurityTypes37 = record {
 	count: uint8;
 	types: uint8[count];
 } &let {
 	proc: bool = $context.connection.handle_security_types37(this);
-};
+} &length=(count + 1);
 
 type RFBAuthTypeSelected = record {
 	type: uint8;
 } &let {
 	proc: bool = $context.connection.handle_auth_type_selected(this);
 	proc2: bool = $context.flow.proc_security_types37(this);
-};
+} &length=1;
 
 type RFBSecurityResult = record {
 	result: uint32;
 } &let {
 	proc: bool = $context.connection.handle_security_result(this);
 	proc2: bool = $context.flow.proc_handle_security_result(result);
-};
+} &length=4;
-
-type RFBSecurityResultReason = record {
-	len: uint32;
-	reason: bytestring &length=len;
-};
 
 type RFBVNCAuthenticationRequest = record {
 	challenge: bytestring &length=16;
 } &let {
 	proc: bool = $context.connection.handle_auth_request();
-};
+} &length=16;
 
 type RFBVNCAuthenticationResponse = record {
 	response: bytestring &length= 16;
 } &let {
 	proc: bool = $context.connection.handle_auth_response();
-};
+} &length=16;
 
 type RFBSecurityARDChallenge = record {
-	challenge: bytestring &restofdata;
+	# TODO: Not sure if this all is complete/accurate, could not find the spec.
+	generator: uint16;
+	key_length: uint16;
+	prime_mod: bytestring &length=key_length;
+	publickey: bytestring &length=key_length;
 } &let {
-	proc: bool = $context.connection.handle_ard_challenge();
+	proc: bool = $context.connection.handle_ard_challenge(this);
-}
+} &length=(4 + (2 * key_length));
 
 type RFBSecurityARDResponse = record {
-	response: bytestring &restofdata;
+	publickey: bytestring &length=$context.connection.get_ard_key_length();
+	creds: bytestring &length=$context.connection.get_ard_key_length();
 } &let {
 	proc: bool = $context.connection.handle_ard_response();
-}
+} &length=(2 * $context.connection.get_ard_key_length());
 
 type RFBClientInit = record {
 	shared_flag: uint8;
 } &let {
 	proc: bool = $context.connection.handle_client_init(this);
 	proc2: bool = $context.flow.proc_rfb_share_flag(shared_flag);
-}
+} &length=1;
 
 type RFBServerInit = record {
 	width: uint16;
 	height: uint16;
-	pixel_format: bytestring &length= 16;
+	pixel_format: uint8[16];
-	len : uint32;
+	len: uint32 &enforce(len < MAX_DATA_LENGTH);
-	name: bytestring &length = len;
+	name: uint8[len];
 } &let {
 	proc: bool = $context.connection.handle_server_init(this);
 	proc2: bool = $context.flow.proc_handle_server_params(this);
-};
+} &length=24 + len;
 
-type RFB_PDU_request = record {
+type InvalidData(orig: bool) = record {
-	request: case state of {
+	invalid: uint8;
-		AWAITING_CLIENT_BANNER -> version: RFBProtocolVersion(true);
-		AWAITING_CLIENT_RESPONSE -> response: RFBVNCAuthenticationResponse;
-		AWAITING_CLIENT_SHARE_FLAG -> shareflag: RFBClientInit;
-		AWAITING_CLIENT_AUTH_TYPE_SELECTED37 -> authtype: RFBAuthTypeSelected;
-		AWAITING_CLIENT_ARD_RESPONSE -> ard_response: RFBSecurityARDResponse;
-		RFB_MESSAGE -> ignore: bytestring &restofdata &transient;
-		default -> data: bytestring &restofdata &transient;
-	} &requires(state);
 } &let {
-		state: uint8 = $context.connection.get_state(true);
+	proc: bool = $context.connection.handle_invalid_data(orig);
-};
+} &length=1;
 
-type RFB_PDU_response = record {
+type WaitData(orig: bool) = record {
-	request: case rstate of {
+	nothing: bytestring &length = 0;
-		AWAITING_SERVER_BANNER -> version: RFBProtocolVersion(false);
-		AWAITING_SERVER_AUTH_TYPES -> auth_types: RFBSecurityTypes;
-		AWAITING_SERVER_AUTH_TYPES37 -> auth_types37: RFBSecurityTypes37;
-		AWAITING_SERVER_CHALLENGE -> challenge: RFBVNCAuthenticationRequest;
-		AWAITING_SERVER_AUTH_RESULT -> authresult : RFBSecurityResult;
-		AWAITING_SERVER_ARD_CHALLENGE -> ard_challenge: RFBSecurityARDChallenge;
-		AWAITING_SERVER_PARAMS -> serverinit: RFBServerInit;
-		RFB_MESSAGE -> ignore: bytestring &restofdata &transient;
-		default -> data: bytestring &restofdata &transient;
-	} &requires(rstate);
 } &let {
-		rstate: uint8 = $context.connection.get_state(false);
+	proc: bool = $context.connection.handle_wait_data(orig);
+} &length=0;
+
+type ClientMessageType = record {
+	type: uint8;
+} &let {
+	proc: bool = $context.connection.handle_client_message_type(type);
+} &length=1;
+
+type ClientMessage(type: uint8) = case type of {
+	0 -> set_pixel_format: ClientSetPixelFormat;
+	2 -> set_encodings: ClientSetEncodings;
+	3 -> framebuffer_update_request: ClientFramebufferUpdateRequest;
+	4 -> key_event: ClientKeyEvent;
+	5 -> pointer_event: ClientPointerEvent;
+	6 -> cut_text: ClientCutText;
+} &let {
+	proc: bool = $context.connection.handle_client_message(type);
 };
 
-type RFB_PDU(is_orig: bool) = record {
+type ClientSetPixelFormat = record {
-	payload: case is_orig of {
+	pad: uint8[3];
-		true -> request: RFB_PDU_request;
+	pixel_format: uint8[16];
-		false -> response: RFB_PDU_response;
+} &let {
+	proc: bool = $context.connection.handle_client_set_pixel_format(this);
+} &length=19;
+
+type ClientSetEncodings = record {
+	pad: uint8;
+	num_encodings: uint16;
+	encodings: uint32[num_encodings];
+} &let {
+	proc: bool = $context.connection.handle_client_set_encodings(this);
+} &length=3 + (4 * num_encodings);
+
+type ClientFramebufferUpdateRequest = record {
+	incremental: uint8;
+	xpos: uint16;
+	ypos: uint16;
+	width: uint16;
+	height: uint16;
+} &let {
+	proc: bool = $context.connection.handle_client_framebuffer_update_request(this);
+} &length=9;
+
+type ClientKeyEvent = record {
+	down_flag: uint8;
+	pad: uint16;
+	key: uint32;
+} &let {
+	proc: bool = $context.connection.handle_client_key_event(this);
+} &length=7;
+
+type ClientPointerEvent = record {
+	button_mask: uint8;
+	xpos: uint16;
+	ypos: uint16;
+} &let {
+	proc: bool = $context.connection.handle_client_pointer_event(this);
+} &length=5;
+
+type ClientCutText = record {
+	pad: uint8[3];
+	len: uint32 &enforce(len < MAX_DATA_LENGTH);
+	text: bytestring &length=len;
+} &let {
+	proc: bool = $context.connection.handle_client_cut_text(this);
+} &length=(7 + len);
+
+type ServerMessageType = record {
+	type: uint8;
+} &let {
+	proc: bool = $context.connection.handle_server_message_type(type);
+} &length=1;
+
+type ServerMessage(type: uint8) = case type of {
+	0 -> framebuffer_update: ServerFramebufferUpdate;
+	1 -> set_color_map_entries: ServerSetColorMapEntries;
+	2 -> bell: ServerBell;
+	3 -> cut_text: ServerCutText;
+} &let {
+	proc: bool = $context.connection.handle_server_message(type);
 };
+
+type PixelData(encoding: int32, x: uint16, y: uint16, w: uint16, h: uint16) = case encoding of {
+	   0   -> raw: PD_Raw(w, h);
+	   1   -> copy_rec: PD_CopyRec;
+	   2   -> rre: PD_RRE;
+	   5   -> hextile: PD_Hextile;
+	  15   -> trle: PD_TRLE;
+	  16   -> zrle: PD_ZRLE;
+	# TODO: binpac is not happy with negative values here
+	#-239   -> cursor_pseudo: PD_PsuedoCursor;
+	#-223   -> desktop_size: PD_PsuedoDesktopSize;
+};
+
+type PD_Raw(w: uint16, h: uint16) = record {
+	pixels: bytestring &length=(w * h * $context.connection.get_bytes_per_pixel()) &transient;
+} &length=(w * h * $context.connection.get_bytes_per_pixel());
+
+type PD_CopyRec = record {
+	xpos: uint16;
+	ypos: uint16;
+} &length=4;
+
+type RRE_Subrect = record {
+	pixel: bytestring &length=$context.connection.get_bytes_per_pixel();
+	xpos: uint16;
+	ypos: uint16;
+	width: uint16;
+	height: uint16;
+} &length=$context.connection.get_bytes_per_pixel() + 8;
+
+type PD_RRE = record {
+	num_subrects: uint32;
+	bg_pixel: bytestring &length=$context.connection.get_bytes_per_pixel();
+	subrects: RRE_Subrect[num_subrects] &transient;
+} &length=4 + $context.connection.get_bytes_per_pixel() + (num_subrects * ($context.connection.get_bytes_per_pixel() + 8));
+
+type PD_Hextile = record {
+	# TODO
+	nothing: empty;
+} &length=0;
+
+type PD_TRLE = record {
+	# TODO
+	nothing: empty;
+} &length=0;
+
+type PD_ZRLE = record {
+	len: uint32;
+	zlib_data: bytestring &length=len &transient;
+} &length=(4 + len);
+
+type PD_PsuedoCursor(w: uint16, h: uint16) = record {
+	pixels: bytestring &length=(w * h * $context.connection.get_bytes_per_pixel()) &transient;
+	bitmask: bytestring &length=(h * ((w + 7) / 8)) &transient;
+} &length=(w * h * $context.connection.get_bytes_per_pixel()) + (h * ((w + 7) / 8))
+
+type PD_PsuedoDesktopSize = record {
+	# Actually no further data
+	nothing: empty;
+} &length=0;
+
+type Rectangle = record {
+	xpos: uint16;
+	ypos: uint16;
+	width: uint16;
+	height: uint16;
+	encoding: int32;
+	pixel_data: PixelData(encoding, xpos, ypos, width, height);
+	# TODO add in pixel_data length to &length
+} &length=12;
+
+type ServerFramebufferUpdate = record {
+	pad: uint8;
+	num_rects: uint16;
+	rects: Rectangle[num_rects];
+	# TODO add in Rectangle[] length to &length
+} &length=3;
+
+type RGB_Value = record {
+	red: uint16;
+	green: uint16;
+	blue: uint16;
+} &length=6;
+
+type ServerSetColorMapEntries = record {
+	pad: uint8;
+	first_color: uint16;
+	num_colors: uint16;
+	colors: RGB_Value[num_colors];
+} &length=5 + (num_colors * 6)
+
+type ServerBell = record {
+	nothing: empty;
+} &length=0;
+
+type ServerCutText = record {
+	pad: uint8[3];
+	len: uint32 &enforce(len < MAX_DATA_LENGTH);
+	text: bytestring &length=len;
+} &length=(7 + len);
+
+type RFB_PDU_request(state: uint8) = case state of {
+	CLIENT_WAIT -> wait: WaitData(true);
+	CLIENT_INVALID -> invalid: InvalidData(true);
+
+	CLIENT_VERSION -> version: RFBProtocolVersion(true);
+	CLIENT_AUTH_SELECTION -> authtype: RFBAuthTypeSelected; # version 3.7+
+	CLIENT_AUTH_VNC_RESPONSE -> response: RFBVNCAuthenticationResponse;
+	CLIENT_AUTH_ARD_RESPONSE -> ard_response: RFBSecurityARDResponse;
+	CLIENT_INIT -> shareflag: RFBClientInit;
+
+	CLIENT_MESSAGE_TYPE -> msg_type: ClientMessageType;
+	CLIENT_MESSAGE -> msg: ClientMessage($context.connection.get_next_msg_type(true));
+};
+
+type RFB_PDU_response(state: uint8) = case state of {
+	SERVER_WAIT -> wait: WaitData(false);
+	SERVER_INVALID -> invalid: InvalidData(false);
+
+	SERVER_VERSION -> version: RFBProtocolVersion(false);
+	SERVER_AUTH_TYPE -> auth_type: RFBSecurityType;
+	SERVER_AUTH_TYPE37 -> auth_types37: RFBSecurityTypes37;
+	SERVER_AUTH_FAILURE -> fail_reason: RFBFailReasonString;
+	SERVER_AUTH_VNC_CHALLENGE -> challenge: RFBVNCAuthenticationRequest;
+	SERVER_AUTH_ARD_CHALLENGE -> ard_challenge: RFBSecurityARDChallenge;
+	SERVER_AUTH_RESULT -> authresult : RFBSecurityResult;
+	SERVER_INIT -> serverinit: RFBServerInit;
+
+	SERVER_MESSAGE_TYPE -> msg_type: ServerMessageType;
+	# TODO: server message parsing (framebuffer update) is not completely implemented
+	#       as it is mostly uninteresting
+	SERVER_MESSAGE -> msg: ServerMessage($context.connection.get_next_msg_type(false));
+};
+
+type RFB_PDU(is_orig: bool) = case is_orig of {
+	true -> request: RFB_PDU_request($context.connection.get_state(true));
+	false -> response: RFB_PDU_response($context.connection.get_state(false));
 } &byteorder = bigendian;

src/analyzer/protocol/rfb/rfb.pac

@@ -24,7 +24,7 @@ connection RFB_Conn(bro_analyzer: BroAnalyzer) {
 
 # Now we define the flow:
 flow RFB_Flow(is_orig: bool) {
-	datagram = RFB_PDU(is_orig) withcontext(connection, this);
+	flowunit = RFB_PDU(is_orig) withcontext(connection, this);
 };
 
 %include rfb-analyzer.pac

testing/btest/Baseline/scripts.base.protocols.rfb.vnc-scanner/rfb.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	rfb
+#open	2019-04-03-20-57-33
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	client_major_version	client_minor_version	server_major_version	server_minor_version	authentication_method	auth	share_flag	desktop_name	width	height
+#types	time	string	addr	port	addr	port	string	string	string	string	string	bool	bool	string	count	count
+1551120432.417278	CHhAvVGS1DHFjwGM9	192.168.0.11	46381	10.0.0.149	5900	003	008	003	008	VNC	F	-	-	-	-
+#close	2019-04-03-20-57-33

testing/btest/scripts/base/protocols/rfb/vnc-scanner.bro

@@ -0,0 +1,4 @@
+# @TEST-EXEC: zeek -C -r $TRACES/rfb/vnc-scanner.pcap
+# @TEST-EXEC: btest-diff rfb.log
+
+@load base/protocols/rfb