This also fixes the GRE analyzer to forward into the IEEE 802.11 analyzer
if it encounters Aruba packets with the proper protocol types. This way
the QoS header can be handled correctly.
This fixes a bug for AYIYA, Geneve and VXLAN forwarding encapsulated
content only if it's longer than their header. A new weird is introduced
to indicate empty tunnels.
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.
* ekoyle/add-protocol-pbb:
Update seemingly-unrelated btests
Use a default analyzer
Simplify PBB analyzer by using Ethernet analyzer
Add btest for PBB and update baselines
Use constexpr instead of #define
Cleanup and add customer MAC addresses
Add PBB (802.1ah) support
After the first 4 bytes, this traffic actually just looks like Ethernet.
Rather than try to re-implement the ethernet analyzer, just check the
length, skip 4 bytes, and pass it on.
This mimics how the Teredo analyzer is already doing it, including
sending a weird if the session is invalid and bailing out if the
protocol was already violated.
* 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.
* microsoft/master: (71 commits)
Clang formatting
Mask ports before inserting them into the map
Fix compiler warning from applied patch
Remove statistics plugin in favor of stats bif
Add EventHandler version of stats plugin
Mark a few EventHandler methods const
Changed implementation from std::map to std::unordered_map of Val.cc
Removed const, Windows build is now working
Added fixes suggested in PR
Update src/packet_analysis/protocol/ip/IP.cc
Apply suggestions from code review
Clang format again but now with v13.0.1
Rewrote usages of define(_MSC_VER) to ifdef _MSC_VER
Clang format it all
Fixed initial CR comments
Add NEWS entry about Windows port
Add a couple of extra unistd.h includes to fix a build failure
Use std::chrono instead of gettimeofday
Update libkqueue submodule [nomail]
Don't call tokenize_string if the input string is empty
...
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
Before, that API was part of the analyzers themselves, which meant we
couldn't disable a packet analyzer before it had been instantiated.
That's different from protocol/file analyzers, where we disable them
through the corresponding component. The lack of the component-side
API prevented Spicy from replacing packet analyzers at startup.
The reason we had put this into analyzer originally was performance so
that we don't need a component lookup for every packet. This change
keeps that optimization by caching the on/off state in the analyzer
itself as well, but now with the component being the one controlling
it.
There was a check generating an internal warning if a new packet
analyzer mapping overwrote an existing one. However, replacing
an existing analyzer with a new one seems a legitimate use case (and
will be supported by Spicy soon). So removing that warning.
This adds machinery to the packet_analysis manager for disabling
and enabling packet analyzers and implements two low-level bifs
to use it.
Extend Analyzer::enable_analyzer() and Analyzer::disable_analyzer()
to transparently work with packet analyzers, too. This also allows
to add packet analyzers to Analyzer::disabled_analyzers.
Passing nullptr sessions to AnalyzerConfirmation and AnalyzerViolation
of protocol analyzers previously blew up - protect from that.
Related to zeek/spicy-plugin#133.
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.
It turns out that this can be *very* spammy on networks where we're receiving
lots of these packets, and can fill up the reporter log very quickly. Weirds are
already reported in all of these cases anyways, so it doesn't make sense to log
a reporter warning too.
