Fix parsing of version field in SSLv2 client hello

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.

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
+df37cbcef57db5aeb09da8045deed0141d471507

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

@@ -1 +1 @@
-2a6b523ed423a550cc897cb2cb6a2d6e0cdaea22
+4e30d6b89edf12d99e1165fd5c5e193d1320e371