Merge remote-tracking branch 'origin/topic/johanna/gh-4237'

* origin/topic/johanna/gh-4237:
  RDP: cookie is optional

CHANGES

@@ -1,3 +1,12 @@
+7.2.0-dev.253 | 2025-03-04 13:38:32 +0000
+
+  * RDP: cookie is optional (Johanna Amann, Corelight)
+
+    RDP connections are now parsed even if the optional cookie value
+    is not present.
+
+    Fixes GH-4237
+
 7.2.0-dev.250 | 2025-03-04 12:07:01 +0100
 
   * Update zeek.bif documentation for to_count and to_int (Mark Overholser)

NEWS

@@ -89,6 +89,9 @@ Changed Functionality
   warnings instead of builtin errors when hitting trouble. This allows Zeek to
   continue gracefully in case of such problems, particularly during zeek_init().
 
+- The RDP analyzer now also parses connections that do not contain the cookie
+  field, which were previously rejected.
+
 Removed Functionality
 ---------------------
 

VERSION

@@ -1 +1 @@
-7.2.0-dev.250
+7.2.0-dev.253

src/analyzer/protocol/rdp/events.bif

@@ -39,7 +39,7 @@ event rdp_native_encrypted_data%(c: connection, orig: bool, len: count%);
 ##
 ## c: The connection record for the underlying transport-layer session/flow.
 ##
-## cookie: The cookie included in the request.
+## cookie: The cookie included in the request; empty if no cookie was provided.
 ##
 ## flags: The flags set by the client.
 event rdp_connect_request%(c: connection, cookie: string, flags: count%);

src/analyzer/protocol/rdp/rdp-analyzer.pac

@@ -10,9 +10,15 @@ refine flow RDP_Flow += {
 		%{
 		if ( rdp_connect_request )
 			{
+			zeek::StringValPtr cookie_value;
+			if ( ${cr.cookie} )
+				cookie_value = to_stringval(${cr.cookie.cookie_value});
+			else
+				cookie_value = zeek::val_mgr->EmptyString();
+
 			zeek::BifEvent::enqueue_rdp_connect_request(connection()->zeek_analyzer(),
 			                                      connection()->zeek_analyzer()->Conn(),
-			                                      to_stringval(${cr.cookie_value}),
+			                                      cookie_value,
 			                                      ${cr.rdp_neg_req} ? ${cr.rdp_neg_req.flags} : 0);
 			}
 

src/analyzer/protocol/rdp/rdp-protocol.pac

@@ -74,17 +74,26 @@ type Data_Block = record {
 # Client X.224
 ######################################################################
 
+type RDP_Connect_Request_cookie = record {
+	cookie_value:          RE/[^\x0d]*/;
+	cookie_terminator:     RE/\x0d\x0a/;
+} &byteorder=littleendian;
+
 type Connect_Request(cotp: COTP) = record {
 	destination_reference: uint16;
 	source_reference:      uint16;
 	flow_control:          uint8;
-	cookie_mstshash:       RE/Cookie: mstshash\=/;
-	cookie_value:          RE/[^\x0d]*/;
-	cookie_terminator:     RE/\x0d\x0a/;
+	cookie_mstshash:       RE/(Cookie: mstshash\=)?/;
+	# Cookie is optional as per
+	# https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/18a27ef9-6f9a-4501-b000-94b1fe3c2c10
+	switch1: case (sizeof(cookie_mstshash) > 0 ) of {
+		0       -> none1:          empty;
+		default -> cookie:         RDP_Connect_Request_cookie;
+	};
 	# Terrifying little case statement to figure out if there
 	# is any data left in the COTP structure.
-	switch1:   case (offsetof(switch1) + 2 - cotp.cotp_len - 1) of {
-		0       -> none:        empty;
+	switch2:   case (offsetof(switch2) + 2 - cotp.cotp_len - 1) of {
+		0       -> none2:          empty;
 		default -> rdp_neg_req:    RDP_Negotiation_Request;
 	};
 } &byteorder=littleendian;

testing/btest/Baseline/scripts.base.protocols.rdp.rdp-no-cookie-msthash/rdp.log

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	rdp
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cookie	result	security_protocol	client_channels	keyboard_layout	client_build	client_name	client_dig_product_id	desktop_width	desktop_height	requested_color_depth	cert_type	cert_count	cert_permanent	encryption_level	encryption_method
+#types	time	string	addr	port	addr	port	string	string	string	vector[string]	string	string	string	string	count	count	string	string	count	bool	string	string
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	10.128.36.245	50204	10.132.153.76	3389	(empty)	encrypted	HYBRID_EX	-	-	-	-	-	-	-	-	-	0	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.rdp.rdp-no-cookie-msthash/ssl.log

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ssl
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	version	cipher	curve	server_name	resumed	last_alert	next_protocol	established	ssl_history	cert_chain_fps	client_cert_chain_fps	subject	issuer	sni_matches_cert
+#types	time	string	addr	port	addr	port	string	string	string	string	bool	string	string	bool	string	vector[string]	vector[string]	string	string	bool
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	10.128.36.245	50204	10.132.153.76	3389	TLSv12	TLS_RSA_WITH_AES_256_GCM_SHA384	-	-	F	-	-	T	CsxnGIi	2f24fa5efe1c62447b32aa41af38c4176d03a62ca9c3c8d491946ed283b86cca	(empty)	CN=CLK-456040-D.one.phoenix.gov	CN=CLK-456040-D.one.phoenix.gov	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/scripts/base/protocols/rdp/rdp-no-cookie-msthash.zeek

@@ -0,0 +1,12 @@
+# Test a trace that does not have a cookie field.
+
+# @TEST-EXEC: zeek -b -r $TRACES/rdp/rdp-no-cookie-mstshash.pcap %INPUT
+# @TEST-EXEC: btest-diff rdp.log
+# @TEST-EXEC: btest-diff ssl.log
+# @TEST-EXEC: test ! -f analyzer.log
+# @TEST-EXEC: test ! -f dpd.log
+
+@load base/protocols/rdp
+@load base/protocols/ssl
+
+redef SSL::log_include_server_certificate_subject_issuer=T;