Change x509 log - now certificates are only logged once per hour.

Add parsing of several more types to SAN extension.

Make error messages of x509 file analyzer more useful.

Fix file ID generation.

You apparently have to be very careful which EndOfFile function of
the file analysis framework you call... otherwhise it might try
to close another file id. This took me quite a while to find.

addresses BIT-953, BIT-760, BIT-1150

scripts/base/files/x509/main.bro

@@ -6,12 +6,15 @@ module X509;
 export {
 	redef enum Log::ID += { LOG };
 
+	## Set that keeps track of the certificates which were logged recently.
+	global cert_hashes: set[string] &create_expire=1hrs &synchronized &redef;
+
 	type Info: record {
 		## current timestamp
-		ts: time &log &default=network_time();
+		ts: time &log;
 
-		## file id of this certificate
+		## SHA-1 hash of this certificate
-		id: string &log;
+		sha1: string &log &optional;
     
 		## Basic information about the certificate
 		certificate: X509::Certificate &log;
@@ -24,7 +27,7 @@ export {
 		extensions: vector of X509::Extension &default=vector();
 
 		## Subject alternative name extension of the certificate
-		san: string_vec &optional &log;
+		san: X509::SubjectAlternativeName &optional &log;
 
 		## Basic constraints extension of the certificate
 		basic_constraints: X509::BasicConstraints &optional &log;
@@ -45,9 +48,20 @@ redef record Files::Info += {
 	x509: X509::Info &optional;
 };
 
+# Either, this event arrives first - then info$x509 does not exist 
+# yet and this is a no-op, and the sha1 value is set in x509_certificate.
+# Or the x509_certificate event arrives first - then the hash is set here.
+event file_hash(f: fa_file, kind: string, hash: string)
+	{
+	if ( f$info?$x509 && kind == "sha1" )
+		f$info$x509$sha1 = hash;
+	}
+
 event x509_certificate(f: fa_file, cert_ref: opaque of x509, cert: X509::Certificate) &priority=5
 	{
-	f$info$x509 = [$id=f$id, $certificate=cert, $handle=cert_ref];
+	f$info$x509 = [$ts=f$info$ts, $certificate=cert, $handle=cert_ref];
+	if ( f$info?$sha1 )
+		f$info$x509$sha1 = f$info$sha1;
 	}
 
 event x509_extension(f: fa_file, ext: X509::Extension) &priority=5
@@ -62,10 +76,10 @@ event x509_ext_basic_constraints(f: fa_file, ext: X509::BasicConstraints) &prior
 		f$info$x509$basic_constraints = ext;
 	}
 
-event x509_ext_subject_alternative_name(f: fa_file, names: string_vec) &priority=5
+event x509_ext_subject_alternative_name(f: fa_file, ext: X509::SubjectAlternativeName) &priority=5
 	{
 	if ( f$info?$x509 )
-		f$info$x509$san = names;
+		f$info$x509$san = ext;
 	}
 
 event file_state_remove(f: fa_file) &priority=5
@@ -73,5 +87,17 @@ event file_state_remove(f: fa_file) &priority=5
 	if ( ! f$info?$x509 )
 		return;
 
+	if ( ! f$info$x509?$sha1 )
+		{
+		Reporter::error(fmt("Certificate without a hash value. Logging skipped. File-id: %s", f$id));
+		return;
+		}
+
+	if ( f$info$x509$sha1 in cert_hashes )
+		# we already have seen & logged this certificate
+		return;
+
+	add cert_hashes[f$info$x509$sha1];
+
 	Log::write(LOG, f$info$x509);
 	}

scripts/base/init-bare.bro

@@ -2750,7 +2750,7 @@ export {
 
 module X509;
 export {
-	type X509::Certificate: record {
+	type Certificate: record {
 		version: count;	##< Version number.
 		serial: string;	##< Serial number.
 		subject: string;	##< Subject.
@@ -2767,7 +2767,7 @@ export {
 		#path_len: count &optional; ##< indicates the path_length value in the X509v3 BasicConstraints extension
 	} &log;
 
-	type X509::Extension: record {
+	type Extension: record {
 		name: string; ##< long name of extension. oid if name not known
 		short_name: string &optional; ##< short name of extension if known.
 		oid: string; ##< oid of extension
@@ -2775,13 +2775,21 @@ export {
 		value: string; ##< extension content parsed to string for known extensions. Raw data otherwise.
 	};
 
-	type X509::BasicConstraints: record {
+	type BasicConstraints: record {
 		ca: bool; ##< CA flag set?
-		path_len: count &optional;
+		path_len: count &optional; ##< maximum path length
 	} &log;
 
+	type SubjectAlternativeName: record {
+		dns: string_vec &optional &log; ##< list of DNS entries in SAN
+		uri: string_vec &optional &log; ##< list of URI entries in SAN
+		email: string_vec &optional &log; ##< list of email entries in SAN
+		ip: addr_vec &optional &log; ##< list of IP entries in SAN
+		other_fields: bool; ##< true if the certificate contained other, not recognized or parsed name fields
+	};
+	
 	## Result of an X509 certificate chain verification
-	type X509::Result: record {
+	type Result: record {
 		## OpenSSL result code
 		result:	count;
 		## Result as string

scripts/base/protocols/ssl/files.bro

@@ -11,29 +11,34 @@ export {
 		## complete signing chain.
 		cert_chain: vector of Files::Info &optional;
 
-		## An ordered vector of all certicate file unique IDs for the
+		## An ordered vector of all certicate sha1 hashes for the
 		## certificates offered by the server.
-		cert_chain_fuids: vector of string &optional &log;
+		cert_chain_sha1s: vector of string &optional &log;
 
 		## Chain of certificates offered by the client to validate its
 		## complete signing chain.
 		client_cert_chain: vector of Files::Info &optional;
 		
-		## An ordered vector of all certicate file unique IDs for the
+		## An ordered vector of all certicate sha1 hashes for the
 		## certificates offered by the client.
-		client_cert_chain_fuids: vector of string &optional &log;
+		client_cert_chain_sha1s: vector of string &optional &log;
 		
 		## Subject of the X.509 certificate offered by the server.
-		subject:          string           &log &optional;
+		subject: string &log &optional;
 		## Subject of the signer of the X.509 certificate offered by the
 		## server.
-		issuer:   string           &log &optional;
+		issuer: string &log &optional;
 		
 		## Subject of the X.509 certificate offered by the client.
-		client_subject:          string           &log &optional;
+		client_subject: string &log &optional;
 		## Subject of the signer of the X.509 certificate offered by the
 		## client.
-		client_issuer:   string           &log &optional;
+		client_issuer: string &log &optional;
+
+		## current number of certificates seen from either side. Used
+		## to create file handles
+		server_depth: count &default=0;
+		client_depth: count &default=0;
 	};
 
 	## Default file handle provider for SSL.
@@ -45,7 +50,22 @@ export {
 
 function get_file_handle(c: connection, is_orig: bool): string
 	{
-	return cat(Analyzer::ANALYZER_SSL, c$start_time);
+	set_session(c);
+
+	local depth: count;
+
+	if ( is_orig )
+		{
+		depth = c$ssl$client_depth;
+		++c$ssl$client_depth;
+		}
+	else
+		{
+		depth = c$ssl$server_depth;
+		++c$ssl$server_depth;
+		}
+
+	return cat(Analyzer::ANALYZER_SSL, c$start_time, is_orig, id_string(c$id), depth);
 	}
 
 function describe_file(f: fa_file): string
@@ -54,7 +74,19 @@ function describe_file(f: fa_file): string
 	if ( f$source != "SSL" )
 		return "";
 
-	# Fixme!
+	# It is difficult to reliably describe a certificate - especially since
+	# we do not know when this function is called (hence, if the data structures
+	# are already populated).
+	#
+	# Just return a bit of our connection information and hope that that is good enough.
+	for ( cid in f$conns )
+		{
+		if ( f$conns[cid]?$ssl )
+			{
+			local c = f$conns[cid];
+			return cat(c$id$resp_h, ":", c$id$resp_p);
+			}
+		}
 
 	return "";
 	}
@@ -75,30 +107,22 @@ event file_over_new_connection(f: fa_file, c: connection, is_orig: bool) &priori
 		{
 		c$ssl$cert_chain = vector();
 		c$ssl$client_cert_chain = vector();
-		c$ssl$cert_chain_fuids = string_vec();
-		c$ssl$client_cert_chain_fuids = string_vec();
 		}	
 
 	if ( is_orig )
-		{
 		c$ssl$client_cert_chain[|c$ssl$client_cert_chain|] = f$info;
-		c$ssl$client_cert_chain_fuids[|c$ssl$client_cert_chain_fuids|] = f$id;
-		}
 	else
-		{
 		c$ssl$cert_chain[|c$ssl$cert_chain|] = f$info;
-		c$ssl$cert_chain_fuids[|c$ssl$cert_chain_fuids|] = f$id;
-		}
 
 	Files::add_analyzer(f, Files::ANALYZER_X509);
-	# always calculate hashes for certificates
+	# always calculate hashes. SHA1 is always required for certificates.
 	Files::add_analyzer(f, Files::ANALYZER_MD5);
 	Files::add_analyzer(f, Files::ANALYZER_SHA1);
 	}
 
 event ssl_established(c: connection) &priority=6
 	{
-	# update subject and issuer information
+	# update subject and issuer information as well as sha1 hashes
 	if ( c$ssl?$cert_chain && |c$ssl$cert_chain| > 0 )
 		{
 		c$ssl$subject = c$ssl$cert_chain[0]$x509$certificate$subject;
@@ -110,4 +134,19 @@ event ssl_established(c: connection) &priority=6
 		c$ssl$client_subject = c$ssl$client_cert_chain[0]$x509$certificate$subject;
 		c$ssl$client_issuer = c$ssl$client_cert_chain[0]$x509$certificate$issuer;
 		}
+
+
+	if ( c$ssl?$cert_chain )
+		{
+		c$ssl$cert_chain_sha1s = string_vec();
+		for ( i in c$ssl$cert_chain )
+			c$ssl$cert_chain_sha1s[i] = c$ssl$cert_chain[i]$x509$sha1;
+		}
+
+	if ( c$ssl?$client_cert_chain )
+		{
+		c$ssl$client_cert_chain_sha1s = string_vec();
+		for ( i in c$ssl$client_cert_chain )
+			c$ssl$client_cert_chain_sha1s[i] = c$ssl$client_cert_chain[i]$x509$sha1;
+		}
 	}

src/analyzer/protocol/ssl/events.bif

@@ -25,7 +25,7 @@
 ##          :bro:id:`SSL::cipher_desc` table maps them to descriptive names.
 ##
 ## .. bro:see:: ssl_alert  ssl_established ssl_extension ssl_server_hello
-##    ssl_session_ticket_handshake x509_certificate x509_error x509_extension
+##    ssl_session_ticket_handshake x509_certificate
 event ssl_client_hello%(c: connection, version: count, possible_ts: time, client_random: string, session_id: string, ciphers: index_vec%);
 
 ## Generated for an SSL/TLS server's initial *hello* message. SSL/TLS sessions
@@ -58,7 +58,7 @@ event ssl_client_hello%(c: connection, version: count, possible_ts: time, client
 ##              standardized as part of the SSL/TLS protocol.
 ##
 ## .. bro:see:: ssl_alert ssl_client_hello ssl_established ssl_extension
-##    ssl_session_ticket_handshake x509_certificate x509_error x509_extension
+##    ssl_session_ticket_handshake x509_certificate
 event ssl_server_hello%(c: connection, version: count, possible_ts: time, server_random: string, session_id: string, cipher: count, comp_method: count%);
 
 ## Generated for SSL/TLS extensions seen in an initial handshake.  SSL/TLS
@@ -92,7 +92,7 @@ event ssl_extension%(c: connection, is_orig: bool, code: count, val: string%);
 ## c: The connection.
 ##
 ## .. bro:see:: ssl_alert ssl_client_hello  ssl_extension ssl_server_hello
-##    ssl_session_ticket_handshake
+##    ssl_session_ticket_handshake x509_certificate
 event ssl_established%(c: connection%);
 
 ## Generated for SSL/TLS alert records. SSL/TLS sessions start with an

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

@@ -235,9 +235,9 @@ refine connection SSL_Conn += {
 			{
 			const bytestring& cert = (*certificates)[i];
 
-			file_mgr->DataIn(reinterpret_cast<const u_char*>(cert.data()), cert.length(), 
+			string fid = file_mgr->DataIn(reinterpret_cast<const u_char*>(cert.data()), cert.length(),
 					bro_analyzer()->GetAnalyzerTag(), bro_analyzer()->Conn(), ${rec.is_orig});
-			file_mgr->EndOfFile(bro_analyzer()->GetAnalyzerTag(), bro_analyzer()->Conn());
+			file_mgr->EndOfFile(fid);
 			}
 		return true;
 		%}

src/file_analysis/Manager.h

@@ -108,7 +108,7 @@ public:
 	 *         cached and passed back in to a subsequent function call in order
 	 *         to avoid costly file handle lookups (which have to go through
 	 *         the \c get_file_handle script-layer event).  An empty string
-	 *         indicates the associate file is not going to be analyzed further.
+	 *         indicates the associated file is not going to be analyzed further.
 	 */
 	std::string DataIn(const u_char* data, uint64 len, analyzer::Tag tag,
 	                   Connection* conn, bool is_orig,

src/file_analysis/analyzer/x509/X509.cc

@@ -45,7 +45,7 @@ bool file_analysis::X509::EndOfFile()
 	::X509* ssl_cert = d2i_X509(NULL, &cert_char, cert_data.size());
 	if ( !ssl_cert )
 		{
-		reporter->Error("Could not parse X509 certificate");
+		reporter->Error("Could not parse X509 certificate. fuid %s", GetFile()->GetID().c_str());
 		return false;
 		}
 
@@ -222,7 +222,7 @@ void file_analysis::X509::ParseBasicConstraints(X509_EXTENSION* ex)
 	BASIC_CONSTRAINTS *constr = (BASIC_CONSTRAINTS *) X509V3_EXT_d2i(ex);
 	if ( !constr ) 
 		{
-		reporter->Error("Certificate with invalid BasicConstraint");
+		reporter->Error("Certificate with invalid BasicConstraint. fuid %s", GetFile()->GetID().c_str());
 		}
 	else 	
 		{
@@ -246,41 +246,96 @@ void file_analysis::X509::ParseSAN(X509_EXTENSION* ext)
 	GENERAL_NAMES *altname = (GENERAL_NAMES*)X509V3_EXT_d2i(ext);
 	if ( !altname )
 		{
-		reporter->Error("could not parse subject alternative names");
+		reporter->Error("Could not parse subject alternative names. fuid %s", GetFile()->GetID().c_str());
 		return;
 		}
 
-	VectorVal* names = new VectorVal(internal_type("string_vec")->AsVectorType());
+	VectorVal* names = 0;
+	VectorVal* emails = 0;
+	VectorVal* uris = 0;
+	VectorVal* ips = 0;
+
+	unsigned int otherfields = 0;
 
-	int j = 0;
 	for ( int i = 0; i < sk_GENERAL_NAME_num(altname); i++ ) 
 		{
 		GENERAL_NAME *gen = sk_GENERAL_NAME_value(altname, i);
 		assert(gen);
 
-		if ( gen->type == GEN_DNS )
+		if ( gen->type == GEN_DNS || gen->type == GEN_URI || gen->type == GEN_EMAIL )
 			{
 			if (ASN1_STRING_type(gen->d.ia5) != V_ASN1_IA5STRING) 
 				{
-				reporter->Error("DNS-field does not contain an IA5String");
+				reporter->Error("DNS-field does not contain an IA5String. fuid %s", GetFile()->GetID().c_str());
 				continue;
 				}
 			const char* name = (const char*) ASN1_STRING_data(gen->d.ia5);
 			StringVal* bs = new StringVal(name);
-			names->Assign(j, bs);
+
-			j++;
+			switch ( gen->type )
+				{
+				case GEN_DNS:
+					if ( names == 0 )
+						names = new VectorVal(internal_type("string_vec")->AsVectorType());
+					names->Assign(names->Size(), bs);
+					break;
+
+				case GEN_URI:
+					if ( uris == 0 )
+						uris = new VectorVal(internal_type("string_vec")->AsVectorType());
+					uris->Assign(uris->Size(), bs);
+					break;
+
+				case GEN_EMAIL:
+					if ( emails == 0 )
+						emails = new VectorVal(internal_type("string_vec")->AsVectorType());
+					emails->Assign(emails->Size(), bs);
+					break;
+				}
+			}
+		else if ( gen->type == GEN_IPADD )
+			{
+				if ( ips == 0 )
+					ips = new VectorVal(internal_type("addr_vec")->AsVectorType());
+
+				uint32* addr = (uint32*) gen->d.ip->data;
+				if(gen->d.ip->length == 4 )
+					{
+					ips->Assign(ips->Size(), new AddrVal(*addr));
+					}
+				else if ( gen->d.ip->length == 16 )
+					{
+					ips->Assign(ips->Size(), new AddrVal(addr));
+					}
+				else
+					{
+					reporter->Error("Weird IP address length %d in subject alternative name. fuid %s", gen->d.ip->length, GetFile()->GetID().c_str());
+					continue;
+					}
 			}
 		else 
 			{
-			// we should perhaps sometime parse out ip-addresses
+			//reporter->Error("Subject alternative name contained unsupported fields. fuid %s", GetFile()->GetID().c_str());
-			reporter->Error("Subject alternative name contained non-dns fields");
+			// This happens quite often - just mark it
+			otherfields = 1;
 			continue;
 			}
 		}
 
+		RecordVal* sanExt = new RecordVal(BifType::Record::X509::SubjectAlternativeName);
+		if ( names != 0 )
+			sanExt->Assign(0, names);
+		if ( uris != 0 )
+			sanExt->Assign(1, uris);
+		if ( emails != 0 )
+			sanExt->Assign(2, emails);
+		if ( ips != 0 )
+			sanExt->Assign(3, ips);
+		sanExt->Assign(4, new Val(otherfields, TYPE_BOOL));
+
 		val_list* vl = new val_list();
 		vl->append(GetFile()->GetVal()->Ref());
-		vl->append(names);
+		vl->append(sanExt);
 
 		mgr.QueueEvent(x509_ext_subject_alternative_name, vl);
 	}

src/file_analysis/analyzer/x509/events.bif

@@ -1,4 +1,57 @@
+## Generated for encountered X509 certificates, e.g., in the clear SSL/TLS 
+## connection handshake.
+##
+## See `Wikipedia <http://en.wikipedia.org/wiki/X.509>`__ for more information
+## about the X.509 format.
+##
+## f: The file.
+##
+## cert_ref: An opaque pointer to the underlying OpenSSL data structure of the
+##           certificate.
+##
+## cert: The parsed certificate information.
+##
+## .. bro:see:: x509_extension x509_ext_basic_constraints
+##              x509_ext_subject_alternative_name x509_parse x509_verify
+##              x509_get_certificate_string
 event x509_certificate%(f: fa_file, cert_ref: opaque of x509, cert: X509::Certificate%);
+
+## Generated for X509 extensions seen in a certificate.
+##
+## See `Wikipedia <http://en.wikipedia.org/wiki/X.509>`__ for more information
+## about the X.509 format.
+##
+## f: The file.
+##
+## ext: The parsed extension.
+##
+## .. bro:see:: x509_certificate x509_ext_basic_constraints
+##              x509_ext_subject_alternative_name x509_parse x509_verify
+##              x509_get_certificate_string
 event x509_extension%(f: fa_file, ext: X509::Extension%);
+
+## Generated for the X509 basic constraints extension seen in a certificate.
+## This extension can be used to identify the subject of a certificate as a CA.
+##
+## f: The file.
+##
+## ext: The parsed basic constraints extension.
+##
+## .. bro:see:: x509_certificate x509_extension
+##              x509_ext_subject_alternative_name x509_parse x509_verify
+##              x509_get_certificate_string
 event x509_ext_basic_constraints%(f: fa_file, ext: X509::BasicConstraints%);
-event x509_ext_subject_alternative_name%(f: fa_file, names: string_vec%);
+
+## Generated for the X509 subject alternative name extension seen in a certificate.
+## This extension can be used to allow additional entities to be bound to the subject
+## of the certificate. Usually it is used to specify one or multiple DNS names for 
+## which a certificate is valid.
+##
+## f: The file.
+##
+## ext: The parsed subject alternative name extension.
+##
+## .. bro:see:: x509_certificate x509_extension x509_ext_basic_constraints
+##              x509_parse x509_verify
+##              x509_get_certificate_string
+event x509_ext_subject_alternative_name%(f: fa_file, ext: X509::SubjectAlternativeName%);

src/file_analysis/analyzer/x509/functions.bif

@@ -42,7 +42,9 @@ RecordVal* x509_error_record(uint64_t num, const char* reason)
 ##
 ## Returns: A X509::Certificate structure
 ##
-## .. bro:see:: x509_verify
+## .. bro:see:: x509_certificate x509_extension x509_ext_basic_constraints
+##              x509_ext_subject_alternative_name x509_verify
+##              x509_get_certificate_string
 function x509_parse%(cert: opaque of x509%): X509::Certificate
 	%{
 	assert(cert);
@@ -60,7 +62,9 @@ function x509_parse%(cert: opaque of x509%): X509::Certificate
 ##      (false).
 ##
 ## Returns: X509 certificate as a string
-
+##
+## .. bro:see:: x509_certificate x509_extension x509_ext_basic_constraints
+##              x509_ext_subject_alternative_name x509_parse x509_verify
 function x509_get_certificate_string%(cert: opaque of x509, pem: bool &default=F%): string
 	%{
 	assert(cert);
@@ -101,7 +105,9 @@ function x509_get_certificate_string%(cert: opaque of x509, pem: bool &default=F
 ## Returns: A record of type X509::Result containing the result code of the verify
 ##          operation. In case of success also returns the full certificate chain.
 ##
-## .. bro:see:: x509_parse
+## .. bro:see:: x509_certificate x509_extension x509_ext_basic_constraints
+##              x509_ext_subject_alternative_name x509_parse
+##              x509_get_certificate_string
 function x509_verify%(certs: x509_opaque_vector, root_certs: table_string_of_string%): X509::Result
 	%{
 	X509_STORE* ctx = 0;

src/file_analysis/analyzer/x509/types.bif

@@ -1,5 +1,5 @@
 type X509::Certificate: record;
 type X509::Extension: record;
 type X509::BasicConstraints: record;
+type X509::SubjectAlternativeName: record;
 type X509::Result: record;
-