Merge remote-tracking branch 'origin/topic/johanna/dpd-changes'

* origin/topic/johanna/dpd-changes:
  DPD: failed services logging alignment
  DPD: update test baselines; change options for external tests.
  DPD: change policy script for service violation logging; add NEWS
  DPD changes - small script fixes and renames.
  Update public and private test suite for DPD changes.
  Allow to track service violations in conn.log.
  Make conn.log service field ordered
  DPD: change handling of pre-confirmation violations, remove max_violations
  DPD: log analyzers that have confirmed
  IRC analyzer - make protocol confirmation more robust.

scripts/base/frameworks/analyzer/dpd.zeek

@@ -26,14 +26,8 @@ export {
 		failure_reason: string          &log;
 	};
 
-	## Ongoing DPD state tracking information.
-	type State: record {
-		## Current number of protocol violations seen per analyzer instance.
-		violations: table[count] of count;
-	};
-
-	## Number of protocol violations to tolerate before disabling an analyzer.
-	option max_violations: table[Analyzer::Tag] of count = table() &default = 5;
+	## Deprecated, please see https://github.com/zeek/zeek/pull/4200 for details
+	option max_violations: table[Analyzer::Tag] of count = table() &deprecated="Remove in v8.1: This has become non-functional in Zeek 7.2, see PR #4200" &default = 5;
 
 	## Analyzers which you don't want to throw
 	option ignore_violations: set[Analyzer::Tag] = set();
@@ -41,14 +35,16 @@ export {
 	## Ignore violations which go this many bytes into the connection.
 	## Set to 0 to never ignore protocol violations.
 	option ignore_violations_after = 10 * 1024;
+
+	## Add removed services to conn.log, with a - in front of them.
+	option track_removed_services_in_connection = F;
 }
 
 redef record connection += {
 	dpd: Info &optional;
-	dpd_state: State &optional;
 	## The set of services (analyzers) for which Zeek has observed a
 	## violation after the same service had previously been confirmed.
-	service_violation: set[string] &default=set();
+	service_violation: set[string] &default=set() &ordered;
 };
 
 event zeek_init() &priority=5
