This is similar to GH-3206. There do not seem to be practical
consequences - but we should still fix it.
This also includes the udp-testcase that was forgotten in GH-3206.
Avoids loosing state on a connection value when a connection is flipped.
Fixes up the NTP baseline as well where this was visible: analyzer_confirmation_info()
was raised for a connection value which was immediately forgotten due to
the subsequent connection flipping.
Closed#3028
For "individually different but deterministic" runs specifying Zeek's
seed as an environment variable eases setups as one can avoid creating
extra seed files for each of the individual processes.
It is an error to specify the new ZEEK_SEED_VALUES variable together
with the existing ZEEK_SEED_FILE and -G. ZEEK_SEED takes precedence over
deterministic mode (-D) like ZEEK_SEED_FILE does today already.
I wasn't able to reproduce this locally, but after looking at
-B main-loop,tm for a bit it dawned that if the manager is sending
ticks too fast, the Broker IO source may consume two ticks in one go
before expiring timers and that would explain the observed baseline
differences.
Solve this by removing the reliance on realtime delays and switch to
a request-reply pattern instead.
Also fix indentation and bogus messages.
Fixes#3013
In #2464 the warning when overriding a packet analyzer mapping was
removed. While a warning seems indeed excessive, some info would still
be nice to have.
Remove the special case of initializing network time if it hasn't
happened yet. The argument about broker.log containing 0.0 timestamps
is more a problem of the log, not something that would justify modifying
network time globally. For broker.log and possibly cluster.log, it might
be more reasonable to use current time, anyway.
I was a bit wary about tables backed by broker stores being populated
with network_time set to 0.0, but there seems to exist logic and assumptions
that this is okay: It should be the same as if one populates a table with
expirations set within zeek_init().
In fact, staring a bit more, *not setting* network time might be more correct
as workers that don't see packets would never set zeek_start_network_time
which is used within the expiration computation.
This tests that timer expiration happens after a call to set_network_time()
upon the next time around the loop. This should be fairly stable, but
suspect major changes in the main loop or around timer expiration may
subtly change behavior things.
This tested that timers continue working even if one calls
suspend_processing() in zeek -r mode. The new behavior is
that timers do not function in that scenario and the test
invalid.
These seem to have been deprecated since 2018, so do it now.
Unfortunately the message didn't contain a version when it'll
be removed, but it's been long enough.
Mostly, treat HTTP0.9 completely separate. Because we're doing raw
delivery of a body directly, fake enough (connection_close=1, and finish
headers manually) so that the MIME infrastructure thinks it is seeing a
body.
This deals better with the body due to accounting for the first line. Also
it avoids the content line analyzer to strip CRLF/LF and the analyzer
then adding CRLF unconditionally by fully bypassing the content line
analyzer.
Concretely, the vlan-mpls test case contains a HTTP response with LF only,
but the previous implementation would use CRLF, accounting for two many bytes.
Same for the http.no-version test which would previously report a body
length of 280 and now is at 323 (which agrees with wireshark).
Further, the mime_type detection for the http-09 test case works because
it's now seeing the full body.
Drawback: We don't extract headers when a server actually replies with
a HTTP/1.1 message, but grrr, something needs to give I guess.
* origin/topic/robin/gh-2426-flipping:
Fixing productive connections with missing SYN still considered partial after flipping direction.
Add some missing bits when flipping endpoints.
In https://github.com/zeek/zeek/pull/2191, we added endpoint flipping
for cases where a connection starts with a SYN/ACK followed by ACK or
data. The goal was to treat the connection as productive and go ahead
and parse it. But the TCP analyzer could continue to consider it
partial after flipping, meaning that app layers would bail out. #2426
shows such a case: HTTP gets correctly activated after flipping
through content inspection, but it won't process anything because
`IsPartial()` returns true. As the is-partial state reflects
whether we saw the first packets each in direction, this patch now
overrides that state for the originally missing SYN after flipping.
We actually had the same problem at a couple of other locations already
as well. One of that only happened to work because of the originally
inconsistent state flipping that was fixed in the previous commit. The
corresponding unit test now broke after that change. This commit
updates that logic as well to override the state.
This fix is a bit of a hack, but the best solution I could think of
without introducing larger changes.
Closes#2426.
This started with reverting commit 52cd02173d
and then rewriting it to be per handler rather than handler identifier
and adding support for hooks as well as adding implicit module groups.
This seems to be an age-old bug. Reported by mchen on discourse [1].
The TCPSessionAdapter decides in AddExtraAnalyzers() whether to enable
reassembly or not. When dpd_reassemble_first_packets is F, this boils down to
! GetChildren().empty(). The intention being that if any analyzers have been
added to the connection based on known ports, reassembly is to be enabled.
However, GetChildren() does not take into account new_children and so
! GetChildren().empty() is always false here and reassembly solely
based on dpd_reassemble_first_packets=F (or the tcp_content... options).
Ouch.
Call AppendNewChildren() before AddExtraAnalyzers() as a fix. Without this,
the new test does not produce an http.log and service "http" isn't in conn.log.
[1] https://community.zeek.org/t/how-to-activate-an-application-layer-analyzer-when-signature-dpd-reassemble-first-packets-is-off/6763
Add a test parsing a malformed PE file showing that analyzer_violation_info
is raised with the fa_file object set.
It could be interesting to pass through an optional connection if one
exists, but access is provided through f$conns, too.
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.
There's a logic error in the packet analyzer's AnalyzerConfirmation()
method that causes analyzer_confirmation() events to be raised for every
packet rather than stopping after the first confirmation which appears to
have been the intention. This affects, for example, VXLAN and Geneve tunnels.
The optional arg_tag parameter was used for short-circuit'ing, but the return
value of GetAnalyzerTag() used for setting the session state causing the
disconnect.
In scenarios where Zeek receives purely tunneled monitoring traffic, this may
result in a non-negligible performance impact.
Somewhat related, ensure the session state is set to violated before
short-circuiting if no analyzer_violations are installed.
Suggesting this as a 5.0.3 candidate.
Now that it's loaded in bare mode, no need to load it explicitly.
The main thing that tests were relying on seems to be tracking of
c$service for conn.log baselines. Very few were actually checking
for dpd.log
