Merge remote-tracking branch 'origin/topic/seth/ssl-fixes'

* origin/topic/seth/ssl-fixes:
  More bugfixs, cleanup, and test for SSL analyzer
  Fixed parsing of TLS server extensions.

Closes #817.

CHANGES

@@ -1,4 +1,22 @@
 
+2.0-319 | 2012-05-03 13:24:44 -0700
+
+  * SSL bugfixes and cleanup. (Seth Hall)
+
+    - SSL related files and classes renamed to remove the "binpac" term.
+
+    - A small fix for DPD scripts to make the DPD log more helpful if
+      there are multiple continued failures.
+
+    - Fixed the SSL analyzer to make it stop doing repeated violation
+      messages for some handshake failures.
+
+    - Added a $issuer_subject to the SSL log.
+      
+    - Created a basic test for SSL.
+
+    - Fixed parsing of TLS server extensions. (Seth Hall)
+
 2.0-315 | 2012-05-03 11:44:17 -0700
 
   * Add two more TLS extension values that we see in live traffic.

VERSION

@@ -1 +1 @@
-2.0-315
+2.0-319

scripts/base/frameworks/dpd/main.bro

@@ -105,5 +105,8 @@ event protocol_violation(c: connection, atype: count, aid: count,
 				reason: string) &priority=-5
 	{
 	if ( c?$dpd )
+		{
 		Log::write(DPD::LOG, c$dpd);
+		delete c$dpd;
+		}
 	}

scripts/base/protocols/ssl/main.bro

@@ -24,6 +24,8 @@ export {
 		session_id:       string           &log &optional;
 		## Subject of the X.509 certificate offered by the server.
 		subject:          string           &log &optional;
+		## Subject of the signer of the X.509 certificate offered by the server.
+		issuer_subject:   string           &log &optional;
 		## NotValidBefore field value from the server certificate.
 		not_valid_before: time             &log &optional;
 		## NotValidAfter field value from the serve certificate.
@@ -146,6 +148,7 @@ event x509_certificate(c: connection, is_orig: bool, cert: X509, chain_idx: coun
 
 		# Also save other certificate information about the primary cert.
 		c$ssl$subject = cert$subject;
+		c$ssl$issuer_subject = cert$issuer;
 		c$ssl$not_valid_before = cert$not_valid_before;
 		c$ssl$not_valid_after = cert$not_valid_after;
 		}

src/Analyzer.cc

@@ -34,7 +34,7 @@
 #include "Portmap.h"
 #include "POP3.h"
 #include "SSH.h"
-#include "SSL-binpac.h"
+#include "SSL.h"
 #include "Syslog-binpac.h"
 #include "ConnSizeAnalyzer.h"
 
@@ -121,8 +121,8 @@ const Analyzer::Config Analyzer::analyzer_configs[] = {
 		HTTP_Analyzer_binpac::InstantiateAnalyzer,
 		HTTP_Analyzer_binpac::Available, 0, false },
 	{ AnalyzerTag::SSL, "SSL",
-		SSL_Analyzer_binpac::InstantiateAnalyzer,
-		SSL_Analyzer_binpac::Available, 0, false },
+		SSL_Analyzer::InstantiateAnalyzer,
+		SSL_Analyzer::Available, 0, false },
 	{ AnalyzerTag::SYSLOG_BINPAC, "SYSLOG_BINPAC",
 		Syslog_Analyzer_binpac::InstantiateAnalyzer,
 		Syslog_Analyzer_binpac::Available, 0, false },

src/CMakeLists.txt

