Fixing removal of support analyzers, plus some tweaking and cleanup of

CONNECT code.

Removal of support analyzers was broken. The code now actually doesn't
delete them immediately anymore but instead just flags them as
disabled. They'll be destroyed with the parent analyzer later.

Also includes a new leak tests exercising the CONNECT code.

Lines starting # with '#' will be ignored, and an empty message aborts
the commit. # On branch topic/robin/http-connect # Changes to be
committed: # modified: scripts/base/protocols/http/main.bro #
modified: scripts/base/protocols/ssl/consts.bro # modified:
src/analyzer/Analyzer.cc # modified: src/analyzer/Analyzer.h #
modified: src/analyzer/protocol/http/HTTP.cc # new file:
testing/btest/core/leaks/http-connect.bro # modified:
testing/btest/scripts/base/protocols/http/http-connect.bro # #
Untracked files: # .tags # changes.txt # conn.log # debug.log # diff #
mpls-in-vlan.patch # newfile.pcap # packet_filter.log # reporter.log #
src/PktSrc.cc.orig # weird.log #
This commit is contained in:
Robin Sommer 2014-03-02 13:52:32 -08:00
parent dd0856a57f
commit 338d521003
7 changed files with 128 additions and 65 deletions

View file

@ -218,7 +218,7 @@ event http_reply(c: connection, version: string, code: count, reason: string) &p
c$http$info_code = code;
c$http$info_msg = reason;
}
if ( c$http?$method && c$http$method == "CONNECT" && code == 200 )
{
# Copy this conn_id and set the orig_p to zero because in the case of CONNECT proxies there will

View file

@ -86,6 +86,7 @@ export {
[13172] = "next_protocol_negotiation",
[13175] = "origin_bound_certificates",
[13180] = "encrypted_client_certificates",
[30031] = "channel_id",
[65281] = "renegotiation_info"
} &default=function(i: count):string { return fmt("unknown-%d", i); };

View file

@ -209,11 +209,11 @@ void Analyzer::NextPacket(int len, const u_char* data, bool is_orig, int seq,
if ( skip )
return;
// If we have support analyzers, we pass it to them.
if ( is_orig && orig_supporters )
orig_supporters->NextPacket(len, data, is_orig, seq, ip, caplen);
else if ( ! is_orig && resp_supporters )
resp_supporters->NextPacket(len, data, is_orig, seq, ip, caplen);
SupportAnalyzer* next_sibling = FirstSupportAnalyzer(is_orig);
if ( next_sibling )
next_sibling->NextPacket(len, data, is_orig, seq, ip, caplen);
else
{
try
@ -232,11 +232,11 @@ void Analyzer::NextStream(int len, const u_char* data, bool is_orig)
if ( skip )
return;
// If we have support analyzers, we pass it to them.
if ( is_orig && orig_supporters )
orig_supporters->NextStream(len, data, is_orig);
else if ( ! is_orig && resp_supporters )
resp_supporters->NextStream(len, data, is_orig);
SupportAnalyzer* next_sibling = FirstSupportAnalyzer(is_orig);
if ( next_sibling )
next_sibling->NextStream(len, data, is_orig);
else
{
try
@ -255,11 +255,11 @@ void Analyzer::NextUndelivered(int seq, int len, bool is_orig)
if ( skip )
return;
// If we have support analyzers, we pass it to them.
if ( is_orig && orig_supporters )
orig_supporters->NextUndelivered(seq, len, is_orig);
else if ( ! is_orig && resp_supporters )
resp_supporters->NextUndelivered(seq, len, is_orig);
SupportAnalyzer* next_sibling = FirstSupportAnalyzer(is_orig);
if ( next_sibling )
next_sibling->NextUndelivered(seq, len, is_orig);
else
{
try
@ -278,11 +278,10 @@ void Analyzer::NextEndOfData(bool is_orig)
if ( skip )
return;
// If we have support analyzers, we pass it to them.
if ( is_orig && orig_supporters )
orig_supporters->NextEndOfData(is_orig);
else if ( ! is_orig && resp_supporters )
resp_supporters->NextEndOfData(is_orig);
SupportAnalyzer* next_sibling = FirstSupportAnalyzer(is_orig);
if ( next_sibling )
next_sibling->NextEndOfData(is_orig);
else
EndOfData(is_orig);
}
@ -558,31 +557,17 @@ void Analyzer::AddSupportAnalyzer(SupportAnalyzer* analyzer)
void Analyzer::RemoveSupportAnalyzer(SupportAnalyzer* analyzer)
{
SupportAnalyzer** head =
analyzer->IsOrig() ? &orig_supporters : &resp_supporters;
SupportAnalyzer* prev = 0;
SupportAnalyzer* s;
for ( s = *head; s && s != analyzer; prev = s, s = s->sibling )
;
if ( ! s )
return;
if ( prev )
prev->sibling = s->sibling;
else
*head = s->sibling;
DBG_LOG(DBG_ANALYZER, "%s removed support %s",
DBG_LOG(DBG_ANALYZER, "%s disabled %s support analyzer %s",
fmt_analyzer(this).c_str(),
analyzer->IsOrig() ? "originator" : "responder",
fmt_analyzer(analyzer).c_str());
if ( ! analyzer->finished )
analyzer->Done();
delete analyzer;
// We mark the analyzer as being removed here, which will prevent it
// from being used further. However, we don't actually delete it
// before the parent gets destroyed. While we woulc do that, it's a
// bit tricky to do at the right time and it doesn't seem worth the
// trouble.
analyzer->removing = true;
return;
}
@ -596,6 +581,19 @@ bool Analyzer::HasSupportAnalyzer(Tag tag, bool orig)
return false;
}
SupportAnalyzer* Analyzer::FirstSupportAnalyzer(bool orig)
{
SupportAnalyzer* sa = orig ? orig_supporters : resp_supporters;
if ( ! sa )
return 0;
if ( ! sa->Removing() )
return sa;
return sa->Sibling(true);
}
void Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig,
int seq, const IP_Hdr* ip, int caplen)
{
@ -782,16 +780,32 @@ void Analyzer::Weird(const char* name, const char* addl)
conn->Weird(name, addl);
}
SupportAnalyzer* SupportAnalyzer::Sibling(bool only_active) const
{
if ( ! only_active )
return sibling;
SupportAnalyzer* next = sibling;
while ( next && next->Removing() )
next = next->sibling;
return next;
}
void SupportAnalyzer::ForwardPacket(int len, const u_char* data, bool is_orig,
int seq, const IP_Hdr* ip, int caplen)
{
// We do not call parent's method, as we're replacing the functionality.
if ( GetOutputHandler() )
GetOutputHandler()->DeliverPacket(len, data, is_orig, seq,
ip, caplen);
else if ( sibling )
SupportAnalyzer* next_sibling = Sibling(true);
if ( next_sibling )
// Pass to next in chain.
sibling->NextPacket(len, data, is_orig, seq, ip, caplen);
next_sibling->NextPacket(len, data, is_orig, seq, ip, caplen);
else
// Finished with preprocessing - now it's the parent's turn.
Parent()->DeliverPacket(len, data, is_orig, seq, ip, caplen);
@ -800,12 +814,15 @@ void SupportAnalyzer::ForwardPacket(int len, const u_char* data, bool is_orig,
void SupportAnalyzer::ForwardStream(int len, const u_char* data, bool is_orig)
{
// We do not call parent's method, as we're replacing the functionality.
if ( GetOutputHandler() )
GetOutputHandler()->DeliverStream(len, data, is_orig);
else if ( sibling )
SupportAnalyzer* next_sibling = Sibling(true);
if ( next_sibling )
// Pass to next in chain.
sibling->NextStream(len, data, is_orig);
next_sibling->NextStream(len, data, is_orig);
else
// Finished with preprocessing - now it's the parent's turn.
Parent()->DeliverStream(len, data, is_orig);
@ -814,12 +831,15 @@ void SupportAnalyzer::ForwardStream(int len, const u_char* data, bool is_orig)
void SupportAnalyzer::ForwardUndelivered(int seq, int len, bool is_orig)
{
// We do not call parent's method, as we're replacing the functionality.
if ( GetOutputHandler() )
GetOutputHandler()->Undelivered(seq, len, is_orig);
else if ( sibling )
SupportAnalyzer* next_sibling = Sibling(true);
if ( next_sibling )
// Pass to next in chain.
sibling->NextUndelivered(seq, len, is_orig);
next_sibling->NextUndelivered(seq, len, is_orig);
else
// Finished with preprocessing - now it's the parent's turn.
Parent()->Undelivered(seq, len, is_orig);

View file

@ -587,7 +587,7 @@ protected:
void RemoveTimer(Timer* t);
/**
* Returnsn true if the analyzer has associated an SupportAnalyzer of a given type.
* Returns true if the analyzer has associated an SupportAnalyzer of a given type.
*
* @param tag The type to check for.
*
@ -595,6 +595,14 @@ protected:
*/
bool HasSupportAnalyzer(Tag tag, bool orig);
/**
* Returns the first still active support analyzer for the given
* direction, or null if none.
*
* @param orig True if asking about the originator side.
*/
SupportAnalyzer* FirstSupportAnalyzer(bool orig);
/**
* Adds a a new child analyzer with the option whether to intialize
* it. This is an internal method.
@ -616,6 +624,12 @@ protected:
*/
void AppendNewChildren();
/**
* Returns true if the analyzer has been flagged for removal and
* shouldn't be used otherwise anymore.
*/
bool Removing() const { return removing; }
private:
// Internal method to eventually delete a child analyzer that's
// already Done().
@ -718,6 +732,14 @@ public:
*/
bool IsOrig() const { return orig; }
/**
* Returns the analyzer's next sibling, or null if none.
*
* only_active: If true, this will skip siblings that are still link
* but flagged for removal.
*/
SupportAnalyzer* Sibling(bool only_active = false) const;
/**
* Passes packet input to the next sibling SupportAnalyzer if any, or
* on to the associated main analyzer if none. If however there's an
@ -749,11 +771,6 @@ public:
*/
virtual void ForwardUndelivered(int seq, int len, bool orig);
/**
* Returns the analyzer next sibling, or null if none.
*/
SupportAnalyzer* Sibling() const { return sibling; }
protected:
friend class Analyzer;

View file

@ -950,7 +950,7 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
if ( pia )
{
// There will be a PIA instance if this connection has been identified
// There will be a PIA instance if this connection has been identified
// as a connect proxy.
ForwardStream(len, data, is_orig);
return;
@ -1066,14 +1066,10 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
HTTP_Reply();
InitHTTPMessage(content_line,
reply_message, is_orig,
ExpectReplyMessageBody(),
len);
if ( connect_request && reply_code == 200 )
{
pia = new pia::PIA_TCP(Conn());
if ( AddChildAnalyzer(pia) )
{
pia->FirstPacket(true, 0);
@ -1084,13 +1080,22 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
// need to be removed.
RemoveSupportAnalyzer(content_line_orig);
RemoveSupportAnalyzer(content_line_resp);
return;
}
else
{
// Shouldn't really happen.
delete pia;
pia = 0;
}
}
InitHTTPMessage(content_line,
reply_message, is_orig,
ExpectReplyMessageBody(),
len);
}
else
{
@ -1422,6 +1427,12 @@ void HTTP_Analyzer::HTTP_Request()
{
ProtocolConfirmation();
const char* method = (const char*) request_method->AsString()->Bytes();
int method_len = request_method->AsString()->Len();
if ( strcasecmp_n(method_len, method, "CONNECT") == 0 )
connect_request = true;
if ( http_request )
{
val_list* vl = new val_list;
@ -1436,9 +1447,6 @@ void HTTP_Analyzer::HTTP_Request()
// DEBUG_MSG("%.6f http_request\n", network_time);
ConnectionEvent(http_request, vl);
}
if ( strcasecmp_n(request_method->AsString()->Len(), (const char*) (request_method->AsString()->Bytes()), "CONNECT") == 0 )
connect_request = true;
}
void HTTP_Analyzer::HTTP_Reply()

View file

@ -0,0 +1,14 @@
# Needs perftools support.
#
# @TEST-GROUP: leaks
#
# @TEST-REQUIRES: bro --help 2>&1 | grep -q mem-leaks
#
# @TEST-EXEC: HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local btest-bg-run bro bro -b -m -r $TRACES/http/connect-with-smtp.trace %INPUT
# @TEST-EXEC: btest-bg-wait 15
@load base/protocols/conn
@load base/protocols/http
@load base/protocols/smtp
@load base/protocols/tunnels
@load base/frameworks/dpd

View file

@ -6,6 +6,9 @@
# @TEST-EXEC: btest-diff smtp.log
# @TEST-EXEC: btest-diff tunnel.log
# The base analysis scripts are loaded by default.
#@load base/protocols/http
@load base/protocols/conn
@load base/protocols/http
@load base/protocols/smtp
@load base/protocols/tunnels
@load base/frameworks/dpd