These have been lingering for a while and they generally annoy
everyone because of the sheer volume. They also don't really add
any useful information for debugging and they were generated differently
than most other weirds anyway (which was a little weird...).
This commit rewrites the way that weirds are logged and fixes a number
of issues on the way. Most prominently, flow weirds now actually log
information about the flow that they occur in (before this change, they
only logged the name of the weird, which is only marginally helpful).
Besides restructuring how weird logging works internally, weirds can now
also be generated by calling Weird::weird with the info record directly,
allowing more fine-granular passing of information. This is e.g. used
for DNS weirds, which do not have the connection record available any
more when they are generated (before data like the connection ID was
just not logged in these instances).
Addresses BIT-1578
According to RFC 2535, RFC 3655, and RFC 4035, the Z field has been
partitioned into three 1-bit fields. Therefore, we cannot claim in
the documentation that it always has the value zero.
I added the $path to the create_stream() calls inside doc/ as well.
* origin/topic/jsiwek/bit-1324:
Allow logging filters to inherit default path from stream.
BIT-1324: #merged
This allows the path for the default filter to be specified explicitly
when creating a stream and reduces the need to rely on the default path
function to magically supply the path.
The default path function is now only used if, when a filter is added to
a stream, it has neither a path nor a path function already.
Adapted the existing Log::create_stream calls to explicitly specify a
path value.
Addresses BIT-1324
The "dns_TXT_reply" event now uses a "vector of strings" as the final
parameter instead of just a "string" in order to support DNS TXT
resource records that contain multiple character-strings.
The format in which the TXT answers are logged by default is now changed
to be a list of strings of the form `fmt("TXT %d %s", |str|, str)`, one
for each character-string in the RR and delimited by a space (' ')
character.
The NetBios name service RFC (1002) specified NBSTAT (NetBios Status)
resource records to have identifier 0x0021. The DNS SRV RFC specified
SRV records to have identifier 33. Unfortunately those are the
same number. :)
We now check the resp port to handle this situation better so that
we won't be attempting to parse NBSTAT records as SRV (which
causes several weird messages).
- DNS SRV responses never had the code written to actually
generate the dns_SRV_reply event. Adding this required
extending the event a bit to add extra information. SRV responses
now appear in the dns.log file correctly.
- Fixed an issue where some Microsoft NetBIOS Name Service lookups
would exceed the max label length for DNS and cause an incorrect
"DNS_label_too_long" weird.
The amount of timers involved in DNS::PendingMessage tables'
expiration attributes have a significant performance hit. Instead the
script now relies solely on maximum thresholds for pending message
quantities to limit amount of accumulated state. There's a new option,
"DNS::max_pending_query_ids", to limit the number outstanding messages
across all DNS query IDs ("DNS::max_pending_msgs" still limits number
of outstanding messages for a *given* query ID).
The previous method of matching queries with replies was still
unreliable in cases where the reply contains no answers. The new code
also takes extra measures to avoid pending state growing too large in
cases where the condition to match a query with a corresponding reply is
never met, but yet DNS messages continue to be exchanged over the same
connection 5-tuple (preventing cleanup of the pending state).
The scope of dns.log is now only standard queries (OPCODE == 0). Other
kinds of queries (e.g. inverse query) were not handled correctly and
could interfere with the state tracking of the default DNS scripts.
- Fix parsing of empty question sections (when QDCOUNT == 0). In this
case, the DNS parser would extract two 2-byte fields for use in either
"dns_query_reply" or "dns_rejected" events (dependent on value of
RCODE) as qclass and qtype parameters. This is not correct, because
such fields don't actually exist in the DNS message format when
QDCOUNT is 0. As a result, these events are no longer raised when
there's an empty question section. Scripts that depends on checking
for an empty question section can do that in the "dns_message" event.
- Add a new "dns_unknown_reply" event, for when Bro does not know how
to fully parse a particular resource record type. This helps fix a
problem in the default DNS scripts where the logic to complete
request-reply pair matching doesn't work because it's waiting on more
RR events to complete the reply. i.e. it expects ANCOUNT number of
dns_*_reply events and will wait until it gets that many before
completing a request-reply pair and logging it to dns.log. This could
cause bogus replies to match a previous request if they happen to
share a DNS transaction ID.
- Each transaction ID within a connection is now maintained as
a queue of DNS::Info logging records.
- New function added to the queue.bro script to support
peeking at the new gettable item in the queue without removing it.
All tests pass with one exception: some Broxygen tests are broken
because dpd_config doesn't exist anymore. Need to update the mechanism
for auto-documenting well-known ports.
This is a larger internal change that moves the analyzer
infrastructure to a more flexible model where the available analyzers
don't need to be hardcoded at compile time anymore. While currently
they actually still are, this will in the future enable external
analyzer plugins. For now, it does already add the capability to
dynamically enable/disable analyzers from script-land, replacing the
old Analyzer::Available() methods.
There are three major parts going into this:
- A new plugin infrastructure in src/plugin. This is independent
of analyzers and will eventually support plugins for other parts
of Bro as well (think: readers and writers). The goal is that
plugins can be alternatively compiled in statically or loadead
dynamically at runtime from a shared library. While the latter
isn't there yet, there'll be almost no code change for a plugin
to make it dynamic later (hopefully :)
- New analyzer infrastructure in src/analyzer. I've moved a number
of analyzer-related classes here, including Analyzer and DPM;
the latter now renamed to Analyzer::Manager. More will move here
later. Currently, there's only one plugin here, which provides
*all* existing analyzers. We can modularize this further in the
future (or not).
- A new script interface in base/framework/analyzer. I think that
this will eventually replace the dpm framework, but for now
that's still there as well, though some parts have moved over.
I've also remove the dpd_config table; ports are now configured via
the analyzer framework. For exmaple, for SSH:
const ports = { 22/tcp } &redef;
event bro_init() &priority=5
{
...
Analyzer::register_for_ports(Analyzer::ANALYZER_SSH, ports);
}
As you can see, the old ANALYZER_SSH constants have more into an enum
in the Analyzer namespace.
This is all hardly tested right now, and not everything works yet.
There's also a lot more cleanup to do (moving more classes around;
removing no longer used functionality; documenting script and C++
interfaces; regression tests). But it seems to generally work with a
small trace at least.
The debug stream "dpm" shows more about the loaded/enabled analyzers.
A new option -N lists loaded plugins and what they provide (including
those compiled in statically; i.e., right now it outputs all the
analyzers).
This is all not cast-in-stone yet, for some things we need to see if
they make sense this way. Feedback welcome.
no reply package was seen.
In the fixed version rcode is only set when a reply packet was seen.
Updates for the baseline have been commited separately in the
topic/bernhard/dns-fix branch.
Internally, all BROv6 preprocessor switches were removed and
addr/subnet representations wrapped in the new IPAddr/IPPrefix classes.
Some script-layer changes of note:
- dns_AAAA_reply event signature changed: the string representation
of an IPv6 addr is easily derived from the addr value, it doesn't
need to be another parameter. This event also now generated directly
by the DNS analyzer instead of being "faked" into a dns_A_reply event.
- removed addr_to_count BIF. It used to return the host-order
count representation of IPv4 addresses only. To make it more
generic, we might later add a BIF to return a vector of counts
in order to support IPv6.
- changed the result of enclosing addr variables in vertical pipes
(e.g. |my_addr|) to return the bit-width of the address type which
is 128 for IPv6 and 32 for IPv4. It used to function the same
way as addr_to_count mentioned above.
- remove bro_has_ipv6 BIF
- Answers and TTLs are now vectors.
- The warning that was being generated (dns_reply_seen_after_done)
from transaction ID reuse is fixed.
- Updated the single failing btest baseline.
- Log path's are generated in the scripting land
now. The default Log stream ID to path string
mapping works like this:
- Notice::LOG -> "notice"
- Notice::POLICY_LOG -> "notice_policy"
- TestModule::LOG -> "test_module"
- Logging streams updated across all of the shipped
scripts to be more user friendly. Instead of
the logging stream ID HTTP::HTTP, we now have
HTTP::LOG, etc.
- The priorities on some bro_init handlers have
been adjusted to make the process of applying
filters or disabling streams easier for users.
- policy/ renamed to scripts/
- By default BROPATH now contains:
- scripts/
- scripts/policy
- scripts/site
- *Nearly* all tests pass.
- All of scripts/base/ is loaded by main.cc
- Can be disabled by setting $BRO_NO_BASE_SCRIPTS
- Scripts in scripts/base/ don't use relative path loading to ease use of BRO_NO_BASE_SCRIPTS (to copy and paste that script).
- The scripts in scripts/base/protocols/ only (or soon will only) do logging and state building.
- The scripts in scripts/base/frameworks/ add functionality without causing any additional overhead.
- All "detection" activity happens through scripts in scripts/policy/.
- Communications framework modified temporarily to need an environment variable to actually enable (ENABLE_COMMUNICATION=1)
- This is so the communications framework can be loaded as part
of the base without causing trouble when it's not needed.
- This will be removed once a resolution to ticket #540 is reached.
