From 0fa1ecce8f9ce6a2a81a412df987b50b55cfd890 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Wed, 5 Feb 2025 16:42:26 +0000 Subject: [PATCH] DPD: change policy script for service violation logging; add NEWS This commit renames the `service_violation` column that can be added via a policy script to `failed_service`. This expresses the intent of it better - the column contains services that failed and were removed after confirmation. Furthermore, the script is fixed so it actually does this - before it would sometimes add services to the list that were not actually removed. In the course of this, the type of the column was changed from a vector to an ordered set. Due to the column rename, the policy script itself is also renamed. Also adds a NEWS entry for the DPD changes. --- NEWS | 40 +++++++++++++++++++ .../conn/failed-service-logging.zeek | 38 ++++++++++++++++++ .../conn/service-violation-logging.zeek | 27 ------------- scripts/test-all-policy.zeek | 2 +- .../conn.log | 4 +- .../analyzer/dpd-logging-configuration.zeek | 2 +- testing/external/scripts/testing-setup.zeek | 3 +- 7 files changed, 84 insertions(+), 32 deletions(-) create mode 100644 scripts/policy/protocols/conn/failed-service-logging.zeek delete mode 100644 scripts/policy/protocols/conn/service-violation-logging.zeek diff --git a/NEWS b/NEWS index 74a5faf1b7..1d40791499 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,46 @@ New Functionality Changed Functionality --------------------- +- The ``service`` field in the connection log is now sorted in the order that + protocol analyzers raise their confirmation events. + Since the time at which the protocol confirmation is raised depends on the + indivivual implementation of each analyzer, there is no specific meaning + to the order that the services appear. However, the order should be + deterministic between runs. It also will in many cases represent + the order in which layered protocols are parsed (e.g. "quic,ssl"). + +- The way that protocol violations are handled by the dynamic protocol + detection (DPD) changed. Now, a violation that is raised by an analyzer + before it is confirmed will immediately disable the analyzer. This adjusts + the behavior back to the historically desired state, and aligns it with + the treatment of confirmed analyzers. + + As a consequence of this, the option ``DPD::max_violations`` is no longer used. + It will be retained till Zeek 8.1 to prevent script errors, and raises a + deprecation warning. + + The way failed services interact with the ``service`` field in the connection + log also changed. In the past, protocol analyzers that were confirmed and + later failed were removed from the ``service`` field in some cases. This + commonly lead to the case that a protocol log exists, while the service is + not listed in the connection.log - so, e.g., an etry in ``http.log`` existing + without ``http`` showing up in the connection log. + + Now, protocol analyzers that raised a confirmation event will always be added to + the ``service`` field in the connection log, and the entry will be retained + even if the analyzer raises a violation later. + + To extend the visibility of protocol violations, a new option + ``DPD::track_removed_services_in_connection`` was added. Enabling it causes + failed analyzers to be logged to the ``service`` field of the connection log, + with a prepended "-". So a connection that attached the ``ssl`` analyzer + which later failed due to a protocol error will be logged as ``ssl,-ssl``. + + This change also adds a new policy script, + ``protocols/conn/failed-service-logging.zeek``. Loading this script adds the + column ``failed_service`` to the connection.log. This column contains the + list of protocol analyzers that failed due to a protocol error. + - Command line options processing will no longer print usage whenever there is an error. Instead, issues in command line processing will print an error, then prompt to use --help. The --help usage will now print to standard output diff --git a/scripts/policy/protocols/conn/failed-service-logging.zeek b/scripts/policy/protocols/conn/failed-service-logging.zeek new file mode 100644 index 0000000000..291b44de19 --- /dev/null +++ b/scripts/policy/protocols/conn/failed-service-logging.zeek @@ -0,0 +1,38 @@ +##! 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 + +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 ) + 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]; + } diff --git a/scripts/policy/protocols/conn/service-violation-logging.zeek b/scripts/policy/protocols/conn/service-violation-logging.zeek deleted file mode 100644 index 54376c8a4e..0000000000 --- a/scripts/policy/protocols/conn/service-violation-logging.zeek +++ /dev/null @@ -1,27 +0,0 @@ -##! This script adds the new column ``service_violation`` 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 - -module Conn; - -redef record Conn::Info += { - ## 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. - service_violation: vector of string &log &optional; -}; - -# Not using connection removal hook, as this has to run for every connection. -event connection_state_remove(c: connection) &priority=4 - { - if ( c?$conn && |c$service_violation| > 0 ) - { - c$conn$service_violation = {}; - local sv: string; - for ( sv in c$service_violation) - c$conn$service_violation += to_lower(sv); - } - } diff --git a/scripts/test-all-policy.zeek b/scripts/test-all-policy.zeek index 2e3083315e..79db7e235b 100644 --- a/scripts/test-all-policy.zeek +++ b/scripts/test-all-policy.zeek @@ -98,7 +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/service-violation-logging.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 diff --git a/testing/btest/Baseline/scripts.base.frameworks.analyzer.dpd-logging-configuration/conn.log b/testing/btest/Baseline/scripts.base.frameworks.analyzer.dpd-logging-configuration/conn.log index 4df75f2392..1906d2690b 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.analyzer.dpd-logging-configuration/conn.log +++ b/testing/btest/Baseline/scripts.base.frameworks.analyzer.dpd-logging-configuration/conn.log @@ -5,7 +5,7 @@ #unset_field - #path conn #open XXXX-XX-XX-XX-XX-XX -#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig local_resp missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents ip_proto service_violation -#types time string addr port addr port enum string interval count count string bool bool count string count count count count set[string] count vector[string] +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig local_resp missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents ip_proto failed_service +#types time string addr port addr port enum string interval count count string bool bool count string count count count count set[string] count set[string] XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 51354 127.0.0.1 21 tcp ftp,-ftp 9.891089 34 71 SF T T 0 ShAdDaFf 13 718 10 599 - 6 ftp #close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/scripts/base/frameworks/analyzer/dpd-logging-configuration.zeek b/testing/btest/scripts/base/frameworks/analyzer/dpd-logging-configuration.zeek index 9dc69ad66a..2f1914efa7 100644 --- a/testing/btest/scripts/base/frameworks/analyzer/dpd-logging-configuration.zeek +++ b/testing/btest/scripts/base/frameworks/analyzer/dpd-logging-configuration.zeek @@ -2,6 +2,6 @@ # @TEST-EXEC: zeek -r $TRACES/ftp/ftp-invalid-reply-code.pcap %INPUT # @TEST-EXEC: btest-diff conn.log -@load policy/protocols/conn/service-violation-logging +@load policy/protocols/conn/failed-service-logging redef DPD::track_removed_services_in_connection = T; diff --git a/testing/external/scripts/testing-setup.zeek b/testing/external/scripts/testing-setup.zeek index b429adbb49..e165d38ef2 100644 --- a/testing/external/scripts/testing-setup.zeek +++ b/testing/external/scripts/testing-setup.zeek @@ -1,7 +1,8 @@ # Sets some testing specific options. @load external-ca-list -@load protocols/conn/service-violation-logging + +@load protocols/conn/failed-service-logging @ifdef ( SMTP::never_calc_md5 ) # MDD5s can depend on libmagic output.