Let our TCP-based application analyzers operate without any TCP parent analyzer.

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 is contained in:
Robin Sommer 2022-01-28 12:02:56 +01:00
parent 0793a38cc5
commit 9b0d525728
No known key found for this signature in database
GPG key ID: 6BEDA4DA6B8B23E3
27 changed files with 58 additions and 79 deletions

View file

@ -36,10 +36,7 @@ void BitTorrent_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
// punt on partial.
if ( TCP() && TCP()->IsPartial() )
return;
if ( this_stop )

View file

@ -80,10 +80,7 @@ void BitTorrentTracker_Analyzer::DeliverStream(int len, const u_char* data, bool
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
// punt on partial.
if ( TCP() && TCP()->IsPartial() )
return;
if ( orig )

View file

@ -50,8 +50,6 @@ void DCE_RPC_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( had_gap )
// If only one side had a content gap, we could still try to
// deliver data to the other side if the script layer can handle this.

View file

@ -45,12 +45,15 @@ void FTP_Analyzer::Done()
{
analyzer::tcp::TCP_ApplicationAnalyzer::Done();
if ( TCP() )
{
if ( nvt_orig->HasPartialLine() &&
(TCP()->OrigState() == analyzer::tcp::TCP_ENDPOINT_CLOSED ||
TCP()->OrigPrevState() == analyzer::tcp::TCP_ENDPOINT_CLOSED) )
// ### should include the partial text
Weird("partial_ftp_request");
}
}
static uint32_t get_reply_code(int len, const char* line)
{

View file

@ -38,8 +38,6 @@ void GSSAPI_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
try
{
interp->NewData(orig, data, data + len);

View file

@ -47,8 +47,7 @@ void IMAP_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
return;
}
assert(TCP());
if ( TCP()->IsPartial() )
if ( TCP() && TCP()->IsPartial() )
return;
if ( had_gap )

View file

@ -39,8 +39,7 @@ void KRB_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
if ( TCP() && TCP()->IsPartial() )
return;
if ( had_gap )

View file

@ -123,8 +123,8 @@ void Login_Analyzer::NewLine(bool orig, char* line)
if ( state == LOGIN_STATE_AUTHENTICATE )
{
if ( TCP()->OrigState() == analyzer::tcp::TCP_ENDPOINT_PARTIAL ||
TCP()->RespState() == analyzer::tcp::TCP_ENDPOINT_PARTIAL )
if ( TCP() && (TCP()->OrigState() == analyzer::tcp::TCP_ENDPOINT_PARTIAL ||
TCP()->RespState() == analyzer::tcp::TCP_ENDPOINT_PARTIAL) )
state = LOGIN_STATE_CONFUSED; // unknown login state
else
{

View file

@ -34,10 +34,13 @@ Contents_Rsh_Analyzer::~Contents_Rsh_Analyzer() { }
void Contents_Rsh_Analyzer::DoDeliver(int len, const u_char* data)
{
auto* tcp = static_cast<analyzer::tcp::TCP_ApplicationAnalyzer*>(Parent())->TCP();
assert(tcp);
int endp_state;
int endp_state = IsOrig() ? tcp->OrigState() : tcp->RespState();
if ( auto* tcp = static_cast<analyzer::tcp::TCP_ApplicationAnalyzer*>(Parent())->TCP() )
endp_state = IsOrig() ? tcp->OrigState() : tcp->RespState();
else
endp_state = tcp::TCP_ENDPOINT_ESTABLISHED; // no TCP parent, assume somebody's feeding us a
// legitimate stream
for ( ; len > 0; --len, ++data )
{

View file

@ -30,10 +30,13 @@ Contents_Rlogin_Analyzer::~Contents_Rlogin_Analyzer() { }
void Contents_Rlogin_Analyzer::DoDeliver(int len, const u_char* data)
{
auto* tcp = static_cast<analyzer::tcp::TCP_ApplicationAnalyzer*>(Parent())->TCP();
assert(tcp);
int endp_state;
int endp_state = IsOrig() ? tcp->OrigState() : tcp->RespState();
if ( auto* tcp = static_cast<analyzer::tcp::TCP_ApplicationAnalyzer*>(Parent())->TCP() )
endp_state = IsOrig() ? tcp->OrigState() : tcp->RespState();
else
endp_state = tcp::TCP_ENDPOINT_ESTABLISHED; // no TCP parent, assume somebody's feeding us a
// legitimate stream
for ( ; len > 0; --len, ++data )
{

View file

@ -37,8 +37,6 @@ void MQTT_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
try
{
interp->NewData(orig, data, data + len);

View file

@ -38,8 +38,7 @@ void MySQL_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
if ( TCP() && TCP()->IsPartial() )
return;
if ( had_gap )

View file

@ -180,8 +180,12 @@ void Contents_NCP_Analyzer::DeliverStream(int len, const u_char* data, bool orig
if ( ! resync_set )
{
resync_set = true;
if ( tcp )
resync = (IsOrig() ? tcp->OrigState() : tcp->RespState()) !=
analyzer::tcp::TCP_ENDPOINT_ESTABLISHED;
else
resync = false;
}
if ( tcp && tcp->HadGap(orig) )

View file

@ -37,8 +37,6 @@ void NTLM_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
try
{
interp->NewData(orig, data, data + len);

View file

@ -39,8 +39,7 @@ void RDP_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
if ( TCP() && TCP()->IsPartial() )
return;
if ( had_gap )

View file

@ -36,8 +36,8 @@ void RFB_Analyzer::EndpointEOF(bool is_orig)
void RFB_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
if ( TCP() && TCP()->IsPartial() )
return;
if ( had_gap )

View file

@ -446,16 +446,16 @@ bool Contents_RPC::CheckResync(int& len, const u_char*& data, bool orig)
// is fully established we are in sync (since it's the first chunk
// of data after the SYN if its not established we need to
// resync.
auto* tcp = static_cast<analyzer::tcp::TCP_ApplicationAnalyzer*>(Parent())->TCP();
assert(tcp);
resync_state = INSYNC;
if ( auto* tcp = static_cast<analyzer::tcp::TCP_ApplicationAnalyzer*>(Parent())->TCP() )
{
if ( (IsOrig() ? tcp->OrigState() : tcp->RespState()) !=
analyzer::tcp::TCP_ENDPOINT_ESTABLISHED )
{
NeedResync();
}
else
resync_state = INSYNC;
}
}
if ( resync_state == INSYNC )

View file

@ -41,8 +41,7 @@ void SIP_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
if ( TCP() && TCP()->IsPartial() )
return;
if ( had_gap )

View file

@ -61,8 +61,6 @@ void SMB_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
// It we need to resync and we don't have an SMB header, bail!
if ( need_sync && ! HasSMBHeader(len, data) )
return;

View file

@ -46,10 +46,7 @@ void SOCKS_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
// punt on partial.
if ( TCP() && TCP()->IsPartial() )
return;
if ( orig_done && resp_done )

View file

@ -44,8 +44,7 @@ void SSH_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
if ( TCP() && TCP()->IsPartial() )
return;
if ( had_gap )

View file

@ -79,10 +79,7 @@ void Syslog_Analyzer::DeliverPacket(int len, const u_char* data, bool orig, uint
// {
// analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
//
// assert(TCP());
//
// if ( TCP()->IsPartial() || TCP()->HadGap(orig) )
// // punt-on-partial or stop-on-gap.
// if ( TCP() && TCP()->IsPartial() )
// return;
//
// interp->NewData(orig, data, data + len);

View file

@ -331,9 +331,7 @@ void ContentLine_Analyzer::CheckNUL()
// had been an initial SYN, so we check for whether
// the connection has at most two bytes so far.
auto* tcp = static_cast<TCP_ApplicationAnalyzer*>(Parent())->TCP();
if ( tcp )
if ( auto* tcp = static_cast<TCP_ApplicationAnalyzer*>(Parent())->TCP() )
{
TCP_Endpoint* endp = IsOrig() ? tcp->Orig() : tcp->Resp();
if ( endp->state == TCP_ENDPOINT_PARTIAL && endp->LastSeq() - endp->StartSeq() <= 2 )

View file

@ -36,12 +36,13 @@ void TCP_ApplicationAnalyzer::Init()
void TCP_ApplicationAnalyzer::AnalyzerViolation(const char* reason, const char* data, int len)
{
auto* tcp = TCP();
if ( tcp && (tcp->IsPartial() || tcp->HadGap(false) || tcp->HadGap(true)) )
if ( auto* tcp = TCP() )
{
if ( tcp->IsPartial() || tcp->HadGap(false) || tcp->HadGap(true) )
// Filter out incomplete connections. Parsing them is
// too unreliable.
return;
}
Analyzer::AnalyzerViolation(reason, data, len);
}

View file

@ -44,8 +44,7 @@ void XMPP_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
return;
}
assert(TCP());
if ( TCP()->IsPartial() )
if ( TCP() && TCP()->IsPartial() )
return;
if ( had_gap )

View file

@ -38,8 +38,7 @@ void FOO_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
{
tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
if ( TCP() && TCP()->IsPartial() )
return;
if ( had_gap )

View file

@ -36,10 +36,7 @@ void Foo::DeliverStream(int len, const u_char* data, bool orig)
{
zeek::analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig);
assert(TCP());
if ( TCP()->IsPartial() )
// punt on partial.
if ( TCP() && TCP()->IsPartial() )
return;
try