This commit revamps the handling of analyzer violations that happen
before an analyzer confirms the protocol.
The current state is that an analyzer is disabled after 5 violations, if
it has not been confirmed. If it has been confirmed, it is disabled
after a single violation.
The reason for this is a historic mistake. In Zeek up to versions 1.5,
analyzers were unconditianally removed when they raised the first
protocol violation.
When this script was ported to the new layout for Zeek 2.0 in
b4b990cfb5, a logic error was introduced
that caused analyzers to no longer be disabled if they were not
confirmed.
This was the state for ~8 years, till the DPD::max_violations options
was added, which instates the current approach of disabling unconfirmed
analyzers after 5 violations. Sadly, there is not much discussion about
this change - from my hazy memory, I think this was discovered during
performance tests and the new behavior was added without checking into
the history of previous changes.
This commit reinstates the originally intended behavior of DPD. When an
analyzer that has not been confirmed raises a protocol violation, it is
immediately removed from the connection. This also makes a lot of sense
- this allows the analyzer to be in a "tasting" phase at the beginning
of the connection, and to error out quickly once it realizes that it was
attached to a connection not containing the desired protocol.
This change also removes the DPD::max_violations option, as it no longer
serves any purpose after this change. (In practice, the option remains
with an &deprecated warning, but it is no longer used for anything).
There are relatively minimal test-baseline changes due to this; they are
mostly triggered by the removal of the data structure and by less
analyzer errors being thrown, as unconfirmed analyzers are disabled
after the first error.
This switches the DPD logic to always log analyzers that raised a
protocol confirmation.
The logic is that, once a protocol has been confirmed - and thus there
probably is log output - it does not make sense to later remove it from
the log. It does make sense to somehow flag it as failed - but that
seems like a secondary step.
* 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
224.0.0.0/24 (and 6to4 conversion 2002:e000::/40) from RFC5771 "Multicast Local Network Control Block" defined as non-routable.
239.0.0.0/8 (and 6to4 conversion 2002:ef00::/24) from RFC2365 "Administratively Scoped IP Multicast"
fec0::/10 from RFC3879 "Deprecated Site Local Addresses"
(cherry picked from commit 821ab2dbed)
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
This fixes instances where `zeek:see` was used incorrectly so it was not
rendered correctly. All these instances have been found by looking for
`zeek:see` in the generated HTML where it should not be visible anymore.
I also removed a doc reference to `paraglob_add` which never existed.
This caused confusion and I don't think it's very intuitive. If called
with a name that does not exist, this returns without a value, not even
an error value. Changing that seems like it could be more deprecation
work.
* origin/topic/awelzel/move-broker-to-cluster-publish:
netcontrol: Move to Cluster::publish()
openflow: Move to Cluster::publish()
netcontrol/catch-and-release: Move to Cluster::publish()
config: Move to Cluster::publish()
ssl/validate-certs: Move to Cluster::publish()
irc: Move to Cluster::publish()
ftp: Move to Cluster::publish()
dhcp: Move to cluster publish
notice: Move to Cluster::publish()
intel: Move to Cluster::publish()
sumstats: Move to Cluster::publish()
* origin/topic/awelzel/fix-cluster-publish-any:
cluster/Backend: Handle unspecified table/set
cluster: Fix Cluster::publish() of Broker::Data
cluster: Be noisy when attempting to connect to an unknown node
Mostly due to spending too much time wondering why nodes didn't connect
when there was a mismatch between "manager" and "manager-1" in the
cluster layout. Remove manager from test-all-policy-cluster test to
avoid connection attempts in this test.
* topic/christian/disconnect-slow-peers:
Bump cluster testsuite to pull in Broker backpressure tests
Expand documentation of Broker events.
Add sleep() BiF.
Add backpressure disconnect notification to cluster.log and via telemetry
Remove unneeded @loads from base/misc/version.zeek
Add Cluster::nodeid_to_node() helper function
Support re-peering with Broker peers that fall behind
Add Zeek-level configurability of Broker slow-peer disconnects
Bump Broker to pull in disconnect feature and infinite-loop fix
No need to namespace Cluster:: functions in their own namespace
This adds a Broker-specific script to the cluster framework, loaded only when
Zeek is running in cluster mode. It adds logging in cluster.log as well as
telemetry via a metrics counter for Broker-observed backpressure disconnects.
The new zeek_broker_backpressure_disconnects counter, labeled by the neighboring
peer that the reporting node has determined to be unresponsive, counts the
number of unpeerings for this reason.
Here the node "worker" has observed node "proxy" falling behind once:
# HELP zeek_broker_backpressure_disconnects_total Number of Broker peering drops due to a neighbor falling too far behind in message I/O
# TYPE zeek_broker_backpressure_disconnects_total counter
zeek_broker_backpressure_disconnects_total{endpoint="worker",peer="proxy"} 1
Includes small btest baseline update to reflect @load of a new script.
This module is loaded by the telemetry framework, which we're now loading via
the cluster framework, i.e. also in bare mode. The resulting additional
thread (for creating reporter.log) trips up a number of btest baselines.
version.zeek doesn't use any of the string helper functions.
This adds re-peering at the Broker level for peers that Broker decided to
unpeer. We keep this at the Broker level since this behavior is specific to
it (as opposed to other cluster backends).
Includes baseline updates for btests that pick up on the new script's @load.
* origin/topic/johanna/sqlite-pragmas:
Options for SQLite log writer, eliminate duplicate definitions
Test synchronous/journal mode options for SQLite log writer
Added default options for synchronous and journal mode
Support for synchronous and journal_mode
