Merge remote branch 'remotes/origin/topic/policy-scripts-new'

* remotes/origin/topic/policy-scripts-new:
  Fixed another SSL analyzer memory leak.
  Attempting to fix another SSL bug.
  Fixing a ref counting bug in the SSL analyzer that I just introduced.
  Fixing memory leaks in SSL analyzer.
  Fixed a parsing bug in the SSL analyzer thanks to tracefile from Aashish Sharma.
  Removing my fix from earlier.  This is indicating the script-land generated events priority problem.
  Updates to the DPD framework.
  Fixed a bug in the auth-addl DNS script.

Conflicts:
	src/bro.bif

policy/frameworks/dpd/__load__.bro

@@ -1,3 +1,2 @@
 @load dpd/base
-@load dpd/dyn-disable
 @load dpd/packet-segment-logging

policy/frameworks/dpd/base.bro

@@ -1,4 +1,5 @@
-##! Activates port-independent protocol detection.
+##! Activates port-independent protocol detection and selectively disables
+##! analyzers if protocol violations occur.
 
 @load functions
 @load signatures
@@ -6,19 +7,34 @@
 module DPD;
 
 ## Add the DPD signatures to the signature framework.
-redef signature_files += "dpd/dpd.sig";
+redef signature_files += "frameworks/dpd/dpd.sig";
 
 export {
 	redef enum Log::ID += { DPD };
 
 	type Info: record {
+		## Timestamp for when protocol analysis failed.
 		ts:             time            &log;
+		## Connection unique ID.
 		uid:            string          &log;
+		## Connection ID.
 		id:             conn_id         &log;
+		## Transport protocol for the violation.
 		proto:          transport_proto &log;
+		## The analyzer that generated the violation.
 		analyzer:       string          &log;
+		## The textual reason for the analysis failure.
 		failure_reason: string          &log;
+		
+		## Disabled analyzer IDs.  This is only for internal tracking 
+		## so as to not attempt to disable analyzers multiple times.
+		# TODO: This is waiting on ticket #460 to remove the '0'.
+		disabled_aids:  set[count]      &default=set(0);
 	};
+	
+	## Ignore violations which go this many bytes into the connection.
+	## Set to 0 to never ignore protocol violations.
+	const ignore_violations_after = 10 * 1024 &redef;
 }
 
 redef record connection += {
@@ -29,6 +45,7 @@ event bro_init()
 	{
 	Log::create_stream(DPD, [$columns=Info]);
 	
+	# Populate the internal DPD analysis variable.
 	for ( a in dpd_config )
 		{
 		for ( p in dpd_config[a]$ports )
@@ -42,31 +59,53 @@ event bro_init()
 
 event protocol_confirmation(c: connection, atype: count, aid: count) &priority=10
 	{
-	if ( fmt("-%s",analyzer_name(atype)) in c$service )
+	local analyzer = analyzer_name(atype);
-		delete c$service[fmt("-%s", analyzer_name(atype))];
+	
+	if ( fmt("-%s",analyzer) in c$service )
+		delete c$service[fmt("-%s", analyzer)];
 
-	add c$service[analyzer_name(atype)];
+	add c$service[analyzer];
 	}
 
 event protocol_violation(c: connection, atype: count, aid: count,
-                         reason: string) &priority=5
+                         reason: string) &priority=10
 	{
-	if ( analyzer_name(atype) in c$service )
+	local analyzer = analyzer_name(atype);
-		delete c$service[analyzer_name(atype)];
+	# If the service hasn't been confirmed yet, don't generate a log message
-	add c$service[fmt("-%s", analyzer_name(atype))];
+	# for the protocol violation.
+	if ( analyzer !in c$service )
+		return;
+		
+	delete c$service[analyzer];
+	add c$service[fmt("-%s", analyzer)];
 	
 	local info: Info;
 	info$ts=network_time();
 	info$uid=c$uid;
 	info$id=c$id;
 	info$proto=get_conn_transport_proto(c$id);
-	info$analyzer=analyzer_name(atype);
+	info$analyzer=analyzer;
 	info$failure_reason=reason;
 	c$dpd = info;
 	}
 
+event protocol_violation(c: connection, atype: count, aid: count, reason: string) &priority=5
+	{
+	if ( !c?$dpd || aid in c$dpd$disabled_aids )
+		return;
+
+	local size = c$orig$size + c$resp$size;
+	if ( ignore_violations_after > 0 && size > ignore_violations_after )
+		return;
+
+	# Disable the analyzer that raised the last core-generated event.
+	disable_analyzer(c$id, aid);
+	add c$dpd$disabled_aids[aid];
+	}
+
 event protocol_violation(c: connection, atype: count, aid: count,
 				reason: string) &priority=-5
 	{
-	Log::write(DPD, c$dpd);
+	if ( c?$dpd )
-	}
+		Log::write(DPD, c$dpd);
+	}

policy/frameworks/dpd/dyn-disable.bro

@@ -1,42 +0,0 @@
-##! When this script is loaded, analyzers that raise protocol_violation events
-##! are disabled for the affected connection.
-
-@load dpd/base
-@load notice
-
-module DPD;
-
-export {
-	redef enum Notice::Type += {
-		ProtocolViolation
-	};
-	
-	redef record DPD::Info += {
-		## Disabled analyzer IDs.
-		# TODO: This is waiting on ticket #460 to remove the '0'.
-		disabled_aids: set[count] &default=set(0);
-	};
-	
-	## Ignore violations which go this many bytes into the connection.
-	const max_data_volume = 10 * 1024 &redef;
-}
-
-
-event protocol_violation(c: connection, atype: count, aid: count,
-				reason: string) &priority=5
-	{
-	if ( aid in c$dpd$disabled_aids )
-		return;
-
-	local size = c$orig$size + c$resp$size;
-	if ( max_data_volume > 0 && size > max_data_volume )
-		return;
-
-	# Disable the analyzer that raised the last core-generated event.
-	disable_analyzer(c$id, aid);
-	add c$dpd$disabled_aids[aid];
-
-	NOTICE([$note=ProtocolViolation, $conn=c,
-	        $msg=fmt("%s disabled due to protocol violation", analyzer_name(atype)),
-	        $sub=reason, $n=atype]);
-	}

policy/frameworks/dpd/packet-segment-logging.bro

@@ -10,6 +10,8 @@ module DPD;
 
 export {
 	redef record Info += {
+		## A chunk of the payload the most likely resulted in the protocol 
+		## violation.
 		packet_segment: string &optional &log;
 	};
 
@@ -21,5 +23,7 @@ export {
 event protocol_violation(c: connection, atype: count, aid: count,
                          reason: string) &priority=4
 	{
+	if ( ! c?$dpd ) return;
+	
 	c$dpd$packet_segment=fmt("%s", sub_bytes(get_current_packet()$data, 0, packet_segment_size));
 	}

policy/protocols/dns/auth-addl.bro

@@ -6,10 +6,12 @@ redef dns_skip_all_addl = F;
 
 module DNS;
 
-redef record Info += {
+export {
-	auth:       set[string] &log &optional;
+	redef record Info += {
-	addl:       set[string] &log &optional;
+		auth:       set[string] &log &optional;
-};
+		addl:       set[string] &log &optional;
+	};
+}
 
 event do_reply(c: connection, msg: dns_msg, ans: dns_answer, reply: string) &priority=4
 	{

policy/tuning/defaults/remove-high-volume-notices.bro

@@ -5,7 +5,6 @@
 
 # Load the policy scripts where the notices are defined.
 @load frameworks/notice/weird
-@load dpd
 
 # Remove these notices from logging since they can be too noisy.
 redef Notice::ignored_types += {
@@ -13,5 +12,4 @@ redef Notice::ignored_types += {
 	Weird::AckAboveHole,
 	Weird::RetransmissionInconsistency,
 	Weird::WeirdActivity, # Only allow these to go in the weird log.
-	DPD::ProtocolViolation,
 };

src/bro.bif

@@ -3410,7 +3410,7 @@ function x509_verify%(der_cert: string, cert_stack: string_vec, root_certs: tabl
 	BroString* s = convert_index_to_string(root_certs);
 	if ( x509_stores.count(*s) > 0 )
 		ctx = x509_stores[*s];
-
+	
 	if ( ! ctx ) // lookup to see if we have this one built already!
 		{
 		ctx = X509_STORE_new();
@@ -3431,10 +3431,20 @@ function x509_verify%(der_cert: string, cert_stack: string_vec, root_certs: tabl
 				}
 			X509_STORE_add_cert(ctx, x);
 			}
+		delete idxs;
 
 		// Save the newly constructed certificate store into the cacheing map.
 		x509_stores[*s] = ctx;
 		}
+	delete s;
+	
+	const uint8 *cert_data = der_cert->Bytes();
+	X509* cert = d2i_X509_(NULL, &cert_data, der_cert->Len());
+	if ( ! cert )
+		{
+		builtin_run_time(fmt("Certificate error: %s", ERR_error_string(ERR_peek_last_error(),NULL)));
+		return new Val((uint64) ERR_get_error(), TYPE_COUNT);
+		}
 
 	STACK_OF(X509)* untrusted_certs = sk_X509_new_null();
 	if ( ! untrusted_certs )
@@ -3451,15 +3461,16 @@ function x509_verify%(der_cert: string, cert_stack: string_vec, root_certs: tabl
 		X509* x = d2i_X509_(NULL, &data, sv->Len());
 		if ( ! x )
 			{
-			builtin_error(fmt("Untrusted certificate stack creation error: %s", ERR_error_string(ERR_peek_last_error(),NULL)));
+			X509_free(cert);
+			sk_X509_pop_free(untrusted_certs, X509_free);
+			builtin_run_time(fmt("Untrusted certificate stack creation error: %s", ERR_error_string(ERR_peek_last_error(),NULL)));
 			return new Val((uint64) ERR_get_error(), TYPE_COUNT);
 			}
 		sk_X509_push(untrusted_certs, x);
 		}
 
-	const uint8 *cert_data = der_cert->Bytes();
-
 	X509_STORE_CTX csc;
+<<<<<<< HEAD
 	X509* cert = d2i_X509_(NULL, &cert_data, der_cert->Len());
 	if ( ! cert )
 		{
@@ -3467,6 +3478,8 @@ function x509_verify%(der_cert: string, cert_stack: string_vec, root_certs: tabl
 		return new Val((uint64) ERR_get_error(), TYPE_COUNT);
 		}
 
+=======
+>>>>>>> remotes/origin/topic/policy-scripts-new
 	X509_STORE_CTX_init(&csc, ctx, cert, untrusted_certs);
 	X509_STORE_CTX_set_time(&csc, 0, (time_t) network_time);
 
@@ -3475,6 +3488,7 @@ function x509_verify%(der_cert: string, cert_stack: string_vec, root_certs: tabl
 
 	if ( untrusted_certs )
 		sk_X509_pop_free(untrusted_certs, X509_free);
+	X509_free(cert);
 
 	return new Val((uint64) csc.error, TYPE_COUNT);
 	%}

src/ssl-analyzer.pac

@@ -72,22 +72,12 @@ function version_ok(vers : uint16) : bool
 	}
 	%}
 
-function convert_ciphers_uint24(ciph : uint24[]) : int[]
-	%{
-	vector<int>* newciph = new vector<int>();
-
-	std::transform(ciph->begin(), ciph->end(),
-		std::back_inserter(*newciph), to_int());
-
-	return newciph;
-	%}
 
 function convert_ciphers_uint16(ciph : uint16[]) : int[]
 	%{
 	vector<int>* newciph = new vector<int>();
 
-	std::copy(ciph->begin(), ciph->end(),
+	std::copy(ciph->begin(), ciph->end(), std::back_inserter(*newciph));
-		std::back_inserter(*newciph));
 
 	return newciph;
 	%}
@@ -140,7 +130,8 @@ refine connection SSL_Conn += {
 	function proc_client_hello(rec: SSLRecord,
 					version : uint16, ts : double,
 					session_id : uint8[],
-					cipher_suites : int[]) : bool
+					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",
@@ -150,13 +141,15 @@ refine connection SSL_Conn += {
 		if ( ! version_ok(version) )
 			bro_analyzer()->ProtocolViolation(fmt("unsupported client SSL version 0x%04x", version));
 
+		vector<int>* cipher_suites = new vector<int>();
+		if ( cipher_suites16  )
+			std::copy(cipher_suites16->begin(), cipher_suites16->end(), std::back_inserter(*cipher_suites));
+		else
+			std::transform(cipher_suites24->begin(), cipher_suites24->end(), std::back_inserter(*cipher_suites), to_int());
+
 		if ( ssl_client_hello )
 			{
-			BroType* count_t = base_type(TYPE_COUNT);
+			TableVal* cipher_set = new TableVal(internal_type("count_set")->AsTableType());
-			TypeList* set_index = new TypeList(count_t);
-			set_index->Append(count_t);
-			SetType* s = new SetType(set_index, 0);
-			TableVal* cipher_set = new TableVal(s);
 			for ( unsigned int i = 0; i < cipher_suites->size(); ++i )
 				{
 				Val* ciph = new Val((*cipher_suites)[i], TYPE_COUNT);
@@ -168,6 +161,8 @@ refine connection SSL_Conn += {
 							version, ts,
 							to_string_val(session_id),
 							cipher_set);
+							
+			delete cipher_suites;
 			}
 
 		return true;
@@ -176,7 +171,8 @@ refine connection SSL_Conn += {
 	function proc_server_hello(rec: SSLRecord,
 					version : uint16, ts : double,
 					session_id : uint8[],
-					cipher_suite : uint16,
+					cipher_suites16 : uint16[],
+					cipher_suites24 : uint24[],
 					comp_method : uint8) : bool
 		%{
 		if ( state_ == STATE_TRACK_LOST )
@@ -184,6 +180,13 @@ refine connection SSL_Conn += {
 				orig_label(${rec.is_orig}).c_str(),
 				state_label(old_state_).c_str()));
 
+		vector<int>* ciphers = new vector<int>();
+		
+		if ( cipher_suites16 )
+			std::copy(cipher_suites16->begin(), cipher_suites16->end(), std::back_inserter(*ciphers));
+		else
+			std::transform(cipher_suites24->begin(), cipher_suites24->end(), std::back_inserter(*ciphers), to_int());
+
 		if ( ! version_ok(version) )
 			bro_analyzer()->ProtocolViolation(fmt("unsupported server SSL version 0x%04x", version));
 
@@ -193,9 +196,10 @@ refine connection SSL_Conn += {
 							bro_analyzer()->Conn(),
 							version, ts,
 							to_string_val(session_id),
-							cipher_suite, comp_method);
+							ciphers->size()==0 ? 0 : ciphers->at(0), comp_method);
 			}
 
+		delete ciphers;
 		bro_analyzer()->ProtocolConfirmation();
 		return true;
 		%}
@@ -223,7 +227,6 @@ refine connection SSL_Conn += {
 
 		if ( x509_certificate )
 			{
-			X509* pCert = 0;
 			for ( unsigned int i = 0; i < certificates->size(); ++i )
 				{
 				const bytestring& cert = (*certificates)[i];
@@ -266,11 +269,6 @@ refine connection SSL_Conn += {
 				// Are there any X509 extensions?
 				if ( x509_extension && X509_get_ext_count(pTemp) > 0 )
 					{
-					BroType* count_t = base_type(TYPE_COUNT);
-					TypeList* set_index = new TypeList(count_t);
-					set_index->Append(count_t);
-					SetType* s = new SetType(set_index, 0);
-					TableVal* x509ex = new TableVal(s);
 					int num_ext = X509_get_ext_count(pTemp);
 					for ( int k = 0; k < num_ext; ++k )
 						{
@@ -295,6 +293,7 @@ refine connection SSL_Conn += {
 							}
 						}
 					}
+				X509_free(pTemp);
 				}
 			}
 		return true;
@@ -396,26 +395,26 @@ refine typeattr ApplicationData += &let {
 refine typeattr ClientHello += &let {
 	proc : bool = $context.connection.proc_client_hello(rec, client_version,
 				gmt_unix_time,
-				session_id, convert_ciphers_uint16(csuits))
+				session_id, csuits, 0)
 		&requires(state_changed);
 };
 
 refine typeattr V2ClientHello += &let {
 	proc : bool = $context.connection.proc_client_hello(rec, client_version, 0,
-				session_id, convert_ciphers_uint24(ciphers))
+				session_id, 0, ciphers)
 		&requires(state_changed);
 };
 
 refine typeattr ServerHello += &let {
 	proc : bool = $context.connection.proc_server_hello(rec, server_version,
-			gmt_unix_time, session_id, cipher_suite,
+			gmt_unix_time, session_id, cipher_suite, 0,
 			compression_method)
 		&requires(state_changed);
 };
 
 refine typeattr V2ServerHello += &let {
 	proc : bool = $context.connection.proc_server_hello(rec, server_version, 0, 0,
-				convert_ciphers_uint24(ciphers)[0], 0)
+				0, ciphers, 0)
 		&requires(state_changed);
 
 	cert : bool = $context.connection.proc_v2_certificate(rec, cert_data)

src/ssl-protocol.pac

@@ -433,7 +433,7 @@ type ServerHello(rec: SSLRecord) = record {
 	random_bytes : bytestring &length = 28 &transient;
 	session_len : uint8;
 	session_id : uint8[session_len];
-	cipher_suite : uint16;
+	cipher_suite : uint16[1];
 	compression_method : uint8;
 } &let {
 	state_changed : bool =
@@ -609,6 +609,16 @@ type CertificateVerify(rec: SSLRecord) = record {
 
 # The finished messages are always sent after encryption is in effect,
 # so we will not be able to read those message.
+type Finished(rec: SSLRecord) = record {
+	cont : bytestring &restofdata &transient;
+} &let {
+	state_changed : bool =
+		$context.connection.transition(STATE_SERVER_HELLO_DONE,
+					STATE_COMM_ENCRYPTED, rec.is_orig, true) ||
+		$context.connection.transition(STATE_CLIENT_FINISHED,
+					STATE_COMM_ENCRYPTED, rec.is_orig, false) ||
+		$context.connection.lost_track();
+};
 
 
 ######################################################################
@@ -635,6 +645,9 @@ type Handshake(rec: SSLRecord) = record {
 		SERVER_HELLO_DONE ->	server_hello_done : ServerHelloDone(rec);
 		CERTIFICATE_VERIFY ->	certificate_verify : CertificateVerify(rec);
 		CLIENT_KEY_EXCHANGE ->	client_key_exchange : ClientKeyExchange(rec);
+		FINISHED            ->  finished : Finished(rec);
+		CERTIFICATE_URL     ->  certificate_url : bytestring &restofdata &transient;
+		CERTIFICATE_STATUS  ->  certificate_status : bytestring &restofdata &transient;
 		default ->		unknown_handshake : UnknownHandshake(this, rec.is_orig);
 	} &length = to_int()(length);
 };
@@ -679,7 +692,7 @@ type CiphertextRecord(rec: SSLRecord, is_orig: bool) = record {
 ######################################################################
 
 type SSLPDU(is_orig: bool) = record {
-	records : SSLRecord(is_orig)[] &until($element == 0);
+	records : SSLRecord(is_orig)[] &until($element <= 0);
 } &byteorder = bigendian;