@@ -79,12 +75,11 @@ event analyzer_violation_info(atype: AllAnalyzers::Tag, info: AnalyzerViolationI
 
 	local c = info$c;
 	local analyzer = Analyzer::name(atype);
-	# If the service hasn't been confirmed yet, don't generate a log message
-	# for the protocol violation.
-	if ( analyzer !in c$service )
+	# If the service hasn't been confirmed yet, or already failed,
+	# don't generate a log message for the protocol violation.
+	if ( analyzer !in c$service || analyzer in c$service_violation )
 		return;
 
-	delete c$service[analyzer];
 	add c$service_violation[analyzer];
 
 	local dpd: Info;
@@ -125,24 +120,16 @@ event analyzer_violation_info(atype: AllAnalyzers::Tag, info: AnalyzerViolationI
 	if ( ignore_violations_after > 0 && size > ignore_violations_after )
 		return;
 
-	if ( ! c?$dpd_state )
+	local disabled = disable_analyzer(c$id, aid, F);
+
+	# add "-service" to the list of services on removal due to violation, if analyzer was confirmed before
+	if ( track_removed_services_in_connection && disabled && Analyzer::name(atype) in c$service )
 		{
-		local s: State;
-		c$dpd_state = s;
+		local rname = cat("-", Analyzer::name(atype));
+		if ( rname !in c$service )
+			add c$service[rname];
 		}
 
-	if ( aid in c$dpd_state$violations )
-		++c$dpd_state$violations[aid];
-	else
-		c$dpd_state$violations[aid] = 1;
-
-	if ( c?$dpd || c$dpd_state$violations[aid] > max_violations[atype] )
-		{
-		# Disable an analyzer we've previously confirmed, but is now in
-		# violation, or else any analyzer in excess of the max allowed
-		# violations, regardless of whether it was previously confirmed.
-		disable_analyzer(c$id, aid, F);
-		}
 	}
 
 event analyzer_violation_info(atype: AllAnalyzers::Tag, info: AnalyzerViolationInfo ) &priority=-5

scripts/base/frameworks/analyzer/logging.zeek

@@ -214,10 +214,5 @@ hook Analyzer::disabling_analyzer(c: connection, atype: AllAnalyzers::Tag, aid:
 
 	populate_from_conn(rec, c);
 
-	if ( c?$dpd_state && aid in c$dpd_state$violations )
-		{
-		rec$failure_data = fmt("Disabled after %d violations", c$dpd_state$violations[aid]);
-		}
-
 	Log::write(LOG, rec);
 	}

scripts/base/init-bare.zeek

@@ -795,7 +795,7 @@ type connection: record {
 	## principle it is possible that more than one protocol analyzer is able
 	## to parse the same data. If so, all will be recorded. Also note that
 	## the recorded services are independent of any transport-level protocols.
-	service: set[string];
+	service: set[string] &ordered;
 	history: string;	##< State history of connections. See *history* in :zeek:see:`Conn::Info`.
 	## A globally unique connection identifier. For each connection, Zeek
 	## creates an ID that is very likely unique across independent Zeek runs.
@@ -954,10 +954,6 @@ type AnalyzerViolationInfo: record {
 ##
 ## An analyzer generating this many violations is unlikely parsing
 ## the right protocol or potentially buggy.
-##
-## See also :zeek:see:`DPD::max_violations` which controls disabling
-## analyzers through script logic after a certain number of violations
-## was observed.
 const max_analyzer_violations = 1000 &redef;
 
 ## Fields of a SYN packet.

scripts/base/protocols/conn/main.zeek

@@ -27,8 +27,10 @@ export {
 		id:           conn_id         &log;
 		## The transport layer protocol of the connection.
 		proto:        transport_proto &log;
-		## An identification of an application protocol being sent over
-		## the connection.
+		## A comma-separated list of confirmed protocol(s).
+		## With :zeek:see:DPD::track_removed_services_in_connection, the list
+		## includes the same protocols prefixed with "-" to record that Zeek
+		## dropped them due to parsing violations."
 		service:      string          &log &optional;
 		## How long the connection lasted.
 		##

scripts/policy/protocols/conn/failed-service-logging.zeek

@@ -0,0 +1,46 @@
+##! This script adds the new column ``failed_service`` to the connection log.
+##! The column contains the list of protocols in a connection that raised protocol
+##! violations causing the analyzer to be removed. Protocols are listed in order
+##! that they were removed.
+
+@load base/protocols/conn
+@load base/frameworks/analyzer/dpd
+
+module Conn;
+
+redef record Conn::Info += {
+	## List of analyzers in a connection that raised violations
+	## causing their removal.
+	## Analyzers are listed in order that they were removed.
+	failed_service: set[string] &log &optional &ordered;
+};
+
+hook Analyzer::disabling_analyzer(c: connection, atype: AllAnalyzers::Tag, aid: count) &priority=-1000
+	{
+	if ( ! is_protocol_analyzer(atype) && ! is_packet_analyzer(atype) )
+		return;
+
+	# Only add if previously confirmed
+	if ( Analyzer::name(atype) !in c$service || Analyzer::name(atype) !in c$service_violation )
+		return;
+
+	# Only log if dpd.zeek will disable
+	if ( atype in DPD::ignore_violations )
+		return;
+
+	local size = c$orig$size + c$resp$size;
+	if ( DPD::ignore_violations_after > 0 && size > DPD::ignore_violations_after )
+		return;
+
+	set_conn(c, F);
+
+	local aname = to_lower(Analyzer::name(atype));
+	# No duplicate logging
+	if ( c$conn?$failed_service && aname in c$conn$failed_service )
+		return;
+
+	if ( ! c$conn?$failed_service )
+		c$conn$failed_service = set();
+
+	add c$conn$failed_service[aname];
+	}

scripts/test-all-policy.zeek

@@ -98,6 +98,7 @@
 @load misc/unknown-protocols.zeek
 @load protocols/conn/community-id-logging.zeek
 @load protocols/conn/disable-unknown-ip-proto-support.zeek
+@load protocols/conn/failed-service-logging.zeek
 @load protocols/conn/ip-proto-name-logging.zeek
 @load protocols/conn/known-hosts.zeek
 @load protocols/conn/known-services.zeek