A std::unordered_map takes 56 bytes on my system. Switch to a unique_ptr
an initialize weird_state lazily. That saves ~48 bytes per connection.
Particularly for scan or non-weird traffic, this should allow some
memory savings.
When the stem process terminates and SIGCHLD is sent to the supervisor,
the signal might be handled by the main thread or any other threads that
aren't blocking SIGCHLD explicitly. Convert last_signal to a std::atomic<int>
such that non-main threads can safely set last_signal without triggering
data race as reported by TSAN. This doesn't make it less racy to work
last_signal, but it appears we only use it for debug printing anyhow and
another option might have been to just remove last_signal altogether.
Follow-up for #4849
* origin/topic/timw/merge-3rdparty-repo-into-main: (30 commits)
Ignore src/3rdparty for pre-commit
src/3rdparty: Port doctest fix for including <ciso646> from upstream
src/3rdparty: Update doctest to v2.4.12
src/3rdparty: Move jthread/stop_token out of std namespace to prevent collisions
src/3rdparty: Fix clang-tidy bugprone-casting-through-void warning
src/3rdparty: Add jthread and stop_token headers
src/3rdparty: Update SQLite to 3.47.1
src/3rdparty: Upgrade sqlite3 to 3.45.0
src/3rdparty: modp: Disable deprecation warning for sprintf
src/3rdparty: Update SQLite to 3.41.2
src/3rdparty: Handle zeek-inet-ntop snprintf() return value correctly
src/3rdparty: Rework inet-ntop change to build on Windows
src/3rdparty: Use snprintf in zeek_inet_ntop.c to silence macOS compiler warnings
src/3rdparty: Use snprintf in patricia.cc to silence macOS compiler warnings
src/3rdparty: Update ConvertUTF Unicode license as per request from LLVM upstream
src/3rdparty: Update doctest to 2.4.9 and sqlite to 3.39.4
src/3rdparty: Fix rampant off-by-one error in last change
src/3rdparty: numeric conversion functions now return the number of characters added
src/3rdparty: Fix GCC 12.2 warning in access to patricia_t members
src/3rdparty: Rebase patricia.{h,c} on upstream version
...
This has been failing on our weekly builds for a bit. It's fixed upstream
but there's no idea when they'll release a new version that includes it.
This cherry-picks 5eb20178ba
Access to members of the larger prefix_t, while safe, throws warnings
when the compiler knows it previously allocated the smaller prefix4_t.
Adapted from 11ae12079ca716f8cfac42cccfd905dad45da18f in the upstream repo.
If Cluster::init() hasn't been invoked yet, Cluster::subscribe() with the
ZeroMQ backend would block because the main_inproc socket didn't
yet have a connection from the child thread. Prevent this by connecting
the main and child socket pair at construction time.
This will queue the subscriptions and start processing them once the
child thread has started.
An URI containing a bracketed or non-bracketed IPv6 address of the form
http://[::1]:42 was previously split on the first colon for port extraction,
causing a subsequent to_count() call to fail. Harden this to check for a
digits in the last :[0-9]+ component.
Fixes#4842
build
Whatever is used with recent OpenSSL is also available with latest LibreSSL
on OpenBSD 7.8-beta as of today.
Some of these hunks have been in the net/bro port for years, others I
recently added whilst gradually updating from 6.0.5 to 8.0.1.
* origin/topic/timw/4742-skip-storage-thread:
Avoid starting up storage expiration thread with zero backends
Remove a couple of obsolete TODOs in storage manager
* origin/topic/timw/windows-zeromq-support:
Reword comment about when ZeroMQ is required for the build
CI: Add label for enabling the Windows build in PRs
Enable building ZeroMQ support on Windows
Add a test/example forwarding all new_connection() events produced during
`zeek -r wikipedia.trace` as my_new_connection() to a WebSocket client.
This is mostly to demonstrate and verify usage of suspend_processing(),
websocket_client_added(), resume_processing(), Pcap::file_done(),
websocket_client_lost() and terminate() together.
The upstream recently merge Christian's fixes for the fd_map allocations
so that patch doesn't need to be in our fork anymore. I also tested removing
the patch about timers from our fork, and it seems to only matter for Linux
kernels earlier than 5.3. All of our supported platforms are newer than
that.
* origin/topic/vern/emphasize-IDPtrs:
fixup! fixup! shift much of the internal use of ID* identifier pointers over to IDPtr objects
fixup! shift much of the internal use of ID* identifier pointers over to IDPtr objects
fixup! shift much of the internal use of ID* identifier pointers over to IDPtr objects
fixup! shift much of the internal use of ID* identifier pointers over to IDPtr objects
annotate a number of BTests as unsuited for -O gen-C++ testing due to multiple Zeek runs
BTest baseline updates for -O gen-C++ - all minor tweaks
BTest updates for script optimization tracking of BiFs
regression test for former ASAN issue with script optimization of lambdas
shift much of the internal use of ID* identifier pointers over to IDPtr objects
maintenance update for script optimization's knowledge of BiFs
logger fix for interoperability with -O gen-C++ code
Add full support for RFC 9460's SvcParams list.
Amend the existing `dns_svcb_rr` record by a vector of new
`dns_svcb_param` records containing aptly typed SvcParamKey and
SvcParamValue pairs. Example output:
```
@load base/protocols/dns
event dns_HTTPS( c: connection , msg: dns_msg , ans: dns_answer , https: dns_svcb_rr ) {
for (_, param in https$svc_params)
print to_json(param); # filter uninitialised values
}
```
```
$ dig https cloudflare-ech.com +short | tr [:space:] \\n
1
.
alpn="h3,h2"
ipv4hint=104.18.10.118,104.18.11.118
ech=AEX+DQBBHgAgACBGL2e9TiFwjK/w1Zg9AmRm7mgXHz3PjffP0mTFNMxmDQAEAAEAAQASY2xvdWRmbGFyZS1lY2guY29tAAA=
ipv6hint=2606:4700::6812:a76,2606:4700::6812:b76
```
```
{"key":1,"alpn":["h3","h2"]}
{"key":4,"hint":["104.18.10.118","104.18.11.118"]}
{"key":5,"ech":"AEX+DQBBHgAgACBGL2e9TiFwjK/w1Zg9AmRm7mgXHz3PjffP0mTFNMxmDQAEAAEAAQASY2xvdWRmbGFyZS1lY2guY29tAAA="}
{"key":6,"hint":["2606:4700::6812:a76","2606:4700::6812:b76"]}
```
Values with malformed data or belonging to invalid/reserved keys
are passed raw bytes in network order for script-level inspection.
Follow up to "Initial Support to DNS SVCB/HTTPS RR"
https://github.com/zeek/zeek/pull/1808
* origin/topic/timw/readability-container-contains:
Fix a few more random clang-tidy findings
Use std::numbers::pi instead of hard-coded value
Use std::scoped_lock instead of std::lock_guard
Use .contains() instead of .find() or .count()
* topic/christian/management-framework-tls:
Bump zeek-testing-cluster to pull in WebSocket TLS updates
Bump zeek-client to pull in TLS config updates
Management framework: add TLS options for controller's websocket server
* origin/topic/timw/switch-spicy-zeek-builds-to-ubuntu-24:
Move benchmarking to Ubnutu 24 task, add to normal PR builds
Reorder ubuntu builds so 22.04 comes last
Rename Ubuntu-based tasks to have the full version name
Switch Zeek Spicy builds to Ubuntu 24
* origin/topic/timw/tools-clang-tidy:
gen-zam: Move source files up a directory
bifcl: Fix clang-tidy warnings
bifcl: Run clang-format on c++ code embedded in builtin-func.{l,y}
binpac: Fix a large number of clang-tidy warnings
binpac: Replace delete_list macro uses
binpac: Run clang-format on c++ code embedded in pac_parse.{ll,yy}
* origin/topic/awelzel/cluster-event-metadata-fixes-for-8.0:
cluster/Backend: Fallback to current network time when current event has not timestamp
cluster/serializer/broker: Do not send empty metadata vectors around
We install test data which we also make available under an alternative
path for backwards compatibility. The installation of this symlink did
not take `DESTDIR` installs like used by Zeek's packaging into account
which caused installations from packages to behave different from
installs from source.
This patch fixes the symlink to respect a possible `DESTDIR`.
Closes#3266.
When a WebSocket client sends an event to Zeek without explicit network
timestamp metadata, Zeek would use -1.0 as a timestamp for any events
published while handling this event. Instead, it seems far more sensible
to use the current network time in that scenario.
Event when there's no metadata attached to an event, we'd still use the
constructor passing an empty metadata vector, resulting in an on-the-wire
representation with an empty trailing vector.
Particularly visible when just snooping events via websocat. There also
seems to be some bug with the timestamp -1 handling.
* origin/topic/timw/move-submodules-to-main-repo-take-2: (343 commits)
Add NEWS entries for submodule moves
Remove configure --with-gen-zam argument and the CMake summaries
af_packet: Remove submodule, adapt CMake/code for Zeek build
af_packet: pre-commit fixes
af_packet: Fix initialization
af_packet: Ensure all of the member fields get initialized
af_packet: Require CMake 3.15 to match Zeek's requirement
af_packet: Note that Zeek ships with a built-in version.
af_packet: Use cstdint instead of stdint.h
af_packet: Use override for overriding parent methods
af_packet: Use 'pragma once' instead of include guards
af_packet: AF_Packet: Fix wrong vlan when PCP or DEI bits are set in tp_vlan_tci
af_packet: AF_Packet: Check interface for upness
af_packet: AF_Packet: Use negative socket_fd for error indication
af_packet: AF_Packet: Remove usages of inline
af_packet: Add guarded zeek/zeek-version.h include.
af_packet: RX_Ring: Add include for string
af_packet: Increase version number.
af_packet: Add info if TP_STATUS_CSUM_VALID is not defined.
af_packet: Define TP_STATUS_CSUM_VALID when not defined
...
A user reported vlan ids > 4095 being logged by Zeek [1]. For populating
packet->vlan, mask away Priority Code Point (PCP) and Drop Eligible
Indicator (DEI) bits from the tp_vlan_tci field, else we're not setting
the correct value on the packet.
Fixes#60
[1] https://community.zeek.org/t/zeek-reporting-vlan-ids-above-4095-bug-found/7000
When using af_packet with an interface that was not up, the following
non-informative error was reported:
$ /opt/zeek-5.2/bin/zeek -i af_packet::replay
fatal error: problem with interface af_packet::replay (Invalid argument)
With this change, the error now includes information about the
interface being down:
$ ZEEK_PLUGIN_PATH=$(pwd)/build zeek -Ci af_packet::replay
fatal error: problem with interface af_packet::replay (interface is down)
Fixes#51
Technically, socket() can return 0, so shouldn't use it as an
indication of a non existent / closed socket.
I'm not 100% sure about the Close() contract here: If something
goes haywire with a packet source Zeek calls FatalError without
calling Close() nor properly destructing the PktSrc. Oh yikes.
With zeek/zeek#2802, zeek-config.h will not provide ZEEK_VERSION_NUMBER
when a plugin is compiled as a builtin/static plugin into Zeek. This is
done to avoid tree-wide ccache busting when just the version changes.
When compiling with clang and libc++ like done when using Google's
hongfuzz in the oss-fuzz setup, the following errors are produced:
Step #3 - "compile-honggfuzz-address-x86_64": /src/zeek/auxil/zeek-af_packet-plugin/src/RX_Ring.cc:19:9: error: no matching conversion for functional-style cast from 'const char[15]' to 'RX_RingException'
Step #3 - "compile-honggfuzz-address-x86_64": throw RX_RingException("invalid socket");
Step #3 - "compile-honggfuzz-address-x86_64": ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Step #3 - "compile-honggfuzz-address-x86_64": /src/zeek/auxil/zeek-af_packet-plugin/src/RX_Ring.h:14:7: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'const char[15]' to 'const RX_RingException' for 1st argument
Step #3 - "compile-honggfuzz-address-x86_64": class RX_RingException : public std::runtime_error {
Step #3 - "compile-honggfuzz-address-x86_64": ^
Step #3 - "compile-honggfuzz-address-x86_64": /src/zeek/auxil/zeek-af_packet-plugin/src/RX_Ring.h:14:7: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'const char[15]' to 'RX_RingException' for 1st argument
Step #3 - "compile-honggfuzz-address-x86_64": /src/zeek/auxil/zeek-af_packet-plugin/src/RX_Ring.h:16:2: note: candidate constructor not viable: no known conversion from 'const char[15]' to 'const std::string' (aka 'const basic_string<char, char_traits<char>, allocator<char>>') for 1st argument
Step #3 - "compile-honggfuzz-address-x86_64": RX_RingException(const std::string& what_arg) : std::runtime_error(what_arg) {}
Step #3 - "compile-honggfuzz-address-x86_64":
The problem can be reproduced outside of hongfuzz by compiling this
plugin with with clang/libc++:
export CXX=clang++-14
export CXXFLAGS=-stdlib=libc++
./configure
make
Include <string> in RX_Ring.h to ensure the required const char * to
std::string conversion are available to any users of RX_RingException.
On some older Linux distributions (CentOS 7), the if_packet.h header does
not yet include TP_STATUS_CSUM_VALID (introduced in March 2015). Simply
define it if it's not there.
This lowers the default timeout from 100msec to 10msec and increases the default
block_size from 16KB to 32KB. Both are aligned with what Suricata uses as defaults.
The block_size is likely too conservative for high-performance, tuning
guides recommend starting with 1MB block size.
Fixes#37.
There's been some wondering why kernel headers are required to compile
this plugin as it's not providing a kernel module or otherwise provides
functionality related to kernel APIs. AF_PACKET sockets are provided
through user-space APIs.
There may have been historical reasons, but let's move forward and
remove the dependency.
Fixes#29#24
The plugin mechanism automatically picks up the scripts folder for
plugin-related Zeek scripts. When a package's zkg.meta also declares the scripts
folder the location for package-level scripts, the scripts get installed
redundantly in two places. Zeek cannot recognize them as duplicates, triggering
various errors relating to double-defined symbols.
This moves the package-level scripts to a subdirectory with a placeholder. The
placeholder avoids the zkg install-time warning, whose original fix in e3aea1
introduced this problem.
Under MSVC regex objects are multiline by default and there is no apparant way of changing this behaviour. Under clang/gcc regex is singleline unless std::regeX_constants::multiline is specificied.
This behaviour is assumed in Gen-ZAM.cc when auto-generating header files.
Example: https://godbolt.org/z/aP59x3EhT
1491335: AUTO_CAUSES_COPY due to a for loop using auto& instead of const auto&
1491338: AUTO_CAUSES_COPY due to a for loop using auto& instead of const auto&
Currently, Zeek disables any static type checking for var_arg bifs.
However, the generated preamble for var_args bifs assume that
typed positional arguments are correctly typed and blindly calls
the type converters on them. This easily triggers abort()s at runtime
currently when a script mistakenly uses the wrong types for var_arg
bifs. For example, calling publish_rr() with a port instead of a string
causes a hard-abort with Zeek 5.0.8.
$ zeek -e 'Cluster::publish_rr(Cluster::Pool(), 80/tcp)'
fatal error in <no location>: Val::CONVERTER (port/string) (80/tcp)
Aborted (core dumped)
Extend bifcl so that for var_arg functions and the types that bifcl understands,
we render a runtime type check and explicit early return to avoid the abort().
For any/other types, the implementer of the bif continuous to be responsible
for type checking.
This isn't solving the var_args situation generally, but avoids some
ad-hoc fixes trickling in current bif implementations.
Some references:
https://github.com/zeek/zeek/issues/1523https://github.com/zeek/zeek/issues/2425https://github.com/zeek/zeek/issues/2935https://github.com/zeek/zeek/pull/2950
I'm not sure why this was restricted to only two components,
the following appears functional in Zeek scripts.
module A::B::C;
export {
type MyRecord: record {
a: string;
};
}
Closes#25.
This fixes the DWARF information gets output by the compiler, and allows debuggers
to use the full path name to display contextual information when a session stops
inside of BIF code.
The Yacc grammar treats comments like other whitespace and when seeing
the first definition in a file would previously emit all whitespace
before emitting the `export` section containing the definition. This
lead to the first definition being separated from their zeekygen
documention (separated by `export {`).
With this patch we start the export section before emitting whitespace.
While this might now pull more "whitespace" into the exported part, it
avoids breaking the association between zeekygen comments and
definitions.
Closes#15.
This avoids potential problems with libc++ 8+ on case-insensitive file
systems due to inclusion of a new header called <version> which will end
up conflicting with the VERSION file if the search path includes the
project root.
Binpac generates a lot of switch statements with repeated blocks in
them (typically empty blocks). Running clang-tidy on the generated code
with bugprone-branch-clone generates a lot of warnings. Instead of
doing a ton of analysis in binpac to avoid generating the duplicates,
just mark any switch generated with an annotation to avoid reporting
them.
Elsewhere (zeek/zeek#2482), it was observed that when binpac encounters
include failures, it still exits with 0 indicating success. Subsequent
compilation of the produced .h and .cc files likely fails.
Exit with 1 on include errors to make pin pointing issues easier by
having make/ninja stop earlier.
CMake documentation says that CMAKE_CFG_INTDIR is no longer necessary to
find the right binary for the configuration and is in fact deprecated in
recent versions of CMake.
For example:
inum: uint32 = case (ed & 0x0f) of {
0x00 -> n_8; # n_8 is a uint8
0x01 -> n_16; # n_16 is a uint16
0x02 -> n_32; # n_32 is a uint32
default -> 0;
};
Previously, the temporary storage used for evaluating the
case-expression was based on whatever type the first case yields, which
is a uint8 in the above example. That behavior can lead to a narrowing
conversion whenever the 0x01 or 0x02 cases occur.
The new behavior is to base the temporary storage's type on the largest
numeric type that the case-expression can yield, which is uint32 in the
above example.
Incremental flowbuffer parsing sought to first parse the "minimum header
length" required to get the full frame length, possibly from a record
field, but generating the logic to parse that field could greedily
bundle in additional boundary-checks for all subsequent fields of
known-size.
E.g. for flowunit parsing of this:
type HDR = record {
version: uint8;
reserved: uint8;
len: uint16;
} &byteorder=bigendian;
type FOO_PDU(is_orig: bool) = record {
hdr: HDR;
plen: uint8;
ptype: uint8;
something: bytestring &restofdata;
} &byteorder=bigendian, &length=hdr.len;
The flowbuffer was correctly seeking to buffer 4 bytes and parse the
"hdr.len" field, but the generated parsing logic for "hdr.len" included
a boundary check all the way up to include "plen" and "ptype".
This causes out-of-bounds exceptions to be thrown for inputs that should
actually be possible to incrementally parse via flowbuffer.
OpenBSD shared library names are like "libfoo.so.major.minor" and
binpac was previously letting the post-release number into the name
like "libbinpac.so.0.54-7", which isn't compatible with that scheme.
Related to https://github.com/zeek/zeek/issues/649
Array lengths use signed integer storage, so multiplication of that
by the element size for purpose of bounds checking against available
data may produce a signed integer overlow, which is undefined behavior.
The type used to store the index for a case-type now tracks the
type of the index expression rather than always using an "int".
The case fields also now have some checking done at code-gen-time to
ensure the constants used for cases does not exceed the numeric limit
of the type used in the case's index expression. Then, assuming, it
looks safe, the C++ case labels are generated with casts to the type
of the Binpac case's index expression to ensure compilers accept it
(since all Binpac numbers use "int" for storage/printing internally).
This allows for tunability of the following behaviors:
* Minimum flowbuffer capacity to use when parsing a new unit
* Threshold at which flowbuffer capacity is contracted back to the
minimum after parsing a complete unit and before parsing the next
* Maximum flowbuffer capacity to allow when parsing a given unit
Failed flowbuffer allocations due to reaching maximum capacity or any
other reason now throw ExceptionFlowBufferAlloc.
For arrays that are fields within a record, the bounds check was based
on a pointer to the start of the record rather than the start of the
array field.
For arrays with unknown element size, it used to cap the array length to
be the maximum it could be for the given input buffer, assuming 1-byte
elements. An exception is instead now raised for cases where the
maximum array size (number of elements) exceeds what's possibly in the
buffer. Using an exception versus capping the length may help prevent
protocol analyzers from unintentionally accessing array indices that
were not actually parsed even if the evauluated-length-expression for
that given array implies it may have been.
The former is easy to misuse by accidentally storing the contents of
the temporary string return value and accessing it later. There's also
potential pitfalls in changing it to return a pointer into a static
buffer, so instead start using strfmt() uniformly across the codebase
and change some methods to use strings instead of char*.
In this case, the bounds checking for individual elements can be
optimized out of the parsing-loop in favor of a single, array-wide
bounds check beforehand.
For arrays with a length expression (e.g. uint16[size] instead of
uint16[]), the parsing loop would consider reaching the end of the
data buffer as a successful loop termination condition even if it's
not yet parsed the required number of elements.
Now, for such arrays, the loop will only terminate based on the loop
counter (derived from the length expression) or else it will throw an
OOB exception when trying to parse an element and finding not enough
data in the buffer.
Credit to Tomas Bortoli for reporting the problem and proposing
patches.
It should only suppress the parsing-loop boundary check in the case
where array elaments are a single byte in length and thus covered by
the boundary check (generated as a result of &length) that is placed
before the parsing-loop.
The issue is that t_begin_of_data + %s can sometimes overflow.
Bug reported and patch proposed by
Philippe Antoine <p.antoine@catenacyber.fr> from Catena cyber.
How to reproduce:
>>>code
int32 n = 0xF71B0000;
int32 n1 = pac_swap(n);
code<<<
n1 becomes 0xFFFFFFF7 instead of 0x00001BF7
Reason: Undefined behaviour after bit shift operation because of
negative value of the argument. See C++ standard (2011) 5.8.2 (Shift
operators).
A common BinPAC construct for parsing records is a switch statement,
with no breaks between the cases, as control is expected to fall
through.
Coverity raises an error about this; this commit should fix that.
Specifying &length on a record no longer skips generating boundary
checks for individual fields. E.g. a record field that specifies a
&length that extends beyond the &length of the record containing it
should throw binpac::ExceptionOutOfBound, the usual way of handling
out-of-bounds conditions.
Field lengths derived from other data in the input could potentially
lead to reading from outside the bounds of the input buffer.
Reported by John Villamil and Chris Rohlf - Yahoo Paranoids
before anything else.
Internally, this function compiles all regular expressions, avoiding
to that inside the regexp constructore. The code is a bit hackish due
to the way the regexp code depends on the Bro header.
This helps ensure the availability of PRI* macros from .pac files,
which cannot create this definition themselves since the inclusion
of binpac.h is hardcoded to be placed very early in the generated
code and already includes inttypes.h itself.
Strings with a constant &length expression can be checked for negative
length values while generating the parser instead of in the parser
itself (which likely just ends up being dead code).
The code generated for types w/ &refcount will subclass RefCount and
Unref definitely deletes via a pointer to that base class so it needs a
virtual dtor.
parsers.
This consists of two parts:
1. The generated Flow classes expose their flow buffers via a new
method flow_buffer().
2. Flow buffers get two new methods:
// Interface for delayed parsing. Sometimes BinPAC doesn't get the
// buffering right and then one can use these to feed parts
// individually and assemble them internally. After calling
// FinishBuffer(), one can send the uppper-layer flow an FlowEOF()
// to trigger parsing.
void BufferData(const_byteptr data, const_byteptr end);
void FinishBuffer();
Switch to using a no-argument throw to preserve the dynamic type of
the binpac exception. Otherwise, the exception is "sliced" and can only
be subsequently handled as binpac::Exception and not a derived type.
This allows analyzers to define their own types of the same name
without mistakingly overshadowing the usages of binpac::Exception
and its derived types in the generated parser code.
If set, parsed elements won't actually be added to the array, and read
access to the array aren't permitted. This is helpful to save memory
in the case of large arrays for which elements don't need (or can't)
be buffered.
Also set some convenience variables that mimic what a FindBinPAC
module would set. These could be used by a superproject that
wants to decide whether to use an existing BinPAC installation
or (re)build from source.
- Moved find_package() calls up to top directory so failure can occur
right away if any dependencies are missing
- Got rid of a bunch of header checks and preprocessor definitions that
aren't ever used; not sure why I had them there in the first place
- Renamed VERSION preprocessor definition to not conflict with Bro's.
- Removed check for HAVE_CONFIG_H; config.h will always be generated in the
binpac build directory, but if a parent directory has generated a config.h
and added that to compiler include search paths via include_directories(),
then that one should be used -- include_directories() appends paths by
default and the parent CMakeLists.txt will have called it first.
* origin/topic/awelzel/no-null-strings-in-threading-vals:
logging/Manager: Also pass non-null vector and set
logging/Manager: Non-null strings for empty strings
After #4724, empty strings would result in nullptrs being stored in the
threading::Value's string_val.data field instead of a valid pointer to
an empty strings. This upsets UBSAN's nonnull check for memcpy()
[01:29:45.807] ../../src/SerializationFormat.cc:80:33: runtime error: null pointer passed as argument 2, which is declared to never be null
[01:29:45.807] /usr/include/string.h:44:28: note: nonnull attribute specified here
[01:29:45.807] #0 0x5b2e9c933a3f in zeek::detail::SerializationFormat::WriteData(void const*, unsigned long) /zeek/build/src/../../src/SerializationFormat.cc:80:5
[01:29:45.807] #1 0x5b2e9c935184 in zeek::detail::BinarySerializationFormat::Write(char const*, int, char const*) /zeek/build/src/../../src/SerializationFormat.cc:371:40
Continue to allocate the empty string for now as a fix.
* 'master' of https://github.com/blightzero/zeek:
Changed behavior of var-extraction-uri.zeek from policy/protocol/http to extract only the URI parameter names. Do not include the path in the first parameter name. Only extract uri vars if parameters actually exist.
Previously, Zeek treated the receipt of `AuthenticationOk` as a
successful login. However, according to the PostgreSQL
Frontend/Backend Protocol, the startup phase is not complete until
the server sends `ReadyForQuery`. It is still possible for the server
to emit an `ErrorResponse` (e.g. ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION)
after `AuthenticationOk` but before `ReadyForQuery`.
This change updates the PostgreSQL analyzer to defer reporting login
success until `ReadyForQuery` is observed. This prevents false
positives in cases where authentication succeeds but session startup
fails.
* origin/topic/awelzel/4754-double-wrapped-broker-data-records:
cluster/serializer/broker: Do not special case Broker::Data anymore
broker/Data: Support unwrapping Broker::Data records
The previous approach ignored the fact that nested / inner values might
also be Broker::Data values. I'm not super sure about the validity of
the test, because it's essentially demonstrating any-nesting, but
it's not leading to extra Broker::Data encoding.
Calling val_to_data() on a Broker::Data ends up wrapping the
Broker::Data record instead of using the contained broker::value
directly.
Seems this should be the default behavior and wonder if the flag
even makes sense, but for a 8.0 backport that seems more reasonable.
* origin/topic/timw/move-submodules-to-main-repo:
Fix some clang-tidy findings in generated BIF code
Fix clang-tidy and pre-commit warnings for gen-zam code files
Move gen-zam code into the main Zeek repository
Move zeek-af_packet-plugin code into the main Zeek repository
Move the bifcl code into the main Zeek repository
Fix clang-tidy findings in the binpac lib code
Add copyright headers to all of the binpac source files
Move binpac code into the main Zeek repository
Parameters relied on is_record for a couple of validations, but they are
not records and should not be treated as such. This way we can validate
&optional better.
There was some confusing behavior with &optional and locals, so this
should get rid of that by making it an error. However, there is a case
where function parameters are still allowed to have &optional - this is
because there are checks for &default in parameters as well.
* origin/topic/awelzel/docker-trixie:
ci: Run zeekctl and builtin tasks with Debian 13, too
ci: Prepend timestamps to output
ci: Enable Spicy for arm_debian13
ci: Add Debian 13.0 (trixie)
docker: Bump to debian:trixie-slim
* origin/topic/timw/limit-string-and-container-lengths-in-logs:
Add NEWS entry for field length limiting
Tag truncated values with a flag, plus pack threading::Value better
Remove length limiting on string fields for HTTP
Make total_size counter a member in logging::Manager
Remove using numeric_limits and just check for zero instead
Expand the size of the log-size filters for x509
Add options to filter at the stream level as well as globally
Add a weird that gets emitted when strings/containers are over the limits
Add metrics to track string and container fields limited by length
Replace unused stream argument from RecordToLogRecord with WriterInfo
Implement string- and container-length filtering at the log record level
* origin/topic/timw/remove-8.1-deprecations:
Pass DNS complete_flag along as a uint8_t instead of a String
Update docs submodule with 8.1 deprecation removals
Update zeekjs submodule with 8.1 deprecation fixes
Remove deprecations tagged for v8.1
A user provided a SMB2 pcap with the reserved1 field of a ReadResponse
set to 1 instead of 0. This confused the padding computation due to
including this byte into the offset. Properly split data_offset and
reserved1 into individual byte fields.
Closes#4730
* origin/topic/timw/cleanup-warnings-from-plugin-btest-builds:
Update zeek-aux to remove BRO_DIST from plugin skeleton
cmake_minimum_required() should come before project()
* origin/topic/awelzel/tap-analyzer-take-four-thanks-clang-tidy:
btest/tap-analyzer: Update existing test and add new one for UpdateConnVal()
SessionAdapter: Keep tap_analyzers until destruction
tcp,udp,icmp adapters: Move TapPacket() to earlier
tcp,udp,icmp adapters: Fix UpdateConnVal() superclass call
This also changes the output of connection UIDs from the tap analyzer to be
prefixed with C for easier correlation with other logs.
Relates to #4337#4725#4734#4737
connection_state_remove() is invoked after Done(), so it's not a good
idea to remove the tap analyzers before in case they have up-to-date
information for the connection val.
Relates to #4337#4725#4734#4737
Writing a test, the packet was tapped after protocol analysis at least
for TCP. Ensure tapping happens before. The adapter->Process() moving
after pkt->session made me a bit wondering if things are underspecified
here, but seems reasonable to set the session on pkt before adapter->Process().
Relates to #4337#4725#4734#4737
Now that SessionAdapter implements UpdateConnVal(), the individual
adapters need to call that instead of Analyzer::UpdateConnVal()
Thanks clang-tidy.
Relates to #4337#4725#4734#4737
This commit introduces a mechanism to attach light weight analyzers to
the root analyzer of sessions in order to tap into the packets delivered
to child analyzer.
* origin/topic/awelzel/defer-more-stuff:
RecordType: Ensure &default fields are always re-initialized
Attr: Deprecate using &default and &optional together on record fields
RecordType: Allow deferring &default=vector(), set(), table() fields
This moves c$service_violation to the deprecated-dpd-log policy script.
This is the only script in the distribution that uses the field, and it
is unlikely to be used externally. It is also responsible for a
significant amount of memory use by itself.
This also restores the field being populated, which was broken in
GH-4362
The conn protocol scripts now assume that new_connection is run before
connection_state_remove. Update the DNS analyzer to raise the
new_connection event.
This class is a parameter of virtual methods of the Backend API for users
to implement and also a parameter to the HookPublishEvent() API. Seems it
shouldn't be in detail and instead we should own it.
Alternatively, could mark the cluster APIs as not-stable-yet, but I
think we can move forward and make it non-detail for 8.0.
* origin/topic/awelzel/4431-zeromq-drop-policy-v2:
cluster.bif: Improve Cluster::publish() docstring
btest/cluster/zeromq: Add tests for overload behavior
cluster/zeromq: Metric for msg errors
cluster/zeromq: Drop events when overloaded
cluster/zeromq: Comments and move lookups to InitPostScript()
cluster/zeromq: Rework lambdas to member functions
cluster/zeromq: Support local XPUB/XSUB hwm and buf configurability
cluster/OnLoop: Support DontBlock and Force flags for queueing
cluster/ThreadedBackend: Injectable OnLoopProcess instance
The overload-drop.zeek and overload-no-drop.zeek tests have proxy,
worker-1 and worker-2 publish to the manager topic. For the drop
case, we verify that both, the senders, but also the manager drops
events. For the no-drop test, the HWMs are set such that all events
are buffered.
The overload-worker-proxy-topic*.zeek tests are similar, but instead
of publishing to the manager topic, proxy, worker-1 and worker-2 publish
to the proxy and worker topics to overload each other. This had
previously resulted in lockups and these tests verify that this doesn't
happen anymore.
When either the XPUB socket's hwm is reached, or the onloop queue is
full, drop the events. Users can set ths xpub_sndhwm and
onloop_queue_hwm to 0 to avoid these drops at the risk of unbounded
memory growth.
Now that Conn::set_conn is guaranteed to be called at the beginning and
at the end of the connection, we can skip re-setting the elements that
we know will not have changed. This prevents repeated lookups, e.g. to
check that addresses are in the local networks.
During `connection_state_remove`, only the duration, number of
packets, service, and history fields are updated.
local_orig and local_resp are updated when the connection is flipped. A
test was added for that purpose. It uses the already existing
http.zeek-image-post-1080-8000-x.pcap, which was slightly rewritten for
this, so that one side of the connection has IP addresses different from
127.0.0.1.
The existing history-flip test also was updated to have one side being
in a local-net, to check that the flipping of local_orig and local_resp
works correctly at the beginning of a connection.
c$conn is often needed for connection events, but it being established
in connection_state_removed can be a problem because event handlers have
to remember to call Con::set_conn().
This commit moves to call Conn::set_conn() in new_connection.
Addresses GH-4202
update logs
* origin/topic/timw/lazy-allocate-id-option-handlers:
Move ID::type in structure to fill memory padding
Use sorted forward_list instead of multimap for ID option change handlers
* origin/topic/timw/cpp20-modernization:
Remove intermediate cipher vectors in ssl-analyzer.pac
Reduce the size of Func::Body quite a bit with some extra packing
Switch to using std::ranges algorithms
Enable modernize-std-numbers clang-tidy checker, fix findings
Switch to using c++20 constraints instead of std::enable_if
Not sure it's the best place to put, but we don't have packet analysis stats
bif and also num_packets is already there, so seems reasonable to put the
num_packets_unprocessed into
* origin/topic/awelzel/conn-id-ctx-singleton:
RecordType: Allow field init deferral of deferrable record constructors
Conn: Use conn_id_ctx singleton
Conn: Add InitPostScript() and conn_id_ctx singleton
ID: Add conn_id_ctx
The ctx: conn_id_ctx &default=conn_id_ctx() field was not optimized
as deferrable even though by default its an empty record and so deferring
initialization seems safe. Open-code the special record constructor
expression case in ExprFieldInit so that the ctx field is not default
initialized at record construction anymore. I am wondering a bit if the
same applies to &default=set() and &default=table().
The EDNS rcode was incorrectly calculated. The extended rcode is formed
by taking the upper 8 bits of the extended rcode field, plus the lower 4
bits of the existing rcode.
This also adds a new trace with an extended rcode, and a testcase
parsing it.
Reported by dwhitemv25.
Fixes GH-4656
When a conn_id represents a ICMP "connection", we need to determine the
is_one_way flag for InitTuple() in order to skip any flipping of address
and ports for one-way ICMP connections.
Fixes#4645
* origin/topic/timw/coverity-fixes:
Fix a few other minor issues reported by Coverity
Add a few extra null checks, plus a missing initialization that led to a bad null check
Fix some integer overflow issues reported by Coverity
Ignore a couple of known-unused results reported by Coverity
Fix some bit-shifting overflow/UB issues reported by Coverity
Reset the value of a status variable in SQLite backend before using it in a loop
Fix a potential memory leak reported by Coverity
Avoid some string copies in IRC analyzer
Add some additional std::moves reported by Coverity
Fix an unsigned integer comparison reported by Coverity
Fix uninitialized class member Coverity findings
Handle uncaught exception during setup
Update gen-zam submodule for Coverity findings
* origin/topic/timw/storage-forced-sync:
Fix swapped storage metrics names
Add flag to force synchronous mode when calling storage script-land functions
* origin/topic/awelzel/control-switch-to-cluster:
NEWS: ZeekControl, ZeroMQ and WebSocket
Update zeekctl module for ClusterBackend and UseWebSocket
control: Use Cluster::publish() for replying
This adds a new PacketAnalyzer::PPPoE::session_id bif, which extracts
the PPPoE session ID from the current packet.
Furthermore, a new policy script is added which adds the pppoe session
id to the connection log.
Related to GH-4602
Switching to ZeroMQ as cluster backend and dabbling with zeekctl
and WebSocket, replies didn't arrive due to the usage of
Broker::publish() rather than Cluster::publish(). Additionally,
add the node name to the topic on which we reply so that the
receiver can figure out which node sent the reply. It could've
been a separate event parameter, but the topic appears just fine.
This allows injecting a custom onloop process to configure the
max_queue_size at instantiation time. Also allow access to the
instance directly and deprecate the QueueForProcessing() helper
* origin/topic/timw/storage-metrics:
Add SQLite page_count and file_size metrics
Add btests to cover storage metrics
Add storage metrics for operations, expirations, data transferred
Fix ordering of telemtry metrics when running under test
Make RunPragma take an optional value parser to return data
Make SQLite::Step take a callback function for parsing result data
The util:: versions of these methods remain as a thin wrapper around them so
they can be used with const char* arguments. Otherwise callers have to manually
make string_view objects from the input.
s Please enter the commit message for your changes. Lines starting
* origin/topic/robin/gh-4481-test-analyzer:
Spicy: Fix missing include.
Bump Spicy.
Spicy: Add functions to check if Zeek provides an analyzer of a given name.
Specifically, set a MIME part's parent_id to the rfc822_msg_fuid if it
is set and take into account the current rfc822_msg_fuid for describe_file()
to avoid fuid collisions of the top-level RFC822 message and the first
MIME part.
* origin/topic/bbannier/sqlite-no-stale-entries:
Make PUT on SQLite backend implicitly overwrite expired entries
Prevent SQLite storage backend from serving expired entries
The backend does not serve expired but still present entries so to a
user they do not exist. When they put new data over such an entry their
expecation is that the value is overwritten, even if not explicitly
requested.
The SQLite storage backend implements expiration by hand and garbage
collection is done in `DoExpire`. This previously relied exclusively on
gets not running within `Storage::expire_interval` of the put, otherwise
we would potentially serve expired entries.
With this patch we explictly check that entries are not expired before
serving them so that the SQLite backend should never serve expired
entries.
```
## Checks if there is a Zeek analyzer of a given name.
##
## analyzer: the Zeek-side name of the analyzer to check for
## if_enabled: if true, only checks for analyzers that are enabled
##
## Returns the type of the analyzer if it exists, or ``Undef`` if it does not.
public function has_analyzer(analyzer: string, if_enabled: bool = True): bool &cxxname="zeek::spicy::rt::has_analyzer";
## Differentiates between the types of analyzers Zeek provides.
public type AnalyzerType = enum { Protocol, File, Packet, };
## Returns the type of a Zeek analyzer of a given name.
##
## analyzer: the Zeek-side name of the analyzer to check
## if_enabled: if true, only checks for analyzers that are enabled
##
## Returns the type of the analyzer if it exists, or ``Undef`` if it does not.
public function analyzer_type(analyzer: string, if_enabled: bool = True): AnalyzerType &cxxname="zeek::spicy::rt::analyzer_type";
```
Closes#4481.
* origin/topic/timw/std-filesystem:
Fix some missing #includes resulting from removal of ghc::filesystem
Remove ghc::filesystem submodule, switch to std::filesystem
* origin/topic/timw/cpp20-clang-tidy-updates:
Use ranges::reverse_view to fix a few reverse ranged-for loops
Disable a few new modernize clang-tidy checkers, enabled by C++20
While we support initializing records via coercion from an expression
list, e.g.,
local x: X = [$x1=1, $x2=2];
this can sometimes obscure the code to readers, e.g., when assigning to
value declared and typed elsewhere. The language runtime has a similar
overhead since instead of just constructing a known type it needs to
check at runtime that the coercion from the expression list is valid;
this can be slower than just writing the readible code in the first
place, see #4559.
With this patch we use explicit construction, e.g.,
local x = X($x1=1, $x2=2);
* topic/timw/c++20:
Reduce some false-positive warnings from GCC to warnings with -Werror
Add zeek-namespaced alias for jthread
Include RequireCXXStd.cmake from main CMakeLists.txt
Update submodules to C++20-enabled versions
Apple Clang (as of version 17.0.0) does not support jthread unless
you enable experimental libcpp features by defining
_LIBCPP_ENABLE_EXPERIMENTAL or passing -fexperimental_library to
the compiler. Even if you don't do those, our 3rdparty jthread
library fails to build because of a bunch of namespace/naming
collisions. I moved our 3rdparty one to the `nonstd` namespace, and
am using an alias under __APPLE__ to avoid the collisions.
Zeek's analyzer API makes it hard to determine during analyzer
shutdown whether a regular end-of-data has been reached, or if we're
aborting in the middle of a session (e.g., because Zeek missed the
remaining packets): the corresponding analyzer method, `EndOfData()`
gets called in both cases.
In an earlier change, we had stopped signaling Spicy analyzers a
regular finish when that `EndOfData()` method executes, because doing
so could trigger a parse error if it wasn't a regular shutdown—-which
isn't desired, a user request was to just silently stop processing in
this case.
However, that behavior now seems unfortunate in the case that one
deliberately calls `zeek::protocol_handle_close()` to terminate an
analyzer: this feels like a regular shutdown that should just
immediately happen. We achieve this now in this function by
additionally signaling the shutdown at the TCP layer as an "end of
file", which, for Spicy analyzers, happens to run the final, orderly
tear-down.
Not exactly great, but ti seems to thread the needle to achieve the
desired semantics in both cases.
pre-commit ignores Cargo.lock files for Rust projects, so any movement
in a Rust project's dependencies can break a hook, even if no code in
the hook changed. I have tried to work with upstream on a fix, but they
basically told me they weren't interested and to get lost.
This bumps the `spicy-format` pre-commit hook to a new version which
explicitly deals with bumps of its dependencies. Having to do this
semi-regularly is not fun, and ideally somebody interested in using this
hook would help set up infrastructure in the hook so it just pulls
pre-built binaries. This is not directly supported by pre-commit, but
many projects work around this by declaring a Python module which then
pulls pre-build binaries which already exist for spicy-format.
This changes the PPPoE parser so that it doesn't forward extra bytes
that might be appended after the payload. Instead, it raises a weird if
the payload size doesn't match the size indicated by the header.
This is in line with what other protocol parsers (like UDP) are doing.
Two tests needed to be updated - with this change, the traffic in
pppoe-over-qinq.pcap is now valid TLS. A new trace was introduced for
the confirmation-violation-info test.
Addresses GH-4602
* origin/topic/vern/line-number-ordering:
Bump ZeekJS to work with new Location constructor
remove non-functional column information from Location objects
isolate Location specifics to private class variables to enforce correct line number ordering
The .rst generation doesn't escape the trailing `_` and the docs build
gets upset due to using `type` as a reference target then.
For the better or worse, revert to using tpe. Though I acknowledge this
means we need to be careful with trailing underscores because our docs
build is so fragile.
Partly reverts b9eabbabba.
* origin/topic/awelzel/4605-conn-id-context:
NEWS: Adapt for conn_id$ctx introduction
conn_key/fivetuple: Drop support for non conn_id records
Conn: Move conn_id init and flip to IPBasedConnKey
IPBasedConnKey: Add GetTransportProto() helper
input/Manager: Ignore empty record types
external: Bump commit hashes for external suites
ip/vlan_fivetuple: Populate nested conn_id_context, not conn_id
ConnKey: Extend DoPopulateConnIdVal() with ctx
btest: Update tests and baselines after adding ctx to conn_id
init-bare: Add conn_id_ctx to conn_id
Previously, we supported any records that happened to have orig_h,
resp_h, etc. fields, but it's not exactly clear why we ever did. Users
that relied on this can instantiate an explicit conn_id instance, too.
This loosens the coupling of the script-layer conn_id record and
the code in Conn a bit, moving more into the IPBasedConnKey class.
I'm not quite sure whether moving the flipping logic is worth it,
but assuming Conn could become non-IP in the future, it might.
Somewhere record types with zero fields get the optional attribute
apparently. The input/sqlite/basic test failed due to complaining
that ctx is optional. It isn't optional and when it has zero fields
we can just ignore it, too.
Also adds a input framework test with an explicit empty record type
get_file_handle() may include c$id and perturbs their values when adding new
fields. I think that's reasonable, as files transferred in one VLAN should
be treated separate from files transferred in a different VLAN.
This prepares the move where ConnKey implementations should fill out
ctx rather than filling conn_id directly. The API continues to receive
both, conn_id and ctx, as adding fields to `conn_id` is reasonable
use-case even if it's just for logging purposes.
This nested record can be used to discriminate orig_h or resp_h being
observed in different "contexts". A context can be based on VLAN tags,
but any custom ConnKey implementation should populate the ctx field,
allowing to write context-aware Zeek scripts without needing to know
what the context really is.
Changes in endpoint stats are a side-effect caused by the ConnSize
analyzer updating the conn record triggering the threshold event. The
phenomenon is described in https://github.com/zeek/zeek/issues/4214.
* origin/topic/johanna/ssl-small-tweaks:
Spicy SSL analyzer: move unknown version out of possible range
Spicy SSL analyzer: move exceptions to &requires & throw
* origin/topic/etyp/redis-resp3:
Touchup TODOs in the Redis analyzer
Handle more Redis RESP3 protocol pieces
Stringify all Redis-RESP serialized data
Handle Redis protocol `message` separately
Add Redis analyzer array stringification
Closes#4504
Messages are not typical responses, so they need special handling. This
is different between RESP2 and 3, so this is the first instance where
the script layer needs to tell the difference.
* origin/topic/bbannier/readability-isolate-declaration:
Make clang-tidy warnings report as errors to cause CI build to fail
Fix a clang-tidy finding in cluster telemetry code
Fix clang-tidy readability-isolate-declaration warnings
I missed one of these in review so a machine is probably better at
catching them.
I fixed the existing instances which where largely in code which look
dated. Where possible I slightly reorganized the code so we do not have
to leave values uninitialized, but did not touch up anything else.
A bit ad-hoc formatting for the log, but that's mostly because cluster.log
only has message field and I don't think having a dedicated application_name
column is worth it. That could also be added by custom scripts if it's really
wanted for a given deployment.
* origin/topic/awelzel/cluster-telemetry-follow-up:
Bump cluster test suite
cluster/Telemetry: Cache CallExpr locations
cluster/Telemetry: Avoid unneeded StringVal() construction
Val: Switch TablePatternMatcher to std::string_view
RE: Add MatchAll() and MatchSet() for std::string_view
cluster/websocket: Fix and test for invalid X-Application-Name
cluster/telemetry: Move topic_normalization redef to zeromq
This moves the negotiated version outside of the possible range of
protocol values. I don't think it was possible to cause a problem with
this in the past - but this approach seems safer.
The Spicy based SSL analyzer was, so far, more permissive with the
record layer versions that it would accept.
This change brings the parsing of record layer versions in line with the
binpac based analyzer. This behavioral difference was discovered due to
a test that changed with the recent dpd log changes.
* origin/topic/awelzel/1474-cluster-telemetry:
btest/cluster/telemetry: Add smoke testing for telemetry
cluster/WebSocket: Fetch X-Application-Name header as app label
cluster/WebSocket: Pass X-Application-Name to dispatcher
broker/WebSocketShim: Add calls to Telemetry hooks
cluster/WebSocket: Configure telemetry for WebSocket backends
broker: Hook up generic cluster telemetry
cluster: Introduce telemetry component
One bug fix removing static from a variable that shouldn't be static.
* origin/topic/christian/extensible-conntuples:
btest/plugins: Add test for custom ConnKey factory
NEWS updates for pluggable connection tuples.
Add a VLAN-aware flow tuple implementation.
Deprecate ConnTuple and related APIs.
Deprecate the old Connection constructor and detail::ConnKey class.
Switch to virtualized use of new zeek::ConnKey class tree
Provide a connkey factory for Zeek's default five-tuples.
Add IP-specific ConnKey implementation.
Establish plugin infrastructure for ConnKey factories.
Add new ConnKey abstraction.
Given IP-aware ConnKeys, ConnTuples aren't really required any more. ConnTuple
had two benefits:
- It preserved the original src/dst orientation from the packet headers it was
based on, which IPBasedConnKey now tracks and provides accessor methods for.
- In IPBasedAnalyzer::AnalyzePacket() its instance survived past the std:move()
of the key into NewConn(), which we sidestep by keeping the original src address
and port around until we need after the connection is obtained.
Since the base factory is pure virtual this is now the first full
implementation, but still a bit of a special case because it implements Zeek's
default behavior and doesn't add "custom" content to the tuple.
Using a label named "endpoint" is not intuitive and requires explaining to
users that it's really just the Cluster::node value. Change the label to
"node", so that we don't need to do the explaining.
This probably breaks some existing users of the Prometheus metrics, but after
looking more at metrics recently, "endpoint" really is a thorn in my eye.
This commit fixes the parsing of the data field in the SSL analyzer. So
far, this field contained two extra bytes at the beginning, which
contain the length of the following data.
Now, the data passed to the event only contains the actual value of the
session ticket.
The Spicy analyzer already contains the correct handling of this field,
and does not need to be updated. A test that uses the event and
exhibited the bug was added.
* origin/topic/awelzel/4586-zeromq-ipv6:
cluster/zeromq: Short-circuit DoPublishLogWrite() when not initialized
cluster/zeromq: Hook up and enable IPV6 by default
cluster/zeromq/connect: Make failures fatal
cluster/zeromq: Move log_push creation to DoInit()
After moving the log_push initialization from the constructor to the
DoInit() method, it's now possible that DoPublishLogWrites() is invoked
even if DoInit() was never called. Handle this by short-circuiting. This
is sort of an error, but can happen during tests if scripts are loaded
somewhat arbitrarily.
ZeroMQ's IPv6 support isn't enabled by default, resulting in
"No such device" errors when attempting to listen on an IPv6
address. This change adds a ipv6 option to the ZeroMQ module
and enables it by default. Further, adds a test configuring
everything to listen on IPv6 ::1 as well, and one test to provoke
the original error. This also regularizes some error messages.
The addr_to_uri() calls weren't actually needed, but they apparently do
not hurt and the result is easier on the eyes, so use them :-)
* origin/topic/johanna/default-canonifier-only-first-timestamp:
Default canonifier change to only remove first timestamp in line
Align SMB timestamp calculation between operating systems
This one is weird. This checker complains because we tend to use PList,
which sets the type of T to a pointer. The checker is making note that
we're doing sizeof() on a pointer, which always returns the same value
every time. It's asking whether we meant to do that, or if we meant to
pass the actual type to sizeof.
The schema of cluster WebSocket error messages deviated from the
existing one in Broker which breaks seamless migration from the Broker
WebSocket bindings.
This patch aligns the serialization in cluster with the one in Broker.
This is technically a breaking change of the cluster schema, but since
it never worked like documented and is still experimental this is
probably fine.
Closes#4594.
The cluster is borked if the initialization fails, so may as well just
completely abort Zeek at that point with a fatal error. There's no real
point in continuing to run.
In the past, we used a default canonifier, which removes everything that
looks like a timestamp from log files. The goal of this is to prevent
logs from changing, e.g., due to local system times ending up in log
files.
This, however, also has the side-effect of removing information that is
parsed from protocols which probably should be part of our tests.
There is at least one test (1999 certificates) where the entire test
output was essentially removed by the canonifier.
GH-4521 was similarly masked by this.
This commit changes the default canonifier, so that only the first
timestamp in a line is removed. This should skip timestamps that are
likely to change while keeping timestamps that are parsed
from protocol information.
A pass has been made over the tests, with some additional adjustments
for cases which require the old canonifier.
There are some cases in which we probably could go further and not
remove timestamps at all - that, however, seems like a follow-up
project.
The way that SMB timestamps were calculated used a "long double" for
the conversion calculation.
OS-X and Linux seem to have different sizes for long doubles. On the
Linux distributions that I have available, a long double is 128bits, vs
64 bits in OS-X.
This leads to slight discrepancies in the timestamps. This commit
changes this calculation to use a double on both systems, which is 64
bits and eliminates this difference.
Not the not_before/not_after fields output GMT based times.
Also adds a new btest diff canonifier which only removes the first
timestamp in a line.
Fixes GH-4521
The Spicy SSL analyzer was using references pretty heavily, probably to
work around now fixed issues with `inout` parameters in older Spicy
versions. At least for units this seems not needed anymore, and was also
partially incorrect, e.g., the Spicy docs call out that when using
`inout` parameters, passed and expected types should match exactly so
passing a reference as an `inout` value seems incorrect. Additionally,
one use case for references in Spicy is to use their interior
mutability, i.e., a reference never needs to be passed `inout` since
their can always be mutated.
Internally units are stored as reference-counted values, and references
to units are not much cheaper since they also need to be refcounted.
With that there seems litle reason to use references in this analyzer at
all, and this patch drops their use completely; instead we either pass
values, or values declared `inout`.
We leave the use of references for sharing sinks in place.
When looking up the postprocessor function from shadow files, id::find_func()
would abort() if the function wasn't available instead of falling back
to the default postprocessor.
Fix by using id::find() and checking the type explicitly and also adding a
strict type check while at it.
This issue was tickled by loading the json-streaming-logs package,
Zeek creating shadow files containing its custom postprocessor function,
then restarting Zeek without the package loaded.
Closes#4562
* origin/topic/timw/util-types:
Remove some unused #includes from spicy code
Remove using util.h in various headers in favor of util-types.h
Move type definitions/aliases from util.h to a separate file
After a public discussion and also chatting with Vern directly, disable the
ZAM bif tracking test to avoid an update every time new functions are
added. Usually these aren't performance critical and the defaults
characterization is fine. If they are performance critical, then Vern
is currently best positioned to properly integrate an optimized version.
* topic/christian/btest-trace-cleanup:
Btests: don't use -C in Zeek invocations that don't actually need it
Remove executable file permission bits from a bunch of our pcaps
The response to BDAT LAST was never recognized, resulting in the
BDAT LAST commands not being logged in a timely fashion and receiving
the wrong status.
This likely doesn't handle complex pipeline scenarios, but it fixes
the wrong behavior for smtp_reply() not handling simple BDAT commands
responses.
Thanks @cccs-jsjm for the report!
Closes#4522
Add `net-tools` and `procps` to the final.Dockerfile, to avoid warning in
zeekctl commands like: "failed to find local IP addresses [...]" and the
error in `zeekctl top` command caused respectively by missing `ifconfig`
and `top` commands.
Nobody generates docs at startup while then moving on to regular Zeek operation,
and the generated runtime output when loading scripts/zeekygen is both noisy and
confusing.
* origin/topic/timw/storage-expire-contention:
Add busy_timeout script-level option, override any busy_timeout pragma
Handle potential contention when running sqlite expiration
Add expiration to sqlite-cluster.btest
Use unique_ptr to avoid needing to call sqlite3_reset manually
Move Deferred class from ZeroMQ to util
* origin/topic/johanna/new-style-analyzer-log:
NEWS entries for analyzer log changes
Move detect-protocol from frameworks/dpd to frameworks/analyzer
Introduce new c$failed_analyzers field
Settle on analyzer.log for the dpd.log replacement
dpd->analyzer.log change - rename files
Analyzer failure logging: tweaks and test fixes
Introduce analyzer-failed.log, as a replacement for dpd.log
Rename analyzer.log to analyzer.debug log; move to policy
Move dpd.log to policy script
Currently, coverage/bare-mode-errors is one of the slowest tests in the
entire test suite. This is caused by the fact that it has to repeatedly
launch Zeek for every script that we ship. This is done sequentially.
This commit changes this test to use xargs to spawn 20 parallell
processes.
detect-protocol.zeek was the last non-deprecated script left in
policy/frameworks/dpd. It was moved to policy/frameworks/analyzer. A
script that loads the script from the new location with a deprecation
warning was added.
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.
* origin/topic/timw/4350-redis-passwords:
Redis: Add support for sending AUTH commands during connection
Redis: disconnect cleanly if INFO request fails
Fix segfault if storage sync open_backend returns bad code
Add ToStdString and ToStdStringView to ZeekString
To address review feedback in GH-4362: rename analyzer-failed-log.zeek
to loggig.zeek, analyzer-debug-log.zeek to debug-logging.zeek and
dpd-log.zeek to deprecated-dpd-log.zeek.
Includes respective test, NEWS, etc updates.
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.
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.
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.
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.
Something is off with the libhiredis package that's installed
as part of the current image, and 'zypper patch' is failing to
update it during the prepare step. Force a rebuild so the
updated package is part of the image to start with.
* origin/topic/awelzel/4177-4178-custom-event-metadata-part-2:
Event: Bail on add_missing_remote_network_timestamp without add_network_timestamp
btest/plugin: Test custom metadata publish
NEWS: Add note about generic event metadata
cluster: Remove deprecated Event constructor
cluster: Remove some explicit timestamp handling
broker/Manager: Fetch and forward all metadata from events
Event/init-bare: Add add_missing_remote_network_timestamp logic
cluster/Backend/DoProcessEvent: Use generic metadata, not just timestamps
cluster/Event: Support moving args and metadata from event
cluster/serializer/broker: Support generic metadata
cluster/Event: Generic metadata support
Event: Use -1.0 for undefined/unset timestamps
cluster: Use shorter obj_desc versions
Desc: Add obj_desc() / obj_desc_short() overloads for IntrusivePtr
Also use the generic metadata version for publishing, keep the
ts-based API for now, but only add timestamps when
EventMetadata::add_network_timestamp is T. I'm not sure what the
right way forward here is, maybe deprecating Broker's publish event
variations and funneling through cluster.
Instead of a timestamp attribute, switch to holding a EventMetadataVectorPtr
like zeek::Event instances do. Keep the old constructor until the end of
the patch series.
This can happen if either there's no network timestamp associated with
an event, or there's currently no event being dispatched. Using 0.0
isn't great as it's the normal start timestamp before reading a network
packet. Using -1.0 gives the caller a chance to check and realize what's
going on.
* origin/topic/vern/CPP-maint.May25:
minor BTest maintenance updates for -O gen-C++
fix for more robustly finding BTests to assess for -O gen-C++
fix for -O gen-C++ dealing with type constants of unnamed compound types
This change adds two new hooks to the Intel framework that can be used
to intercept added and removed indicators and their type.
These hooks are fairly low-level. One immediate use-case is to count the
number of indicators loaded per Intel::Type and enable and disable the
corresponding event groups of the intel/seen scripts.
I attempted to gauge the overhead and while it's definitely there, loading
a file with ~500k DOMAIN entries takes somewhere around ~0.5 seconds hooks
when populated via the min_data_store store mechanism. While that
doesn't sound great, it actually takes the manager on my system 2.5
seconds to serialize and Cluster::publish() the min_data_store alone
and its doing that serially for every active worker. Mostly to say that
the bigger overhead in that area on the manager doing redundant work
per worker.
Co-authored-by: Mohan Dhawan <mohan@corelight.com>
* origin/topic/vern/ZAM-maint.May25:
fix for crash when interpreting transformed ASTs that include multi-field record assignments/additions
Remove unused ZAM compiler method
* origin/topic/timw/ci-clang-tidy:
CI: Add new task to run clang-tidy as part of nightly builds
CI: Update to clang 19 on ubuntu 24.04, add clang-tidy package
This only changes the script-layer API, but keeps the std::string host
in the C++ layer's ServerOptions. Mostly because the ixwebsocket library
takes host as std::string. Also, maybe at some point we'd want to
support something scheme-based like unix:///var/run/zeek.sock and placing
that in a string could not be totally wrong.
Add tests for IPV6, too.
* origin/topic/timw/coverity:
Add a few std::moves to fix Coverity warnings
Fix possible memory leak in broker::Manager
fixup! Use SCN* constants when reading into sized integer types
from_json: Statically lookup err index in result type
Use SCN* constants when reading into sized integer types
Remove setting unused variable value in FTP::parse_eftp
This one renames the Modbus Exception binpac type to ExcResponse. clang-tidy
insisted that anything named started with Exception was actually an exception
and needed to be thrown.
One instance of this method is noexcept and one isn't. The version
that is noexcept uses std::vector::emplace_back, which may throw
exceptions. Instead of adding a try/catch block, opt for just making
the two functions able to throw exceptions.
This fixes a clang-tidy bugprone-exception-escape warning.
* origin/topic/etyp/redis-analyzer:
spicy-redis: Add NEWS entry
spicy-redis: Separate error replies from success
spicy-redis: Cleanup scripts and tests
spciy-redis: Bring Redis analyzer into Zeek proper
spicy-redis: Abort parsing if server data comes first
spicy-redis: Add recursion depth to server data
spicy-redis: Make client data only accept bulk strings
spicy-redis: Add dpd signature and clean pcaps
spicy-redis: Add some commands and touch up parsing
spicy-redis: Add some script logic for logging
spicy-redis: Separate client/server
spicy-redis: Touchup logging and Spicy issues
spicy-redis: Add synchronization and pipeline support
spicy-redis: Begin Spicy Redis analyzer
- Recomputes checksums for pcaps to keep clean
- Removes some tests that had big pcaps or weren't necessary
- Cleans up scripting names and minor points
- Comments out Spicy code that causes a build failure now with a TODO to
uncomment it
Redis seems to only want client data first to request server data. The
DPD signature seems to pick up on some cases where server data comes
first, but is otherwise "valid" RESP. See if this helps lower FP rates.
* origin/topic/awelzel/fix-no-zero-timestamp-metadata:
btest: Add test for Cluster::hello zero-timestamp
EventMgr/Enqueue: Add automatic timestamp metadata to local events, only
cluster and broker: Propagate zero-timestamp as metadata, too.
It seems less surprising if only local events receive automatic network
timestamp metadata. For remote events the automatic value will most
likely be misleading.
This will be cleaned up later to just pass all contained metadata from
a cluster event to the queued event, but for now do this here, otherwise
we break some internal tests.
* origin/topic/awelzel/4177-4178-custom-event-metadata-part-1:
Event: Move meta after args
Event: Use IntrusivePtr to manage obj refcount
btest/zam: Update for new EventMetadata bifs
broker and cluster: Switch to new Enqueue() API
Event/zeek.bif: Add EventMetadata current() and current_values() accessors
Event: Deprecate default network timestamp metadata
Event: Store timestamp in metadata vector
EventRegistry/zeek.bif/init-bare: Add event metadata infrastructure
EventMgr: Add CurrentEvent() accessor
This deprecates the Event constructor and the ``ts`` parameter of Enqueue()
Instead, versions are introduced that take a detail::MetadataVectorPtr which
can hold the network timestamp metadata and is meant to be allocated by the
caller instead of automatically during Enqueue() or within the Event
constructor.
This also introduces a BifConst ``EventMetadata::add_network_timestamp`` to
opt-in adding network timestamps to events globally. It's disabled by
default as there are not a lot of known use cases that need this.
This removes the ts attribute from Event and instead allocates a vector for
storing metadata. By default, adds the network time as a TimeVal. Later
patches will make the allocation of the vector optional by introducing a
different constructor so that users that are not interested in network
timestamp metadata do not take the allocation hit.
Moving the explicit ``ts`` out of the event is done in order to treat it
just as generic metadata, too. However, the Time() accessor is adapted to
lookup the value from the metadata vector instead.
* origin/topic/timw/sqlite-cluster-test-followup:
SQLite: Add TODO note about possibly using sqlite3_busy_timeout
SQLite: Fix typo in variable name causing pragmas not to retry on busy
SQLite: Use tableval iteration instead of ToMap for pragmas
SQLite: Fix logging/error messages around executing pragmas
* origin/topic/timw/sqlite-cluster-test:
SQLite: Move integrity_check to pragma table
SQLite: Add backend option for pragma timeout
SQLite: Rename tuning_params to pragma_commands, move running pragmas to utility method
SQLite: Retry pragma statements at startup to avoid contention
SQLite: Check for locked database as well as busy databases
SQLite: Fix some string-sizing issues
SQLite: Run pragmas on connection before creating table
SQLite: Add busy_timeout pragma to default options
Prefix sqlite-based btests with sqlite- to match redis tests
Add sqlite cluster storage btest
* origin/topic/timw/3913-parse-port-invalid-read:
Add extra input files to ftp fuzzer corpus
Use bool instead of int flag in FTP analyzer's parse_eftp method
Fix undefined behavior in FTP analyzer's parse_port method
Fix invalid-read in FTP analyzer's parse_port method
* origin/topic/awelzel/btest-cluster-layout-replacement:
btest remaining: Use generic cluster-layout.zeek
btest/frameworks/logging: Use generic cluster-layout.zeek
btest/files/x509: Use generic cluster-layout.zeek
btest/frameworks/notice: Use generic cluster-layout.zeek
btest/policy: Use generic cluster-layout.zeek
btest/frameworks/intel: Use generic cluster-layout.zeek
btest/frameworks/sumstats: Use generic cluster-layout.zeek
btest/frameworks/cluster: Use generic cluster-layout.zeek
btest/frameworks/config: Use generic cluster-layout.zeek
btest/broker: Use generic cluster-layout.zeek
btest/cluster: Use generic cluster-layout.zeek
testing/btest/Files: Add generic broker/cluster-layout.zeek
This is a cluster-layout.zeek template that can be copied into a testing
directory if needed. The idea is that a developer sets environment
variables within their btest and the Cluster::nodes variable is
implicitly extended by appropriate nodes.
For example, using @TEST-PORT BROKER_LOGGER1_PORT will add an appropriate
logger-1 node to Cluster::nodes, based on the existence of the
BROKER_LOGGER1_PORT environment variable.
* origin/topic/vern/http-sqli-replacement:
site/local: Switch to detect-sql-injection
Add a revised script for detecting HTTP SQL injection, deprecate original
New test triggered the following error:
runtime error: downcast of address 0x57021a323ea0 which does not point to an object of type 'const FileVal' 0x57021a323ea0: note: object is of type 'zeek::FuncVal'
* 'smoot-improve-from_json' of github.com:/stevesmoot/zeek:
update baseline for zam
Update src/zeek.bif
Change from_json to return an error rather than print it.
* origin/topic/timw/include-cleanup-part-2:
Remove unnecessary #includes in script_opt
Remove unnecessary #includes in telemetry and supervisor
Remove unnecessary #includes in cluster/broker/iosource/probabilistic/session
Remove unnecessary #includes in zeekygen and base plugin files
Remove unnecessary #includes in input/logging/threading
Remove unnecessary #includes in analyzer/packet analyzer/file analyzer source files
Remove unnecessary #includes in analyzer/packet analyzer/file analyzer headers
Remove unnecessary #includes in base files in repo
Coverity complains about a missing fclose() in a non-existing
destructor. Also sprinkle in a strerror() call for fopen() to
provide a bit of a hint what might have gone wrong.
* origin/topic/timw/include-cleanup-part-1:
Remove including <cinttypes> from util.h
Remove telemetry #includes from OpaqueVal.h
Reduce includes in plugin/Component.h
Remove zeek/Stats.h include from NetVar.h
Include StmtBase/StmtEnums in Func.h instead of Stmt.h
Use modern names for standard headers
Remove fix for CentOS 7 from TCP_Flags.h
Fix usage of std::string in http analyzer
Reorder top section of net_util.h to batch includes together
Use quotes instead of <> for zeek includes
Fix Obj.h include in IntrusivePtr.h to have full path
* origin/topic/awelzel/4440-websocket-pong-timeout:
btest/cluster/websocket: Update tests for new event signature
cluster/websocket: Propagate code and reason to websocket_client_lost()
cluster/websocket: Support configurable ping interval
IXWebSocket: Bump to improve O^2 rxbuf erase()
* origin/topic/bbannier/fix-spicy-main:
Fix clang-tidy `bugprone-inc-dec-in-conditions` report in Spicy plugins glue compiler
Fix clang-tidy `performance-enum-size` reports in Spicy plugin's glue compiler
Remove unneeded copies in Spicy plugin glue compiler
Adjust for renamed function flavor in Spicy development version
With zeek/spicy#2048 the flavor for functions was renamed from
`Standard` to `Function`. This patch adapts the code for that while
still allowing using earlier Spicy versions.
* origin/topic/awelzel/ci-fedora-42:
probabilistic/BitVector: Add include <cstdint>
Bump spicy to fix build with GCC 15.1
CI: Drop fedora-40
CI: Add fedora-42
When a node restarts or a peering between two nodes starts over for other
reasons, the internal tracking in the Broker manager resets its state (since
it's per-peering), and thus the message overflow counter. The script layer was
unaware of this, and threw errors when trying to reset the corresponding counter
metric down to zero at sync time.
We now track past buffer overflows via a separate epoch table, using Broker peer
ID comparisons to identify new peerings, and set the counter to the sum of past
and current overflows.
I considered just making this a gauge, but it seems more helpful to be able to
look at a counter to see whether any messages have ever been dropped over the
lifetime of the node process.
As an aside, this now also avoids repeatedly creating the labels vector,
re-using the same one for each metric.
Thanks to @pbcullen for identifying this one!
I believe there's a bug/usage issue in the websockets library
where during send(), EOF is detected and stored, but the receiving
thread is then discarding the last received frame. Avoid the bug
by replacing the close_socket() implementation of the websockets
library just for that test and leave detecting the EOF condition
to the receiving thread.
The terminate-while-queueing test added for #4428 failed spuriously
indicating that sometimes WebSocket clients receive code 1000 instead of 1001.
This happens if the ixwebsocket server is shutdown before the reply thread had a
chance to process queued close messages.
Fix by signaling and waiting for the dispatcher's reply thread to terminate
before returning from Terminate().
In GH-4422 it was pointed out that the protocols/conn/failed-service-logging.zeek
policy script only works when
`DPD::track_removed_services_in_connection=T` is set.
This was caused by a logic error in the script. This commit fixes this
logic error and introduces an additional test that checks that
failed-service-logging works even when the option is not set to true.
Terminate() is called when Zeek shuts down. If WebSocket client threads
were blocked in QueueForProcessing() due to reaching queue limits, these
previously would not exit QueueForProcessing() and instead block
indefinitely, resulting in the ixwebsocket library blocking and its
garbage collection thread running at 100%. Not great.
Closing the onloop instance will unblock the WebSocket client threads
for a timely shutdown.
Closes#4420
* origin/topic/timw/storage-check-redis-server-version:
Add commands to the static methods for the Redis implementation
Redis: Check server version when connecting
Ensure the client side also uses the initial destination connection ID
for decryption purposes instead of the one from the current long header
packet. PCAP from local WiFi hotspot.
Instead of sending the accumulated CRYPTO frames after processing an
INITIAL packet, add logic to determine the total length of the TLS
Client or Server Hello (by peeking into the first 4 byte). Once all
CRYPTO frames have arrived, flush the reassembled data to the TLS
analyzer at once.
The payload is already consumed within the InitialPacket unit. Consuming
it again resulted in UDP datagrams with multiple packets to ignore
the remaining packets in the same UDP datagram. The baseline changes
showing I being followed by a new H indicates that the INITIAL packet
was followed by a HANDSHAKE packet, but previously Zeek discarded
these.
* origin/topic/awelzel/event-publish-hook:
NEWS: Add HookPublishEvent() note
btest/plugin: Test for PublishEventHook()
broker and cluster: Wire up HookPublishEvent
plugin: Add HookPublishEvent hook
Vern didn't have ZeroMQ installed and the test was skipped for him.
Generally would recommend anyone working on core Zeek to install
libzmq-dev or the equivalent for their environment, but until it is a
real required dependency, loosen the requirements on the test.
* origin/topic/timw/update-ct-ca-lists:
External tests: add removed logs to CT list to prevent baseline changes
Update Mozilla CA list and CT list to NSS 3.110
Instead of simply removing holes from vectors or lists when converting
from Val to Broker format, error out as the receiver has no chance to
reconstruct where the hole might have been.
We could encode holes with broker::none, but this will put unnecessary
burden on language bindings and users due to the potential optionality.
Think a std::vector<uint64_t> that technically needs to be a
std::vector<std::optional<uint64_t>> to represent optional elements
properly.
Closes#3045
Coverity complains proc is set under a lock, but accessed in Process()
without a lock. Fix this by setting it in Close() also without locking.
The proc member should only ever be accessed my the main thread.
* origin/topic/timw/seven-two-news:
Updates for the various Broker changes
Add versions of bundled dependencies
Fix a few typos.
Additional user contributions for NEWS
NEWS addition for cluster backends
NEWS additions for 7.2
Reformat 7.2 NEWS entries for consistent line lengths
* topic/christian/broker-tuning:
Lower listen/connect retry intervals in Broker and the cluster framework to 1sec
Bump cluster testsuite
Switch Broker's default backpressure policy to drop_oldest, bump buffer sizes
Deprecate Broker::congestion_queue_size and stop using it internally
At every site where we've dug into backpressure disconnect findings, it has been
the case that the default values were too small. 8192, so 4x the old default,
suffices at every site to drown out premature disconnects.
With metrics now available for the send buffers regardless of backpressure
overflow policy, this also switches the default from "disconnect" to
"drop_oldest" (for both peers and websockets), meaning that peerings remain
untouched but the oldest queued message simply gets dropped when a new message
is enqueued. With this policy, the number of backpressure overflows is then
simply the count of discarded messages, something that users can tune to see
drop to zero in everyday use. Another benefit is that marginal overflows cause
less message loss than when an entire buffer's worth (plus potentially more
in-flight messages) gets thrown out with a disconnect.
Since a reorg in the Broker library (commit b04195183) that revamped flow
control and that we pulled in with Zeek 5.0, this setting hasn't done
anything. Broker's endpoint::make_subscriber() and
endpoint::make_status_subscriber() take a queue size argument (with a default
value) that simply gets dropped in the eventual subscriber::make() call. See:
b041951835 (diff-5c0d2baa7981caeb6a4080708ddca6ad929746d10c73d66598e46d7c2c03c8deL34-R178)
* topic/christian/broker-backpressure-metrics:
Add basic btest to verify that Broker peering telemetry is available.
Add cluster framework telemetry for Broker's send-buffer use
Add peer buffer update tracking to the Broker manager's event_observer
Rename the Broker manager's LoggerAdapter
Avoid race in the cluster/broker/publish-any btest
This hooks into Telemetry::sync() to update Broker-level metrics tracking the
peerings' send buffer state. We do this in the cluster framework so we can label
the resulting metrics with Zeek cluster node names, not Broker's endpoint IDs.
The ZeroMQ heuristic for "ready to publish" is to create an unique and
ephemeral subscription using the XSUB socket and observe it arrive on the
XPUB socket. At this point, visibility into other node's subscriptions
is provided.
Provide a mechanism to allow a cluster backend report when it is ready
for publish operations. This is primarily useful for ZeroMQ which has
sender-side filtering and is only really ready for publishing when it
has learned about subscriptions from other nodes.
* origin/topic/awelzel/comment-out-broker-websocket-shim-two-endpoint-tests:
broker/WebSocketShim/tests: Comment out two endpoint tests
broker/WebSocketShim/tests: Replace hard-coded timeout values with vars
This implements basic tracking of each peering's current fill level, the maximum
level over a recent time interval (via a new Broker::buffer_stats_reset_interval
tunable, defaulting to 1min), and the number of times a buffer overflows. For
the disconnect policy this is the number of depeerings, but for drop_newest and
drop_oldest it implies the number of messages lost.
This doesn't use "proper" telemetry metrics for a few reasons: this tracking is
Broker-specific, so we need to track each peering via endpoint_ids, while we
want the metrics to use Cluster node name labels, and the latter live in the
script layer. Using broker::endpoint_id directly as keys also means we rely on
their ability to hash in STL containers, which should be fast.
This does not track the buffer levels for Broker "clients" (as opposed to
"peers"), i.e. WebSockets, since we currently don't have a way to name these,
and we don't want to use ephemeral Broker IDs in their telemetry.
To make the stats accessible to the script layer the Broker manager (via a new
helper class that lives in the event_observer) maintains a TableVal mapping
Broker IDs to a new BrokerPeeringStats record. The table's members get updated
every time that table is requested. This minimizes new val instantiation and
allows the script layer to customize the BrokerPeeringStats record by redefing,
updating fields, etc. Since we can't use Zeek vals outside the main thread, this
requires some care so all table updates happen only in the Zeek-side table
updater, PeerBufferState::GetPeeringStatsTable().
On very busy machines the hardwired scheduling of the ping batches could move
around among the arriving pongs, causing baseline deviations. We now wait for
each batch to complete before triggering the next one.
The internal ZeroMQ thread would call QueueForProcessing() thereby
accessing the onloop member. As ThreadedBackend::DoTerminate() unsets it,
this was a) reported as a data race by TSAN and b) potentially caused
missed events that were still to be queued.
* origin/topic/awelzel/websocket-empty-subscriptions:
cluster/websocket: Short-circuit clients without subscriptions
cluster/websocket: Factor out active subscription handling
* origin/topic/timw/no-islower-before-toupper:
Statically lookup field offsets for connection values in UDP and ICMP analyzers
Skip calling islower before toupper
Ubuntu 20.04's default Python doesn't deal well with the type annotations
used in btest/Files/wstest.py. Given that it's about to be EOL, just remove it.
Limit the number WebSocket events queued from external clients to
dispatcher instances to produce back pressure to the clients if
Zeek's IO loop is overloaded.
Explicitly notify the internal thread about the shutdown via the
inproc socket pair. This ensures that the internal thread processes
all previous messages on the inproc socket before terminating.
This fixes the scenario where a backend is created, a few messages published
and then immediately terminated as can be done with WebSocket clients.
Previously, some of the messages published might have still been in the
inproc socket's queue and were simply discarded.
Adds the same test for Broker and ZeroMQ backends.
When Cluster::backend is configured with CLUSTER_BACKEND_BROKER, switch
WebSocketClients to CLUSTER_BACKEND_BROKER_WEBSOCKET_SHIM instead.
Instead of the special case, we could also add something to Backend
called NewWebSocketBackend(), but if it only affects broker, I think
the special case is okay for now.
This adds a cluster backend implementation using broker's hub primitive
to connect WebSocket clients with the local broker endpoint for pub/sub
functionality.
* topic/christian/gh4318-track-broker-peerings:
Use Broker peering directionality when re-peering after backpressure overflows
Expand Broker APIs to allow tracking directionality of peering establishment
* origin/topic/timw/test-cmake-krb5-fixes:
Use longer path when including krb5.h to match the cmake lookup
Update cmake submodule for krb5 fixes [nomail]
* topic/timw/clang-tidy-iwyu-for-all-targets:
Update src/3rdparty submodule to disable clang-format
Disable linting for files generated by bison
Make sure clang-tidy and iwyu are added to all targets
Clang warns about declared but unused parameters somewhere in the guts of
IXWebSocket (internal code, not its headers). We are not interested in
this or similar warnings since we do not control this code, so suppress
all warnings for this target.
* origin/topic/awelzel/4275-ldap-gss-spnego-auth-miss:
ldap: Clean up from code review
ldap: Add Sicily Authentication constants
ldap: Only switch into MS_KRB5 mode if responseToken exists
If the server doesn't include a responseToken within negTokenResp,
assume there won't be signing or sealing happening on the
connection. Don't switch into MS_KRB5 mode.
Closes#4275
It looks like there is no reason to keep sessions in the map beyond the
point where the session manager is considered done. This hopefully
simplifies the shutdown control flow a tiny bit.
* origin/topic/timw/storage-serialization:
Add STORAGE_ prefixes for backends and serializers
Add versioning to JSON serializer
Remove unnecessary includes in Val.h
Move byte_buffer types from cluster and storage into util
Remove unnecessary <array> and <memory> includes from util.h
Mark storage classes as final where appropriate
Add JSON storage serializer, use with existing backends/tests
Make ValFromJSON return zeek::expected instead of a variant
Ground work for pluggable storage serializers
fc005a5 Plugin: Drop unneeded zeekjs.bif.h
cbf737f Plugin: Remove unneeded zeek::Args copy
2129feb Plugin/Nodejs: Remove intermediate Event instance
5de78b4 Ignore the typescript test.
550a0f3 Force users to specify to Node that they want to run typescript.
7a44aad Add a test for typescript support.
cda2d69 Begin support for the Node's new experimental typescript support
* origin/topic/awelzel/generic-metadata-pre-work:
cluster/Backend: Add name and lookup component tag
cluster/Event: Hide members behind accessors
cluster/PublishEvent:: Make event non-const
broker/Manager: Re-use broker serializer for conversion
EventMgr: Add Dispatch() with handler and args
plugin/Manager: Fix MetaHookPre and MetaHookPost using HOOK_CALL_FUNCTION
For scheduled events, the event timestamp is the intended timestamp. If
we force timer expiration, the timestamp might be in the future. Today,
this happens on shutdown. This change guarantees that event timestamps
are never set beyond network time.
Allow users to call event_mgr.Dispatch(handler, args) instead of
constructing the Event instance themselves. Deprecate the old API
and replace users.
There's a subtle change that net_done() may be propagated via
auto_publish() now, but that still needs opt-in from script land
and likely no one did that, or else they'd expected to have it
work anyhow.
This test is very sensitive to new enum values that perturb the final
order of the output. Listing the commits that modify that file in the
past clearly shows changes to the baseline happen when a new component
with an enum value is introduced.
40f04d4ccf broker: Add WebSocketShim backend
51836d08ae protocol: Add StreamEvent analyzer
fdde1e9841 cluster/serializer: Add binary-serialization-format
6fb73aa9da broker: Add shim plugin adding a backend component
85ca59484b postgresql: Initial parser implementation
efc2681152 WebSocket: Introduce new analyzer and log
...
That's just distraction. Point fix the test to sort the analyzers before
writing the final log line.
I've previously attempted this in a more complete fashion in PR #3365
(issue #3364), but seems this direct fix might just make more sense.
This test has become flaky after #4295. It hasn't been failing regularly
enough locally to make it approachable. Annotate with
BTEST_BG_RUN_SLEEP=1 to revert to pre #4295 behavior, hopefully
fixing the flakiness as a side-effect.
After further testing with the fuzzer corpus, std::unreachable isn't
necessary here. It's fine to just let the default case break to fix
the Coverity warning.
* topic/timw/coverity-fixes:
Add move assignment operator to ZeekString, which already had a move constructor
Don't initialize telemetry listener at all if ZEEKCTL_CHECK_CONFIG is set
Avoid extra temporary strings in concatenation
Use const auto references to avoid some copies
Clean up initialization of TunnelEncapsulation to use default member values
Remove some unnecessary std::move calls in broker::Manager
Avoid divide-by-zero in CardinalityCounter::Size
Minor fixes in POP3 analyzer based on flycheck warnings
More std::move changes based on Coverity findings
Fix a potential memory leak in Debug function
* origin/topic/timw/4281-unconditionally-forward-out-of-snap:
Make SNAP analyzer use both OUI and protocol for forwarding
Change packet analyzer identifiers to be 64-bit
* origin/topic/awelzel/set-val-on-ids-for-enums:
broker/main: Adapt enum values to agree with comm.bif
btest/plugin: Add plugin testing enum identifiers
Var/add_type: Do not clone EnumType when declared for the first time
type/id/zeekygen: Add EnumVal to enum identifiers
Logic to detect this error already existed, but due to enum identifiers
not having a value set, it never triggered before.
Should probably backport this one.
EnumType receives the name into its constructor. Even for the first declaration
the name is not empty and instead the same as the identifier's name. Due to that,
add_type() previously took the else path and created a shallow clone of the
initial type instead of using it. This lead to buggy behavior where enum value
identifiers declared within an enum's first body have a different TypePtr
associated than the one that is found via `zeek:🆔:find_type()`. It also
means that enum identifiers added via redef later would have a different
TypePtr than those in the initial pointer.
Since commit 0a813a53c7,
zeek::detail::stmts is set to a nullptr when before it was usually
an empty list of statements. This caused traverse_all() to be
short-circuited unless global statements were available.
Fixes#4323
The logic was template-based so far, which wasn't great because: (1)
conceptually, it models the Spicy types at the wrong layer (C++ rather
than HILTI types), and (2) stopped working with some recent Spicy
updates (which we have temporarily reverted in the meantime to keep
Zeek working).
The new code is based on HILTI's runtime type information and the
corresponding introspection API, pretty much like `spicy-dump` works
as well. This is the recommended approach for working with HILTI
values, and generally much cleaner.
This is on top of https://github.com/zeek/zeek/pull/4300.
* origin/topic/timw/storage-timeout-testing:
Fix handling of timeout conditions from storage backends
Reformat plugin.storage btest to be more consistent with other storage tests
The ts parameter was only added to Call() for the Broker::auto_publish()
functionality and propagating the network timestamp. By now, the auto-publish
functionality is deprecated, so it'd be good to cleanup that signature. There
won't be any need for no_remote in the future either.
Allow users to just use Call() instead.
Avoid proliferation of various members on EventMgr by storing the
pointer of the current event instead.
This subtly changes the behavior of some builtin functions as they would
have returned the prior event's data when executed outside of event
draining (e.g. C++ level hook invocations), but I think that's actually
for the better.
Calling collect_metrics() from a script would not invoke metric
callbacks, resulting in most of the process metrics to be zero
when a Zeek process isn't scraped via Prometheus.
Fixes#4309
The busy polling wasn't clever and usually resulted in delays. For now,
switch to mutex/condition variable and log an error if the timeouts are
immense.
Due to prefix matching, worker-1's node_topic() also matched worker-10,
worker-11, etc. Suffix the node topic with a `.`. The original implementation
came from NATS, where subjects are separated by `.`.
Adapt nodeid_topic() for consistency.
WebSocket clients that connected with the wrong URL do not have
a backend attached. If a dispatcher is terminated while these
clients are still connected, a null deref would happen.
This was found while running all cluster/websocket tests in a loop
for a long time, tickling a segfault during the bad-url test.
These test were very sensible to the speed at which ZeroMQ distributes
subscriptions in the cluster and showed to be unreliably when testing with
zeek/btest#113.
The main fix here is to have individual WebSocket clients subscribe to unique
topics, e.g /test/client-0 and /test/client-1, instead of just a shared topic.
This ensures the WebSocket handshake completes only when they observed their
own subscriptions and not prematurely when observing the shared topic.
This seems mainly relevant for tests: In the real world one shouldn't
rely on subscription visibility - you miss messages if you're too late
to the party.
When two workers connect to zeek.cluster.worker, the central ZeroMQ
proxy would not propagate unsubscription information to other nodes
once they both left. Set ZMQ_XPUB_VERBOSER on the proxies XPUB socket
for visibility.
* origin/topic/timw/storage-framework-followup:
Redis: Handle other errors from requests, fix KEY_EXISTS for put operations
SQLite: handle existing keys when overwrite=F correctly
Remove unnecessary type aliases from storage btests
Avoid thread-leak in scripts.base.frameworks.file-analysis.bifs.enable-disable btest
Fix data-race with calling DBG_LOG from a separate thread
Fix data-race with ReturnCode objects in Sqlite::DoExpire
Fix data race with calling run_state::network_time from a separate thread
Add NEWS entry for Storage, reduce CHANGES spam
Fix Coverity findings in the SQLite backend
Remove inclusion of non-existent expected-lite path during dynamic plugin builds
Squash code from OperationResultCallback into ResultCallback
Add hiredis to generate-docs workflow to enable Redis backend
This btest uses the exit() BIF to shut down, which immediately calls
::exit() and kills Zeek without doing any shutdown. This will sometimes
leave the thread running the storage manager, which causes TSan to
complain about a thread leak. Switch to use the terminate() BIF instead
which cleanly shuts down all of Zeek.
Bump to the latest btest master version and set BTEST_BG_RUN_SLEEP
environment variable and to 0. This makes btest-bg-run return
immediately instead of delaying by 1 second.
* origin/topic/vern/CPP-maint.Mar25:
minor baseline updates for -O gen-C++
-O gen-C++ support for pattern-to-pattern comparison
-O gen-C++ support for pattern vector comparisons
-O gen-C++ support for type expressions
* origin/topic/johanna/ssh-server-banners-can-be-wild:
SSH analyzer - tiny aesthetic fixes
SSH: make banner parsing more robust
SSH: split banner into client/server parts
These functions reside in base/utils/addrs.zeek and were not tested. We don't
use has_valid_octets() anywhere right now, normalize_mac() is used in the RADIUS
analyzer, and addr_to_uri() is used in the signatures/dpd.zeek tests.
* origin/topic/timw/storage-framework: (52 commits)
Update docs submodule [nomail]
Cleanup/update comments across the storage C++ files
Split storage.bif file into events/sync/async, add more comments
Update comments in script files, run zeek-format on all of them
Allow sync methods to be called from when conditions, add related btest
Redis: Handle disconnection correctly via callback
Redis: Fix sync erase, add btest for it
Remove default argument for callbacks, reorder function arguments
Remove file-local expire_running variable
Pass network time down to Expire()
Add IN_PROGRESS return code, handle for async backends
Store sqlite3_stmts directly instead of looking up from a map
Reduce code duplication in storage.bif
Add OperationResult::MakeVal, use it to reduce some code duplication
Rearrange visibility of Backend methods, add DoPoll/DoExpire, add return comments
Implement Storage::backend_opened and Storage::backend_lost events
SQLite: expand expiration test
SQLite: Handle other return values from sqlite3_step
Redis: Fix thread-contention issues with Expire(), add more tests
Change how redis-server is run during btests, removing redis.conf
...
- New erase/overwrite tests
- Change existing sqlite-basic test to use async
- Test passing bad keys to validate backend type checking
- New test for compound keys and values
The number of args being passed to the put() methods was getting to be
fairly long, with more on the horizon. Changing to a record means simplifying
things a little bit.
This change revamps SSH banner parsing. The previous behavior was both
a bit too strict in some regards, and too permissive in other.
Specifically, clients are now required to send a line starting with
"SSH-" as the first line. This is in line with the RFC, as well with
observed behavior. This also prevents the creation of `ssh.log` for
non-SSH traffic on port 22.
For the server side, we now accept text before the SSH banner. This
previously led to a protocol violation but is allowed by the spec.
New tests are added to cover these cases.
* origin/topic/awelzel/4136-cluster-websocket-support:
ci/opensuse-tumpleweed: Bust cache
ci/macos/prepare: Install python@3 explicitly
cluster/websocket: Implement WebSocket server
cluster/websocket: Add IXWebsocket submodule
ci/alpine: Install openssl package for testing
ci: Install websockets from pip for all distros
auxil/libunistd: Bump for ssize_t typedef
auxil/broker: Bump to latest master version
cluster/zeromq: Catch log_push.send() exception
cluster/zeromq: Catch exceptions as const zmq::error_t&
cluster/zeromq: No assert on inproc handling
cluster/zeromq: Support configuring IO threads for proxy thread
cluster/zeromq: Move variable lookups from DoInit() to DoInitPostScript()
cluster/zeromq: Handle EINTR at shutdown
cluster/zeromq: Queue one message at a time
cluster/Backend: Queue a single message only
cluster/zeromq: Adapt for OnLoopProcess changes
cluster/ThreadedBackend: Switch to OnLoopProcess
cluster/OnLoop: Introduce helper template class
serializer/broker: Expose to_broker_event() and to_zeek_event()
Got a build failure because the old container images had python3-devel
for Python3.11 or something older, but then prepare.sh would install
Python 3.13 and Zeek's configure failing due to trying to find the
devel headers from python313-devel which wasn't installed by prepare.sh
It seems Homebrew's Python 3.12 doesn't install default symlinks or
python3 symlinks[1]. I believe this results in prepare.sh using the
system's Python rather than Homebrew's. Install python@3 explicitly
to put the symlinks in place.
[1] https://stackoverflow.com/a/77655631
Using file_state_remove() or file_sniff() would previously crash with the
spicy-zip file analyzer. We don't have spicy-zip here, so it's not a
proper reproducer, but still makes sense to test these events.
f5ff7c4 version: 0.15.0
2a48143 docker/fedora: Bust cache and ensure sqlite-libs is updated
05e5f57 Executor: Unblock SIGTERM in thread
46496b9 Instance: Remove unsued variable
c3b3c86 Instance: Offload JavaScript invocations to executor thread
09e311f Introduce Executor helper
Mainly, run JavaScript on a separate thread for interoperability with
Spicy fiber stacks.
Closes#4239
The cluster/websocket tests were developed against websockets 14.2,
but Ubuntu and Alpine ship too old versions. Switch to installing
the latest version from pip instead, so we don't need to bother making
tests compatible with very old Python packages shipped by distributions.
The ZeroMQ backend would accumulate multiple messages and enqueue them
all at once. However, as this could potentially result in huge batches
of events being queued into the event loop at once, switch to a one
message at a time model. If there's too many messages queued already,
OnLoop::QueueForProcessing() will block the ZeroMQ thread until
there's room available again.
The current raise(SIGTERM) sends a signal to the calling thread. If
terminate_processing() is called from a non-main thread and that thread
was created at InitPostScript() time when SIGTERM is blocked, or has
blocked SIGTERM manually, terminate_processing() has no effect.
Switching to kill(getpid(), SIGTERM) guarantees that a thread that doesn't
block the signal (minimally the main thread) will run the handler.
* origin/topic/timw/iwyu-clang-tidy:
Fix indentation in .clang-tidy
Remove existing hand-written clang-tidy setup
Add ability to enable iwyu and clang-tidy at configure time
Using dnsmasq --address covers an entire domain, so dns.example.com
AAAA queries would also return IPv6 addresses for example.com
Instead, this was always meant to use host entries.
* origin/topic/timw/bifs-tocount-zam-baseline:
CI: Add release-build ZAM task for baseline checks during PRs
Fix language/assert btest baselines for to_count/to_int changes
Update the bifs.to_count baseline for ZAM
For a plugin loaded via @load-plugin, create a YY_BUFFER_STATE holding
the required loads for the implicitly loaded files. In loaded scripts,
this generated file will show up with a path of the shared object file
of the plugin with the __preload__.zeek and __load__.zeek files loaded
by it.
Closes#2311
* origin/topic/timw/few-btest-jobs-for-zam-ci-tasks:
CI: Use the right variable for the number of test jobs for zeekctl_debian11_task tasks
CI: Use fewer btest jobs for ZAM tasks
* markoverholser/master:
Fix incorrect syntax for static_cast in to_count that I introduced in a previous merge
Update baselines after changes to to_count and to_int
fix to_count in zeek.bif to resolve conflict from bbanier's adjustment to static casting of the return from strtoull
Fix tests in to_count to reflect appropriate function; add tests for 0x-formatted hex values for to_count and to_int
update BIFs to_int() and to_count() to accept optional 'base' argument; allows more more exotic conversions from hex, octal, binary
Fixes#4076; update BIFs to_int() and to_count() to accept optional 'base' argument for more easy conversions of strings representing (for example) hexadecimal, octal, or binary numbers
Do not just track the analyzer instance in the stack, but also the
data span it is given. This allows to extract more information on-demand
during event processing.
TrackAnalyzer() is technically a public API, but no one should use it
outside of the Analyzer's Forward methods itself.
* origin/topic/bbannier/integer-conversion-error-handling:
Also trim trailing spaces in `to_count`/`to_int` inputs
Align error handling in `to_int` with existing behavior of `to_count`
Baseline handling of leading/trailing spaces in `to_count`/`to_int`.
Add error messages to `to_count`/`to_int` baselines
Previously we would already trim leading spaces in inputs to `to_count`
and `to_int`, effectively by just passing the behavior of the low-level
functions used in their implementations to the user. While this was
useful it was also inconsistent in that we did not allow trailing
spaces which we enable with this patch.
Previously `to_int` would silently ignore invalid inputs and simply
return `0` while `to_count` would return an error; this patch changes
`to_int` to behave like `to_count`.
This introduces a breaking change in that `to_int` now raises an error
for trailing spaces (but still accepts leading spaces) where it
previously would have silently accepted it. This is consistent with
the behavior of `to_count`, but one could also argue that both of
these should only accept properly trimmed input; I did not go that route
since that would introduce breaking changes for both these functions
instead of for just one of them.
Currently `to_count` reports an error for trailing spaces (but not for
leading ones) while `to_int` silently accepts them. This patch adds
baselines capture the current behavior.
CI is currently failing with:
{ "error": { "code": 404, "message": "The resource 'projects/freebsd-org-cloud-dev/global/images/family/freebsd-14-1' was not found" ...
The new Broker API allows us to provide a custom logger to Broker that
pulls previously unattainable context information out of Broker to put
them into broker.log for users of Zeek.
Since Broker log events happen asynchronously, we cache them in a queue
and use a flare to notify Zeek of activity. Furthermore, the Broker
manager now implements the `ProcessFd` function to avoid unnecessary
polling of the new log queue. As a side effect, data stores are polled
less as well.
* origin/topic/johanna/dpd-changes:
DPD: failed services logging alignment
DPD: update test baselines; change options for external tests.
DPD: change policy script for service violation logging; add NEWS
DPD changes - small script fixes and renames.
Update public and private test suite for DPD changes.
Allow to track service violations in conn.log.
Make conn.log service field ordered
DPD: change handling of pre-confirmation violations, remove max_violations
DPD: log analyzers that have confirmed
IRC analyzer - make protocol confirmation more robust.
There were some special cases in which the failed-service-logging policy
script might log a service being removed that was not removed due to an
analyzer violation. This change should fix these cases.
As services are sorted by default now, this disables the canonifier that
sorts the service field for the external baseline.
This also adds the tracking of disabled services in the service field
via DPD::track_removed_services_in_connection - to make such changes
more visible, and check that the feature works as desired.
This commit renames the `service_violation` column that can be added via
a policy script to `failed_service`. This expresses the intent of it
better - the column contains services that failed and were removed after
confirmation.
Furthermore, the script is fixed so it actually does this - before it
would sometimes add services to the list that were not actually removed.
In the course of this, the type of the column was changed from a vector
to an ordered set.
Due to the column rename, the policy script itself is also renamed.
Also adds a NEWS entry for the DPD changes.
This allows Zeek execution to continue gracefully in the presence of such
errors, particularly at zeek_init() time. Includes a tweak to expand the
bifs.directory_operations test to check continuation after errors.
Resolves#3595.
* origin/topic/awelzel/zeromq-fix-fmt-call-thanks-tsan:
cluster/zeromq: Call DoTerminate() in destructor
cluster/zeromq: Use lambda for thread trampoline
cluster/zeromq: Do not call util::fmt() from thread
Normal life-cycle is that Terminate() / DoTerminate() is called
by zeek-setup code. If that doesn't happen, shutdown and join
threads during destructor.
try { } catch (...) suggested by Benjamin.
...util::fmt() uses a static buffer, so this is problematic.
I've dabbled a bit replacing std::thread with using threading::BasicThread
which would offer Fmt(), but this makes things more complicated. Primarily
as BasicThread is registered with the thread manager and the shutdown
interactions become entangled. The thread might be terminated before the
backend, or vice-versa. Seems nicer for the thread to be owned by the backend.
* origin/topic/awelzel/4136-cluster-backend-pre-work:
cluster/zeromq: Fix Unsubscribe() bug caused by \x00 prefix
cluster: Add SubscribeCallback support
cluster/zeromq: Fix XSUB threading issues
cluster/zeromq: Use NodeId(), drop my_node_id
cluster/Backend: Pass node_id via Init()
cluster/Backend: Make backend event processing customizable
cluster/broker/Serializer: Fix adaptor to adapter
cluster/Backend: Do not use const std::string_view&
cluster/serializer/broker: Fix handler lookup
broker/Manager: Move name in PublishEvent()
btest/zeromq/test-bootstrap: Fix port parsing
EventHandler: Support operator!=
This allows callers of Subscribe() to pass in a callback that will be invoked
once the subscription is established or failed to establish. It is the
backend's responsibility to execute the callback on the main thread either
synchronously, or preferably asynchronously at a later point, by
scheduling a task on the IO main loop.
This turns on ZMQ_XPUB_VERBOSE for ZeroMQ so that notifications about
subscriptions are raised even if the subscriptions has previously been
observed.
It is not safe to use the same socket from different threads, but the
current code used the xsub socket directly from the main thread (to setup
subscriptions) and from the internal thread for polling and reading.
Leverage the PAIR socket already in use for forwarding publish operations
to the internal thread also for subscribe and unsubscribe.
The failure mode is/was a bit annoying. Essentially, closing of the
context would hang indefinitely in zmq_ctx_term().
This allows configurability at the code level to decide what to do with
a received remote events and events produced by a backend. For now, only
enqueue events into the process's script layer, but for the WebSocket
interface, the action would be to send out the event on a WebSocket
connection instead.
This also includes some test baseline updates, due to recent QUIC
changes.
* origin/master: (39 commits)
Update doc submodule [nomail] [skip ci]
Bump cluster testsuite to pull in resilience to agent connection timing [skip ci]
IPv6 support for detect-external-names and testcase
Add `skip_resp_host_port_pairs` option.
util/init_random_seed: write_file implies deterministic
external/subdir-btest.cfg: Set OPENSSL_ENABLE_SHA1_SIGNATURES=1
btest/x509_verify: Drop OpenSSL 1.0 hack
testing/btest: Use OPENSSL_ENABLE_SHA1_SIGNATURES
Add ZAM baseline for new scripts.base.protocols.quic.analyzer-confirmations btest
QUIC/decrypt_crypto: Rename all_data to data
QUIC: Confirm before forwarding data to SSL
QUIC: Parse all QUIC packets in a UDP datagram
QUIC: Only slurp till packet end, not till &eod
Remove unused SupervisedNode::InitCluster declaration
Update doc submodule [nomail] [skip ci]
Bump cluster testsuite to pull in updated Prometheus tests
Make enc_part value from kerberos response available to scripts
Management framework: move up addition of agent IPs into deployable cluster configs
Support multiple instances per host addr in auto metrics generation
When auto-generating metrics ports for worker nodes, get them more uniform across instances.
...
This commit builds on top of GH-4183 and adds IPv6 support for
policy/protocols/dns/detect-external-names.
Additionally it adds a test-case for this file testing it with mDNS
queries.
This makes Zeek run in deterministic mode with --save-seeds usage
and reworks all the extra indirections used in init_random_seed()
to make it easier to follow the control flow.
Fixes#4209
* origin/topic/awelzel/4035-btest-openssl-sha1-certs:
external/subdir-btest.cfg: Set OPENSSL_ENABLE_SHA1_SIGNATURES=1
btest/x509_verify: Drop OpenSSL 1.0 hack
testing/btest: Use OPENSSL_ENABLE_SHA1_SIGNATURES
This reverts the call to update-crypto-policies in the Fedora 41 image
and instead sets OPENSSL_ENABLE_SHA1_SIGNATURES in the individual tests.
This allows RHEL 10 or Fedora 41 users to run the tests in question
without needing to fiddle with system settings.
Fixes#4035
* origin/topic/timw/add-note-about-pe-pcap:
Add note to Traces/README about possible malware in pe/pe.trace
Fix formatting of Traces/README entry for modbus-eit.trace
* origin/topic/awelzel/4198-4201-quic-maintenance:
QUIC/decrypt_crypto: Rename all_data to data
QUIC: Confirm before forwarding data to SSL
QUIC: Parse all QUIC packets in a UDP datagram
QUIC: Only slurp till packet end, not till &eod
A UDP datagram may contain multiple QUIC packets, but the parser so far
handled only the very first packet, ignoring any subsequent packets.
Fixes#4198
This doesn't change behavior, but avoids slurping in more data than
needed. A UDP packet an contain multiple QUIC packets and we'd read
all following ones instead just the one we're interested in.
* topic/christian/management-multinode-metrics-ports:
Bump cluster testsuite to pull in updated Prometheus tests
Management framework: move up addition of agent IPs into deployable cluster configs
Support multiple instances per host addr in auto metrics generation
When auto-generating metrics ports for worker nodes, get them more uniform across instances.
Since the changes to port autoassignment in the preceding commits leverage agent
IP address information, we need to ensure that this information is available at
the time of autoassignment. The controller learns IP addresses from connecting
agents, and previously used that information at deploy time. This moves the
augmentation of the cluster config up to port autoassignment time.
- Analyzer: Reduce from 208 bytes to 192 bytes, remove one cache line
- EventGroup: Reduce from 104 bytes to 96 bytes
- Packet: Reduce from 200 bytes to 184 bytes, remove one cache line
- threading::Value: Reduce from 48 bytes to 40 bytes
- ConnTuple: push hole to the end of struct
- TCP_Reassembler: Reduce from 240 bytes to 232 bytes
The changes are mostly quite minor. The main change reasons are:
* analyzers that were confirmed, and later removed now show up in the
conn.log.
* a couple of removed lines in analyzer.log, because non-confirmed
analyzers get removed more quickly.
* in some cases there are additional lines in analyzer.log. These are
cases in which an analyzer gets removed due to a violation and then
re-attached because of a later signature match, which replays the
violating content. In all examples that I have so far, this is caused
by both sides of a connection speaking a differing protocol. There
probably should be a better way to handle this - but it works.
* new column for failed analyzers 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.
This changes service set in the connection record, and thus also the
conn.log service field to being ordered. Speficically, the order of the
entries in the service field will be the same order in which protocols
will be confirmed. This means that it now is possible to see which
protocols were layered over each other in which order by looking at the
respective conn.log entry.
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.
I noticed that the IRC analyzer always confirms connections, pretty much
without regard what happened in it. This commit changes the logic of the
IRC analyzer to check for valid commands before confirming.
Closes#4173
This allows types to be used in expressions, but they can't be
reassigned. Note that this was meant to be a special "type expression" -
but that is unnecessary complexity.
Type expressions would allow access to the type without going through
its constant value, but the constant value is never changed, so it's
simply a few more checks if necessary when functionality gets expanded.
This way, ZAM and other code will not need updates, so the potential for
increased work in the future is probably not worth caring about.
Closes#4173
This allows types to be used in expressions, but they can't be
reassigned. Note that this was meant to be a special "type expression" -
but that is unnecessary complexity.
Type expressions would allow access to the type without going through
its constant value, but the constant value is never changed, so it's
simply a few more checks if necessary when functionality gets expanded.
This way, ZAM and other code will not need updates, so the potential for
increased work in the future is probably not worth caring about.
* origin/topic/timw/merge-init-bare-sections:
Minor whitespace cleanup in init-bare.zeek
Add missing export blocks to init-bare.zeek
Merge some GLOBAL module sections in init-bare
Merge Tunnel module sections in init-bare
A command like this would segfault:
zeek -b test.zeek --debug
The issue was that `getopt_long` was using a null element to determine
what the end of the options array is. If it saw a non-null element after
`--debug` it would say it's the argument for optarg, even if it's beyond
`zeek_args.size()`. Instead, just make sure the array is
null-terminated.
* origin/topic/awelzel/lookup-connection-tweaks:
session/Manager: Emit explicit errors for FindConnection() with proto=65535
IPAddr/ConnKey: Protect from uninitialized conn_id
IPAddr/ConnKey: Promote transport to uint16_t
session/Manager: Header cleanup
We silently broke users constructing conn_id records manually and
subsequently using them with lookup_connection() or connection_exists().
This is an attempt to at least report a runtime error about the situation
so it doesn't go completely unnoticed.
Check if the non-default fields exist using HasField()
and use GetField() for proto such that it'll initialize
the default value which GetFieldAs<> doesn't do.
default
Instead of a separate bool field which is also stored in the session
table, promote the transport field to uint16_t and encode an invalid
ConnKey as transport 2**16-2
* 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
If a plugin provides a write hook, the invocation for HookLogWrite() would
redo looking up the writer's name from the enum value and instantiating
a new std::string instance for every write. Avoid doing this.
There's two instances of WriterBackend::WriterInfo for a given
writer. One in Manager::WriterInfo that's accessible via
stream.writers and a copy within WriterFrontend.
Commit 78999d147d switched to use the
address of the frontend's info instance for HookLogWrite() invocations,
breaking users using the address for identification purposes.
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)
Namely, this will:
- print --help to stdout rather than stderr
- not print full usage when errors happen
- add a prompt to use --help for more info on error
This should make it more greppable, clearer, and less spammy.
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
* origin/topic/timw/coverity-upload-large-builds:
CI: Fix coverity workflow to support large uploads
CI: Remove use of wget from coverity workflow
CI: Upgrade coverity workflow to ubuntu24, add jq
The previous setup only allowed uploads of up to 500MB, which we
eclipsed a long time ago. They recently started enforcing it, so
this switches over to use a call-and-response API for uploading
larger files.
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/vern/ZAM-field-assign-in-op:
pre-commit: Bump spicy-format to 0.23
fix for ZAM optimization of assigning a record field to result of "in" operation
* security/topic/awelzel/217-quic-decrypt-crash:
QUIC/decrypt_crypto: Actually check if decryption was successful
QUIC/decrypt_crypto: Limit payload_length to 10k
QUIC/decrypt_crypto: Fix decrypting into too small stack buffer
The fatal error is actually triggered at runtime, so it's a bit
dangerous for users, but not sure there's many use-cases to
compare vectors of patterns.
Closes#4084
Given we dynamically allocate memory for decryption, employ a limit
that is unlikely to be hit, but allows for large payloads produced
by the fuzzer or jumbo frames.
A QUIC initial packet larger than 1500 bytes could lead to crashes
due to the usage of a fixed size stack buffer for decryption.
Allocate the necessary memory dynamically on the heap instead.
* origin/topic/vern/ZAM-assert-streamlining:
btest/supervisor: Bump one timeout to 30 seconds
btest/cluster: Bump timeouts to 30 seconds
btest/validate-ZAM: Update baseline, run always
* origin/topic/awelzel/3287-push-rc-container-images:
cirrus/container_image_manifest: match RC tags, too
cirrus: Move additional tag "computation" into separate script
* origin/topic/vern/CPP-standalone-fixes:
BTest updates in support of compiling-scripts-to-C++
-O gen-standalone-C++ fixes for recent more aggressive AST profiling
The nodes-experimental/manager.zeek file ends up calling Broker::publish()
unconditionally, resulting in a warning. Skip running that code when
generating documentation.
* 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
Same as what we do in Broker. Use the expected type if publishing
a table() or set() parameter.
This fixes issues when switching sumstats to Cluster::publish()
The broker serializer leverages the existing data_to_val() function.
During unserialization, if the destination type is any, the logic
simply wraps the broker::data value into a Broker::Data record.
Therefore, events with any parameters are currently exposed to
the Broker::Data type.
There is a bigger issue in that re-publishing such Broker::Data
instances would encode them as a normal record. Explicitly prevent
this by serializing the contained data value directly instead, similar
to what Broker already did when publishing a record.
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.
ZAM's implementation deviates from Zeek's in consideration of remappings, which
means that running ZAM produced conn.log service names prefixed with "spicy_"
whereas regular Zeek doesn't.
* origin/topic/timw/python-upgrade: (65 commits)
Swap pre-commit yapf for ruff/ruff-format, fix findings
Upgrade package-manager submodule to get python upgrade
Upgrade broker submodule to get python upgrade
Upgrade zeekctl submodule to get python upgrade
Upgrade zeek-client submodule to get python upgrade
Upgrade btest submodule to get python upgrade
Require Python 3.9 in CMakeLists.txt
CI: Install python 3.9 on ubuntu 20
btest: Avoid loading ZeroMQ if not compiled in
btest/coverage: Avoid warnings in test-all-policy-cluster
Broker::publish: Warn on using Broker::publish() when inactive
Update doc submodule [nomail] [skip ci]
Update zeek-testing and zeek-testing-cluster commit hashes
Update ZAM BiF-tracking
configure: Add --disable-cluster-backend-zeromq
CMakeLists: Cluster backends output
zeromq: Conditionally enable by default
btest/generic: Add publish_hrw(), publish_rr() and logging tests
generate-docs: Run on Ubuntu 24.04, add cppzmq
docker: Add cppzmq/libzmq dependencies
...
* origin/topic/awelzel/broker-publish-warn:
btest: Avoid loading ZeroMQ if not compiled in
btest/coverage: Avoid warnings in test-all-policy-cluster
Broker::publish: Warn on using Broker::publish() when inactive
...at the same time, add some `TEST-REQUIRES: have-zeromq` which
unfortunately means that developers will usually want libzmq
installed on their system.
This is a cluster backend implementation using a central XPUB/XSUB proxy
that by default runs on the manager node. Logging is implemented leveraging
PUSH/PULL sockets between logger and other nodes, rather than going
through XPUB/XSUB.
The test-all-policy-cluster baseline changed: Previously, Broker::peer()
would be called from setup-connections.zeek, causing the IO loop to be
alive. With the ZeroMQ backend, the IO loop is only alive when
Cluster::init() is called, but that doesn't happen anymore.
Not all supported platforms provide a recent enough cppzmq version,
add a fallback as submodule. cppzmq is a header-only library, so there's
no build step involved.
Also, run the ZAM-bif-tracking test in non-ZAM environments so
failures are caught immediately. There's nothing overly ZAM specific about
running this test.
I'm not sure I like the fact that any new contributor adding a BiF
will need to dig into this... it might be a bit intimidating.
* 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
* origin/topic/awelzel/license-header-cleanup:
Add missing copyright line to headers and cc files
pre-commit: Add license-header check inspired by Spicy
Add missing "COPYING" in file comments
* origin/topic/vern/CPP-streamlining:
BTest baseline updates for compile-to-C++
mark ZAM regression BTests as not suitable for compile-to-C++
fix for -O gen-C++ maintenance helper to skip BTest intermediary files
introduced simplified initialization for non-standalone -O gen-C++ code tied -O gen-standalone-C++ to use of --optimize-files
streamline generated -O C++ code by relying on per-function profiles rather than aggregate profile
when reporting available/unavailble C++ script bodies, flag those that are skipped
modified AST profiling to mark (and fully skip) non-optimizable functions
modified merge_types() to skip work if given identical types, which also preserves type names (useful for -O gen-C++)
Using network_time to calculate packet lag will produce wrong results
when there is no packet available but network time does not (yet) fall
back to wall clock.
This now picks up additional typical misspellings, but also triggers on
more identifiers we use. I opted for fixing the obvious misspellings and
updated the allowlist for anything else.
* origin/topic/vern/script-opt-uncompilable-AST:
ScriptOpt: Fail compilation if known exprs/stmts is outdated
skip optimization of functions with AST nodes unknown to script optimization
Yes, really. :-) We've hit the need for this on occasion in very specific
settings and always worked around it via ugly nested loops or similars.
This has ample warning that folks normally won't want to use this.
Not sure that ZAM btest should baseline the number of BiFs.
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.
This analyzer can be used to transport raw stream data for a given
connection to the script layer. For example, adding this analyzer into
the HTTP::upgrade_analyzer or using it to configure a child WebSocket
analyzer allows to get access to the raw stream data in script land
when no more appropriate protocol analyzer is available.
* origin/topic/vern/script-opt-keep-asserts:
ZAM documentation updates for asserts and event handler run-time errors
BTest updates for ZAM support of (optionally) keeping "assert" statements
command-line options for controlling script optimization: keeping asserts, avoiding event handler coalescence
ZAM support for option to not coalesce event handlers
ZAM support for keeping "assert" statements
internal support for script optimization options for keeping asserts, not consolidating event handlers
ZAM operations to support asserts
simplified "assert" by not trying to catch messages that themselves have errors
Fixed some TEST-REQUIRES "${ZEEK_ZAM}" == "1" to use "=" instead to
be /bin/sh compatible.
* origin/topic/vern/zam-pattern-comparison:
update of BTest that tracks number of (and validates) ZAM operations
ZAM support for pattern equality/inequality operations
expanded ZAM operations for bit-shifting to allow for int/count shift values
added type coercion for bit-shifting expressions
The pcap comes from the following dataset [1]:
CTU-SME-11: a labeled dataset with real benign and malicious network
traffic mimicking a small medium-size enterprise environment
[1] https://zenodo.org/records/7958259
* origin/topic/awelzel/pluggable-cluster-backends-part3:
init-bare/zeek-setup: Groundwork for instantiating cluster backends
cluster/serializer: Add binary-serialization-format
logging/WriterFrontend: Add logic for non-broker cluster backends
logging/WriterBackend: Include logging/Types.h
logging/Manager: Implement new WriteBatchFromRemote()
logging/WriterFrontend: Add LogWriteHeader as member
logging: Add filter_name to WriterInfo
This is a serializer for log records that is using SerialTypes
for serializing and un-serializing rather. Essentially, this is
similar to what broker does except for the envelope.
If cluster::backend isn't broker_mgr, use the WriterFrontend's buffering
logic and send a whole batch of log writes during FlushWriteBuffer().
This is a different path than broker's own logging logic.
Preferably we adapt broker to a model where it isn't
buffering either.
...with this change, it'll be possible to identify WriterFrontend's
based on (stream, filter_name, path) pairs in addition to (stream,
writer, path) pairs.
* origin/topic/timw/json-interval-conversion:
Add interval_as_double argument to control how intervals are converted to JSON
Add btest for round-trip JSON conversion
Allow comparing two PatternVals
Handle conversion between data from Val::ToJSON and ValFromJSON better
The old `c_str_safe` utility function allowed Zeek to operator on
`broker::data` and `broker::variant`. The former grants access to actual
`std::string` objects while the latter only provides access to fields
via `std::string_view`. Since the Zeek formatting functions need null
terminated strings, we need to copy the characters into a
null-terminated container first.
After removing support for `broker::data` and `broker::variant` from the
same code paths, we can drop `c_str_safe` and always do the copying
(since we are always dealing with `broker::variant` now).
Since the transition to broker::variant has been long finalized, there
is no more need to be able to go back to a pre-variant version of
Broker. Hence, we can drop various utilities that allow Zeek to run with
older Broker releases.
zeek-6.0 shipped with spicy-1.8, but we still have dedicated handling
for earlier versions of Spicy in the code. This patch cleans them up
since these versions are unsupported.
* 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
Broker's JSON serialization is TZ dependent (which seems a bug). For now
do the same as we do in btest.cfg and run doctests with TZ set to UTC.
Reported in zeek/broker#434.
This was added previously in the 7.1 cycle. Now that MakeEvent() was
removed from cluster::Backend, there's no need for Broker to provide
this version.
* origin/topic/awelzel/pluggable-cluster-backends-part1:
btest: Test Broker::make_event() together with Cluster::publish_hrw()
btest: Add cluster dir, minimal test for enum value
broker: Add shim plugin adding a backend component
zeek-setup: Instantiate backend::manager
cluster: Add to src/CMakeLists.txt
cluster: Add Components and ComponentManager for new components
cluster/Backend: Interface for cluster backends
cluster/Serializer: Interface for event and log serializers
logging: Introduce logging/Types.h
SerialTypes/Field: Allow default construction and add move constructor
DebugLogger: Add cluster debugging stream
plugin: Add component enums for pluggable cluster backends
broker: Pass frame to MakeEvent()
Header and implementation for types shared between the cluster and
logging framework. The logging framework will be adapted later to
use these. For now, the new cluster components will simply reference
them.
The continued line was incorrectly marked up (as a plain comment instead
of a Zeekygen comment) which caused only half of the docstring to be
rendered by Zeekygen.
This ensures that the normal test setup is run, which enables greedy
mode on Cirrus allowing us to over-provision the testing. This should
help speed up the ZAM builds a bit.
41ec748 cmake: Use foreach range
2b90a56 tests: Update new-event to run with 7.1+, remove ip_proto elsewhere
a596f6f Makefile: Update outdated commands
b954661 Enable `cmake/FindNodejs.cmake` to find more libnode versions
This ordering fixes a test failure we're seeing on Alpine for the
signatures/tcp-end-of-match btest, since discrepancies in rule match traversal
could lead to discrepancies in corresponding event ordering.
It looks safe to rely on across platforms since the index is driven by signature
load order, which shouldn't deviate. If this somehow doesn't hold in the future,
we'll only wind up with a test failure, not incorrect match behavior.
(Correction to 2e03fbb8b0, which I pushed
accidentally.)
@Sheco reported that standalone epoch processing may exclude scheduled
events when the final sumstat epoch runs before. For example, this easily
happens when attempting to do sumstat observations within connection_state_remove().
Delay final epoch processing to zeek_done() instead.
This doesn't deal with the clustered version - this would need something
more elaborate and potentially a mechanism to delay the shutdown of
other cluster nodes until/after sumstat processing completed.
Currently, plugins do not have a way to further inspect or even mutate
script functions after ZAM optimization ran. One use-case here is
zeek-perf-support [1]. This plugin wraps Stmt instances of functions,
events and hooks hooks with a small assembly stub to support JIT map
files [2] and for integration with perf tools.
This change introduces a new InitPreExecution() hook that runs after
ZAM optimization completed, just before the zeek_init() event is enqueued.
Additionally, remove the existing CPP_activation_hook. It doesn't seem
to be used. If it becomes necessary in the future, the new
InitPreExecution() hook can be leveraged instead.
[1] https://github.com/zeek/zeek-perf-support
[2] https://github.com/torvalds/linux/blob/master/tools/perf/Documentation/jit-interface.txt
* origin/topic/christian/debug-stream-tweaks:
Make debug stream names use "-", and handle "_" transparently
Make "-B all" apply to plugin streams as well.
Sort streams in "-B help" output, and match case-insensitively throughout
We already accept that the packets coming into this analyzer won't
be processed, so forwarding out of it will just result in a failure.
Forwarding will also report a weird for every packet which just
results in extra noise.
Matching of plugins' debug streams was still case-sensitive. Also contains some
minor output tweaks.
It'd be nice to only list plugin debug streams actually _used_ by plugins. I
didn't see a quick way to do that so that's for another time.
* origin/topic/awelzel/log-broker-io-telemetry-rules-includes:
zeek-setup: Remove some unused headers
clang-format: Sort doctest header at the bottom
RuleMatcher: Move plugin/Manager.h include from .h to .cc
iosource/Manager: Remove superflous includes
telemetry/Manager: Remove broker header include, add fnmatch.h
logging/Manager: Fix using filename from input.h in debug log
GetFieldAs() does not initialize an optional or default field. Apparently,
for ZAM that makes a difference, possibly the [] record construction is
optimized, not initializing proto until actually accessed. Not quite
sure why that's not happening for classic script interpretation
though.
The zeek_binpac.h one isn't used directly, but keeping it over
<binpac.h> include. Also do some std prefixing. binpac.h has
'using namespace std', so these slip through :-(
Establishing reliable ordering fixes a test failure we're seeing on Alpine for
the signatures/tcp-end-of-match btest, since discrepancies in rule match
traversal could lead to discrepancies in corresponding event ordering.
Discussed with @J-Gras, calling Broker::publish() within a scheduled
should use the "intended timestamp" implicitly.
This is subtle, but supposedly more expected when running
a pcap replay cluster.
* topic/christian/ci-updates:
CI: Use FEDORA40 crypto policy in Fedora 41
Bump zeekjs to 0.13.0
CI: bump FreeBSD 13 to 13.4, released in September
CI: drop Fedora 39, add 41
* origin/topic/timw/3915-unknown-ip-protocol:
Add NEWS entry for ip_proto feature
Move IP protocol names table out of policy script to init-bare
Minor review nits
Fixes for community ID hashing with new proto values
Use new_connection instead of connection_state_remove
Add policy script to remove ip_proto field, rename protocol naming script
Rename protocol_id field to ip_proto and similar renaming for name field
Increase size of proto fields to uint16_t, add common default value
Disable part of core/dict-iteration-expire5 btest to avoid iteration bug
Add conn.log entries for connections with unhandled IP protocols
* origin/topic/timw/remove-abspath-cleanup:
diff-remove-abspath: Add separate handling of Windows paths
diff-remove-abspath: Remove capture of windows drive letters from POSIX regex
check_pseudo_time() used zeek_start_time which skews things sufficiently
around being in the past when ZAM compilation takes multiple seconds. Switch
to using first_wallclock instead.
Further, move setting of first_timestamp and first_wallclock from PktSrc
into RunState's dispatch_packet(), so it's more centralized now.
The only pseudo_realtime piece left in PktSrc() is in GetNextTimeout() to
determine how long the PktSrc is idle until the next packet is ready.
* origin/topic/vern/zam-asan-fixes:
ZAM fixes for assignments involving "any" record fields
fixes for (mostly ZAM) vector operation issues found by ASAN
Including a fix for mmdb/explicit-open.zeek to avoid using assert.
* origin/topic/timw/update-c-ares-to-latest-release:
DNS_Mgr: Remove processing of dns aliases in general
ci: Add dnsmasq to a few platforms for testing
DNS_Mgr: Fix aliases memory issues
btest: Add integration test for DNS_Mgr
DNS_Mgr: Remove usage of ares_getsock from Lookup
DNS_Mgr: Remove usage of ares_getsock from GetNextTimeout
DNS_Mgr: Switch to ares_set_servers_csv
DNS_Mgr: Use ares_dns_record methods for queries
Update vcpkg submodule to pick up c-ares v1.34.2
Update c-ares submodule to v1.34.2
This wasn't possible before #3028 was fixed, but now it's safe to set
the value in new_connection() and allow other users access to the
field much earlier. We do not have to deal with connection_flipped()
because the community-id hash is symmetric.
So far, when Zeek didn't see a connection's regular tear-down (e.g.,
because its state timed-out before we got to the end), we'd still
signal a regular end-of-data to Spicy parsers. As a result, they would
then typically raise a parse error because they were probably still
expecting data and would now declare it missing. That's not very
useful because semantically it's not really a protocol issue if the
data just doesn't make it over to us; it's a transport-layer issue
that Zeek already handles elsewhere. So we now switch to signaling
end-of-data to Spicy analyzers only if the connection indeed shuts
down regularly. This is also matches how BinPAC handles it.
This also comes with a test exercising various combinations of
end-of-data behavior so that we ensure consistent/desired behavior.
Closes#4007.
typename(table()) apparently always resulted in a set[] type
being rendered. Make the yield type of an unspecified table
ANY so that type->IsSet() ends up false.
While at it, also render unspecified types as table(), set() and
vector() rather than vector of void, set[] or table[] of any which
IMO should help to figure out what's going.
If we added a file but the other side of the connection had already run
into a protocol violation and shut down we could previously have
dereferenced a null cookie. This patch fixes the code so it now throws
in such scenarios.
We would previously assert that it was available which could have lead
to aborts since when the analyzer for either side of a connection shuts
down the connection cookie could get cleared and become nil. This patch
reworks the code slightly so we now never assume it is available. We do
this by either throwing or by making the whole operation requesting the
cookie a noop.
This admittedly is a quite esoteric combination of protocols. But - as
we do correctly support them, it seems nice to have a slightly more
complete testcase that covers this.
Modified ubuntu24_clang_libcpp_task to be skipped on PRs.
* origin/topic/etyp/ubuntu-clang-ci:
CI: Upgrade FreeBSD 14 to 14.1
CI: Add Ubuntu Clang task
Fix up minor warnings in touched files
Fix Clang 19 deprecation failure
* origin/topic/awelzel/data-end-offset-v1:
signatures: Add data_end_offset to signature_match() and custom events
Add pattern_end_offset to signature_state
This feels more like a band-aid solution - it still requires very manual
intervention to get the next builds, but at least this way we don't rely
on whatever Clang/libc++ happens to ship with FreeBSD for the only
libc++ testing. It seems like sanitizer builds don't use libc++, but I'd
rather a dedicated Linux/Clang/libc++ task at least.
Fixes#3994
Clang 19 with libc++ started failing to compile because the default
implementation of `std::char_traits` was removed, making uses of
`std::char_traits<unsigned char>` invalid. This was more of used for
convenience before, but it should be roughly the same behavior with
`char`.
See relevant LLVM commits:
aeecef08c308a0faf4cd
This change tracks the current offset (number of bytes fed into matchers)
on the top-level RuleEndpointState such that we can compute the relative ending
for matched texts individually.
Additionally, it adds the data_end_offset as a new optional parameter to
signature_match().
This commit fixes three issues with Zeek's Modbus message logging:
1 - Some exception responses (e.g., READ_COILS_EXCEPTION) are logged
twice: once without and once with the exception message.
2 - Some exception responses (e.g., PROGRAM_484_EXCEPTION) are not
logged.
3 - Some known but reserved function codes (e.g., PROGRAM_UNITY) are
logged as unk-xxx (e.g., unk-90), while it would be possible to
log their known name.
To address these inconsistencies, the modbus parser has been updated
to parse all exception responses (i.e., all responses where the MSB
of the function code is set) using the already defined Exception
message.
Also, the Modbus main.zeek script has been updated to consistently
demand logging exception responses to the specialized
modbus_exception event, rather than logging some exception responses
in the modbus_message event and others in the modbus_exception event.
Finally, the main.zeek script has been updated to make sure that
for every known function code, the corresponding exception code was
also present, and the enumeration of known function codes in
consts.zeek has been expanded.
Closes#3984
Add pattern_end_offset to signature_state
Update init-bare.zeek
Update RuleMatcher.cc
Update RuleMatcher.h
Update init-bare.zeek
clang format
clang format
clang format
Using Match Offsets List
Temp commit
* origin/topic/vern/CPP-when-capture-naming:
fixed "-O gen-C++" naming of "when" captures to avoid ambiguities due to inlining
Simplified & made more robust maintenance helper script for "-O gen-C++" testing
"-a cpp" baseline updates to reflect recent BTest changes
remove instance of plus sign to account for real plus in sql
account for spaces encoding to plus signs in sqli regex detection
add test cases for sqli space to plus
account for spaces encoding to plus signs in sqli regex detection
forgot semicolon
account for spaces encoding to plus signs in sqli regex detection
* topic/christian/telemetry-make-bifs-primary:
Telemetry framework: move BIFs to the primary-bif stage
Minor comment tweaks for init-frameworks-and-bifs.zeek
Adding a metric for the network time value itself should make it
possible to observe it stopping or growing slowly as compared to
realtime when Zeek isn't able to keep up.
Also, modify the telemetry/log.zeek test to include misc/stats and
log at a higher frequency with a more interesting pcap.
This stops invoking Telemetry::sync() via a scheduled event and instead
only invokes it on-demand. This makes metric collection network time
independent and lazier, too.
With Prometheus scrape requests being processed on Zeek's main thread
now, we can safely invoke the script layer Telemetry::sync() hook.
Closes#3947
This commit prevents most non-Modbus TCP traffic on port 502 to be
reported as Modbus in conn.log as well as in modbus.log.
To do so, we have introduced two &enforce checks in the Modbus
protocol definition that checks that some specific fields of the
(supposedly) Modbus header are compatible with values specified in
the specs.
To ensure non-regression, with this commit we also introduce a
new btest.
Closes#3962
* topic/vern/script-opt-headers-factoring:
factored CPP source's main header into collection of per-source-file headers
renamed script optimization Attrs.h header to prepare for factoring large Compile.h
factored ZAM source's main header into collection of per-source-file headers
This moves the Telemetry framework's BIF-defined functionalit from the
secondary-BIFs stage to the primary one. That is, this functionality is now
available from the end of init-bare.zeek, not only after the end of
init-frameworks-and-bifs.zeek.
This allows us to use script-layer telemetry in our Zeek's own code that get
pulled in during init-frameworks-and-bifs.
This change splits up the BIF features into functions, constants, and types,
because that's the granularity most workable in Func.cc and NetVar. It also now
defines the Telemetry::MetricsType enum once, not redundantly in BIFs and script
layer.
Due to subtle load ordering issues between the telemetry and cluster frameworks
this pushes the redef stage of Telemetry::metrics_port and address into
base/frameworks/telemetry/options.zeek, which is loaded sufficiently late in
init-frameworks-and-bifs.zeek to sidestep those issues. (When not doing this,
the effect is that the redef in telemetry/main.zeek doesn't yet find the
cluster-provided values, and Zeek does not end up listening on these ports.)
The need to add basic Zeek headers in script_opt/ZAM/ZBody.cc as a side-effect
of this is curious, but looks harmless.
Also includes baseline updates for the usual btests and adds a few doc strings.
This pins the generate-docs GH workflow to run on a fixed version of
ubuntu so we get defined semantics of `pip install` wrt system packages.
We were previously using the `ubuntu-latest` image and GH has been
switching this from ubuntu-22.04 to 24.04 and back again, also see
https://github.com/actions/runner-images/issues/10636.
This job uses `ubuntu-latest`, i.e., it will automatically use the
newest version of Ubuntu available. On current versions `pip install`
refuses to install packages into the system since this might break
system packages. Users are expected to explicitly opt into this
potential breakage by passing the `--break-system-packages` flag. This
is an acceptable workaround here since we do not distribute this setup,
and currently things appear to not break.
The GH action `pre-commit/action` installs `pre-commit` into the system
with `pip`.[^1] On newer distributions this is discouraged as it might break
system packages and would require a dedicated flag
`--break-system-packages` in order to pass.
Since the action does not work in such environments yet, instead pin the
distribution we use for the workflow to a fixed, older and working version.
[^1]: 576ff52938/action.yml (L11)
Previously there was logic for doing this, but it was ineffectual
due to the order in which canonicalization was done. This problem
manifested non-deterministically depending on order-of-generation
of "when" lambdas, which is why previous testing didn't catch it.
This isn't a straightforward fix, unfortunately. The existing GetLine()
implementation didn't deal well with input that's incrementally produced
where individually read chunks wouldn't end with the separator.
The prior implementation increased the buffer each time it failed to find
a separator in the current buffer, but then also ended up not searching the
full new buffer size for the terminator, doing that endlessly.
This change reworks the Raw reader to rely only on bufpos for reading
and searching purposes and skip reallocation if the buffer size if it
wasn't actually exhausted.
Closes#3957
Processing out-of-order commands or finishing commands based on invalid
server responses resulted in inconsistent analyzer state, potentially
triggering null pointer references for crafted traffic.
This commit reworks cf9fe91705 such that
too many pending commands are simply discarded, rather than any attempt
being made to process them. Further, invalid server responses do not
result in command completion anymore.
Test PCAP was crafted based on traffic produced by the OSS-Fuzz reproducer.
Closes#215
That test got flaky probably from #3949 on centosstream9 CI. You can
replicate that behavior by increasing the sleep time when waiting for
the file such that the test will attempt to read the missing file again.
Since the one second wait for file is glacially slow for this, speeding
it up should mean that the file gets created sooner and so the test
won't try to open the file again. But, it's always still technically
possible, since the test will wait for 10 seconds and the heartbeat
seems to be 1 second. At least if that happens, it's probably a bug or
massive slowdown of some kind.
It seems like other similar tests get by because they have more "stuff"
before they call `terminate()` most likely. But, to be safe, just
removing the "received termination signal" line seems like the best
approach.
Invalid lines in a file was the one case that would not suppress future
warnings. Just make it suppress warnings too, but clear that suppression
if there is a field in between that doesn't error.
Fixes#3692
* topic/vern/script-opt-maint.Sep24B:
factoring of logic used by ZAM's low-level optimizer when adjusting control flow info
BTest baseline update for more complete function/lambda names
tweak to -O gen-C++ maintenance script to avoid treating plugins as BTests
fixed lambda hash collision bug due to function descriptions lacking full parameter information
fixes (to avoid collisions) for AST profiling's function hash computations
removed unused ZAM cast-to-any operation
fixes for ZAM tracking the return type associated with function calls
ZAM control-flow tracking now explicitly includes the ends of loops
fix for ZAM identification of common subexpressions
"-O dump-final-ZAM" option similar to "dump-ZAM" only prints final version of functions
fix for setting object locations to avoid use-after-free situation
extended "-O allow-cond" to apply to both gen-C++ and gen-standalone-C++
-O gen-C++ fix for run-time warnings for "when" lambdas
fix to -O gen-C++ for recent AST profiling changes for identifying function parameters
fix to -O gen-C++ for dealing with "hidden" parameters
tweak to prevent an incorrect warning for scripts compiled to C++
fixed overly narrow Spicy test for manipulating packet analyzers
fixed memory leak for recursive ZAM functions that exit via an exception
remove unnecessary header include
* origin/topic/awelzel/cluster-backends-pre-work-v1:
NEWS: Update
scripts/base/cluster: Move active node management into node_down()
logging/Manager: Extract another CreateWriter() helper
logging/Manager: Extract path_func invocation into helper
logging: Dedicated log flush timer
all: Change to use Func::GetName()
script_opt: Use Func::GetName()
Func: Add std::string name accessors, deprecate const char* versions
plugin/ComponentManager: Support lookup by EnumValPtr
For other cluster backends, CreateWriter() will use a logger's filter
configuration rather than receiving all configuration through CreateLog.
Extract a helper out from WriteToFilters() for reuse.
Log flushing is currently triggered based on the threading heartbeat timer
of WriterBackends and the hard-coded WRITE_BUFFER_SIZE 1000.
This change introduces a separate timer that is managed by the logger
manager instead of piggy-backing on the heartbeat timer, as well as a
const &redef for the buffer size.
This allows to modify the log flush frequency and batch size independently
of the threading heartbeat interval. Later, this will allow to re-use the
buffering and flushing logic of writer frontends for non-Broker cluster
backends, too.
One change here is that even frontends that do not have a backend will
be flushed regularly. This is wanted for non-Broker backends and should be
very cheap. Possibly, Broker can piggy back on this timer down the road, too,
rather than using its own script-level timer (see Broker::log_flush()).
For debugging btests, it can be convenient to enable debug streams
by setting an environment variable rather than editing zeek invocations
and adding -B selectively.
Sample use case:
$ export ZEEK_DEBUG_LOG_STREAMS=all
$ btest -d core/failing-test.zeek
$ less .tmp/core/failing-test/debug.log
This change makes Zeek's -B option and ZEEK_DEBUG_LOG_STREAMS are additive.
The tests `core.sigterm-regular` and `core.sigterm-stdin` rely on `ps`
to be present which is not the case anymore on OpenSuse Leap; install it
explicitly there.
The cmds list may grow unbounded due to the POP3 analyzer being in
multiLine mode after seeing `AUTH` in a Redis connection, but never
a `.` terminator. This can easily be provoked by the Redis ping
command.
This adds two heuristics: 1) Forcefully process the oldest commands in
the cmds list and cap it at max_pending_commands. 2) Start raising
analyzer violations if the client has been using more than
max_unknown_client_commands commands (default 10).
Closes#3936
* origin/topic/awelzel/teredo-gtpv1-conn-removal-hook:
btest: Update baselines for removal-hooks addition
gtpv1: Replace connection_state_remove() with RemovalHook
teredo: Replace connection_state_remove() with RemovalHook
teredo: Move conn member from analyzer to encapsulation
Remove overhead of unconditionally calling remove_teredo_connection()
for *every* connection by installing a connection removal hook for only
when state was allocated.
There's only a single instance of the Teredo analyzer. Mutating the conn
member for every new packet and leaving it set after processing the
packet is confusing. Move conn into TeredoEncapsulation instead, or pass
it explicitly.
Really, they both should be count. But, they were getting provided as an
integer. Port is easy since it is backed by an unsigned value. Enums
*should* be unsigned, but aren't. This doesn't address that, it just
takes the other name for this operator (absolute value) and makes the
enum value positive if it's negative.
This fixes a case where using the size of operator on enum/port values
in certain contexts (like the default parameter of a struct) would cause
an internal error.
Now that we run callbacks on the main loop, we can move callback support
for Counter and Gauge instances directly into Zeek and don't need to patch
prometheus-cpp anymore.
* origin/topic/johanna/reformat-spicy-ssl:
Bump spicy-format to 0.16.2
Bump spicy-format to 0.16.1
Spicy SSL: reformat with new version of spicy format
* origin/master: (27 commits)
Update doc submodule [nomail] [skip ci]
btest/ldap: Add regression test for #3919
postgresql: Simplify SSL buffering and forwarding
postgresql: Initial parser implementation
testing/external: Update private baselines
analyzer/syslog: Reformat with spicy-format
analyzer/finger: Reformat with spicy-format
scripts/spicy: Reformat with spicy-format
pre-commit: Add spicy-format
Check for netbios to avoid reporting extra bad DNS opcodes
Add weird for unhandled opcodes in DNS analyzer
Bump zeek-aux for zeek/zeek-aux#57
Remove pre-commit exclusions for clang-format
Bump clang-format
Bump auxil/spicy to latest development snapshot
RunState: Drop broker_mgr->Active() usage
script_opt/ZAM/IterInfo.h: Add missing Dict.h dependency
script_opt/ZAM: ZBody.h / Support.h: Cleanup includes, use forward declarations
script_opt/ZAM/Profile: Remove Zeek header includes
script_opt: Extend Support.h to break include dependencies
...
* origin/topic/awelzel/script-opt-less-includes:
script_opt/ZAM/IterInfo.h: Add missing Dict.h dependency
script_opt/ZAM: ZBody.h / Support.h: Cleanup includes, use forward declarations
script_opt/ZAM/Profile: Remove Zeek header includes
script_opt: Extend Support.h to break include dependencies
script_opt: Do not include ZBody.h in ZAM/Validate.cc
This adds a protocol parser for the PostgreSQL protocol and a new
postgresql.log similar to the existing mysql.log.
This should be considered preliminary and hopefully during 7.1 and 7.2
with feedback from the community, we can improve on the events and logs.
Even if most PostgreSQL communication is encrypted in the real-world, this
will minimally allow monitoring of the SSLRequest and hand off further
analysis to the SSL analyzer.
This originates from github.com/awelzel/spicy-postgresql, with lots of
polishing happening in the past two days.
We previously would ignore any files under `auxil/`. Since pre-commit
only formats files tracked in this repo, and `auxil/` today contains
only submodules this exclusion did not exclude anything; if however we
ever end up adding C++ files under `auxil/` we would like them to be
formatted consistently.
This patch contains a bump of the configured clang-format version from
17.0.3 to 18.1.8 and automatically generated C++ source updates. The
main difference we are seeing from this is fixes for previously
incomplete reformats.
* origin/topic/awelzel/less-threading-star-star:
logging/WriterFrontend: No need for explicit CleanupWriteBuffer()
logging: Switch index-assignment of raw pointers to emplace_back()
broker/logging: Change threading::Value** usage std::vector instead
threading/Value: Support move and copy constructors
Rebuilding ZBody.cc is annoyingly slow with UBSAN or ASAN enabled. A rebuild
is currently triggered when touching Manager.h files like logging/Manager.h,
broker/Manager.h or file_analysis/Manager.h and various other headers that are
included from there or directly in ZBody.h.
Add tiny wrapper functions to Support.h that encapsulate the actual Zeek
functionality and move the includes to Support.cc instead. This will
cause frequent Support.cc rebuilds, but that is decently fast.
This comes at the expense of one indirect function call for some ops, but
its overhead should be negligible in comparison to a lookup for a
connection or component or a log_mgr->Write() call.
This allows to leverage automatic memory management, less allocations
and using move semantics for expressing ownership.
This breaks the existing logging and broker API, but keeps the plugin
DoWrite() and HookLogWrite() methods functioning.
It further changes ValToLogVal to return a threading::Value rather than
a threading::Value*. The vector_val and set_val fields unfortunately
use the same pointer-to-array-of-pointers approach. this can'tbe changed
as it'd break backwards compatibility for plugin provided input readers
and log writers.
Touching broker/Manager.h or logging/Manager.h or anything that's
included in ZBody.h results in a rebuild of a bunch of files in ZAM.
Particularly, ZAM/Validate.cc takes much too long to recompile for
me (45+ seconds) and apparently that's mostly by accident.
Clean up the ZAM/Validate.cc includes to prevent this.
The current implementation would only log, if the password contains a
colon, the part before the first colon (e.g., the password
`password:password` would be logged as `password`).
A test has been added to confirm the expected behaviour.
The reason that this is necessary is the end-of-connection-handling of
spicy. If spicy is in the middle of parsing some bytes while the
connection ends, an error is raised. This behavior cannot be changed,
and means that there will be a DPD-log entry, etc. for connections that
are completely valid TLS connections - that just happen to be truncated
and end in the middle.
* origin/master:
Update doc submodule [nomail] [skip ci]
Analyzer: Do not add child analyzers when finished
Fix parsing of version field in SSLv2 client hello
TCP_Reassembler: Fix IsOrig() position in Match() call
Spicy: Register well-known ports through an event handler.
Update doc submodule [nomail] [skip ci]
Revert "Remove deprecated port/ports fields for spicy analyzers"
Make ssl_history work for SSLv2 handshakes/connections
Depending on an analyzer's implementation, its Done() method may
attempt to access analyzer or connection state when executing.
When this happens in the destructor of the parent analyzer during
the process of destructing a connection, this state may have been
deleted, resulting in use-after-free crashes or worse memory
corruption.
The following cases have been observed in the wild for when this happens.
* PIA matching during Done() for undelivered TCP data enables a Spicy
based analyzer which in turn attempts to raise an analyzer violation
during Done()->EndOfData().
* Spicy analyzers attaching new analyzers during their Done() processing
which in turn attempt to use TCP() (to call FindChild()) during Done()
while the analyzer tree / connection is being destructed.
The second scenario was previously found to happen in the HTTP analyzer
and fixed with 6ef9423f3c.
Plug these scenarios by short-circuiting AddChildAnalyzer() if the analyzer
or connection have finished or are being finished.
It turns out that, for probably a long time, we have reported an
incorrect version when parsing an SSLv2 client hello. We always reported
this as SSLv2, no matter which version the client hello actually
contained.
This bug probably went unnoticed for a long time, as SSLv2 is
essentially unused nowadays, and as this field does not show up in the
default logs.
This was found due to a baseline difference when writing the Spicy SSL
analyzer.
This avoids the earlier problem of not tracking ports correctly in
scriptland, while still supporting `port` in EVT files and `%port` in
Spicy files.
As it turns out we are already following the same approach for file
analyzers' MIME types, so I'm applying the same pattern: it's one
event per port, without further customization points. That leaves the
patch pretty small after all while fixing the original issue.
* origin/master: (60 commits)
Update gen-zam submodule [nomail] [skip ci]
Update doc submodule [nomail] [skip ci]
Remove unused wrapper packet analyzer
Add DNS TKEY event
ScriptOpt: Ensure global statements have non-null scope
simpler and more robust identification of function parameters for AST profiling
fixes to limit AST traversal in the face of recursive types
address some script optimization compiler warnings under Linux
fix for -O C++ construction of variable names that use multiple module namespaces
fix for script optimization of "opaque" values that are run-time constants
fix for script optimization of nested switch statements
script optimization fix for complex "in" expressions in conditionals
updates to typos allow-list reflecting ZAM regularization changes
BTest updates for ZAM regularization changes
convert new ZAM operations to use typed operands
complete migration of ZAM to use only public ZVal methods
"-O validate-ZAM" option to validate generated ZAM instructions
internal option to suppress control-flow optimization
exposing some functionality for greater flexibility in structuring run-time execution
rework ZAM compilation of type switches to leverage value switches
...
* origin/topic/johanna/ssl-history-also-for-sslv2-not-only-for-things-that-use-the-more-modern-handshake:
Make ssl_history work for SSLv2 handshakes/connections
The ProfileFunc() logic assumed that GetScope() returned a non-nullptr.
This holds except for the synthetic global statements function.
Fix the latter and add an assert, also add a name to the type so it's
easier to recognize in a debugger what's going on, otherwise the name
is "".
This was found by UBSAN due to it seeing the ->OrderedVars() call on a
nullptr. Elsewhere, num_params == 0 shielded from that access and so
didn't lead to crashes.
* origin/topic/vern/zam-regularization: (33 commits)
simpler and more robust identification of function parameters for AST profiling
fixes to limit AST traversal in the face of recursive types
address some script optimization compiler warnings under Linux
fix for -O C++ construction of variable names that use multiple module namespaces
fix for script optimization of "opaque" values that are run-time constants
fix for script optimization of nested switch statements
script optimization fix for complex "in" expressions in conditionals
updates to typos allow-list reflecting ZAM regularization changes
BTest updates for ZAM regularization changes
convert new ZAM operations to use typed operands
complete migration of ZAM to use only public ZVal methods
"-O validate-ZAM" option to validate generated ZAM instructions
internal option to suppress control-flow optimization
exposing some functionality for greater flexibility in structuring run-time execution
rework ZAM compilation of type switches to leverage value switches
add tracking of control flow information
factoring of ZAM operation specifications into separate files
updates to ZAM operations / gen-zam regularization, other than the operations themselves
type-checking fix for vector-of-string operations
ZVal constructor for booleans
...
The variadic broker messaging BIFs currently convert @ARGS@ into a
ValPList before passing it on to MakeEvent(). This appears historic
plumbing. Implement the same functionality using Span<const ValPtr>
and do the extra copying in the now deprecated MakeEvent().
Further, make passing a frame optional as not all callers may
have one available.
* origin/topic/vern/cpp-init:
Func: Add SetCapturesVec()
marked some recently added BTests as not suitable for -O gen-C++ testing
robustness improvements for -O gen-C++ generation of lambdas / "when"s
speedups for compilation of initializers in -O gen-C++ generated code
fixes for -O gen-C++ generation of floating point constants
-O gen-C++ fix for dealing with use of more than one module qualifier
header tweaks to provide gen-C++ script optimization with more flexibility
fix for script optimization of constants of type "opaque"
fix for script optimization of "in" operations
some minor tidying of -O gen-C++ sources
Add an API to directly set captures_vec for use by C++ compilation. The
current code keys off or asserts on ZAM stmts, making it difficult to
leverage captures_vec in other contexts.
* origin/topic/awelzel/3880-mysql-caching-sha2-password-successful:
mysql: Implement and test COM_CHANGE_USER
mysql: Remove auth plugin state
mysql: Handle server connection phase separately from command phase
This reworks the parser such that COM_CHANGE_USER switches the
connection back into the CONNECTION_PHASE so that we can remove the
EXPECT_AUTH_SWITCH special case in the COMMAND_PHASE. Adds two pcaps
produced with Python that actually do COM_CHANGE_USER as it seems
not possible from the MySQL CLI.
Now that we use CONNECTION_PHASE and interpret AuthSwitchRequest
correctly, we can remove reliance on the plugin discrepancy to
determine expectations.
It turns out that the ssl_history field never was populated with C/S for
SSLv2 connections, or connections using the SSLv2 handshake. In our
testcases, the latter is especially common - with connections up to TLS1
using the old SSLv2 client hello for backwards compatibility.
This change resolves this issue. As the history is not by default
enabled in a lot of locations, baseline impact is minor.
With this commit, the entire Zeek test suite passes using spicy TLS.
Tests that either use a SSLv2 handshake, or DTLS are skipped, as the
parser currently does not support either.
Similarly, tests that rely on behavior we cannot replicate (baseline,
hooks, exact error messages) are passed. Other than that, all the
TLS-based tests pass with 100% the exact same baseline results.
This necessitated a couple of small tweaks to the spicy file - the
testcases uncovered several small problems.
This commit also enables cirrus tests for Spicy SSL/TLS.
* jgras/topic/jgras/packet-analyzer-history-rework:
Deprecate old unknown_protocol event signature
Use raw pointer for packet analyzer history
Update external baselines for analyzer history
Update btest baselines for analyzer history
Add packet analyzer history
The ccache version shipped with Ubuntu 24.04 does not yet recognize
--fprofile-update=atomic, install one that does.
Now that the asan_sanitizer build also includes building Spicy and
running the spicyz test suite, ccache is quite important.
Reference ccache/ccache#1408 and zeek/zeek#3777.
* origin/topic/awelzel/threading-manager-metrics-follow-up:
telemetry/Manager: Check RegisterFd() return value
telemetry/Manager: Track sent_in and sent_out totals without callback
threading/Manager: Switch inf bucket from infinity() to max()
threading/Manager: "lt" to "le" and do not break
UBSAN's vptr sanitize isn't happy with the call to _fmt()
in its member initializer list.
$ zeek -r Traces/ssh/single-conn.trace .tmp/spicy.event-args-mismatch/test.hlto .tmp/spicy.event-args-mismatch/event-args-mismatch.zeek
<...>/src/include/zeek/spicy/runtime-support.h:80:29: runtime error: member call on address 0x511000369540 which does not point to an object of type 'zeek::spicy::rt::ParameterMismatch'
0x511000369540: note: object has invalid vptr
00 00 00 00 be be be be be be be be be be be be be be be be be be be be be be be be be be be be
^~~~~~~~~~~~~~~~~~~~~~~
invalid vptr
#0 0x7f9c9977b019 in zeek::spicy::rt::ParameterMismatch::ParameterMismatch(std::basic_string_view<char, std::char_traits<char>>, zeek::IntrusivePtr<zeek::Type> const&, std::basic_string_view<char, std::char_traits<char>>) <...>/src/include/zeek/spicy/runtime-support.h:80:29
#1 0x7f9c9977a6a2 in zeek::spicy::rt::to_val(hilti::rt::Bytes const&, zeek::IntrusivePtr<zeek::Type> const&) <...>/src/include/zeek/spicy/runtime-support.h:562:15
Initial fuzzing caused a bind response to arrive before a bind request,
resulting in an unset field expression error:
expression error in base/protocols/ldap/main.zeek, line 270: field value missing (LDAP::m$opcode)
Prevent this by ensuring m$opcode is set and raising instead.
LDAP supports both, UDP and TCP as separate analyzers. The corpus
is identical, however. Started to hit the TLS analyzer fairly
quickly, too.
Closes#3860
The current test attempts to instantiate two spicy::SSH_1 protocol
analyzers in the .evt file. The intention likely was to use two
distinct protocol analyzer both trying to replace the builtin SSH
analyzer.
Coincidentally, fixing this happens to workaround TSAN errors tickled
by the FatalError() call while loading the .hlto with two identically
named analyzers.
$ cat .tmp/spicy.replaces-conflicts/output
error: redefinition of protocol analyzer spicy::SSH_1
ThreadSanitizer: main thread finished with ignores enabled
One of the following ignores was not ended (in order of probability)
Ignore was enabled at:
#0 __llvm_gcov_init __linker___d192e45c25d5ee23-484d3e0fc2caf5b4.cc (ssh.hlto+0x34036) (BuildId: 091934ca4da885e7)
#1 __llvm_gcov_init __linker___d192e45c25d5ee23-484d3e0fc2caf5b4.cc (ssh.hlto+0x34036) (BuildId: 091934ca4da885e7)
...
I was tempted to replace FatalError() with Error() and rely on
zeek-setup.cc's early exiting on any reporter errors, but this
seems easier for now.
Relates to #3865.
For terminated threads, the totals would go down once the threads are
removed, which isn't great. Move tracking of sent in and sent out
messages from callback to explicit `Inc()` calls.
Also fixes total_messages_in_metric being initialized twice rather
than total_messages_out_metric.
For uint64_t, std::numeric_limits<T>::has_infinity is false and infinity()
actually returns 0. Use uint64_t's max() instead. We could cast to double
and use the double infinity, but this seems reasonable, too.
This was found while trying to provoke some pending messages and being
confused why all but the "inf" bucket increased.
The buckets are specified as lower-equal (changed from lower-than now),
which means we shouldn't break: The larger "le" bucket contains all previous
buckets, too. The "inf" bucket represents the current number of threads.
For example, with a total of 10 threads, 5 threads with 0 messages pending,
another 4 threads with 50 messages, and on with 2000 messages, the metrics
would end end up as follows:
pending_buckets{le=1} = 5
pending_buckets{le=10} = 5
pending_buckets{le=100} = 9
pending_buckets{le=1000} = 9
pending_buckets{le=10000} = 10
pending_buckets{le=inf} = 10
This might be strange initially, but aligns with the Prometheus
histogram approach (though we're using gauges here).
* origin/topic/timw/3081-prof-log-metrics:
Remove some unnecessary #includes
Avoid capturing 'this' for callback in telemetry::Manager
Make telemetry metrics out of MsgThread statistics
Split cpu time metric into user/system components like prof.log
Move broker statistics to be telemetry metrics
Add timer counts as telemetry metrics
Move thread manager stats to telemetry metric
Add extra metrics to session_mgr
Move dns_mgr stats to telemetry instruments
Move trigger stats to telemetry instruments
Move pulling of global state inside 'expensive' check for stats
This avoids the callbacks from being processed on the worker thread
spawned by Civetweb. It fixes data race issues with lookups involving
global variables, amongst other threading issues.
The ctu-sme-11-win7ad-1-ldap-tcp-50041.pcap file was harvested
from the CTU-SME-11 (Experiment-VM-Microsoft-Windows7AD-1) dataset
at https://zenodo.org/records/7958259 (DOI 10.5281/zenodo.7958258).
Closes#3853
* origin/topic/awelzel/mysql-amazing-pp-issue-2716-additions:
mysql: Simplify length computation
mysql: Improve date and time parsing
btest/mysql: Clean query-attr.pcapng
mysql: Support non-string query attributes
btest/mysql: Add pcap with non-string query attributes
mysql: Introduce mysql_ssl_request event
mysql: Fix EOFIfLegacyThenResultSet
mysql: Add data parameter to mysql_auth_plugin
mysql: Add mysql_auth_plugin, mysql_auth_more_data and mysql_auth_switch_request events
mysql: AuthSwitchRequest: &enforce a 0xfe / 254 status
mysql: Make auth_plugin_ a std::string
mysql: Fix auth_plugin_data_part2 length computation
Refactored connection phase state handling
Add support for "auth switch" and "query attrs"
Add support for parsing the "caching_sha2_password" auth plugin
Pcap was generated as follows. Doesn't seem wireshark even parses
this properly right now.
with common.get_connection() as c:
with c.cursor() as cur:
date1 = datetime.date(1987, 10, 18)
datetime1 = datetime.datetime(1990, 9, 26, 12, 13, 14)
cur.add_attribute("number1", 42)
cur.add_attribute("string1", "a string")
cur.add_attribute("date1", date1)
cur.add_attribute("datetime1", datetime1)
cur.execute("SELECT version()")
result = cur.fetchall()
print("result", result)
Remove caching_sha2_password parsing/state from the analyzer and implement
the generic events. If we actually want to peak into the authentication
mechanism, we could write a separate analyzer for it. For now, treat it
as opaque values that are exposed to script land.
The added tests show the --get-server-public-key in use where
mysql_auth_more_data contains an RSA public key.
* origin/topic/vern/script-opt-maint.Aug24:
minor optimization of boolean comparisons
fix & regression test for GH-3839 (spurious warnings for "when" constructs)
The analyzer now detects partial connections at the beginning of a
connection - and will skip them. This makes behavior more similar to the
binpac analyzer.
The decryption test is skipped.
And some minor refacoring.
Instead of dissecting the GSSAPI handshake, add another heuristic
into MaybeEncrypted to check for the WRAP token identifier.
After this change, the pcap on the following ticket is processed
nicely: https://gitlab.com/wireshark/migration-test/-/issues/9398
@dopheide-esnet provided sample captures where SASL SRP is used as
a SASL mechanism and the follow-up LDAP messages are encrypted. It's
not clear how to determine whether encryption will or will not happen,
so re-add a heuristic to determine this based on the first byte of
the first message *after* the successful bindResponse handshake. If
that byte is 0x30, assume cleartext.
I haven't been able to produce such pcaps, unfortunately, but the
cleartext path is tested via the existing sasl-ntlm.pcap.
PCAP was produced with a local OpenLDAP server configured to support StartTLS.
This puts the Zeek calls into a separate ldap_zeek.spicy file/module
to separate it from LDAP.
* origin/master:
Update broker submodule [nomail]
telemetry: Deprecate prometheus.zeek policy script
input/Manager: Improve type checks of record fields with type any
Bump zeek-testing-cluster to pull in tee SIGPIPE fix
ldap: Remove MessageWrapper with magic 0x30 searching
ldap: Harden parsing a bit
ldap: Handle integrity-only KRB wrap tokens
Bump auxil/spicy to latest development snapshot
CI: Set FETCH_CONTENT_FULLY_DISCONNECTED flag for configure
Update broker and cmake submodules [nomail]
Fix a broken merge
Do not emit hook files for builtin modules
Fix warning about grealpath when running 'make dist' on Linux
Start of 7.1.0 development
Updating submodule(s) [nomail]
Update the scripts.base.frameworks.telemetry.internal-metrics test
Revert "Temporarily disable the scripts/base/frameworks/telemetry/internal-metrics btest"
Bump Broker to pull in new Prometheus support and pass in Zeek's registry
Do not emit hook files for builtin modules
With Cluster::Node$metrics_port being optional, there's not really
a need for the extra script. New rule, if a metrics_port is set, the
node will attempt to listen on it.
Users can still redef Telemetry::metrics_port *after*
base/frameworks/telemetry was loaded to change the port defined
in cluster-layout.zeek.
This unit implements a heuristic to search for the 0x30 sequence
byte if Message couldn't readily be parsed. Remove it with the
idea of explicit and predictable support for SASL mechanisms.
ASN1Message(True) may go off parsing arbitrary input data as
"something ASN.1" This could be GBs of octet strings or just very
long sequences. Avoid this by open-coding some top-level types expected.
This also tries to avoid some of the &parse-from usages that result
in unnecessary copies of data.
Adds a locally generated PCAP with addRequest/addResponse that we
don't currently handle.
Mostly staring at the PCAPs and opened a few RFCs. For now, only if the
MS_KRB5 OID is used and accepted in a bind response, start stripping
KRB5 wrap tokens for both, client and server traffic.
Would probably be nice to forward the GSS-API data to the analyzer...
Closeszeek/spicy-ldap#29.
Despite already having an accessor, X509_ALGOR wasn't made opaque
during OpenSSL 1.1.0 development. It would be nice if this could be
fixed at some point, so avoid reaching into that struct by using the
accessor
This patch bump Spicy to the latest development snapshot. This
introduces a backwards-incompatible change in that it removes support
for a never officially supported syntax to specify unit fields (so I
would argue: not strictly a breaking change).
I merged an old version of the branch on accident and then merged the right
one over top of it, but git ended up including both versions. This fixes
that mistake.
We would previously emit a C++ file with hooks for at least the builtin
`spicy` module even though that module like any other builtin module
never contains implementations of hooks for types in user code.
This patch prevents modules with skipped implementations (such as our
builtin modules) from being added to the compilation which prevents
generating their hook files.
* origin/master: (89 commits)
Update doc submodule [nomail] [skip ci]
Bump cmake submodule [nomail]
testing/btest: Default to HILTI_JIT_PARALLELISM=1
Revert "CI: Use ccache and a single CPU when building spicy analyzers for btests"
Update doc submodule [nomail] [skip ci]
CI: Use ccache and a single CPU when building spicy analyzers for btests
Extend btest for logging of disabled analyzers
Update zeek-aux submodule [nomail]
Add logging of disabled analyzers to analyzer.log
Bump auxil/spicy to latest development snapshot
Management framework: bump cluster testsuite to pull in telemetry tests
Management framework: bump zeek-client
Management framework: augment deployed configs with instance IP addresses
Management framework: add auto-enumeration of metrics ports
Management framework: propagate metrics port from agent
Management framework: add metrics port in management & Supervisor node records
Harden the telemetry manager against unset Telemetry::metrics_address
Comment-only tweaks for telemetry-related settings.
Fix for --display-cmake in configure Moved build directory creation further down in the script so that --display-cmake has a chance to happen before build tree setup.
Update submodules [nomail]
...
We would previously emit a C++ file with hooks for at least the builtin
`spicy` module even though that module like any other builtin module
never contains implementations of hooks for types in user code.
This patch adds a blocklist of builtin modules which are skipped for
generating hook files.
Spicy SSL is now only enabled when specifying the --enable-spicy-ssl
configure-time option.
This should allow merging this into Zeek on an experimental basis.
* origin/master: (173 commits)
Bump Spicy to latest dev snapshot
Update docs submodule [nomail] [skip ci]
Add type aliases for instrument and family shared_ptrs
Update NEWS for double and is_sum changes
Remove is_sum arguments from counters and gauges
Change all instruments to only handle doubles
Add comment to telemetry::Manager::InitPostScript
Remove all of the ZEEK_METRICS_ environment variables
Fix header comments in scripts/policy/frameworks/telemetry/prometheus.zeek
Change all prometheus #includes to use angle brackets
Update zeekctl submodule for metrics_port node.cfg option
Regenerate docs [nomail]
Remove the is_sum argument from BIF histogram creation methods
Update NEWS for Telemetry rework
Remove Telemetry::metrics_export_prefixes option
Validate that label names are constant in non-zeek metrics
Avoid calling Collect() in counter/gauge Value() methods if not needed
Fix some determinism issues with btests
Temporarily disable the scripts/base/frameworks/telemetry/internal-metrics btest
Fix the scripts.policy.frameworks.telemetry.prometheus btest to use the service discovery endpoint
...
* origin/master: (352 commits)
Bump Spicy.
Remove support for old Spicy versions from QUIC analyzer.
Make sure that vcpkg isn't preferred if pcap_root_dir is passed in
Remove some unused Spicy state.
Bump Spicy.
ZAM fix for concretizing vectors in record constructors
improve ZAM's estimation of profiling overheads
CI: Remove commented openssl 1.1 workaround from macOS preparation script
CI: Fix installation of python package on macOS
Address review feedback for configure error change
Raise configure error message for unsupported archives
fix ZAM "cat" of doubles/times to include trailing ".0" per normal BiF behavior
CI: Specify the xcode version of the macOS Sonoma instance
Remove vestigial Conan bit in CMakeLists.txt
When configuring Spicy, be prepated for zeek_lib or zeek_exe targets.
Fix a typo in CMakeLists.txt when building Zeek as a library
Bump Spicy to current `main`.
tie into updates to gen-zam
ZAM documentation updated to reflect finer-grained profiling
ZAM-specific BTest baseline changes for tweak to how ZAM bodies print
...
* origin/master: (73 commits)
Spicy: allow providing file id in zeek::file_begin
Spicy: allow providing file id in zeek::file_begin
Update doc submodule [nomail] [skip ci]
Spicy: allow providing file id in zeek::file_begin
Spicy: allow providing file id in zeek::file_begin
Bump cmake submodule for INCLUDE_DIRS fix
Update doc submodule [nomail] [skip ci]
Integrate review feedback
Add jq to final.Dockerfile
TableVal: Replace raw subnets/pattern_matcher with unique_ptr
TablePatternMatcher: Drop Insert()/Remove(), use Clear()
TableType: Simplify and inline Is...Index tests
NEWS: Add small table[pattern] section
Expr/Val: Add support for in set[pattern]
zeek.bif: Implement table_pattern_matcher_stats() bif for introspection
DFA: Extract inner Stats struct from DFA_State_Cache
Expr: Implement string in table[pattern] of X
RE: Remove RE_DisjunctiveMatcher and re-use MatchAll()
TablePatternMatcher: Use const StringValPtr& instead of const StringVal*
Val: Move TablePatternMatcher into detail namespace
...
Parsing of client/server hellos that do not contain extensions should
now work correctly.
The port registration is now done Zeek-side, wich fixes some test
failures.
Naming the analyzer different than the old one was a mistake that
required unnecessary code changes; keeping the old name makes things
like StartTLS in other protocol work without additional code changes.
* origin/master:
Update doc submodule [nomail] [skip ci]
build_inner_connection: Use the outer packet's timestamp
build_inner_connection: Avoid one extra Init()
packet_analysis: Do not run DetectProtocol() on disabled analyzers
packet_analysis/Dispatcher: Do not index table twice
packet_analysis: Avoid shared_ptr copying for analyzer lookups
Update doc submodule [nomail] [skip ci]
Update doc submodule [nomail] [skip ci]
SSL: Add new extension types and ECH test
Update `.git-blame-ignore-revs`
Format JSON with clang-format
Bump pre-commit hooks
Reformat Zeek in Spicy style
* origin/master: (386 commits)
Normalize version strings in test
Update doc submodule [nomail] [skip ci]
Update external testing baseline hashes
fuzzers: Add DTLS fuzzer
generic-analyzer-fuzzer: Support NextPacket() fuzzing
Require `truncate` for a test using it
Bump outdated baseline
Fix tests so they work both with GNU and BSD tools
Install libmaxminddb in macOS CI
Bump auxil/spicy to latest release
Supervisor: Handle EAGAIN error on stem pipe
fuzzer-setup: Allow customization without recompiling
ssl: Prevent unbounded ssl_history growth
ssl: Cap number of alerts parsed from SSL record
subdir-btest: Allow setting build_dir
Update doc submodule [nomail] [skip ci]
CI: Pass -A flag to btest for cluster-testing builds
Update doc submodule [nomail] [skip ci]
Update baselines
ftp: Do not base seq on number of pending commands
...
This commit mostly changes the ordering of some the events, so that they
fit the event order of the old analyzer (and also are more
representative of the order of how things are happening on the wire).
* origin/master: (82 commits)
Update doc submodule [nomail] [skip ci]
Update zeekjs submodule
Update broker submodule
telemetry: Disable metrics centralization by default
GSSAPI: basic support for MIC/WRAP tokens
CI: Force rebuild of tumbleweed VM to pick up newer version of python
formatters/JSON: Make JSON::NullDoubleWriter use zeek::json::detail version
formatters/JSON: Remove surrounding_braces
formatters/JSON: Prepare to remove rapidjson from installed Zeek headers
Stmt/Assert: Use std::move() for backtrace element
Update cmake submodule [nomail]
Update external testing hashes for 6.1 deprecation changes
Remove leftover baseline files from 6.1 deprecation removal
Update doc submodule [nomail] [skip ci]
Update external testing repo hashes to current master commits
Update dce-rpc constants
-O gen-C++ refinements for BiF failures, negative vector indices, boolean vector operations
Update docs submodule [nomail] [skip ci]
Force -std=c++17 mode for plugin targets, remove use of RequireCXX17.cmake
Add Spicy updates to NEWS for 6.0.
...
* origin/master: (200 commits)
Update link to slack in README.md
Update bifcl, binpac, and broker repos for cmake changes
bison_target argument changes from HEADER to DEFINES_FILE
Update cmake submodule [nomail]
Start of 6.1.0 development
Switch broker submodule back to master
Update broker submodule to 2.6.0 release tag [nomail] [skip ci]
Update docs submodule [nomail] [skip ci]
Updating CHANGES and VERSION.
Update baseline for coverage.test-all-policy-cluster test
catch-and-release: Mark cr_check_rule as is_used
Add test-all-policy-cluster
Revert "Merge remote-tracking branch 'origin/topic/vern/at-if-analyze'"
Simplify code generated for Spicy analyzer port ranges.
Register test analyzer only for required ports.
Update doc submodule [nomail] [skip ci]
Update broker submodule [nomail]
Update ZeekJS submodule to 0.9.1
Fix disappearing unit fields in Spicy type export.
Bump cluster testsuite to latest main
...
* origin/master: (139 commits)
Given the -C flag, set script-layer ignore_checksums to true.
Add btest for "-C" flag vs the script-layer ignore_checksums global.
Update doc submodule [nomail] [skip ci]
Remove references to bro_broker in broker/Manager.h
cmake: Fixup BRO_PLUGIN_INSTALL_PATH references
testing/external: Bump hashes for community_id addition
NEWS: Add entry for Community ID
policy: Import zeek-community-id scripts into protocols/conn frameworks/notice
Add irc_dcc_send_ack event and fix missing fields
Fix install directory for plugins
Update doc submodule [nomail] [skip ci]
Add community_id_v1() based on corelight/zeek-community-id
Update NEWS to cover cluster framework changes.
Add cluster_started restart tests.
Add basic cluster_started tests.
Add cluster_started and node_fully_connected events.
Add hook into cluster connection setup.
Add broadcast_topics set.
Generalize Cluster::worker_count.
Edit pass over the current 6.0 NEWS entries. [nomail] [skip ci]
...
Tencent is pleased to support the open source community by making RapidJSON available.
Copyright (C) 2015 THL A29 Limited, a Tencent company, and Milo Yip. All rights reserved.
If you have downloaded a copy of the RapidJSON binary from Tencent, please note that the RapidJSON binary is licensed under the MIT License.
If you have downloaded a copy of the RapidJSON source code from Tencent, please note that RapidJSON source code is licensed under the MIT License, except for the third-party components listed below which are subject to different license terms. Your integration of RapidJSON into your own projects may require compliance with the MIT License, as well as the other licenses applicable to the third-party components included within RapidJSON. To avoid the problematic JSON license in your own projects, it's sufficient to exclude the bin/jsonchecker/ directory, as it's the only code under the JSON license.
A copy of the MIT License is included in this file.
Other dependencies and licenses:
Open Source Software Licensed Under the BSD License:
Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
* Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.
* Neither the name of copyright holder nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission.
THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS AND CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
Open Source Software Licensed Under the JSON License:
Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
The Software shall be used for Good, not Evil.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
echo "# This is the command used to configure this build" >config.status
echo "# This is the command used to configure this build" >config.status
echo $command >>config.status
echo $command >>config.status
chmod u+x config.status
chmod u+x config.status
if [ $has_disable_archiver -eq 1 ]; then
echo
echo "NOTE: The --disable-archiver argument no longer has any effect and will be removed in v7.1. zeek-archiver is now part of zeek-aux, so consider --disable-auxtools instead."
TIME_MACHINE &deprecated="Remove in v7.1: Unused.",
};
};
## Record type to indicate a node in a cluster.
## Record type to indicate a node in a cluster.
@ -187,12 +189,8 @@ export {
## The port that this node will listen on for peer connections.
## The port that this node will listen on for peer connections.
## A value of ``0/unknown`` means the node is not pre-configured to listen.
## A value of ``0/unknown`` means the node is not pre-configured to listen.
p: port &default=0/unknown;
p: port &default=0/unknown;
## Identifier for the interface a worker is sniffing.
interface: string &optional &deprecated="Remove in v7.1: interface is not required and not set consistently on workers. Replace usages with packet_source() or keep a separate worker-to-interface mapping in a global table.";
## Name of the manager node this node uses. For workers and proxies.
## Name of the manager node this node uses. For workers and proxies.
manager: string &optional;
manager: string &optional;
## Name of a time machine node with which this node connects.
time_machine: string &optional &deprecated="Remove in v7.1: Unused.";
## A unique identifier assigned to the node by the broker framework.
## A unique identifier assigned to the node by the broker framework.
## This field is only set while a node is connected.
## This field is only set while a node is connected.
id: string &optional;
id: string &optional;
@ -257,10 +255,17 @@ export {
## of the cluster that is started up.
## of the cluster that is started up.
const node = getenv("CLUSTER_NODE") &redef;
const node = getenv("CLUSTER_NODE") &redef;
## Function returning this node's identifier.
##
## By default this is :zeek:see:`Broker::node_id`, but can be
## redefined by other cluster backends. This identifier should be
## a short lived identifier that resets when a node is restarted.
global node_id: function(): string = Broker::node_id &redef;
## Interval for retrying failed connections between cluster nodes.
## Interval for retrying failed connections between cluster nodes.
## If set, the ZEEK_DEFAULT_CONNECT_RETRY (given in number of seconds)
## If set, the ZEEK_DEFAULT_CONNECT_RETRY (given in number of seconds)
## environment variable overrides this option.
## environment variable overrides this option.
const retry_interval = 1min &redef;
const retry_interval = 1sec &redef;
## When using broker-enabled cluster framework, nodes broadcast this event
## When using broker-enabled cluster framework, nodes broadcast this event
## to exchange their user-defined name along with a string that uniquely
## to exchange their user-defined name along with a string that uniquely
@ -285,7 +290,7 @@ export {
##
##
## Returns: a topic string that may used to send a message exclusively to
## Returns: a topic string that may used to send a message exclusively to
## a given cluster node.
## a given cluster node.
global node_topic: function(name: string): string;
global node_topic: function(name: string): string &redef;
## Retrieve the topic associated with a specific node in the cluster.
## Retrieve the topic associated with a specific node in the cluster.
##
##
@ -294,9 +299,126 @@ export {
##
##
## Returns: a topic string that may used to send a message exclusively to
## Returns: a topic string that may used to send a message exclusively to
## a given cluster node.
## a given cluster node.
global nodeid_topic: function(id: string): string;
global nodeid_topic: function(id: string): string &redef;
## Retrieve the cluster-level naming of a node based on its node ID,
## a backend-specific identifier.
##
## id: the node ID of a peer.
##
## Returns: the :zeek:see:`Cluster::NamedNode` for the requested node, if
## known, otherwise a "null" instance with an empty name field.
global nodeid_to_node: function(id: string): NamedNode;
## Initialize the cluster backend.
##
## Cluster backends usually invoke this from a :zeek:see:`zeek_init` handler.
##
## Returns: T on success, else F.
global init: function(): bool;
## Subscribe to the given topic.
##
## topic: The topic to subscribe to.
##
## Returns: T on success, else F.
global subscribe: function(topic: string): bool;
## Unsubscribe from the given topic.
##
## topic: The topic to unsubscribe from.
##
## Returns: T on success, else F.
global unsubscribe: function(topic: string): bool;
## An event instance for cluster pub/sub.
##
## See :zeek:see:`Cluster::publish` and :zeek:see:`Cluster::make_event`.
type Event: record {
## The event handler to be invoked on the remote node.
ev: any;
## The arguments for the event.
args: vector of any;
};
## The TLS options for a WebSocket server.
##
## If cert_file and key_file are set, TLS is enabled. If both
## are unset, TLS is disabled. Any other combination is an error.
type WebSocketTLSOptions: record {
## The cert file to use.
cert_file: string &optional;
## The key file to use.
key_file: string &optional;
## Expect peers to send client certificates.
enable_peer_verification: bool &default=F;
## The CA certificate or CA bundle used for peer verification.
## Empty will use the implementations's default when
## ``enable_peer_verification`` is T.
ca_file: string &default="";
## The ciphers to use. Empty will use the implementation's defaults.
ciphers: string &default="";
};
## WebSocket server options to pass to :zeek:see:`Cluster::listen_websocket`.
type WebSocketServerOptions: record {
## The address to listen on, cannot be used together with ``listen_host``.
listen_addr: addr &optional;
## The port the WebSocket server is supposed to listen on.
