Merge remote-tracking branch 'origin/topic/johanna/fix-ssl2-client-hello-version-parsing'

* origin/topic/johanna/fix-ssl2-client-hello-version-parsing:
  Fix parsing of version field in SSLv2 client hello

CHANGES

@@ -1,3 +1,19 @@
+7.1.0-dev.222 | 2024-08-27 13:29:12 +0100
+
+  * Fix parsing of version field in SSLv2 client hello (Johanna Amann, Corelight)
+
+    It turns out that, for probably a long time, we have reported an
+    incorrect version when parsing an SSLv2 client hello. We always reported
+    this as SSLv2, no matter which version the client hello actually
+    contained.
+
+    This bug probably went unnoticed for a long time, as SSLv2 is
+    essentially unused nowadays, and as this field does not show up in the
+    default logs.
+
+    This was found due to a baseline difference when writing the Spicy SSL
+    analyzer.
+
 7.1.0-dev.219 | 2024-08-23 14:18:35 +0200
 
   * Analyzer: Do not add child analyzers when finished (Arne Welzel, Corelight)

VERSION

@@ -1 +1 @@
-7.1.0-dev.219
+7.1.0-dev.222

scripts/base/protocols/ssl/main.zeek

@@ -283,9 +283,11 @@ event ssl_client_hello(c: connection, version: count, record_version: count, pos
 		c$ssl$client_ticket_empty_session_seen = F;
 		}
 
-	# add manually for SSLv2, since the handshake_message event is not raised, as there is no handshake protocol.
-	# We don't really have a direction in that case
-	if ( version == 2 )
+	# add manually for SSLv2 client hello, since the handshake_message event is not raised, as there is no handshake protocol.
+	# We don't really have a direction in that case.
+	# SSLv2 client hello is signified by a record_layer version of 0, as the client-hello itself can indicate
+	# a higher supported maximum version
+	if ( record_version == 0 )
 		add_to_history(c, T, "c");
 	}
 

src/analyzer/protocol/ssl/ssl-protocol.pac

