Merge remote-tracking branch 'origin/topic/johanna/tls13-version'

Closes BIT-1916.

* origin/topic/johanna/tls13-version:
  Recognize TLS 1.3 negotiation correctly.

CHANGES

@@ -1,4 +1,17 @@
 
+2.5-478 | 2018-03-29 12:59:49 -0700
+
+  * Recognize TLS 1.3 negotiation correctly. The way in which TLS 1.3
+    is negotiated was changed slightly in later revisions of the
+    standard. (Johanna Amann)
+
+  * Fix the travis-job script to always run external tests. (Daniel
+    Thayer)
+
+  * Fix information leak in the update-traces script. (Daniel Thayer)
+
+  * Add Coverity scan and private testing to Travis CI. (Daniel Thayer)
+
 2.5-471 | 2018-03-21 13:56:57 -0700
 
   * Fix a memory leak in SMBv1 share mapping. (Corelight)

VERSION

@@ -1 +1 @@
-2.5-471
+2.5-478

scripts/base/protocols/ssl/main.bro

@@ -216,14 +216,29 @@ event ssl_server_hello(c: connection, version: count, possible_ts: time, server_
 	{
 	set_session(c);
 
-	c$ssl$version_num = version;
+	# If it is already filled, we saw a supported_versions extensions which overrides this.
-	c$ssl$version = version_strings[version];
+	if ( ! c$ssl?$version_num )
+		{
+		c$ssl$version_num = version;
+		c$ssl$version = version_strings[version];
+		}
 	c$ssl$cipher = cipher_desc[cipher];
 
 	if ( c$ssl?$session_id && c$ssl$session_id == bytestring_to_hexstr(session_id) )
 		c$ssl$resumed = T;
 	}
 
+event ssl_extension_supported_versions(c: connection, is_orig: bool, versions: index_vec)
+	{
+	if ( is_orig || |versions| != 1 )
+		return;
+
+	set_session(c);
+
+	c$ssl$version_num = versions[0];
+	c$ssl$version = version_strings[versions[0]];
+	}
+
 event ssl_ecdh_server_params(c: connection, curve: count, point: string) &priority=5
 	{
 	set_session(c);

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

@@ -205,6 +205,17 @@ refine connection Handshake_Conn += {
 		return true;
 		%}
 
+	function proc_one_supported_version(rec: HandshakeRecord, version: uint16) : bool
+		%{
+		VectorVal* versions = new VectorVal(internal_type("index_vec")->AsVectorType());
+		versions->Assign(0u, new Val(version, TYPE_COUNT));
+
+		BifEvent::generate_ssl_extension_supported_versions(bro_analyzer(), bro_analyzer()->Conn(),
+			${rec.is_orig}, versions);
+
+		return true;
+		%}
+
 	function proc_psk_key_exchange_modes(rec: HandshakeRecord, mode_list: uint8[]) : bool
 		%{
 		VectorVal* modes = new VectorVal(internal_type("index_vec")->AsVectorType());
@@ -501,6 +512,10 @@ refine typeattr SupportedVersions += &let {
 	proc : bool = $context.connection.proc_supported_versions(rec, versions);
 };
 
+refine typeattr OneSupportedVersion += &let {
+	proc : bool = $context.connection.proc_one_supported_version(rec, version);
+};
+
 refine typeattr PSKKeyExchangeModes += &let {
 	proc : bool = $context.connection.proc_psk_key_exchange_modes(rec, modes);
 };

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

@@ -786,7 +786,7 @@ type SSLExtension(rec: HandshakeRecord) = record {
 
 type SupportedVersionsSelector(rec: HandshakeRecord, data_len: uint16) = case rec.is_orig of {
 	true -> a: SupportedVersions(rec);
-	false -> b: bytestring &length=data_len &transient;
+	false -> b: OneSupportedVersion(rec);
 }
 
 type SupportedVersions(rec: HandshakeRecord) = record {
@@ -794,6 +794,10 @@ type SupportedVersions(rec: HandshakeRecord) = record {
 	versions: uint16[] &until($input.length() == 0);
 } &length=length+1;
 
+type OneSupportedVersion(rec: HandshakeRecord) = record {
+	version: uint16;
+};
+
 type PSKKeyExchangeModes(rec: HandshakeRecord) = record {
 	length: uint8;
 	modes: uint8[] &until($input.length() == 0);

testing/btest/Baseline/scripts.base.protocols.ssl.tls13-experiment/ssl.log

@@ -3,8 +3,8 @@
 #empty_field	(empty)
 #unset_field	-
 #path	ssl
-#open	2017-09-10-05-23-15
+#open	2018-03-27-21-54-13
 #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	cert_chain_fuids	client_cert_chain_fuids	subject	issuer	client_subject	client_issuer
 #types	time	string	addr	port	addr	port	string	string	string	string	bool	string	string	bool	vector[string]	vector[string]	string	string	string	string
-1505019126.007778	CHhAvVGS1DHFjwGM9	192.168.0.2	62873	104.196.219.53	443	TLSv12	TLS_AES_128_GCM_SHA256	x25519	tls.ctf.network	T	-	-	T	-	-	-	-	-	-
+1505019126.007778	CHhAvVGS1DHFjwGM9	192.168.0.2	62873	104.196.219.53	443	unknown-32257	TLS_AES_128_GCM_SHA256	x25519	tls.ctf.network	T	-	-	T	-	-	-	-	-	-
-#close	2017-09-10-05-23-16
+#close	2018-03-27-21-54-13

testing/btest/Baseline/scripts.base.protocols.ssl.tls13-version/ssl.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ssl
+#open	2018-03-27-21-57-37
+#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	cert_chain_fuids	client_cert_chain_fuids	subject	issuer	client_subject	client_issuer
+#types	time	string	addr	port	addr	port	string	string	string	string	bool	string	string	bool	vector[string]	vector[string]	string	string	string	string
+1520820013.621497	CHhAvVGS1DHFjwGM9	192.168.86.23	63449	52.32.149.186	443	TLSv13-draft23	TLS_AES_128_GCM_SHA256	-	tls13.crypto.mozilla.org	T	-	-	T	-	-	-	-	-	-
+#close	2018-03-27-21-57-37

testing/btest/scripts/base/protocols/ssl/tls13-experiment.test

@@ -8,6 +8,9 @@
 #
 # This only seems to happen with Chrome talking to google servers. We do not recognize this as
 # TLS 1.3, but we do not abort when encountering traffic like this.
+#
+# In the meantime this way of establishing TLS 1.3 was standardized. Still keeping the test even
+# though we parse this correctly now.
 
 event ssl_extension(c: connection, is_orig: bool, code: count, val: string)
 	{

testing/btest/scripts/base/protocols/ssl/tls13-version.test

@@ -0,0 +1,4 @@
+# @TEST-EXEC: bro -C -r $TRACES/tls/tls13draft23-chrome67.0.3368.0-canary.pcap %INPUT
+# @TEST-EXEC: btest-diff ssl.log
+
+# Test that we correctly parse the version out of the extension in an 1.3 connection