From 5f3af9e9ebd474f41d2c20d64cd6ac0a37f75782 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 2 Oct 2012 15:13:38 -0500 Subject: [PATCH] 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 | 8 ++++++ src/Teredo.cc | 18 ++++++++++--- src/Teredo.h | 27 +++++++++++++++---- src/const.bif | 1 + .../core.tunnels.false-teredo/dpd.log | 15 ----------- .../known_services.log | 10 +++++++ .../Baseline/core.tunnels.teredo/conn.log | 2 +- .../conn.log | 2 +- .../weird.log | 6 ++--- testing/btest/core/tunnels/false-teredo.bro | 17 +++++++++++- .../core/tunnels/teredo-known-services.test | 11 ++++++++ 11 files changed, 87 insertions(+), 30 deletions(-) delete mode 100644 testing/btest/Baseline/core.tunnels.false-teredo/dpd.log create mode 100644 testing/btest/Baseline/core.tunnels.teredo-known-services/known_services.log create mode 100644 testing/btest/core/tunnels/teredo-known-services.test diff --git a/scripts/base/init-bare.bro b/scripts/base/init-bare.bro index cc3a40f54b..70026394e9 100644 --- a/scripts/base/init-bare.bro +++ b/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 diff --git a/src/Teredo.cc b/src/Teredo.cc index 54676c3255..1f01086090 100644 --- a/src/Teredo.cc +++ b/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(); } } diff --git a/src/Teredo.h b/src/Teredo.h index 84ff8ddf38..e4048d4266 100644 --- a/src/Teredo.h +++ b/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 - * frequently enough to be less than helpful. + * decapsulate a Teredo packet in both directions or if *force* param is + * 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 { diff --git a/src/const.bif b/src/const.bif index 499dc63314..7373403c11 100644 --- a/src/const.bif +++ b/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; diff --git a/testing/btest/Baseline/core.tunnels.false-teredo/dpd.log b/testing/btest/Baseline/core.tunnels.false-teredo/dpd.log deleted file mode 100644 index 3300a3ef95..0000000000 --- a/testing/btest/Baseline/core.tunnels.false-teredo/dpd.log +++ /dev/null @@ -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 diff --git a/testing/btest/Baseline/core.tunnels.teredo-known-services/known_services.log b/testing/btest/Baseline/core.tunnels.teredo-known-services/known_services.log new file mode 100644 index 0000000000..705cd0e956 --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/core.tunnels.teredo/conn.log b/testing/btest/Baseline/core.tunnels.teredo/conn.log index 657e86b8b3..b71e56f073 100644 --- a/testing/btest/Baseline/core.tunnels.teredo/conn.log +++ b/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 diff --git a/testing/btest/Baseline/core.tunnels.teredo_bubble_with_payload/conn.log b/testing/btest/Baseline/core.tunnels.teredo_bubble_with_payload/conn.log index 757eaf62ca..9d4bf86d57 100644 --- a/testing/btest/Baseline/core.tunnels.teredo_bubble_with_payload/conn.log +++ b/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 diff --git a/testing/btest/Baseline/core.tunnels.teredo_bubble_with_payload/weird.log b/testing/btest/Baseline/core.tunnels.teredo_bubble_with_payload/weird.log index 4ead29302f..764b78656a 100644 --- a/testing/btest/Baseline/core.tunnels.teredo_bubble_with_payload/weird.log +++ b/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-06-19-17-39-37 +#close 2012-10-02-16-53-03 diff --git a/testing/btest/core/tunnels/false-teredo.bro b/testing/btest/core/tunnels/false-teredo.bro index 37088e9535..381478bd54 100644 --- a/testing/btest/core/tunnels/false-teredo.bro +++ b/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) { diff --git a/testing/btest/core/tunnels/teredo-known-services.test b/testing/btest/core/tunnels/teredo-known-services.test new file mode 100644 index 0000000000..862930758f --- /dev/null +++ b/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.