While working on a rotation format function, ran into Zeek crashing
when not returning a value from it, fix and recover the same way as
for scripting errors.
* security/topic/awelzel/152-smtp-validate-mail-transactions:
smtp: Validate mail transaction and disable SMTP analyzer if excessive
generic-analyzer-fuzzer: Detect disable_analyzer() from scripts
* security/topic/awelzel/148-ftp-skip-get-pending-commands-multi-line-response:
ftp/main: Special case for intermediate reply lines
ftp/main: Skip get_pending_command() for intermediate reply lines
The medium.trace in the private external test suite contains one
session/server that violates the multi-line reply protocol and
happened to work out fairly well regardless due to how we looked
up the pending commands unconditionally before.
Continue to match up reply lines that "look like they contain status codes"
even if cont_resp = T. This still improves runtime for the OSS-Fuzz
generated test case and keeps the external baselines valid.
The affected session can be extracted as follows:
zcat Traces/medium.trace.gz | tcpdump -r - 'port 1491 and port 21'
We could push this into the analyzer, too, minimally the RFC says:
> If an intermediary line begins with a 3-digit number, the Server
> must pad the front to avoid confusion.
Increasing this value 10x has lowered CPU usage on a Myricom based
deployment significantly with reportedly no adverse side-effects.
After reviewing the Zeek 3 IO loop, my hunch is that previously when
no packets were available, we'd sleep 20usec every loop iteration after
calling ->Process() on the packet source. With current master ->Process()
is called 10 times on a packet source before going to sleep just once
for 20 usec. Likely this explains the increased CPU usage reported.
It's probably too risky to increase the current value, so introduce
a const &redef value for advanced users to tweak it. A middle ground
might be to lower ``io_poll_interval_live`` to 5 and increase the new
``Pcap::non_fd_timeout`` setting to 100usec.
While this doesn't really fix#2296, we now have enough knobs for tweaking.
Closes#2296.
An invalid mail transaction is determined as
* RCPT TO command without a preceding MAIL FROM
* a DATA command without a preceding RCPT TO
and logged as a weird.
The testing pcap for invalid mail transactions was produced with a Python
script against a local exim4 configured to accept more errors and unknown
commands than 3 by default:
# exim4.conf.template
smtp_max_synprot_errors = 100
smtp_max_unknown_commands = 100
See also: https://www.rfc-editor.org/rfc/rfc5321#section-3.3
Intermediate lines of multiline replies usually do not contain valid status
codes (even if servers may opt to include them). Their content may be anything
and likely unrelated to the original command. There's little reason for us
trying to match them with a corresponding command.
OSS-Fuzz generated a large command reply with very many intermediate lines
which caused long processing times due to matching every line with all
currently pending commands.
This is a DoS vector against Zeek. The new ipv6-multiline-reply.trace and
ipv6-retr-samba.trace files have been extracted from the external ipv6.trace.
Add a central place where the decision when it's okay to update network time
to the current time (wallclock) is. It checks for pseudo_realtime and packet
source existence as well as packet source idleness.
A new const &redef allows to completely disable forwarding of network time.
This probably should not be changed by users, but it's useful for
testing and experimentation rather than needing to recompile.
Processing 100 packets without checking an FD based IO source can
actually mean that FD based sources are never checked during a read
of a very small pcap...
There was a misunderstanding whether to include them by default in
the dns.log, so remove them again.
There had also been a discussion and quirk that AD of a request would
always be overwritten by reply in the dns.log unless the reply is
missing. For now, let users extend dns.log themselves for what best
fits their requirements, rather than adding these flags by default.
Add a btest to print AD and CD flags for smoke testing still.
This makes Site::private_address_space work like a subset of Site::local_nets,
to match many user's intuition of how we should treat site locality out of the
box. As config options, changes/redefs to Site::private_address_space propagate
to Site::local_nets, while changes to the latter don't affect the former.
A new global bit `Site::private_address_space_is_local` controls the behavior.
It defaults to true, and redefing to false brings back the original behavior.
* 'dnssec-flag-parse' of github.com:micrictor/zeek-codespace:
Update external testing commit hash for DNS flag changes
Parse DNSSEC AD and CD bits
Updated dump-events baseline which seemed unrelated.
Parse authentic data (AD) and checking disabled (CD) bits according to
RFC 2535. Leaves the Z field as-is, in case users are already handling
this elsewhere and depend on the value being the integer for all 3 bits.
https://www.rfc-editor.org/rfc/rfc2535#section-6.1Fixes#2672
It wasn't possible from script land to determine the total size
of the cache table held by the DNS_Mgr. Add the total and also
also the TEXT entries count.
This change exposes the signature tyope inside the signed portion of an
X.509 certificate. In the past, we only exposed the signature type that
is contained inside the signature, which is outside the signed portion
of the X.509 certificate.
In theory, both signature fields should have the same value; it is,
however, possible to encode differing values in both fields. The new
field is not logged by default.
The user and password fields are replicated to each of the ftp.log
entries. Using a very large username (100s of KBs) allows to bloat
the log without actually sending much traffic. Further, limit the
arg and reply_msg columns to large, but not unbounded values.
After the first 4 bytes, this traffic actually just looks like Ethernet.
Rather than try to re-implement the ethernet analyzer, just check the
length, skip 4 bytes, and pass it on.
When an analyzer calls DataIn(), there's a costly callback construct
going through the event queue. If an analyzer does not have a
get_file_handle() handler installed, the produced file_id would
end up empty and ignored. Consequently, the get_file_handle() callback
was invoked for every new DataIn() invocations.
This is surprising and costly. Log a warning when this happens and
instead set a generically generated file handle value instead to
prevent the repeated get_file_handle() invocations.
We previously used the Spicy plugin's `Spicy::available` to test for
Spicy support. However, having Spicy support does not necessarily mean that we
have built Zeek with its in-tree Spicy analyzers: the Spicy plugin
could have been pulled in from external. The new BIF now reliably
tells us whether the Spicy analyzers are available; its result
corresponds to what `zeek-config --have-spicy-analyzers` returns as
well.
We also move the two current checks over to use this BIF.
(Note: I refrained from renaming the CMake-side `USE_SPICY_ANALYERS`
to `HAVE_SPICY_ANALYZERS`. We should do this eventually for
consistency, but I didn't want to make more changes than necessary
right now.)
As initial examples, this branch ports the Syslog and Finger analyzers
over. We leave the old analyzers in place for now and activate them
iff we compile without any Spicy.
Needs `zeek-spicy-infra` branches in `spicy/`, `spicy-plugin/`,
`CMake/`, and `zeek/zeek-testing-private`.
Note that the analyzer events remain associated with the Spicy plugin
for now: that's where they will show up with `-NN`, and also inside
the Zeekygen documentation.
We switch CMake over to linking the runtime library into the plugin,
vs. at the top-level through object libraries.
Add configurability of synchronous and journal_mode for SQLite backed
Broker data stores. Setting these to synchronous=normal and journal_mode=wal
can significantly improve throughput at the cost of some durability in
the presence of power loss or OS crash. In the context of Zeek, this is
likely more than acceptable.
Additionally, add integrity_check and failure_mode options to support deleting
and re-opening a corrupted SQLite database at store creation.
Closes#2698
b41a4bf06d removed a field from this record
because it had a duplicate name as another field. The field does need to
exist, but it needs the correct name.
We were parsing MySQL using bigendian even though the protocol is
specified as with "least significant byte first" [1]. This is most
problematic when parsing length encoded strings with 2 byte length
fields...
Further, I think, the EOF_Packet parsing was borked, either due to
testing the CLIENT_DEPRECATE_EOF with the wrong endianness, or due to
the workaround in Resultset processing raising mysql_ok(). Introduce a
new mysql_eof() that triggers for EOF_Packet's and remove the fake
mysql_ok() Resultset invocation to fix. Adapt the mysql script and tests
to account for the new event.
This is a quite backwards incompatible change on the event level, but
due to being quite buggy in general, doubt this matters to many.
I think there is more buried, but this fixes the violation of the simple
"SHOW ENGINE INNODB STATUS" and the existing tests continue to
succeed...
[1] https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_dt_integers.html
* jeff-bb/patch-2:
Log raw keyboard value on best guess
Avoid excessive fmt calls, return default behavior on unknown
"Best Guess" unknown keyboard / language variants
When disabling_analyzer() was introduced, it was added to the GLOBAL
module. The awkward side-effect is that implementing a hook handler
in another module requires to prefix it with GLOBAL. Alternatively, one
can re-open the GLOBAL module and implement the handler in that scope.
Both are not great, and prefixing with GLOBAL is ugly, so move the
identifier to the Analyzer module and ask users to prefix with Analyzer.
Using "in" to query the language const. This also handles the case of not having a best guess and continue using the existing behavior.
Given
keyboard_layout = 1033 (0x0409), "keyboard-English - United States"
keyboard_layout = 66569 (0x00010409), "keyboard-English - United States (Best Guess)"
keyboard_layout = 12345 (0x3039), "keyboard-12345"
If the lookup table does not have an entry, it will just log as the raw decimal language/keyboard code. With this change, if we do not have an entry in the lookup table, we'll look at the low order / 4 least significant bits to see if we have a match. The high order / 4 most significant bits are flags/modifiers to the base language/keyboard code. We'll append that it is a "Best Guess"
(This is my first attempt at Zeek scripting, apologies upfront if I'm missing obvious language features. I feel like the const language lookup should return a success/fail return code that we would key off of, but unsure how to accomplish that so instead went for string matching on value in == value out).
* origin/topic/awelzel/analyzer-log:
btest/net-control: Use different expiration times for rules
analyzer: Add analyzer.log for logging violations/confirmations
By default this only logs all the violations, regardless of the
confirmation state (for which there's still dpd.log). It includes
packet, protocol and file analyzers.
This uses options, change handlers and event groups for toggling
the functionality at runtime.
Closes#2031
In certain deployment scenarios, all analyzers are disabled by default.
However, conditionally/optionally loaded scripts may rely on analyzers
functioning and declare a request for them.
Add a global set set to the Analyzer module where external scripts can record
their requirement/request for a certain analyzer. Analyzers found in this
set are enabled at zeek_init() time.
This commit adds an optional event_groups field to the Logging::Stream record
to associated event groups with logging streams.
This can be used to disable all event groups of a logging stream when it is
disabled. It does require making an explicit connection between the
logging stream and the involved groups, however.
When a fa_file object is created through the use of Input::add_analysis(),
the fa_file's source is likely not valid representation of an analyzer's
tag and a Files::describe() should not error and instead return an empty
description.
Add a new Analyzer::is_tag() helper that can be used to pre-check `f$source`.
* When a file is transferred over multiple connection, have
create_file_info() just pick the first one instead of none.
* Do not unconditionally assume cid and cuid as set on a
Notice::FileInfo object.
