diff --git a/scripts/base/protocols/http/main.bro b/scripts/base/protocols/http/main.bro index 27257be2d6..ed20bc6586 100644 --- a/scripts/base/protocols/http/main.bro +++ b/scripts/base/protocols/http/main.bro @@ -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 diff --git a/scripts/base/protocols/ssl/consts.bro b/scripts/base/protocols/ssl/consts.bro index 55289a7419..b81aebfbbb 100644 --- a/scripts/base/protocols/ssl/consts.bro +++ b/scripts/base/protocols/ssl/consts.bro @@ -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); }; diff --git a/src/analyzer/Analyzer.cc b/src/analyzer/Analyzer.cc index 03734f1a22..63462c0049 100644 --- a/src/analyzer/Analyzer.cc +++ b/src/analyzer/Analyzer.cc @@ -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); diff --git a/src/analyzer/Analyzer.h b/src/analyzer/Analyzer.h index f7ca07ca51..578020082b 100644 --- a/src/analyzer/Analyzer.h +++ b/src/analyzer/Analyzer.h @@ -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; diff --git a/src/analyzer/protocol/http/HTTP.cc b/src/analyzer/protocol/http/HTTP.cc index 93dbfbcb2e..0d49bb037f 100644 --- a/src/analyzer/protocol/http/HTTP.cc +++ b/src/analyzer/protocol/http/HTTP.cc @@ -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() diff --git a/testing/btest/core/leaks/http-connect.bro b/testing/btest/core/leaks/http-connect.bro new file mode 100644 index 0000000000..e9a47d00a2 --- /dev/null +++ b/testing/btest/core/leaks/http-connect.bro @@ -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 diff --git a/testing/btest/scripts/base/protocols/http/http-connect.bro b/testing/btest/scripts/base/protocols/http/http-connect.bro index d8157e2c49..7073d88ac2 100644 --- a/testing/btest/scripts/base/protocols/http/http-connect.bro +++ b/testing/btest/scripts/base/protocols/http/http-connect.bro @@ -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