Small fixes and changes for the recent ssl-keyexchange-event merge.

This commit fixes a few small issues.

* server key exchange parameters are only parsed when a named curve is
  given.

* I removed the ssl-verbose.bro and moved the functionality into the
  testcase.

  The information that we get with these events is likely irrelevant to
  the majority of Bro users; I do not think that we have to ship a
  script that uses them by default. A script like this would be
  something to publish via the Bro package manager instead; this is the
  approach that we have taken with a number of the recent SSL addition.

* I marked the ssl_server_curve event as deprecated. More information is
  contained in the new ssl_ecdh_server_params event.

  This is an events that is probably seldomly (or never) directly used
  by anyone; I plan to completely remove it right after the 2.6 release.

CHANGES

@@ -1,4 +1,19 @@
 
+2.5-358 | 2017-11-28 12:28:14 -0800
+
+  * Extend the TLS analyzer with several events containing cryptographic
+    parameters from the client and server key exchanges.
+
+   The new events are:
+
+    ssl_ecdh_server_params, ssl_dh_server_params, ssl_server_signature,
+    ssl_ecdh_client_params, ssl_dh_client_params, ssl_rsa_client_pms
+
+    Since ssl_ecdh_server_params contains more information than the old
+    ssl_server_curve event, ssl_server_curve is now marked as deprecated.
+
+    (Luke Valenta)
+
 2.5-352 | 2017-11-21 13:21:51 -0600
 
   * Fix assignments to event arguments becoming visible to subsequent

NEWS

@@ -26,6 +26,16 @@ New Functionality
 - The SSL scripts provide a new hook "ssl_finishing(c: connection)"
   to trigger actions after the handshake has concluded.
 
+- New functionality has been added to the TLS parser, adding several
+  events. These events mostly extract information from the server and client
+  key exchange messages. The new events are:
+
+  ssl_ecdh_server_params, ssl_dh_server_params, ssl_server_signature,
+  ssl_ecdh_client_params, ssl_dh_client_params, ssl_rsa_client_pms
+
+  Since ssl_ecdh_server_params contains more information than the old
+  ssl_server_curve event, ssl_server_curve is now marked as deprecated.
+
 Changed Functionality
 ---------------------
 
@@ -34,6 +44,10 @@ Changed Functionality
   "application/x-x509-user-cert" for host certificates and
   "application/x-x509-ca-cert" for CA certificates.
 
+- With the new ssl_ecdh_server_params event, the ssl_server_curve
+  event is considered deprecated and will be removed in a future
+  version of Bro.
+
 Removed Functionality
 ---------------------
 

VERSION

@@ -1 +1 @@
-2.5-352
+2.5-358

scripts/base/protocols/ssl/main.bro

@@ -224,7 +224,7 @@ event ssl_server_hello(c: connection, version: count, possible_ts: time, server_
 		c$ssl$resumed = T;
 	}
 