@@ -8,24 +8,24 @@ type SSLRecord(is_orig: bool) = record {
 	head2 : uint8;
 	head3 : uint8;
 	head4 : uint8;
-	rec : RecordText(this)[] &length=length, &requires(version,content_type,raw_tls_version);
+	rec : RecordText(this)[] &length=length, &requires(record_layer_version,content_type,raw_tls_version);
 } &length = length+5, &byteorder=bigendian,
 	&let {
-	version : int =
+	record_layer_version : int =
 		$context.connection.determine_ssl_record_layer(head0, head1, head2, head3, head4, is_orig);
 
 	# unmodified tls record layer version of this packet. Do not use this if you are parsing SSLv2
-	raw_tls_version: uint16 = case version of {
+	raw_tls_version: uint16 = case record_layer_version of {
 		SSLv20 -> 0;
 		default -> (head1<<8) | head2;
 	} &requires(version);
 
-	content_type : int = case version of {
+	content_type : int = case record_layer_version of {
 		SSLv20 -> head2+300;
 		default -> head0;
 	} &requires(version);
 
-	length : int = case version of {
+	length : int = case record_layer_version of {
 		# fail analyzer if the packet cannot be recognized as TLS.
 		UNKNOWN_VERSION -> 0;
 		SSLv20 -> (((head0 & 0x7f) << 8) | head1) - 3;
@@ -77,7 +77,7 @@ type V2ClientHello(rec: SSLRecord) = record {
 	session_id : uint8[session_len];
 	challenge : bytestring &length = chal_len;
 } &length = 6 + csuit_len + session_len + chal_len, &let {
-	client_version : int = rec.version;
+	client_version : int = (rec.head3 << 8) | rec.head4;
 };
 
 

testing/btest/Baseline/scripts.base.protocols.ssl.dpd/.stdout

@@ -2,7 +2,7 @@
 Start test run
 Client hello, 192.168.4.149, 91.227.4.92, 2
 Start test run
-Client hello, 192.150.187.164, 194.127.84.106, 2
+Client hello, 192.150.187.164, 194.127.84.106, 769
 Client hello, 192.150.187.164, 194.127.84.106, 769
 Client hello, 192.150.187.164, 194.127.84.106, 769
 Start test run

testing/btest/Baseline/scripts.policy.protocols.ssl.ssl-log-ext/ssl-all.log

@@ -27,7 +27,7 @@ XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.18.50	56981	74.125.239.97	443	TLSv12
 #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	sni_matches_cert	server_version	client_version	client_ciphers	ssl_client_exts	ssl_server_exts	ticket_lifetime_hint	dh_param_size	point_formats	client_curves	orig_alpn	client_supported_versions	server_supported_version	psk_key_exchange_modes	client_key_share_groups	server_key_share_group	client_comp_methods	sigalgs	hashalgs
 #types	time	string	addr	port	addr	port	string	string	string	string	bool	string	string	bool	string	vector[string]	vector[string]	bool	count	count	vector[count]	vector[count]	vector[count]	count	count	vector[count]	vector[count]	vector[string]	vector[count]	count	vector[count]	vector[count]	count	vector[count]	vector[count]	vector[count]
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.150.187.164	58868	194.127.84.106	443	TLSv10	TLS_RSA_WITH_RC4_128_MD5	-	-	F	-	-	T	CsxnGIi	ddd0218a34972ceab3d200b78959bd2b4c95eadf37399df35bfd68a5b658bc78,ba352de8d8faa0ecfdbeee560fa308fe192023d3b18d83a68845933bebf28360	(empty)	-	769	2	57,56,53,51,50,4,5,47,22,19,65279,10,21,18,65278,9,100,98,3,6	-	-	-	-	-	-	-	-	-	-	-	-	(empty)	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.150.187.164	58868	194.127.84.106	443	TLSv10	TLS_RSA_WITH_RC4_128_MD5	-	-	F	-	-	T	CsxnGIi	ddd0218a34972ceab3d200b78959bd2b4c95eadf37399df35bfd68a5b658bc78,ba352de8d8faa0ecfdbeee560fa308fe192023d3b18d83a68845933bebf28360	(empty)	-	769	769	57,56,53,51,50,4,5,47,22,19,65279,10,21,18,65278,9,100,98,3,6	-	-	-	-	-	-	-	-	-	-	-	-	(empty)	-	-
 XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	192.150.187.164	58869	194.127.84.106	443	TLSv10	TLS_RSA_WITH_RC4_128_MD5	-	-	F	-	-	T	CsxnGIi	ddd0218a34972ceab3d200b78959bd2b4c95eadf37399df35bfd68a5b658bc78,ba352de8d8faa0ecfdbeee560fa308fe192023d3b18d83a68845933bebf28360	(empty)	-	769	769	57,56,53,51,50,4,5,47,22,19,65279,10,21,18,65278,9,100,98,3,6	-	-	-	-	-	-	-	-	-	-	-	-	0	-	-
 XXXXXXXXXX.XXXXXX	C4J4Th3PJpwUYZZ6gc	192.150.187.164	58870	194.127.84.106	443	TLSv10	TLS_RSA_WITH_RC4_128_MD5	-	-	F	-	-	T	CsxnGIi	ddd0218a34972ceab3d200b78959bd2b4c95eadf37399df35bfd68a5b658bc78,ba352de8d8faa0ecfdbeee560fa308fe192023d3b18d83a68845933bebf28360	(empty)	-	769	769	57,56,53,51,50,4,5,47,22,19,65279,10,21,18,65278,9,100,98,3,6	-	-	-	-	-	-	-	-	-	-	-	-	0	-	-
 #close XXXX-XX-XX-XX-XX-XX

testing/external/commit-hash.zeek-testing

@@ -1 +1 @@
-a1c74b74b8755dc4030dfd6034b2bdce23a07072
+ff3bb79aca72f79ca9cfb35de4cf779d0a6802e3

testing/external/commit-hash.zeek-testing-private

@@ -1 +1 @@
-2a6b523ed423a550cc897cb2cb6a2d6e0cdaea22
+4a2735a9768b124d290a1692d47a25fd8d365320