Add new Tunnel::delay_teredo_confirmation option, default to true.

This option indicates that the Teredo analyzer should wait until
it sees both sides of a connection using a valid Teredo encapsulation
before issuing a protocol_confirmation.  Previous behavior confirmed
on the first instance of a valid encapsulation, which could result
in more false positives (and e.g. bogus entries in known-services.log).

Addresses #890.

scripts/base/init-bare.bro

@@ -2784,6 +2784,14 @@ export {
 	## to have a valid Teredo encapsulation.
 	const yielding_teredo_decapsulation = T &redef;
 
+	## With this set, the Teredo analyzer waits until it sees both sides
+	## of a connection using a valid Teredo encapsulation before issuing
+	## a :bro:see:`protocol_confirmation`.  If it's false, the first
+	## occurence of a packet with valid Teredo encapsulation causes a
+	## confirmation.  Both cases are still subject to effects of
+	## :bro:see:`Tunnel::yielding_teredo_decapsulation`.
+	const delay_teredo_confirmation = T &redef;
+
 	## How often to cleanup internal state for inactive IP tunnels.
 	const ip_tunnel_timeout = 24hrs &redef;
 } # end export

src/Teredo.cc

@@ -138,6 +138,11 @@ void Teredo_Analyzer::DeliverPacket(int len, const u_char* data, bool orig,
 	{
 	Analyzer::DeliverPacket(len, data, orig, seq, ip, caplen);
 
+	if ( orig )
+		valid_orig = false;
+	else
+		valid_resp = false;
+
 	TeredoEncapsulation te(this);
 
 	if ( ! te.Parse(data, len) )
@@ -150,7 +155,7 @@ void Teredo_Analyzer::DeliverPacket(int len, const u_char* data, bool orig,
 
 	if ( e && e->Depth() >= BifConst::Tunnel::max_depth )
 		{
-		Weird("tunnel_depth");
+		Weird("tunnel_depth", true);
 		return;
 		}
 
@@ -162,7 +167,7 @@ void Teredo_Analyzer::DeliverPacket(int len, const u_char* data, bool orig,
 		if ( inner->NextProto() == IPPROTO_NONE && inner->PayloadLen() == 0 )
 			// Teredo bubbles having data after IPv6 header isn't strictly a
 			// violation, but a little weird.
-			Weird("Teredo_bubble_with_payload");
+			Weird("Teredo_bubble_with_payload", true);
 		else
 			{
 			delete inner;
@@ -173,6 +178,11 @@ void Teredo_Analyzer::DeliverPacket(int len, const u_char* data, bool orig,
 
 	if ( rslt == 0 || rslt > 0 )
 		{
+		if ( orig )
+			valid_orig = true;
+		else
+			valid_resp = true;
+
 		if ( BifConst::Tunnel::yielding_teredo_decapsulation &&
 		     ! ProtocolConfirmed() )
 			{
@@ -193,7 +203,7 @@ void Teredo_Analyzer::DeliverPacket(int len, const u_char* data, bool orig,
 				}
 
 			if ( ! sibling_has_confirmed )
-				ProtocolConfirmation();
+				Confirm();
 			else
 				{
 				delete inner;
@@ -203,7 +213,7 @@ void Teredo_Analyzer::DeliverPacket(int len, const u_char* data, bool orig,
 		else
 			{
 			// Aggressively decapsulate anything with valid Teredo encapsulation
-			ProtocolConfirmation();
+			Confirm();
 			}
 		}
 

src/Teredo.h

@@ -6,7 +6,8 @@
 
 class Teredo_Analyzer : public Analyzer {
 public:
-	Teredo_Analyzer(Connection* conn) : Analyzer(AnalyzerTag::Teredo, conn)
+	Teredo_Analyzer(Connection* conn) : Analyzer(AnalyzerTag::Teredo, conn),
+	                                    valid_orig(false), valid_resp(false)
 		{}
 
 	virtual ~Teredo_Analyzer()
@@ -26,18 +27,34 @@ public:
 
 	/**
 	 * Emits a weird only if the analyzer has previously been able to
-	 * decapsulate a Teredo packet since otherwise the weirds could happen
+	 * decapsulate a Teredo packet in both directions or if *force* param is
-	 * frequently enough to be less than helpful.
+	 * set, since otherwise the weirds could happen frequently enough to be less
+	 * than helpful.  The *force* param is meant for cases where just one side
+	 * has a valid encapsulation and so the weird would be informative.
 	 */
-	void Weird(const char* name) const
+	void Weird(const char* name, bool force = false) const
 		{
-		if ( ProtocolConfirmed() )
+		if ( ProtocolConfirmed() || force )
 			reporter->Weird(Conn(), name);
 		}
 
+	/**
+	 * If the delayed confirmation option is set, then a valid encapsulation
+	 * seen from both end points is required before confirming
+	 */
+	void Confirm()
+		{
+		if ( ! BifConst::Tunnel::delay_teredo_confirmation ||
+		     ( valid_orig && valid_resp ) )
+			ProtocolConfirmation();
+		}
+
 protected:
 	friend class AnalyzerTimer;
 	void ExpireTimer(double t);
+
+	bool valid_orig;
+	bool valid_resp;
 };
 
 class TeredoEncapsulation {

src/const.bif

@@ -16,6 +16,7 @@ const Tunnel::enable_ip: bool;
 const Tunnel::enable_ayiya: bool;
 const Tunnel::enable_teredo: bool;
 const Tunnel::yielding_teredo_decapsulation: bool;
+const Tunnel::delay_teredo_confirmation: bool;
 const Tunnel::ip_tunnel_timeout: interval;
 
 const Threading::heartbeat_interval: interval;

testing/btest/Baseline/core.tunnels.false-teredo/dpd.log

@@ -1,15 +0,0 @@
-#separator \x09
-#set_separator	,
-#empty_field	(empty)
-#unset_field	-
-#path	dpd
-#open	2009-11-18-17-59-51
-#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	analyzer	failure_reason
-#types	time	string	addr	port	addr	port	enum	string	string
-1258567191.486869	UWkUyAuUGXf	192.168.1.105	57696	192.168.1.1	53	udp	TEREDO	Teredo payload length [c\x1d\x81\x80\x00\x01\x00\x02\x00\x02\x00\x00\x04amch\x0equestionmarket\x03com\x00\x00\x01\x00...]
-1258578181.516140	nQcgTWjvg4c	192.168.1.104	64838	192.168.1.1	53	udp	TEREDO	Teredo payload length [h\xfd\x81\x80\x00\x01\x00\x02\x00\x03\x00\x02\x08football\x02uk\x07reuters\x03com\x00\x00\x01\x00...]
-1258579063.784919	j4u32Pc5bif	192.168.1.104	55778	192.168.1.1	53	udp	TEREDO	Teredo payload length [j\x12\x81\x80\x00\x01\x00\x02\x00\x04\x00\x00\x08fastflip\x0agooglelabs\x03com\x00\x00\x01\x00...]
-1258581768.898165	TEfuqmmG4bh	192.168.1.104	50798	192.168.1.1	53	udp	TEREDO	Teredo payload length [o\xe3\x81\x80\x00\x01\x00\x02\x00\x04\x00\x04\x03www\x0fnashuatelegraph\x03com\x00\x00\x01\x00...]
-1258584478.989528	FrJExwHcSal	192.168.1.104	64963	192.168.1.1	53	udp	TEREDO	Teredo payload length [e\xbd\x81\x80\x00\x01\x00\x08\x00\x06\x00\x06\x08wellness\x05blogs\x04time\x03com\x00\x00\x01\x00...]
-1258600683.934672	5OKnoww6xl4	192.168.1.103	59838	192.168.1.1	53	udp	TEREDO	Teredo payload length [h\xf0\x81\x80\x00\x01\x00\x01\x00\x02\x00\x00\x06update\x0csanasecurity\x03com\x00\x00\x01\x00...]
-#close	2009-11-19-03-18-03

testing/btest/Baseline/core.tunnels.teredo-known-services/known_services.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	known_services
+#open	2012-10-02-20-10-05
+#fields	ts	host	port_num	port_proto	service
+#types	time	addr	port	enum	table[string]
+1258567191.405770	192.168.1.1	53	udp	TEREDO
+#close	2012-10-02-20-10-05

testing/btest/Baseline/core.tunnels.teredo/conn.log

@@ -22,7 +22,7 @@
 1210953052.202579	nQcgTWjvg4c	192.168.2.16	3797	65.55.158.80	3544	udp	teredo	8.928880	129	48	SF	-	0	Dd	2	185	1	76	(empty)
 1210953060.829233	GSxOnSLghOa	192.168.2.16	3797	83.170.1.38	32900	udp	teredo	13.293994	2359	11243	SF	-	0	Dd	12	2695	13	11607	(empty)
 1210953058.933954	iE6yhOq3SF	0.0.0.0	68	255.255.255.255	67	udp	-	-	-	-	S0	-	0	D	1	328	0	0	(empty)
-1210953052.324629	TEfuqmmG4bh	192.168.2.16	3797	65.55.158.81	3544	udp	teredo	-	-	-	SHR	-	0	d	0	0	1	137	(empty)
+1210953052.324629	TEfuqmmG4bh	192.168.2.16	3797	65.55.158.81	3544	udp	-	-	-	-	SHR	-	0	d	0	0	1	137	(empty)
 1210953046.591933	UWkUyAuUGXf	192.168.2.16	138	192.168.2.255	138	udp	-	28.448321	416	0	S0	-	0	D	2	472	0	0	(empty)
 1210953052.324629	FrJExwHcSal	fe80::8000:f227:bec8:61af	134	fe80::8000:ffff:ffff:fffd	133	icmp	-	-	-	-	OTH	-	0	-	1	88	0	0	TEfuqmmG4bh
 1210953060.829303	qCaWGmzFtM5	2001:0:4137:9e50:8000:f12a:b9c8:2815	128	2001:4860:0:2001::68	129	icmp	-	0.463615	4	4	OTH	-	0	-	1	52	1	52	GSxOnSLghOa,nQcgTWjvg4c

testing/btest/Baseline/core.tunnels.teredo_bubble_with_payload/conn.log

@@ -9,7 +9,7 @@
 1340127577.354166	FrJExwHcSal	2001:0:4137:9e50:8000:f12a:b9c8:2815	1286	2001:4860:0:2001::68	80	tcp	http	0.052829	1675	10467	S1	-	0	ShADad	10	2279	12	11191	j4u32Pc5bif
 1340127577.336558	UWkUyAuUGXf	192.168.2.16	3797	65.55.158.80	3544	udp	teredo	0.010291	129	52	SF	-	0	Dd	2	185	1	80	(empty)
 1340127577.341510	j4u32Pc5bif	192.168.2.16	3797	83.170.1.38	32900	udp	teredo	0.065485	2367	11243	SF	-	0	Dd	12	2703	13	11607	(empty)
-1340127577.339015	k6kgXLOoSKl	192.168.2.16	3797	65.55.158.81	3544	udp	teredo	-	-	-	SHR	-	0	d	0	0	1	137	(empty)
+1340127577.339015	k6kgXLOoSKl	192.168.2.16	3797	65.55.158.81	3544	udp	-	-	-	-	SHR	-	0	d	0	0	1	137	(empty)
 1340127577.339015	nQcgTWjvg4c	fe80::8000:f227:bec8:61af	134	fe80::8000:ffff:ffff:fffd	133	icmp	-	-	-	-	OTH	-	0	-	1	88	0	0	k6kgXLOoSKl
 1340127577.343969	TEfuqmmG4bh	2001:0:4137:9e50:8000:f12a:b9c8:2815	128	2001:4860:0:2001::68	129	icmp	-	0.007778	4	4	OTH	-	0	-	1	52	1	52	UWkUyAuUGXf,j4u32Pc5bif
 1340127577.336558	arKYeMETxOg	fe80::8000:ffff:ffff:fffd	133	ff02::2	134	icmp	-	-	-	-	OTH	-	0	-	1	64	0	0	UWkUyAuUGXf

testing/btest/Baseline/core.tunnels.teredo_bubble_with_payload/weird.log

@@ -3,9 +3,9 @@
 #empty_field	(empty)
 #unset_field	-
 #path	weird
-#open	2012-06-19-17-39-37
+#open	2012-10-02-16-53-03
 #fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	name	addl	notice	peer
 #types	time	string	addr	port	addr	port	string	string	bool	string
+1340127577.341510	j4u32Pc5bif	192.168.2.16	3797	83.170.1.38	32900	Teredo_bubble_with_payload	-	F	bro
 1340127577.346849	UWkUyAuUGXf	192.168.2.16	3797	65.55.158.80	3544	Teredo_bubble_with_payload	-	F	bro
-1340127577.349292	j4u32Pc5bif	192.168.2.16	3797	83.170.1.38	32900	Teredo_bubble_with_payload	-	F	bro
+#close	2012-10-02-16-53-03
-#close	2012-06-19-17-39-37

testing/btest/core/tunnels/false-teredo.bro

@@ -1,8 +1,23 @@
 # @TEST-EXEC: bro -r $TRACES/tunnels/false-teredo.pcap %INPUT >output
 # @TEST-EXEC: test ! -e weird.log
+# @TEST-EXEC: test ! -e dpd.log
 # @TEST-EXEC: bro -r $TRACES/tunnels/false-teredo.pcap %INPUT Tunnel::yielding_teredo_decapsulation=F >output
 # @TEST-EXEC: btest-diff weird.log
-# @TEST-EXEC: btest-diff dpd.log
+# @TEST-EXEC: test ! -e dpd.log
+
+# In the first case, there isn't any weird or protocol violation logged
+# since the teredo analyzer recognizes that the DNS analyzer has confirmed
+# the protocol and yields.
+
+# In the second case, there are weirds since the teredo analyzer decapsulates
+# despite the presence of the confirmed DNS analyzer and the resulting
+# inner packets are malformed (no surprise there).  There's also no dpd.log
+# since the teredo analyzer doesn't confirm until it's seen a valid teredo
+# encapsulation in both directions and protocol violations aren't logged
+# until there's been a confirmation.
+
+# In either case, the analyzer doesn't, by default, get disabled as a result
+# of the protocol violations.
 
 function print_teredo(name: string, outer: connection, inner: teredo_hdr)
 	{

testing/btest/core/tunnels/teredo-known-services.test

@@ -0,0 +1,11 @@
+# @TEST-EXEC: bro -b -r $TRACES/tunnels/false-teredo.pcap base/frameworks/dpd protocols/conn/known-services Tunnel::delay_teredo_confirmation=T "Site::local_nets+={192.168.1.0/24}"
+# @TEST-EXEC: test ! -e known_services.log
+# @TEST-EXEC: bro -b -r $TRACES/tunnels/false-teredo.pcap base/frameworks/dpd protocols/conn/known-services Tunnel::delay_teredo_confirmation=F "Site::local_nets+={192.168.1.0/24}"
+# @TEST-EXEC: btest-diff known_services.log
+
+# The first case using Tunnel::delay_teredo_confirmation=T doesn't produce
+# a known services.log since valid Teredo encapsulations from both endpoints
+# of a connection is never witnessed and a protocol_confirmation never issued.
+
+# The second case issues protocol_confirmations more hastily and so bogus
+# entries in known-services.log are more likely to appear.