oss-fuzz generated "HTTP traffic" containing 250k+ sequences of "T<space>\r\r"
which Zeek then logged as individual HTTP requests. Add a heuristic to bail
on such request lines. It's a bit specific to the test case, but should work.
There are more issues around handling HTTP/0.9, e.g. triggering
"not a http reply line" when HTTP/0.9 never had such a thing, but
I don't think that's worth fixing up.
Fixes#119
* origin/topic/robin/gh-2426-flipping:
Fixing productive connections with missing SYN still considered partial after flipping direction.
Add some missing bits when flipping endpoints.
In https://github.com/zeek/zeek/pull/2191, we added endpoint flipping
for cases where a connection starts with a SYN/ACK followed by ACK or
data. The goal was to treat the connection as productive and go ahead
and parse it. But the TCP analyzer could continue to consider it
partial after flipping, meaning that app layers would bail out. #2426
shows such a case: HTTP gets correctly activated after flipping
through content inspection, but it won't process anything because
`IsPartial()` returns true. As the is-partial state reflects
whether we saw the first packets each in direction, this patch now
overrides that state for the originally missing SYN after flipping.
We actually had the same problem at a couple of other locations already
as well. One of that only happened to work because of the originally
inconsistent state flipping that was fixed in the previous commit. The
corresponding unit test now broke after that change. This commit
updates that logic as well to override the state.
This fix is a bit of a hack, but the best solution I could think of
without introducing larger changes.
Closes#2426.
* microsoft/master: (71 commits)
Clang formatting
Mask ports before inserting them into the map
Fix compiler warning from applied patch
Remove statistics plugin in favor of stats bif
Add EventHandler version of stats plugin
Mark a few EventHandler methods const
Changed implementation from std::map to std::unordered_map of Val.cc
Removed const, Windows build is now working
Added fixes suggested in PR
Update src/packet_analysis/protocol/ip/IP.cc
Apply suggestions from code review
Clang format again but now with v13.0.1
Rewrote usages of define(_MSC_VER) to ifdef _MSC_VER
Clang format it all
Fixed initial CR comments
Add NEWS entry about Windows port
Add a couple of extra unistd.h includes to fix a build failure
Use std::chrono instead of gettimeofday
Update libkqueue submodule [nomail]
Don't call tokenize_string if the input string is empty
...
the data/string handling in smb1-com-transaction.pcac was improved
with c75519ca88, re-use the added
functionality also for smb1-com-transaction-secondary.pac to avoid
buffer overflows.
This PR changes the way in which the SSL analyzer tracks the direction
of connections. So far, the SSL analyzer assumed that the originator of
a connection would send the client hello (and other associated
client-side events), and that the responder would be the SSL servers.
In some circumstances this is not true, and the initiator of a
connection is the server, with the responder being the client. So far
this confused some of the internal statekeeping logic and could lead to
mis-parsing of extensions.
This reversal of roles can happen in DTLS, if a connection uses STUN -
and potentially in some StartTLS protocols.
This PR tracks the direction of a TLS connection using the hello
request, client hello and server hello handshake messages. Furthermore,
it changes the SSL events from providing is_orig to providing is_client,
where is_client is true for the client_side of a connection. Since the
argument positioning in the event has not changed, old scripts will
continue to work seamlessly - the new semantics are what everyone
writing SSL scripts will have expected in any case.
There is a new event that is raised when a connection is flipped. A
weird is raised if a flip happens repeatedly.
Addresses GH-2198.
We could already pass an overriding tag to
`Analyzer::AnalyzerConfirmation()`, but we didn't have that ability
for `AnalyzerViolation`, leading to the two potentially mismatching in
the analyzer they report.
Documentation is missing and will be added in the next couple of hours.
* origin/topic/johanna/tls12-decryption: (24 commits)
TLS decryption: add test, fix small issues
Address PR feedback
TLS decryption: refactoring, more comments, less bare pointers
Small code fix and test baseline update.
SSL decryption: refactor TLS12_PRF
SSL decryption: small style changes, a bit of documentation
Deprecation and warning fixes
Clang-format updates
add missing call to EVP_KDF_CTX_set_params
TLS decryption: remove payload from ssl_encrypted_data again.
TLS 1.2 decryption: adapt OpenSSL 3.0 changes for 1.1
ssl: adapt TLS-PRF to openSSL 3.0
ssl/analyzer: potentially fix memory leaks caused by bytestrings
analyzer/ssl: several improvements
analyzer/ssl: defensive key length check + more debug logging
testing: feature gate ssl/decryption test
testing: add ssl/decryption test
analyzer/ssl: handle missing <openssl/kdf.h>
analyzer/ssl: silence warning in DTLS analyzer
analyzer/ssl: move proc-{client,server}-hello into the respective analyzers
...
This addresses feedback to GH-1814. The most significant change is the
fact that the ChipertextRecord now can remain &transient - which might
lead to improved speed.
Conceptually, a TCP-based application analyzer should not need any
knowledge about the underlying TCP analysis; it's supposed to just
process its reassembled input stream as it's handed over. But our
analyzers break that assumption at a few places because sometimes
knowledge about the TCP state of the connection can be helpful for
heuristics. This is fine as long as there actually *is* a TCP parent
analyzer available. Sometimes, however, there isn't: if the payload
stream is encapsulated inside another application-layer protocol, the
semantic link to TCP is broken. And if the outer connection is even
UDP, then we don't have a TCP analyzer at all.
We didn't handle this situation well so far. Most analyzers needing
TCP state would just crash if there's no TCP analyzer (in debug mode
with an `assert`, in release mode with a null pointer deref ...). Only
HTTP did the right thing already: check if TCP is available and adapt
accordingly.
We know extend that check to all other analyzers as well: all accesses
to `TCP()` are guarded, with reasonable defaults if not available.
It's actually a pretty small change overall, which is evidence for how
little this layering violation actually matters.
The existing behavior is what's causing
https://github.com/corelight/zeek-spicy-openvpn/issues/3.
This commit refactors TLS decryption, adds more comments in scripts and
in C++ source-code, and removes use of bare pointers, instead relying
more on stl data types.