@@ -376,7 +376,7 @@ set(bro_SRCS
     SMB.cc
     SMTP.cc
     SSH.cc
-    SSL-binpac.cc
+    SSL.cc
     Scope.cc
     SerializationFormat.cc
     SerialObj.cc

src/SSL.cc

@@ -1,21 +1,21 @@
-#include "SSL-binpac.h"
+#include "SSL.h"
 #include "TCP_Reassembler.h"
 #include "Reporter.h"
 #include "util.h"
 
-SSL_Analyzer_binpac::SSL_Analyzer_binpac(Connection* c)
+SSL_Analyzer::SSL_Analyzer(Connection* c)
 : TCP_ApplicationAnalyzer(AnalyzerTag::SSL, c)
 	{
 	interp = new binpac::SSL::SSL_Conn(this);
 	had_gap = false;
 	}
 
-SSL_Analyzer_binpac::~SSL_Analyzer_binpac()
+SSL_Analyzer::~SSL_Analyzer()
 	{
 	delete interp;
 	}
 
-void SSL_Analyzer_binpac::Done()
+void SSL_Analyzer::Done()
 	{
 	TCP_ApplicationAnalyzer::Done();
 
@@ -23,23 +23,22 @@ void SSL_Analyzer_binpac::Done()
 	interp->FlowEOF(false);
 	}
 
-void SSL_Analyzer_binpac::EndpointEOF(TCP_Reassembler* endp)
+void SSL_Analyzer::EndpointEOF(TCP_Reassembler* endp)
 	{
 	TCP_ApplicationAnalyzer::EndpointEOF(endp);
 	interp->FlowEOF(endp->IsOrig());
 	}
 
-void SSL_Analyzer_binpac::DeliverStream(int len, const u_char* data, bool orig)
+void SSL_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
 	{
 	TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
 
 	assert(TCP());
-
 	if ( TCP()->IsPartial() )
 		return;
 
 	if ( had_gap )
-		// XXX: If only one side had a content gap, we could still try to
+		// If only one side had a content gap, we could still try to
 		// deliver data to the other side if the script layer can handle this.
 		return;
 
@@ -53,7 +52,7 @@ void SSL_Analyzer_binpac::DeliverStream(int len, const u_char* data, bool orig)
 		}
 	}
 
