Commit graph

15 commits

Author SHA1 Message Date
Christian Kreibich
1dcd13a019 Fix a typo. 2025-06-05 17:51:54 -07:00
Johanna Amann
58613f0313 Introduce new c$failed_analyzers field
This field is used internally to trace which analyzers already had a
violation. This is mostly used to prevent duplicate logging.

In the past, c$service_violation was used for a similar purpose -
however it has slightly different semantics. Where c$failed_analyzers
tracks analyzers that were removed due to a violation,
c$service_violation tracks violations - and doesn't care if an analyzer
was actually removed due to it.
2025-06-04 12:07:13 +01:00
Johanna Amann
af77a7a83b Analyzer failure logging: tweaks and test fixes
The main part of this commit are changes in tests. A lot of the tests
that previously relied on analyzer.log or dpd.log now use the new
analyzer-failed.log.

I verified all the changes and, as far as I can tell, everything
behaves as it should. This includes the external test baselines.

This change also enables logging of file and packet analyzer to
analyzer_failed.log and fixes some small behavior issues.

The analyzer_failed event is no longer raised when the removal of an
analyzer is vetoed.

If an analyzer is no longer active when an analyzer violation is raised,
currently the analyzer_failed event is raised. This can, e.g., happen
when an analyzer error happens at the very end of the connection. This
makes the behavior more similar to what happened in the past, and also
intuitively seems to make sense.

A bug introduced in the failed service logging was fixed.
2025-06-03 15:56:42 +01:00
Johanna Amann
8c814fa88c Introduce analyzer-failed.log, as a replacement for dpd.log
Analyzer-failed.log is, essentially, the replacement for dpd.log. The
name should make more sense, as it does now log analyzer failures. For
protocol analyzers specifically, these are failures that lead to the
analyzer being disabled.
2025-06-03 15:17:26 +01:00
Johanna Amann
c55e21da71 Rename analyzer.log to analyzer.debug log; move to policy
The current analyzer.log is more useful for debugging than for
operational purposes. Hence this is disabled by default, moved to a
policy script, and the log is renamed to analyzer-debug.log.

Furthermore, logging of analyzer confirmations and disabling analyzers
are now enabled by default.
2025-06-03 15:17:26 +01:00
Johanna Amann
6183c5086b Move dpd.log to policy script
This is the first phase of moving from the current dpd log to a more
modern logfile, without some of the weirdnesses that the current dpd log
contains.

Tests will not pass in the current state; this is just splitting out
functionality.
2025-06-03 15:17:26 +01:00
Johanna Amann
b8c135d7cb Remove violating analyzer from services field again
This reverts some of the recent DPD changes; specifically violations
trigger removal from the services field, again, by default.

Discussion in GH-4521
2025-03-04 15:10:49 +00:00
Johanna Amann
e3493bc110 DPD changes - small script fixes and renames.
This addresses review feedback of GH-4200. No functional changes.
2025-02-05 13:55:43 +00:00
Johanna Amann
2f712c3c24 Allow to track service violations in conn.log.
This introduces ian options, DPD::track_removed_services_in_connection.
It adds failed services to the services column, prefixed with a
"-".

Alternatively, this commit also adds
policy/protocols/conn/failed-services.zeek, which provides the same
information in a new column in conn.log.
2025-01-30 16:59:44 +00:00
Johanna Amann
c72c1cba6f DPD: change handling of pre-confirmation violations, remove max_violations
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.
2025-01-30 16:59:44 +00:00
Johanna Amann
e6ed61c47a DPD: log analyzers that have confirmed
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.
2025-01-30 16:59:44 +00:00
Arne Welzel
bc8fd5a4c6 Introduce generic analyzer_confirmation_info and analyzer_violation_info
Introduce two new events for analyzer confirmation and analyzer violation
reporting. The current analyzer_confirmation and analyzer_violation
events assume connection objects and analyzer ids are available which
is not always the case. We're already passing aid=0 for packet analyzers
and there's not currently a way to report violations from file analyzers
using analyzer_violation, for example.

These new events use an extensible Info record approach so that additional
(optional) information can be added later without changing the signature.
It would allow for per analyzer extensions to the info records to pass
analyzer specific info to script land. It's not clear that this would be
a good idea, however.

The previous analyzer_confirmation and analyzer_violation events
continue to exist, but are deprecated and will be removed with Zeek 6.1.
2022-09-27 17:49:51 +02:00
Arne Welzel
7ed6cbeee5 Make connection$service_violation a set[string]
To stay consistent with connection$service.
2022-09-20 23:07:26 +02:00
Arne Welzel
31aeb58e10 dpd: Replace negated service fmt() magic with dedicated field
...the only known cases where the `-` for `connection$service` was
handled is to skip/ignore these analyzers.

Slight suspicion that join_string_set() should maybe become a bif
now determine_service() runs once for each connection.

Closes #2388
2022-09-20 23:07:26 +02:00
Arne Welzel
9e7f2a04c1 frameworks/dpd: Move to frameworks/analyzer/dpd, load by default
* Because frameworks/analyzer is loaded via init-frameworks-and-bifs the
  dpd functionality (really just dpd.log and disabling of analyzers) is
  now enabled even in bare mode.
* Not sure we need to keep frameworks/base/dpd/__load__.zeek around
  or can just remove it right away.
2022-08-31 16:50:47 +02:00
Renamed from scripts/base/frameworks/dpd/main.zeek (Browse further)