Merge remote-tracking branch 'origin/topic/johanna/gh-4061'

* origin/topic/johanna/gh-4061:
  Update BiF-tracking, add is_event_handled
  Address review comments and small updates for DNS warnings
  Raise warnings when for DNS events that are not raised due to dns_skip_all_addl

CHANGES

@@ -1,3 +1,17 @@
+7.2.0-dev.88 | 2025-01-14 14:39:14 +0000
+
+  * Raise warnings when for DNS events that are not raised due to dns_skip_all_addl (Johanna Amann, Corelight)
+
+    By default, dns_skip_all_addl is set to false. This causes several
+    events to not be raised. This change emits warnings when a user defines
+    event handlers for events that will not be raised.
+
+    Furthermore, it adds notes about this behavior to the documentation. We
+    also introduce a new BIF, `is_event_handled`, which checks if an event
+    is handled.
+
+    Fixes GH-4061
+
 7.2.0-dev.84 | 2025-01-14 11:12:52 +0100
 
   * support for record extensions when using -O gen-standalone-C++ (Vern Paxson, Corelight)

NEWS

@@ -12,6 +12,10 @@ Breaking Changes
 New Functionality
 -----------------
 
+- Some DNS events are not raised when ``dns_skip_all_addl`` is set to true.
+  Zeek now raises a warning when a script declares these events while this
+  option is set to true.
+
 Changed Functionality
 ---------------------
 
@@ -21,7 +25,7 @@ Changed Functionality
   rather than standard error.
 
 Removed Functionality
-------------------------
+---------------------
 
 Deprecated Functionality
 ------------------------

VERSION

@@ -1 +1 @@
-7.2.0-dev.84
+7.2.0-dev.88

scripts/base/protocols/dns/__load__.zeek

@@ -1,2 +1,3 @@
 @load ./consts
 @load ./main
+@load ./check-event-handlers

scripts/base/protocols/dns/check-event-handlers.zeek

@@ -0,0 +1,19 @@
+##! This script checks if DNS event handlers that will not be raised
+##! are used and raises a warning in those cases.
+
+module DNS;
+
+event zeek_init() &priority=20
+	{
+	if ( ! dns_skip_all_addl )
+		return;
+
+	local addl_functions = ["dns_TSIG_addl", "dns_EDNS_addl", "dns_EDNS_ecs", "dns_EDNS_tcp_keepalive", "dns_EDNS_cookie"];
+
+	for ( event_name in addl_functions )
+		if ( is_event_handled(event_name) )
+			Reporter::warning(fmt("Used event '%s' will not be raised because 'dns_skip_all_addl' is true", event_name));
+
+	if ( is_event_handled("dns_TKEY") )
+		Reporter::warning("Used event 'dns_TKEY' will not contain any data in 'ans' because 'dns_skip_all_addl' is true");
+	}

src/analyzer/protocol/dns/events.bif

@@ -496,6 +496,11 @@ event dns_unknown_reply%(c: connection, msg: dns_msg, ans: dns_answer%);
 ##
 ## ans: The parsed EDNS reply.
 ##
+## .. note::
+##
+##      Note that this event will only be raised if :zeek:see:`dns_skip_all_addl`
+##      is set to false.
+##
 ## .. zeek:see:: dns_AAAA_reply dns_A_reply dns_CNAME_reply dns_HINFO_reply dns_MX_reply
 ##    dns_NS_reply dns_PTR_reply dns_SOA_reply dns_SRV_reply dns_TSIG_addl
 ##    dns_TXT_reply dns_SPF_reply dns_WKS_reply dns_end dns_mapping_altered
@@ -519,6 +524,11 @@ event dns_EDNS_addl%(c: connection, msg: dns_msg, ans: dns_edns_additional%);
 ##
 ## opt: The parsed EDNS option.
 ##
+## .. note::
+##
+##      Note that this event will only be raised if :zeek:see:`dns_skip_all_addl`
+##      is set to false.
+##
 ## .. zeek:see:: dns_AAAA_reply dns_A_reply dns_CNAME_reply dns_HINFO_reply dns_MX_reply
 ##    dns_NS_reply dns_PTR_reply dns_SOA_reply dns_SRV_reply dns_TSIG_addl
 ##    dns_TXT_reply dns_SPF_reply dns_WKS_reply dns_end dns_mapping_altered