-event ssl_server_curve(c: connection, curve: count) &priority=5
+event ssl_ecdh_server_params(c: connection, curve: count, point: string) &priority=5
 	{
 	set_session(c);
 

scripts/test-all-policy.bro

@@ -101,7 +101,6 @@
 @load protocols/ssl/validate-ocsp.bro
 @load protocols/ssl/validate-sct.bro
 @load protocols/ssl/weak-keys.bro
-@load protocols/ssl/ssl-verbose.bro
 @load tuning/__load__.bro
 @load tuning/defaults/__load__.bro
 @load tuning/defaults/extracted_file_limits.bro

src/analyzer/protocol/ssl/events.bif

@@ -108,10 +108,9 @@ event ssl_extension%(c: connection, is_orig: bool, code: count, val: string%);
 ##    ssl_session_ticket_handshake ssl_extension
 ##    ssl_extension_ec_point_formats ssl_extension_application_layer_protocol_negotiation
 ##    ssl_extension_server_name ssl_server_curve ssl_extension_signature_algorithm
-##    ssl_extension_key_share
+##    ssl_extension_key_share ssl_rsa_client_pms ssl_server_signature
 ##    ssl_extension_psk_key_exchange_modes ssl_extension_supported_versions
 ##    ssl_dh_client_params ssl_ecdh_server_params ssl_ecdh_client_params
-##    ssl_rsa_client_pms ssl_server_signature
 event ssl_extension_elliptic_curves%(c: connection, is_orig: bool, curves: index_vec%);
 
 ## Generated for an SSL/TLS Supported Point Formats extension. This TLS extension
@@ -182,6 +181,9 @@ event ssl_extension_key_share%(c: connection, is_orig: bool, curves: index_vec%)
 ##
 ## curve: The curve.
 ##
+## .. note:: This event is deprecated and superseded by the ssl_ecdh_server_params
+##           event. This event will be removed in a future version of Bro.
+##
 ## .. bro:see:: ssl_alert ssl_client_hello ssl_established ssl_server_hello
 ##    ssl_session_ticket_handshake ssl_extension
 ##    ssl_extension_elliptic_curves ssl_extension_application_layer_protocol_negotiation
@@ -189,11 +191,11 @@ event ssl_extension_key_share%(c: connection, is_orig: bool, curves: index_vec%)
 ##    ssl_extension_psk_key_exchange_modes ssl_extension_supported_versions
 ##    ssl_dh_client_params ssl_ecdh_server_params ssl_ecdh_client_params
 ##    ssl_rsa_client_pms ssl_server_signature
-event ssl_server_curve%(c: connection, curve: count%);
+event ssl_server_curve%(c: connection, curve: count%) &deprecated;
 
-## Generated if a server uses an ECDH-anon or ECDHE cipher suite. This event
-## contains the server ECDH parameters, which are sent in the ServerKeyExchange
-## message as defined in :rfc:`4492`.
+## Generated if a server uses an ECDH-anon or ECDHE cipher suite using a named curve
+## This event contains the named curve name and the server ECDH parameters contained
+## in the ServerKeyExchange message as defined in :rfc:`4492`.
 ##
 ## c: The connection.
 ##
@@ -207,7 +209,7 @@ event ssl_server_curve%(c: connection, curve: count%);
 event ssl_ecdh_server_params%(c: connection, curve: count, point: string%);
 
 ## Generated if a server uses a DH-anon or DHE cipher suite. This event contains
-## the server DH parameters, which are sent in the ServerKeyExchange message as
+## the server DH parameters, contained in the ServerKeyExchange message as
 ## defined in :rfc:`5246`.
 ##
 ## c: The connection.
@@ -225,14 +227,14 @@ event ssl_ecdh_server_params%(c: connection, curve: count, point: string%);
 event ssl_dh_server_params%(c: connection, p: string, q: string, Ys: string%);
 
 ## Generated if a server uses a non-anonymous DHE or ECDHE cipher suite. This event
-## contains the server signature over the key exchange parameters, which is sent in
+## contains the server signature over the key exchange parameters contained in
 ## the ServerKeyExchange message as defined in :rfc:`4492` and :rfc:`5246`.
 ##
 ## c: The connection.
 ##
 ## signed_params: A hash of the server params, with the signature appropriate to
 ##                that hash applied. The private key corresponding to the certified
-##                public key in the server's Certificate message is used for signing.
+##                public key in the server's certificate message is used for signing.
 ##
 ## .. bro:see:: ssl_alert ssl_client_hello ssl_established ssl_server_hello
 ##    ssl_session_ticket_handshake ssl_server_curve ssl_rsa_client_pms
@@ -240,7 +242,7 @@ event ssl_dh_server_params%(c: connection, p: string, q: string, Ys: string%);
 event ssl_server_signature%(c: connection, signed_params: string%);
 
 ## Generated if a client uses an ECDH-anon or ECDHE cipher suite. This event
-## contains the client ECDH public value, which is sent in the ClientKeyExchange
+## contains the client ECDH public value contained in the ClientKeyExchange
 ## message as defined in :rfc:`4492`.
 ##
 ## c: The connection.
@@ -253,7 +255,7 @@ event ssl_server_signature%(c: connection, signed_params: string%);
 event ssl_ecdh_client_params%(c: connection, point: string%);
 
 ## Generated if a client uses a DH-anon or DHE cipher suite. This event contains
-## the client DH parameters, which are sent in the ClientKeyExchange message as
+## the client DH parameters contained in the ClientKeyExchange message as
 ## defined in :rfc:`5246`.
 ##
 ## c: The connection.
@@ -265,7 +267,7 @@ event ssl_ecdh_client_params%(c: connection, point: string%);
 ##    ssl_ecdh_server_params ssl_ecdh_client_params ssl_rsa_client_pms
 event ssl_dh_client_params%(c: connection, Yc: string%);
 
-## Generate if a client uses an RSA key exchange. This event contains the client
+## Generated if a client uses RSA key exchange. This event contains the client
 ## encrypted pre-master secret which is encrypted using the public key of the
 ## server's certificate as defined in :rfc:`5246`.
 ##

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

@@ -272,46 +272,50 @@ refine connection Handshake_Conn += {
 		return true;
 		%}
 
-	function proc_ecdhe_server_key_exchange(rec: HandshakeRecord, curve_type: uint8, curve: uint16, point: bytestring, signed_params: bytestring) : bool
+	function proc_ecdhe_server_key_exchange(kex: EcdheServerKeyExchange) : bool
 		%{
-		if ( curve_type == NAMED_CURVE )
-			BifEvent::generate_ssl_server_curve(bro_analyzer(),
-			  bro_analyzer()->Conn(), curve);
-			BifEvent::generate_ssl_ecdh_server_params(bro_analyzer(),
-			  bro_analyzer()->Conn(), curve, new StringVal(point.length(), (const char*)point.data()));
-			BifEvent::generate_ssl_server_signature(bro_analyzer(),
-			  bro_analyzer()->Conn(), new StringVal(signed_params.length(), (const char*)signed_params.data()));
+		if ( ${kex.curve_type} != NAMED_CURVE )
+			return true;
+
+		BifEvent::generate_ssl_server_curve(bro_analyzer(),
+			bro_analyzer()->Conn(), ${kex.params.curve});
+		BifEvent::generate_ssl_ecdh_server_params(bro_analyzer(),
+			bro_analyzer()->Conn(), ${kex.params.curve}, new StringVal(${kex.params.point}.length(), (const char*)${kex.params.point}.data()));
+		BifEvent::generate_ssl_server_signature(bro_analyzer(),
+			bro_analyzer()->Conn(), new StringVal(${kex.params.signed_params}.length(), (const char*)${kex.params.signed_params}.data()));
 
 		return true;
 		%}
 
-	function proc_ecdh_anon_server_key_exchange(rec: HandshakeRecord, curve_type: uint8, curve: uint16, point: bytestring) : bool
+	function proc_ecdh_anon_server_key_exchange(kex: EcdhAnonServerKeyExchange) : bool
 		%{
-		if ( curve_type == NAMED_CURVE )
-			BifEvent::generate_ssl_server_curve(bro_analyzer(),
-			  bro_analyzer()->Conn(), curve);
-			BifEvent::generate_ssl_ecdh_server_params(bro_analyzer(),
-			  bro_analyzer()->Conn(), curve, new StringVal(point.length(), (const char*)point.data()));
+		if ( ${kex.curve_type} != NAMED_CURVE )
+			return true;
+
+		BifEvent::generate_ssl_server_curve(bro_analyzer(),
+			bro_analyzer()->Conn(), ${kex.params.curve});
+		BifEvent::generate_ssl_ecdh_server_params(bro_analyzer(),
+			bro_analyzer()->Conn(), ${kex.params.curve}, new StringVal(${kex.params.point}.length(), (const char*)${kex.params.point}.data()));
 
 		return true;
 		%}
 
 	function proc_rsa_client_key_exchange(rec: HandshakeRecord, rsa_pms: bytestring) : bool
 		%{
-			BifEvent::generate_ssl_rsa_client_pms(bro_analyzer(), bro_analyzer()->Conn(), new StringVal(rsa_pms.length(), (const char*)rsa_pms.data()));
-			return true;
+		BifEvent::generate_ssl_rsa_client_pms(bro_analyzer(), bro_analyzer()->Conn(), new StringVal(rsa_pms.length(), (const char*)rsa_pms.data()));
+		return true;
 		%}
 
 	function proc_dh_client_key_exchange(rec: HandshakeRecord, Yc: bytestring) : bool
 		%{
-			BifEvent::generate_ssl_dh_client_params(bro_analyzer(), bro_analyzer()->Conn(), new StringVal(Yc.length(), (const char*)Yc.data()));
-			return true;
+		BifEvent::generate_ssl_dh_client_params(bro_analyzer(), bro_analyzer()->Conn(), new StringVal(Yc.length(), (const char*)Yc.data()));
+		return true;
 		%}
 
 	function proc_ecdh_client_key_exchange(rec: HandshakeRecord, point: bytestring) : bool
 		%{
-			BifEvent::generate_ssl_ecdh_client_params(bro_analyzer(), bro_analyzer()->Conn(), new StringVal(point.length(), (const char*)point.data()));
-			return true;
+		BifEvent::generate_ssl_ecdh_client_params(bro_analyzer(), bro_analyzer()->Conn(), new StringVal(point.length(), (const char*)point.data()));
+		return true;
 		%}
 
 	function proc_signedcertificatetimestamp(rec: HandshakeRecord, version: uint8, logid: const_bytestring, timestamp: uint64, digitally_signed_algorithms: SignatureAndHashAlgorithm, digitally_signed_signature: const_bytestring) : bool
@@ -438,11 +442,11 @@ refine typeattr CertificateStatus += &let {
 };
 
 refine typeattr EcdheServerKeyExchange += &let {
-	proc : bool = $context.connection.proc_ecdhe_server_key_exchange(rec, curve_type, curve, point, signed_params);
+	proc : bool = $context.connection.proc_ecdhe_server_key_exchange(this);
 };
 
 refine typeattr EcdhAnonServerKeyExchange += &let {
-	proc : bool = $context.connection.proc_ecdh_anon_server_key_exchange(rec, curve_type, curve, point);
+	proc : bool = $context.connection.proc_ecdh_anon_server_key_exchange(this);
 };
 
 refine typeattr DheServerKeyExchange += &let {

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

@@ -360,10 +360,10 @@ type ServerKeyExchange(rec: HandshakeRecord) = case $context.connection.chosen_c
 # currently supported.
 type EcdheServerKeyExchange(rec: HandshakeRecord) = record {
 	curve_type: uint8;
-	curve: uint16; # only if curve_type = 3 (NAMED_CURVE)
-	point_length: uint8;
-	point: bytestring &length=point_length;
-	signed_params: bytestring &restofdata;
+	named_curve: case curve_type of {
+		NAMED_CURVE -> params: ServerEDCHParamsAndSignature;
+		default -> data: bytestring &restofdata &transient;
+	};
 };
 
 # Parse an ECDH-anon ServerKeyExchange message, which does not contain a
@@ -371,11 +371,18 @@ type EcdheServerKeyExchange(rec: HandshakeRecord) = record {
 # server is not currently supported.
 type EcdhAnonServerKeyExchange(rec: HandshakeRecord) = record {
 	curve_type: uint8;
-	curve: uint16; # only if curve_type = 3 (NAMED_CURVE)
+	named_curve: case curve_type of {
+		NAMED_CURVE -> params: ServerEDCHParamsAndSignature;
+		default -> data: bytestring &restofdata &transient;
+	};
+};
+
+type ServerEDCHParamsAndSignature() = record {
+	curve: uint16;
 	point_length: uint8;
 	point: bytestring &length=point_length;
-	data: bytestring &restofdata &transient;
-};
+	signed_params: bytestring &restofdata; # only present in case of non-anon message
+}
 
 # Parse a DHE ServerKeyExchange message, which contains a signature over the
 # parameters.

testing/btest/scripts/base/protocols/ssl/keyexchange.test

@@ -0,0 +1,116 @@
+# @TEST-EXEC: bro -r $TRACES/tls/dhe.pcap %INPUT
+# @TEST-EXEC: cat ssl.log > ssl-all.log
+# @TEST-EXEC: bro -r $TRACES/tls/ecdhe.pcap %INPUT
+# @TEST-EXEC: cat ssl.log >> ssl-all.log
+# @TEST-EXEC: bro -r $TRACES/tls/ssl.v3.trace %INPUT
+# @TEST-EXEC: cat ssl.log >> ssl-all.log
+# @TEST-EXEC: btest-diff ssl-all.log
+
+# Test the new client and server key exchange events.
+
+@load base/protocols/ssl
+@load base/files/x509
+@load protocols/ssl/extract-certs-pem.bro
+
+module SSL;
+
+export {
+	redef record Info += {
+		# ClientHello
+		client_random: string &log &optional;
+		client_cipher_suites: string &log &optional;
+
+		# ServerHello
+		server_random: string &log &optional;
+
+		# ServerKeyExchange
+		server_dh_p: string &log &optional;
+		server_dh_q: string &log &optional;
+		server_dh_Ys: string &log &optional;
+		server_ecdh_point: string &log &optional;
+		server_signature: string &log &optional;
+
+		# ServerCertificate
+		server_cert_sha1: string &log &optional;
+
+		# ClientKeyExchange
+		client_rsa_pms: string &log &optional;
+		client_dh_Yc: string &log &optional;
+		client_ecdh_point: string &log &optional;
+	};
+
+	## Control if host certificates offered by the defined hosts
+	## will be written to the PEM certificates file.
+	## Choices are: LOCAL_HOSTS, REMOTE_HOSTS, ALL_HOSTS, NO_HOSTS.
+	redef extract_certs_pem = ALL_HOSTS;
+}
+
+event ssl_established(c: connection) &priority=5
+	{
+	if ( ! c$ssl?$cert_chain || |c$ssl$cert_chain| == 0 ||
+	     ! c$ssl$cert_chain[0]?$x509 )
+		return;
+
+	c$ssl$server_cert_sha1 = c$ssl$cert_chain[0]$sha1;
+	}
+
+event ssl_client_hello(c: connection, version: count, possible_ts: time, client_random: string, session_id: string, ciphers: index_vec) &priority=5
+	{
+	set_session(c);
+	c$ssl$client_random = bytestring_to_hexstr(client_random);
+
+	local ciphers_str = "";
+	for (i in ciphers)
+		{
+			ciphers_str += cipher_desc[ciphers[i]];
+			if ( i != |ciphers|-1)
+				{
+					ciphers_str += ",";
+				}
+		}
+	c$ssl$client_cipher_suites = ciphers_str;
+	}
+
+event ssl_server_hello(c: connection, version: count, possible_ts: time, server_random: string, session_id: string, cipher: count, comp_method: count) &priority=5
+	{
+	set_session(c);
+	c$ssl$server_random = bytestring_to_hexstr(server_random);
+	}
+
+event ssl_dh_server_params(c: connection, p: string, q: string, Ys: string) &priority=5
+	{
+	set_session(c);
+	c$ssl$server_dh_p = bytestring_to_hexstr(p);
+	c$ssl$server_dh_q = bytestring_to_hexstr(q);
+	c$ssl$server_dh_Ys = bytestring_to_hexstr(Ys);
+	}
+
+event ssl_ecdh_server_params(c: connection, curve: count, point: string) &priority=5
+	{
+	set_session(c);
+	c$ssl$server_ecdh_point = bytestring_to_hexstr(point);
+	}
+
+event ssl_server_signature(c: connection, signed_params: string) &priority=5
+	{
+	set_session(c);
+	c$ssl$server_signature = bytestring_to_hexstr(signed_params);
+	}
+
+event ssl_rsa_client_pms(c: connection, pms: string) &priority=5
+	{
+	set_session(c);
+	c$ssl$client_rsa_pms = bytestring_to_hexstr(pms);
+	}
+
+event ssl_dh_client_params(c: connection, Yc: string) &priority=5
+	{
+	set_session(c);
+	c$ssl$client_dh_Yc = bytestring_to_hexstr(Yc);
+	}
+
+event ssl_ecdh_client_params(c: connection, point: string) &priority=5
+	{
+	set_session(c);
+	c$ssl$client_ecdh_point = bytestring_to_hexstr(point);
+	}

testing/btest/scripts/policy/protocols/ssl/ssl-verbose.test

@@ -1,9 +0,0 @@
-# @TEST-EXEC: bro -r $TRACES/tls/dhe.pcap %INPUT
-# @TEST-EXEC: cat ssl.log > ssl-all.log
-# @TEST-EXEC: bro -r $TRACES/tls/ecdhe.pcap %INPUT
-# @TEST-EXEC: cat ssl.log >> ssl-all.log
-# @TEST-EXEC: bro -r $TRACES/tls/ssl.v3.trace %INPUT
-# @TEST-EXEC: cat ssl.log >> ssl-all.log
-# @TEST-EXEC: btest-diff ssl-all.log
-
-@load protocols/ssl/ssl-verbose