* 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
* ssh://github.com/fatemabw/zeek:
Update options.zeek
Create out-27
Add files via upload
Update src/packet_analysis/protocol/tcp/TCPSessionAdapter.cc
Updating the weird names to use all lower case
Fixing whitespaces..
Fixing clang pre-commit error
Add check for option 27
Add the parsed fields for TCP option 27
Add TCP options bad length check
Add new syntax for adding and removing attributes from record fields:
redef RecordType$field_name += { &log };
redef RecordType$field_name -= { &log };
For now this only allowed for the &log attribute as the semantics are clear.
For &default and &optional the semantics aren't obvious and no use-cases have
been identified where those would make sense to change.
This enables a mechanism to add potentially interesting fields to the typical
Info records in base scripts, but letting users opt-into actually including
them into their log. At the same time, users that find specific fields in a
standard log uninteresting can opt-out without using `Log::Filter$exclude`
which can be difficult to use correctly. Patching or forking external packages
to remove columns from a log can also be avoided with this mechanism.
Closes#2000.
Due to the asynchronous behavior of the input framework and broker
communication, change handlers were previously called even after
zeek_done() event processing completed and also broker shutdown.
Accessing broker store handles within change handlers this late
triggered invalid Broker store handle messages:
error in ././my_option_store.zeek, line 13: invalid Broker store handle (Broker::put(Test::store, to_any_coercemy_option, to_any_coerceTest::new_value, 0 secs) and broker::store::{})
Fixes#2010
The core.load-stdin test checks Zeek's ability to read scripts from stdin.
core.sigterm-regular and core.sigterm-stdin verify that SIGTERM shuts down a
Zeek process during normal operation and while reading script content from
stdin, respectively. For technical reasons we don't test with SIGINT, as ctrl-c
would trigger -- see comments for details.
This could lead to a noticeable (single-percent) performance
improvement.
Most of the functionality for this is in the packet analyzers that now
cache ignore_chesksums_nets.
Based on a patch by Arne Welzel (Corelight).
* origin/topic/vern/ZAM-prep: (45 commits)
whoops overlooked the need to canonicalize filenames
another set of tweaks per review comments
addressed a number of code review comments
baseline updates for merge
support "any" coercions for "-O gen-C++"
better descriptions for named record constructors
test suite baseline updates for "-a opt" optimize-AST alternative
test suite baseline updates for "-a xform" alternative / AST transformation
error propagation fix for AST reduction
updates to "-a inline" test suite alternative baseline
updates for the main test suite baseline
updates to test suite tests for compatibility with upcoming ZAM functionality
"-O compile-all" option to specify compilation of inlined functions
compile inlined functions if they're also used indirectly
provide ZAM-generated code with low-level access to record fields
fix for cloning records with fields of type "any"
direct access for ZAM to VectorVal internal vector
ZVal constructors, accessors & methods in support of ZAM
switch ZVal representation of types from Type objects to TypeVal's
revised error-reporting interface for ZVal's, to accommodate ZAM inner loop
...
