Merge branch 'topic/robin/http-connect'

* topic/robin/http-connect:
  HTTP fix for output handlers.
  Expanding the HTTP methods used in the signature to detect HTTP traffic.
  Updating submodule(s).
  Fixing removal of support analyzers, plus some tweaking and cleanup of CONNECT code.
  HTTP CONNECT proxy support.

BIT-1132 #merged

CHANGES

@@ -1,4 +1,14 @@
 
+2.2-206 | 2014-03-03 16:52:28 -0800
+
+  * HTTP CONNECT proxy support. The HTTP analyzer now supports
+    handling HTTP CONNECT proxies. (Seth Hall)
+
+  * Expanding the HTTP methods used in the DPD signature to detect
+    HTTP traffic. (Seth Hall)
+
+  * Fixing removal of support analyzers. (Robin Sommer)
+    
 2.2-199 | 2014-03-03 16:34:20 -0800
 
   * Allow iterating over bif functions with result type vector of any.

VERSION

@@ -1 +1 @@
-2.2-199
+2.2-206

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
 }
 

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,17 @@ 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

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,35 @@ 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 )
+		return;
+		}
+
+	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 +817,18 @@ 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 )
+	if ( GetOutputHandler() )
+		{
+		GetOutputHandler()->DeliverStream(len, data, is_orig);
+		return;
+		}
+
+	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 +837,18 @@ 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 )
+	if ( GetOutputHandler() )
+		{
+		GetOutputHandler()->Undelivered(seq, len, is_orig);
+		return;
+		}
+
+	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);

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;
 

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<const char*>(data);
 	const char* end_of_line = line + len;
 
@@ -1055,6 +1066,32 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig)
 
 				HTTP_Reply();
 
+				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);
+
+						return;
+						}
+
+					else
+						{
+						// Shouldn't really happen.
+						delete pia;
+						pia = 0;
+						}
+					}
+
 				InitHTTPMessage(content_line,
 						reply_message, is_orig,
 						ExpectReplyMessageBody(),
@@ -1390,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;

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 '%<hex><hex>'

src/types.bif

@@ -186,6 +186,7 @@ enum Type %{
 	TEREDO,
 	SOCKS,
 	GTPv1,
+	HTTP,
 %}
 
 type EncapsulatingConn: record;

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

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

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	<nhfjenna_neumann@lycos.com>	<thenightwatch@t-online.de>	Tue, 2 Mar 2004 13:57:49 +0100	Sybille Ostermann <nhfjenna_neumann@lycos.com>	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

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

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

testing/btest/scripts/base/protocols/http/http-connect.bro

@@ -0,0 +1,13 @@
+# This tests that the HTTP analyzer handles HTTP CONNECT proxying correctly.
+#
+# @TEST-EXEC: bro -r $TRACES/http/connect-with-smtp.trace %INPUT
+# @TEST-EXEC: btest-diff conn.log
+# @TEST-EXEC: btest-diff http.log
+# @TEST-EXEC: btest-diff smtp.log
+# @TEST-EXEC: btest-diff tunnel.log
+
+@load base/protocols/conn
+@load base/protocols/http
+@load base/protocols/smtp
+@load base/protocols/tunnels
+@load base/frameworks/dpd