From dd0856a57f386b12819b7ef54b141d2d706539d8 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Wed, 12 Feb 2014 22:38:59 -0500 Subject: [PATCH 1/5] HTTP CONNECT proxy support. - The HTTP analyzer now supports handling HTTP CONNECT proxies same as the SOCKS analyzer handles proxying. --- scripts/base/protocols/http/main.bro | 11 ++++++ src/analyzer/protocol/http/HTTP.cc | 35 ++++++++++++++++++ src/analyzer/protocol/http/HTTP.h | 4 ++ src/types.bif | 1 + .../conn.log | 10 +++++ .../http.log | 10 +++++ .../smtp.log | 10 +++++ .../tunnel.log | 10 +++++ .../btest/Traces/http/connect-with-smtp.trace | Bin 0 -> 4191 bytes .../base/protocols/http/http-connect.bro | 11 ++++++ 10 files changed, 102 insertions(+) create mode 100644 testing/btest/Baseline/scripts.base.protocols.http.http-connect/conn.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.http.http-connect/http.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.http.http-connect/smtp.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.http.http-connect/tunnel.log create mode 100644 testing/btest/Traces/http/connect-with-smtp.trace create mode 100644 testing/btest/scripts/base/protocols/http/http-connect.bro diff --git a/scripts/base/protocols/http/main.bro b/scripts/base/protocols/http/main.bro index a164fcd6a6..27257be2d6 100644 --- a/scripts/base/protocols/http/main.bro +++ b/scripts/base/protocols/http/main.bro @@ -4,6 +4,7 @@ @load base/utils/numbers @load base/utils/files +@load base/frameworks/tunnels module HTTP; @@ -217,6 +218,16 @@ 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 + # be potentially many source ports since a new proxy connection is established for each + # proxied connection. We treat this as a singular "tunnel". + local tid = copy(c$id); + tid$orig_p = 0/tcp; + Tunnel::register([$cid=tid, $tunnel_type=Tunnel::HTTP]); + } } event http_header(c: connection, is_orig: bool, name: string, value: string) &priority=5 diff --git a/src/analyzer/protocol/http/HTTP.cc b/src/analyzer/protocol/http/HTTP.cc index f605dce402..93dbfbcb2e 100644 --- a/src/analyzer/protocol/http/HTTP.cc +++ b/src/analyzer/protocol/http/HTTP.cc @@ -889,6 +889,9 @@ HTTP_Analyzer::HTTP_Analyzer(Connection* conn) reply_code = 0; reply_reason_phrase = 0; + connect_request = false; + pia = 0; + content_line_orig = new tcp::ContentLine_Analyzer(conn, true); AddSupportAnalyzer(content_line_orig); @@ -945,6 +948,14 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig) if ( TCP() && TCP()->IsPartial() ) return; + if ( pia ) + { + // There will be a PIA instance if this connection has been identified + // as a connect proxy. + ForwardStream(len, data, is_orig); + return; + } + const char* line = reinterpret_cast(data); const char* end_of_line = line + len; @@ -1059,6 +1070,27 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig) 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); + pia->FirstPacket(false, 0); + + // This connection has transitioned to no longer + // being http and the content line support analyzers + // need to be removed. + RemoveSupportAnalyzer(content_line_orig); + RemoveSupportAnalyzer(content_line_resp); + } + else + { + pia = 0; + } + } + } else { @@ -1404,6 +1436,9 @@ 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/src/analyzer/protocol/http/HTTP.h b/src/analyzer/protocol/http/HTTP.h index a1fedee41d..48a611b63b 100644 --- a/src/analyzer/protocol/http/HTTP.h +++ b/src/analyzer/protocol/http/HTTP.h @@ -5,6 +5,7 @@ #include "analyzer/protocol/tcp/TCP.h" #include "analyzer/protocol/tcp/ContentLine.h" +#include "analyzer/protocol/pia/PIA.h" #include "analyzer/protocol/zip/ZIP.h" #include "analyzer/protocol/mime/MIME.h" #include "binpac_bro.h" @@ -237,6 +238,9 @@ protected: int connection_close; int request_ongoing, reply_ongoing; + bool connect_request; + pia::PIA_TCP *pia; + Val* request_method; // request_URI is in the original form (may contain '%' diff --git a/src/types.bif b/src/types.bif index 2931bf2d22..a44c3c1615 100644 --- a/src/types.bif +++ b/src/types.bif @@ -186,6 +186,7 @@ enum Type %{ TEREDO, SOCKS, GTPv1, + HTTP, %} type EncapsulatingConn: record; diff --git a/testing/btest/Baseline/scripts.base.protocols.http.http-connect/conn.log b/testing/btest/Baseline/scripts.base.protocols.http.http-connect/conn.log new file mode 100644 index 0000000000..8b639edd93 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.http.http-connect/conn.log @@ -0,0 +1,10 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path conn +#open 2014-02-13-03-37-02 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents +#types time string addr port addr port enum string interval count count string bool count string count count count count table[string] +1078232251.833846 CXWv6p3arKYeMETxOg 79.26.245.236 3378 254.228.86.79 8240 tcp http,smtp 6.722274 1685 223 SF - 0 ShADadfF 14 2257 16 944 (empty) +#close 2014-02-13-03-37-02 diff --git a/testing/btest/Baseline/scripts.base.protocols.http.http-connect/http.log b/testing/btest/Baseline/scripts.base.protocols.http.http-connect/http.log new file mode 100644 index 0000000000..4a2cf1ad17 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.http.http-connect/http.log @@ -0,0 +1,10 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path http +#open 2014-02-13-03-37-02 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p trans_depth method host uri referrer user_agent request_body_len response_body_len status_code status_msg info_code info_msg filename tags username password proxied orig_fuids orig_mime_types resp_fuids resp_mime_types +#types time string addr port addr port count string string string string string count count count string count string string table[enum] string string table[string] vector[string] vector[string] vector[string] vector[string] +1078232252.284420 CXWv6p3arKYeMETxOg 79.26.245.236 3378 254.228.86.79 8240 1 CONNECT - mailin03.sul.t-online.de:25 / - - 0 0 200 Connection established - - - (empty) - - - - - - - +#close 2014-02-13-03-37-02 diff --git a/testing/btest/Baseline/scripts.base.protocols.http.http-connect/smtp.log b/testing/btest/Baseline/scripts.base.protocols.http.http-connect/smtp.log new file mode 100644 index 0000000000..e11a7e9ac0 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.http.http-connect/smtp.log @@ -0,0 +1,10 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path smtp +#open 2014-02-13-03-37-02 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p trans_depth helo mailfrom rcptto date from to reply_to msg_id in_reply_to subject x_originating_ip first_received second_received last_reply path user_agent fuids +#types time string addr port addr port count string string table[string] string string table[string] string string string string addr string string string vector[addr] string vector[string] +1078232255.642953 CXWv6p3arKYeMETxOg 79.26.245.236 3378 254.228.86.79 8240 1 208.191.73.21 Tue, 2 Mar 2004 13:57:49 +0100 Sybille Ostermann thenightwatch@t-online.de - - - Hier sind die dicken Girls hemmungloser denn je.. grcu - from mail.iosphere.net (mail.iosphere.net [216.58.97.33]) by mail.netsync.net with esmtp; Mrz, 02 2004 12:55:34 -0700 - 250 Message accepted. 254.228.86.79,79.26.245.236,216.58.97.33 Microsoft Outlook Build 10.0.2616 FVS9k93PUgScEUCOjd +#close 2014-02-13-03-37-02 diff --git a/testing/btest/Baseline/scripts.base.protocols.http.http-connect/tunnel.log b/testing/btest/Baseline/scripts.base.protocols.http.http-connect/tunnel.log new file mode 100644 index 0000000000..9e18e38e03 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.http.http-connect/tunnel.log @@ -0,0 +1,10 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path tunnel +#open 2014-02-13-03-37-02 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p tunnel_type action +#types time string addr port addr port enum enum +1078232252.284420 - 79.26.245.236 0 254.228.86.79 8240 Tunnel::HTTP Tunnel::DISCOVER +#close 2014-02-13-03-37-02 diff --git a/testing/btest/Traces/http/connect-with-smtp.trace b/testing/btest/Traces/http/connect-with-smtp.trace new file mode 100644 index 0000000000000000000000000000000000000000..dba5e69edc0c5c10ecf25db2bb50c46aa6b88ad7 GIT binary patch literal 4191 zcmcgvU2Ggz6~4P(r^%8PDM-{nqMmJETc3 zE|G7u9l~t_d_i$ zBovB-C1Uxl6^K4o4Y!8)&i(;%dc$))5>29l8cTig?1nEO>YGn~E`Is5c^~mY9r0I) zXtamO4xCYrCXVe3Px%}_AK>^VqTZL=GQV{5Iw6PTlSE?uC>0_vmrcYP_J z845^w-?p%}HMb>xD0jb%QEn0|av(yE;JzHS6t752_PURC=F4khwZ#wZ?j@ui5~NfW zhHcJi!DTiF>bUx}$sB_g>%@=zY*T}H&{gdV7k!ogvRY+rD`spbX6(;5XIEzInKw3( zZy;wmnuuHP`NU6u93Z~a7XgWrlDjQ#tYfq0%naQ~09~rNY*$0fkjm_zWui;>}`RVHwtQS2vM8 zD6Sk$YhuSow|uPU16VR!XL4=xFGdcdHAWxJqAhxrWM5L*-le3HHNn1eY;(g0O$Ppl zFPIlJ@EOdViUuCdz99+8O|pFZ@y%ox?#t2SgXBMbEVnPB z5$^aBAx9BOj;5emP!@&Hd}~1VQ^@=kWInt16d@yn`N3H9H<-d>-64`g7364g#L#V@ zTBSYUh*yvr`W8MMdySB;bT>Mp1F(V#7iO4iKv8#fw1~uS3Ad8H*eK-)svLS`@q3bM z<-GvcGsyKKay|O&zffiBdV(i%x@%~#zgAj`8kB@>0WUGm+&QKGk7j}P`a%6)o;;jV zdz7S#3k%oRk%wS<4hn{_xipwAij39{P z9|dU0Hl{G!sTx#JAE>?g)ZtXJM@jc8ecP3;t{MgZCY;xq1< zjN&L%D2Fj7Oa}~VSspK&wu846v4X&Hswhwv1rGrzFH-Tp^U5%bCZ?!xusLhszk^`6 zb}4rg`MD}Z(sb7CteQIG{pciJIMjXQ0p{3=-rjUyBKbfpI-0~&s<~={?in9;dcVBQ%HazrPaL>7_M8@ zIy+}(W)xpwms0NF%a~$0Fik7AWjQX4cnDcATvo=($~ZBFLC4gfM!0Sb7VVF402*xGV_c*Q{YRQZW`#RpAp%yBSKPvB_0)=s@ zD`$^I2TjzFvso010_IU$*EohQtE#A&63C&mb70w~DIMwSrN~X9j$wGra#^+NyBjlH z!S$wB(5W)*lW1>rV^!SYWva;sR=5?HU$I~difu=JJfL6Y4OYpJ%SqQ~D;6u6Me7M( zC{<(8<;E0$bwv|xs-AcJ>RDQ&dJc@>Yt`j(Y)p;nx#@>7lEFhcnwsdYk9{oqaG;4| zB}S~a_~mnzT7S<_$I($3y-=W4mll!e6)er8>2RaWH+TE%vNW5ouP)64BUqZRVh0@? zeu0p$er+PZjCEzb518A7T z9PuYR8dUf{{$%&YFOC1SrY|8 Date: Sun, 2 Mar 2014 13:52:32 -0800 Subject: [PATCH 2/5] 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 # --- scripts/base/protocols/http/main.bro | 2 +- scripts/base/protocols/ssl/consts.bro | 1 + src/analyzer/Analyzer.cc | 114 ++++++++++-------- src/analyzer/Analyzer.h | 29 ++++- src/analyzer/protocol/http/HTTP.cc | 26 ++-- testing/btest/core/leaks/http-connect.bro | 14 +++ .../base/protocols/http/http-connect.bro | 7 +- 7 files changed, 128 insertions(+), 65 deletions(-) create mode 100644 testing/btest/core/leaks/http-connect.bro 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 From ac9c44afd872d0ac837871723f2b8ae0a7042415 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Sun, 2 Mar 2014 13:57:10 -0800 Subject: [PATCH 3/5] Updating submodule(s). [nomail] --- aux/broctl | 2 +- src/3rdparty | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/aux/broctl b/aux/broctl index 66793ec3c6..07349a4593 160000 --- a/aux/broctl +++ b/aux/broctl @@ -1 +1 @@ -Subproject commit 66793ec3c602439e235bee705b654aefb7ac8dec +Subproject commit 07349a459372d72b333bbc50e5f70584520c0bb3 diff --git a/src/3rdparty b/src/3rdparty index 42a4c9694a..e96d95a130 160000 --- a/src/3rdparty +++ b/src/3rdparty @@ -1 +1 @@ -Subproject commit 42a4c9694a2b2677b050fbb7cbae26bc5ec4605a +Subproject commit e96d95a130a572b611fe70b3c3ede2b4727aaa22 From d0f8edb2a4affa843918afc568a4b040d308560a Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Tue, 11 Feb 2014 15:30:22 -0500 Subject: [PATCH 4/5] Expanding the HTTP methods used in the signature to detect HTTP traffic. --- scripts/base/protocols/http/dpd.sig | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/base/protocols/http/dpd.sig b/scripts/base/protocols/http/dpd.sig index 13470f4e95..3e264f0bb3 100644 --- a/scripts/base/protocols/http/dpd.sig +++ b/scripts/base/protocols/http/dpd.sig @@ -1,6 +1,8 @@ +# List of HTTP headers pulled from: +# http://annevankesteren.nl/2007/10/http-methods signature dpd_http_client { ip-proto == tcp - payload /^[[:space:]]*(GET|HEAD|POST)[[:space:]]*/ + payload /^[[:space:]]*(OPTIONS|GET|HEAD|POST|PUT|DELETE|TRACE|CONNECT|PROPFIND|PROPPATCH|MKCOL|COPY|MOVE|LOCK|UNLOCK|VERSION-CONTROL|REPORT|CHECKOUT|CHECKIN|UNCHECKOUT|MKWORKSPACE|UPDATE|LABEL|MERGE|BASELINE-CONTROL|MKACTIVITY|ORDERPATCH|ACL|PATCH|SEARCH|BCOPY|BDELETE|BMOVE|BPROPFIND|BPROPPATCH|NOTIFY|POLL|SUBSCRIBE|UNSUBSCRIBE|X-MS-ENUMATTS|RPC_OUT_DATA|RPC_IN_DATA)[[:space:]]*/ tcp-state originator } @@ -11,3 +13,5 @@ signature dpd_http_server { requires-reverse-signature dpd_http_client enable "http" } + + From 0f4c7080cc81e9bc66168d6612b80b3af7b54514 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Mon, 3 Mar 2014 07:09:38 -0800 Subject: [PATCH 5/5] HTTP fix for output handlers. Had broken that with the CONNECT change. --- src/analyzer/Analyzer.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/analyzer/Analyzer.cc b/src/analyzer/Analyzer.cc index 63462c0049..b280cbb6f8 100644 --- a/src/analyzer/Analyzer.cc +++ b/src/analyzer/Analyzer.cc @@ -798,8 +798,11 @@ void SupportAnalyzer::ForwardPacket(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()->DeliverPacket(len, data, is_orig, seq, ip, caplen); + return; + } SupportAnalyzer* next_sibling = Sibling(true); @@ -816,7 +819,10 @@ 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); + return; + } SupportAnalyzer* next_sibling = Sibling(true); @@ -833,7 +839,10 @@ 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); + return; + } SupportAnalyzer* next_sibling = Sibling(true);