@@ -544,6 +554,11 @@ event dns_EDNS_ecs%(c: connection, msg: dns_msg, opt: dns_edns_ecs%);
 ##
 ## opt: The parsed EDNS Keepalive option.
 ##
+## .. note::
+##
+##      Note that this event will only be raised if :zeek:see:`dns_skip_all_addl`
+##      is set to false.
+##
 ## .. zeek:see:: dns_AAAA_reply dns_A_reply dns_CNAME_reply dns_HINFO_reply dns_MX_reply
 ##    dns_NS_reply dns_PTR_reply dns_SOA_reply dns_SRV_reply dns_TSIG_addl
 ##    dns_TXT_reply dns_SPF_reply dns_WKS_reply dns_end dns_mapping_altered
@@ -569,6 +584,11 @@ event dns_EDNS_tcp_keepalive%(c: connection, msg: dns_msg, opt: dns_edns_tcp_kee
 ##
 ## opt: The parsed EDNS Cookie option.
 ##
+## .. note::
+##
+##      Note that this event will only be raised if :zeek:see:`dns_skip_all_addl`
+##      is set to false.
+##
 ## .. zeek:see:: dns_AAAA_reply dns_A_reply dns_CNAME_reply dns_HINFO_reply dns_MX_reply
 ##    dns_NS_reply dns_PTR_reply dns_SOA_reply dns_SRV_reply dns_TSIG_addl
 ##    dns_TXT_reply dns_SPF_reply dns_WKS_reply dns_end dns_mapping_altered
@@ -592,6 +612,11 @@ event dns_EDNS_cookie%(c: connection, msg: dns_msg, opt: dns_edns_cookie%);
 ##
 ## ans: The parsed TKEY reply.
 ##
+## .. note::
+##
+##      Note that ``ans`` will only be populated if :zeek:see:`dns_skip_all_addl`
+##      is set to false.
+##
 ## .. zeek:see:: dns_TSIG_addl
 event dns_TKEY%(c: connection, msg: dns_msg, ans: dns_tkey%);
 
@@ -609,6 +634,11 @@ event dns_TKEY%(c: connection, msg: dns_msg, ans: dns_tkey%);
 ##
 ## ans: The parsed TSIG reply.
 ##
+## .. note::
+##
+##      Note that this event will only be raised if :zeek:see:`dns_skip_all_addl`
+##      is set to false.
+##
 ## .. zeek:see:: dns_AAAA_reply dns_A_reply dns_CNAME_reply dns_EDNS_addl
 ##    dns_HINFO_reply dns_MX_reply dns_NS_reply dns_PTR_reply dns_SOA_reply
 ##    dns_SRV_reply  dns_TXT_reply dns_SPF_reply dns_WKS_reply dns_end

src/script_opt/FuncInfo.cc

@@ -324,6 +324,7 @@ static std::unordered_map<std::string, unsigned int> func_attrs = {
     {"is_alnum", ATTR_FOLDABLE},
     {"is_alpha", ATTR_FOLDABLE},
     {"is_ascii", ATTR_FOLDABLE},
+    {"is_event_handled", ATTR_IDEMPOTENT}, // can error
     {"is_file_analyzer", ATTR_NO_ZEEK_SIDE_EFFECTS},
     {"is_icmp_port", ATTR_FOLDABLE},
     {"is_local_interface", ATTR_IDEMPOTENT},

src/zeek.bif

@@ -5015,6 +5015,30 @@ function generate_all_events%(%) : bool
 	return zeek::val_mgr->True();
 	%}
 
+## Check if an event is handled. Typically this means that a script defines an event.
+## This currently is mainly used to warn when events are defined that will not be used
+## in certain conditions.
+##
+## Raises an error if the named event does not exist.
+##
+## event_name: event name to check
+##
+## returns: true if the named event is handled.
+function is_event_handled%(event_name: string%) : bool
+	%{
+	auto *h = event_registry->Lookup(event_name->ToStdStringView());
+	if ( ! h )
+		{
+		zeek::emit_builtin_error(zeek::util::fmt("is_event_handled: '%s' is not an event", event_name->CheckString()));
+		return zeek::val_mgr->False();
+		}
+
+	if ( *h )
+		return zeek::val_mgr->True();
+
+	return zeek::val_mgr->False();
+	%}
+
 %%{
 // Autogenerated from CMake bif_target()
 #include "__all__.bif.cc"

testing/btest/Baseline/bifs.is_event_handled/err

@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/is_event_handled.zeek, line 11: is_event_handled: 'myfunc1' is not an event (is_event_handled(myfunc1))
+error in <...>/is_event_handled.zeek, line 12: is_event_handled: 'conn_id' is not an event (is_event_handled(conn_id))

testing/btest/Baseline/bifs.is_event_handled/out

@@ -0,0 +1,5 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+T
+F
+F
+F

testing/btest/Baseline/coverage.default-load-baseline/canonified_loaded_scripts.log

@@ -385,6 +385,7 @@ scripts/base/init-default.zeek
   scripts/base/protocols/dns/__load__.zeek
     scripts/base/protocols/dns/consts.zeek
     scripts/base/protocols/dns/main.zeek
+    scripts/base/protocols/dns/check-event-handlers.zeek
   scripts/base/protocols/finger/__load__.zeek
     scripts/base/protocols/finger/spicy-events.zeek
     scripts/base/protocols/finger/main.zeek

testing/btest/Baseline/opt.ZAM-bif-tracking/output

@@ -1,2 +1,2 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-542 seen BiFs, 0 unseen BiFs (), 0 new BiFs ()
+543 seen BiFs, 0 unseen BiFs (), 0 new BiFs ()

testing/btest/Baseline/scripts.base.protocols.dns.event-handler-warning/.stderr

@@ -0,0 +1,6 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+warning in <...>/check-event-handlers.zeek, line 15: Used event 'dns_EDNS_tcp_keepalive' will not be raised because 'dns_skip_all_addl' is true
+warning in <...>/check-event-handlers.zeek, line 15: Used event 'dns_EDNS_cookie' will not be raised because 'dns_skip_all_addl' is true
+warning in <...>/check-event-handlers.zeek, line 15: Used event 'dns_EDNS_ecs' will not be raised because 'dns_skip_all_addl' is true
+warning in <...>/check-event-handlers.zeek, line 15: Used event 'dns_EDNS_addl' will not be raised because 'dns_skip_all_addl' is true
+warning in <...>/check-event-handlers.zeek, line 18: Used event 'dns_TKEY' will not contain any data in 'ans' because 'dns_skip_all_addl' is true

testing/btest/bifs/is_event_handled.zeek

@@ -0,0 +1,12 @@
+# @TEST-EXEC: zeek -b %INPUT >out 2>err
+# @TEST-EXEC: btest-diff out
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff err
+
+function myfunc1(a: addr, b: addr): int
+	{
+	}
+
+print is_event_handled("zeek_init"); # T
+print is_event_handled("dns_EDNS_cookie"); # F
+print is_event_handled("myfunc1"); # builtin error
+print is_event_handled("conn_id"); # builtin error

testing/btest/opt/ZAM-bif-tracking.zeek

@@ -357,6 +357,7 @@ global known_BiFs = set(
 	"is_alnum",
 	"is_alpha",
 	"is_ascii",
+	"is_event_handled",
 	"is_file_analyzer",
 	"is_icmp_port",
 	"is_local_interface",

testing/btest/scripts/base/protocols/dns/dns-edns-cookie.zeek

@@ -3,6 +3,6 @@
 @load policy/protocols/dns/auth-addl
 
 event dns_EDNS_cookie(c: connection, msg: dns_msg, opt: dns_edns_cookie)
-    {
+	{
-        print opt;
+	print opt;
-    } 
+	}

testing/btest/scripts/base/protocols/dns/event-handler-warning.zeek

@@ -0,0 +1,31 @@
+# Check that warnings are for events that will not be raised
+
+# @TEST-EXEC: zeek -b %INPUT
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+@load base/protocols/dns
+
+event dns_EDNS_addl(c: connection, msg: dns_msg, ans: dns_edns_additional)
+	{
+	print "";
+	}
+
+event dns_EDNS_ecs(c: connection, msg: dns_msg, opt: dns_edns_ecs)
+	{
+	print "";
+	}
+
+event dns_EDNS_tcp_keepalive(c: connection, msg: dns_msg, opt: dns_edns_tcp_keepalive)
+	{
+	print "";
+	}
+
+event dns_EDNS_cookie(c: connection, msg: dns_msg, opt: dns_edns_cookie)
+	{
+	print "";
+	}
+
+event dns_TKEY(c: connection, msg: dns_msg, ans: dns_tkey)
+	{
+	print "";
+	}