-void SSL_Analyzer_binpac::Undelivered(int seq, int len, bool orig)
+void SSL_Analyzer::Undelivered(int seq, int len, bool orig)
 	{
 	TCP_ApplicationAnalyzer::Undelivered(seq, len, orig);
 	had_gap = true;

src/SSL.h

@@ -1,14 +1,13 @@
-#ifndef ssl_binpac_h
-#define ssl_binpac_h
+#ifndef ssl_h
+#define ssl_h
 
 #include "TCP.h"
-
 #include "ssl_pac.h"
 
-class SSL_Analyzer_binpac : public TCP_ApplicationAnalyzer {
+class SSL_Analyzer : public TCP_ApplicationAnalyzer {
 public:
-	SSL_Analyzer_binpac(Connection* conn);
-	virtual ~SSL_Analyzer_binpac();
+	SSL_Analyzer(Connection* conn);
+	virtual ~SSL_Analyzer();
 
 	// Overriden from Analyzer.
 	virtual void Done();
@@ -19,7 +18,7 @@ public:
 	virtual void EndpointEOF(TCP_Reassembler* endp);
 
 	static Analyzer* InstantiateAnalyzer(Connection* conn)
-		{ return new SSL_Analyzer_binpac(conn); }
+		{ return new SSL_Analyzer(conn); }
 
 	static bool Available()
 		{

src/ssl-analyzer.pac

@@ -25,6 +25,7 @@
 	string orig_label(bool is_orig);
 	void free_X509(void *);
 	X509* d2i_X509_binpac(X509** px, const uint8** in, int len);
+	string handshake_type_label(int type);
 	%}
 
 %code{
@@ -46,6 +47,27 @@ string orig_label(bool is_orig)
 		return d2i_X509(px, (u_char**) in, len);
 #endif
 		}
+
+	string handshake_type_label(int type)
+		{
+		switch ( type ) {
+		case HELLO_REQUEST: return string("HELLO_REQUEST");
+		case CLIENT_HELLO: return string("CLIENT_HELLO");
+		case SERVER_HELLO: return string("SERVER_HELLO");
+		case SESSION_TICKET: return string("SESSION_TICKET");
+		case CERTIFICATE: return string("CERTIFICATE");
+		case SERVER_KEY_EXCHANGE: return string("SERVER_KEY_EXCHANGE");
+		case CERTIFICATE_REQUEST: return string("CERTIFICATE_REQUEST");
+		case SERVER_HELLO_DONE: return string("SERVER_HELLO_DONE");
+		case CERTIFICATE_VERIFY: return string("CERTIFICATE_VERIFY");
+		case CLIENT_KEY_EXCHANGE: return string("CLIENT_KEY_EXCHANGE");
+		case FINISHED: return string("FINISHED");
+		case CERTIFICATE_URL: return string("CERTIFICATE_URL");
+		case CERTIFICATE_STATUS: return string("CERTIFICATE_STATUS");
+		default: return string(fmt("UNKNOWN (%d)", type));
+		}
+		}
+
 %}
 
 
@@ -88,15 +110,15 @@ refine connection SSL_Conn += {
 		eof=0;
 	%}
 
-	%eof{
-		if ( ! eof &&
-		     state_ != STATE_CONN_ESTABLISHED &&
-		     state_ != STATE_TRACK_LOST &&
-		     state_ != STATE_INITIAL )
-			bro_analyzer()->ProtocolViolation(fmt("unexpected end of connection in state %s",
-				state_label(state_).c_str()));
-		++eof;
-	%}
+	#%eof{
+	#	if ( ! eof &&
+	#	     state_ != STATE_CONN_ESTABLISHED &&
+	#	     state_ != STATE_TRACK_LOST &&
+	#	     state_ != STATE_INITIAL )
+	#		bro_analyzer()->ProtocolViolation(fmt("unexpected end of connection in state %s",
+	#			state_label(state_).c_str()));
+	#	++eof;
+	#%}
 
 	%cleanup{
 	%}
@@ -133,11 +155,6 @@ refine connection SSL_Conn += {
 					cipher_suites16 : uint16[],
 					cipher_suites24 : uint24[]) : bool
 		%{
-		if ( state_ == STATE_TRACK_LOST )
-			bro_analyzer()->ProtocolViolation(fmt("unexpected client hello message from %s in state %s",
-				orig_label(${rec.is_orig}).c_str(),
-				state_label(old_state_).c_str()));
-
 		if ( ! version_ok(version) )
 			bro_analyzer()->ProtocolViolation(fmt("unsupported client SSL version 0x%04x", version));
 
@@ -175,11 +192,6 @@ refine connection SSL_Conn += {
 					cipher_suites24 : uint24[],
 					comp_method : uint8) : bool
 		%{
-		if ( state_ == STATE_TRACK_LOST )
-			bro_analyzer()->ProtocolViolation(fmt("unexpected server hello message from %s in state %s",
-				orig_label(${rec.is_orig}).c_str(),
-				state_label(old_state_).c_str()));
-
 		if ( ! version_ok(version) )
 			bro_analyzer()->ProtocolViolation(fmt("unsupported server SSL version 0x%04x", version));
 		else
@@ -229,11 +241,6 @@ refine connection SSL_Conn += {
 
 	function proc_certificate(rec: SSLRecord, certificates : bytestring[]) : bool
 		%{
-		if ( state_ == STATE_TRACK_LOST )
-			bro_analyzer()->ProtocolViolation(fmt("unexpected certificate message from %s in state %s",
-				orig_label(${rec.is_orig}).c_str(),
-				state_label(old_state_).c_str()));
-
 		if ( certificates->size() == 0 )
 			return true;
 
@@ -362,6 +369,7 @@ refine connection SSL_Conn += {
 				handshake_type_label(${hs.msg_type}).c_str(),
 				orig_label(is_orig).c_str(),
 				state_label(old_state_).c_str()));
+
 		return true;
 		%}
 

src/ssl-defs.pac

@@ -17,35 +17,6 @@ enum ContentType {
 	UNKNOWN_OR_V2_ENCRYPTED = 400
 };
 
-%code{
-	string* record_type_label(int type)
-		{
-		switch ( type ) {
-		case CHANGE_CIPHER_SPEC:
-			return new string("CHANGE_CIPHER_SPEC");
-		case ALERT:
-			return new string("ALERT");
-		case HANDSHAKE:
-			return new string("HANDSHAKE");
-		case APPLICATION_DATA:
-			return new string("APPLICATION_DATA");
-		case V2_ERROR:
-			return new string("V2_ERROR");
-		case V2_CLIENT_HELLO:
-			return new string("V2_CLIENT_HELLO");
-		case V2_CLIENT_MASTER_KEY:
-			return new string("V2_CLIENT_MASTER_KEY");
-		case V2_SERVER_HELLO:
-			return new string("V2_SERVER_HELLO");
-		case UNKNOWN_OR_V2_ENCRYPTED:
-			return new string("UNKNOWN_OR_V2_ENCRYPTED");
-
-		default:
-			return new string(fmt("UNEXPECTED (%d)", type));
-		}
-		}
-%}
-
 enum SSLVersions {
 	UNKNOWN_VERSION	= 0x0000,
 	SSLv20		= 0x0002,

src/ssl-protocol.pac

@@ -23,7 +23,6 @@ type uint24 = record {
 
 	string state_label(int state_nr);
 	double get_time_from_asn1(const ASN1_TIME * atime);
-	string handshake_type_label(int type);
 %}
 
 extern type to_int;
@@ -268,28 +267,6 @@ enum HandshakeType {
 	CERTIFICATE_STATUS  = 22, # RFC 3546
 };
 
-%code{
-	string handshake_type_label(int type)
-		{
-		switch ( type ) {
-		case HELLO_REQUEST: return string("HELLO_REQUEST");
-		case CLIENT_HELLO: return string("CLIENT_HELLO");
-		case SERVER_HELLO: return string("SERVER_HELLO");
-		case SESSION_TICKET: return string("SESSION_TICKET");
-		case CERTIFICATE: return string("CERTIFICATE");
-		case SERVER_KEY_EXCHANGE: return string("SERVER_KEY_EXCHANGE");
-		case CERTIFICATE_REQUEST: return string("CERTIFICATE_REQUEST");
-		case SERVER_HELLO_DONE: return string("SERVER_HELLO_DONE");
-		case CERTIFICATE_VERIFY: return string("CERTIFICATE_VERIFY");
-		case CLIENT_KEY_EXCHANGE: return string("CLIENT_KEY_EXCHANGE");
-		case FINISHED: return string("FINISHED");
-		case CERTIFICATE_URL: return string("CERTIFICATE_URL");
-		case CERTIFICATE_STATUS: return string("CERTIFICATE_STATUS");
-		default: return string(fmt("UNKNOWN (%d)", type));
-		}
-		}
-%}
-
 
 ######################################################################
 # V3 Change Cipher Spec Protocol (7.1.)
@@ -425,6 +402,10 @@ type ServerHello(rec: SSLRecord) = record {
 	session_id : uint8[session_len];
 	cipher_suite : uint16[1];
 	compression_method : uint8;
+	# This weirdness is to deal with the possible existence or absence
+	# of the following fields.
+	ext_len: uint16[] &until($element == 0 || $element != 0);
+	extensions : SSLExtension(rec)[] &until($input.length() == 0);
 } &let {
 	state_changed : bool =
 		$context.connection.transition(STATE_CLIENT_HELLO_RCVD,

testing/btest/Baseline/scripts.base.protocols.ssl.basic/ssl.log

@@ -0,0 +1,8 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ssl
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	version	cipher	server_name	session_id	subject	issuer_subject	not_valid_before	not_valid_after	last_alert
+#types	time	string	addr	port	addr	port	string	string	string	string	string	string	time	time	string
+1335538392.319381	UWkUyAuUGXf	192.168.1.105	62045	74.125.224.79	443	TLSv10	TLS_ECDHE_RSA_WITH_RC4_128_SHA	ssl.gstatic.com	-	CN=*.gstatic.com,O=Google Inc,L=Mountain View,ST=California,C=US	CN=Google Internet Authority,O=Google Inc,C=US	1334102677.000000	1365639277.000000	-

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

@@ -0,0 +1,4 @@
+# This tests a normal SSL connection and the log it outputs.
+
+# @TEST-EXEC: bro -r $TRACES/tls-conn-with-extensions.trace %INPUT
+# @TEST-EXEC: btest-diff ssl.log