Merge remote-tracking branch 'origin/topic/johanna/dtls-errors'

* origin/topic/johanna/dtls-errors:
  Update DTLS error handling

CHANGES

@@ -1,4 +1,11 @@
 
+2.6-203 | 2019-04-04 16:35:52 -0700
+
+  * Update DTLS error handling (Johanna Amann, Corelight)
+
+    - Adds tuning options: SSL::dtls_max_version_errors and
+      SSL::dtls_max_reported_version_errors
+
 2.6-200 | 2019-04-03 09:44:53 -0700
 
   * Fix reporter net_weird API usage for unknown_mobility_type

VERSION

@@ -1 +1 @@
-2.6-200
+2.6-203

doc

@@ -1 +1 @@
-Subproject commit 5aa921f0f6ce2931e446a11f2a10cffb7f0dbc09
+Subproject commit 6b81dd79ebef6372da961177852e4ea5c65dd5fa

scripts/base/init-bare.bro

@@ -4169,6 +4169,17 @@ export {
 		HashAlgorithm: count; ##< Hash algorithm number
 		SignatureAlgorithm: count; ##< Signature algorithm number
 	};
+
+
+## Number of non-DTLS frames that can occur in a DTLS connection before
+## parsing of the connection is suspended.
+## DTLS does not immediately stop parsing a connection because other protocols
+## might be interleaved in the same UDP "connection".
+const SSL::dtls_max_version_errors = 10 &redef;
+
+## Maximum number of invalid version errors to report in one DTLS connection.
+const SSL::dtls_max_reported_version_errors = 1 &redef;
+
 }
 
 module GLOBAL;

src/analyzer/protocol/ssl/CMakeLists.txt

@@ -8,6 +8,7 @@ bro_plugin_cc(SSL.cc DTLS.cc Plugin.cc)
 bro_plugin_bif(types.bif)
 bro_plugin_bif(events.bif)
 bro_plugin_bif(functions.bif)
+bro_plugin_bif(consts.bif)
 bro_plugin_pac(tls-handshake.pac tls-handshake-protocol.pac tls-handshake-analyzer.pac ssl-defs.pac
 	proc-client-hello.pac
 	proc-server-hello.pac

src/analyzer/protocol/ssl/consts.bif

@@ -0,0 +1,2 @@
+const SSL::dtls_max_version_errors: count;
+const SSL::dtls_max_reported_version_errors: count;

src/analyzer/protocol/ssl/dtls-protocol.pac

@@ -45,15 +45,40 @@ type Handshake(rec: SSLRecord) = record {
 
 refine connection SSL_Conn += {
 
+	%member{
+		uint16 invalid_version_count_;
+		uint16 reported_errors_;
+	%}
+
+	%init{
+		invalid_version_count_ = 0;
+		reported_errors_ = 0;
+	%}
+
 	function dtls_version_ok(version: uint16): uint16
 		%{
 		switch ( version ) {
 		case DTLSv10:
 		case DTLSv12:
+			// Reset only to 0 once we have seen a client hello.
+			// This means the connection gets a limited amount of valid/invalid
+			// packets before a client hello has to be seen - which seems reasonable.
+			if ( bro_analyzer()->ProtocolConfirmed() )
+				invalid_version_count_ = 0;
 			return true;
 
 		default:
+			invalid_version_count_++;
+
+			if ( bro_analyzer()->ProtocolConfirmed() )
+				{
+				reported_errors_++;
+				if ( reported_errors_ <= BifConst::SSL::dtls_max_reported_version_errors )
 					bro_analyzer()->ProtocolViolation(fmt("Invalid version in DTLS connection. Packet reported version: %d", version));
+				}
+
+			if ( invalid_version_count_ > BifConst::SSL::dtls_max_version_errors )
+				bro_analyzer()->SetSkip(true);
 			return false;
 		}
 		%}

src/analyzer/protocol/ssl/dtls.pac

@@ -10,6 +10,7 @@ namespace analyzer { namespace dtls { class DTLS_Analyzer; } }
 typedef analyzer::dtls::DTLS_Analyzer* DTLSAnalyzer;
 
 #include "DTLS.h"
+#include "consts.bif.h"
 %}
 
 extern type DTLSAnalyzer;

testing/btest/Baseline/coverage.bare-load-baseline/canonified_loaded_scripts.log

@@ -3,7 +3,7 @@
 #empty_field	(empty)
 #unset_field	-
 #path	loaded_scripts
-#open	2018-06-08-16-37-15
+#open	2019-04-04-19-22-03
 #fields	name
 #types	string
 scripts/base/init-bare.bro
@@ -149,6 +149,7 @@ scripts/base/init-frameworks-and-bifs.bro
     build/scripts/base/bif/plugins/Bro_SSL.types.bif.bro
     build/scripts/base/bif/plugins/Bro_SSL.events.bif.bro
     build/scripts/base/bif/plugins/Bro_SSL.functions.bif.bro
+    build/scripts/base/bif/plugins/Bro_SSL.consts.bif.bro
     build/scripts/base/bif/plugins/Bro_SteppingStone.events.bif.bro
     build/scripts/base/bif/plugins/Bro_Syslog.events.bif.bro
     build/scripts/base/bif/plugins/Bro_TCP.events.bif.bro
@@ -179,4 +180,4 @@ scripts/base/init-frameworks-and-bifs.bro
     build/scripts/base/bif/plugins/Bro_SQLiteWriter.sqlite.bif.bro
 scripts/policy/misc/loaded-scripts.bro
   scripts/base/utils/paths.bro
-#close	2018-06-08-16-37-15
+#close	2019-04-04-19-22-03

testing/btest/Baseline/coverage.default-load-baseline/canonified_loaded_scripts.log

@@ -3,7 +3,7 @@
 #empty_field	(empty)
 #unset_field	-
 #path	loaded_scripts
-#open	2018-09-05-20-33-08
+#open	2019-04-04-19-22-06
 #fields	name
 #types	string
 scripts/base/init-bare.bro
@@ -149,6 +149,7 @@ scripts/base/init-frameworks-and-bifs.bro
     build/scripts/base/bif/plugins/Bro_SSL.types.bif.bro
     build/scripts/base/bif/plugins/Bro_SSL.events.bif.bro
     build/scripts/base/bif/plugins/Bro_SSL.functions.bif.bro
+    build/scripts/base/bif/plugins/Bro_SSL.consts.bif.bro
     build/scripts/base/bif/plugins/Bro_SteppingStone.events.bif.bro
     build/scripts/base/bif/plugins/Bro_Syslog.events.bif.bro
     build/scripts/base/bif/plugins/Bro_TCP.events.bif.bro
@@ -373,4 +374,4 @@ scripts/base/init-default.bro
   scripts/base/misc/find-filtered-trace.bro
   scripts/base/misc/version.bro
 scripts/policy/misc/loaded-scripts.bro
-#close	2018-09-05-20-33-08
+#close	2019-04-04-19-22-06

testing/btest/Baseline/plugins.hooks/output

@@ -277,7 +277,7 @@
 0.000000   MetaHookPost  CallFunction(Log::__create_stream, <frame>, (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])) -> <no result>
 0.000000   MetaHookPost  CallFunction(Log::__create_stream, <frame>, (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])) -> <no result>
 0.000000   MetaHookPost  CallFunction(Log::__create_stream, <frame>, (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])) -> <no result>
-0.000000   MetaHookPost  CallFunction(Log::__write, <frame>, (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])) -> <no result>
+0.000000   MetaHookPost  CallFunction(Log::__write, <frame>, (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])) -> <no result>
 0.000000   MetaHookPost  CallFunction(Log::add_default_filter, <frame>, (Broker::LOG)) -> <no result>
 0.000000   MetaHookPost  CallFunction(Log::add_default_filter, <frame>, (Cluster::LOG)) -> <no result>
 0.000000   MetaHookPost  CallFunction(Log::add_default_filter, <frame>, (Config::LOG)) -> <no result>
@@ -462,7 +462,7 @@
 0.000000   MetaHookPost  CallFunction(Log::create_stream, <frame>, (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])) -> <no result>
 0.000000   MetaHookPost  CallFunction(Log::create_stream, <frame>, (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])) -> <no result>
 0.000000   MetaHookPost  CallFunction(Log::create_stream, <frame>, (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])) -> <no result>
-0.000000   MetaHookPost  CallFunction(Log::write, <frame>, (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])) -> <no result>
+0.000000   MetaHookPost  CallFunction(Log::write, <frame>, (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])) -> <no result>
 0.000000   MetaHookPost  CallFunction(NetControl::check_plugins, <frame>, ()) -> <no result>
 0.000000   MetaHookPost  CallFunction(NetControl::init, <null>, ()) -> <no result>
 0.000000   MetaHookPost  CallFunction(Notice::want_pp, <frame>, ()) -> <no result>
@@ -678,6 +678,7 @@
 0.000000   MetaHookPost  LoadFile(0, .<...>/Bro_SQLiteWriter.sqlite.bif.bro) -> -1
 0.000000   MetaHookPost  LoadFile(0, .<...>/Bro_SSH.events.bif.bro) -> -1
 0.000000   MetaHookPost  LoadFile(0, .<...>/Bro_SSH.types.bif.bro) -> -1
+0.000000   MetaHookPost  LoadFile(0, .<...>/Bro_SSL.consts.bif.bro) -> -1
 0.000000   MetaHookPost  LoadFile(0, .<...>/Bro_SSL.events.bif.bro) -> -1
 0.000000   MetaHookPost  LoadFile(0, .<...>/Bro_SSL.functions.bif.bro) -> -1
 0.000000   MetaHookPost  LoadFile(0, .<...>/Bro_SSL.types.bif.bro) -> -1
@@ -1179,7 +1180,7 @@
 0.000000   MetaHookPre   CallFunction(Log::__create_stream, <frame>, (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird]))
 0.000000   MetaHookPre   CallFunction(Log::__create_stream, <frame>, (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509]))
 0.000000   MetaHookPre   CallFunction(Log::__create_stream, <frame>, (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql]))
-0.000000   MetaHookPre   CallFunction(Log::__write, <frame>, (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T]))
+0.000000   MetaHookPre   CallFunction(Log::__write, <frame>, (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T]))
 0.000000   MetaHookPre   CallFunction(Log::add_default_filter, <frame>, (Broker::LOG))
 0.000000   MetaHookPre   CallFunction(Log::add_default_filter, <frame>, (Cluster::LOG))
 0.000000   MetaHookPre   CallFunction(Log::add_default_filter, <frame>, (Config::LOG))
@@ -1364,7 +1365,7 @@
 0.000000   MetaHookPre   CallFunction(Log::create_stream, <frame>, (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird]))
 0.000000   MetaHookPre   CallFunction(Log::create_stream, <frame>, (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509]))
 0.000000   MetaHookPre   CallFunction(Log::create_stream, <frame>, (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql]))
-0.000000   MetaHookPre   CallFunction(Log::write, <frame>, (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T]))
+0.000000   MetaHookPre   CallFunction(Log::write, <frame>, (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T]))
 0.000000   MetaHookPre   CallFunction(NetControl::check_plugins, <frame>, ())
 0.000000   MetaHookPre   CallFunction(NetControl::init, <null>, ())
 0.000000   MetaHookPre   CallFunction(Notice::want_pp, <frame>, ())
@@ -1580,6 +1581,7 @@
 0.000000   MetaHookPre   LoadFile(0, .<...>/Bro_SQLiteWriter.sqlite.bif.bro)
 0.000000   MetaHookPre   LoadFile(0, .<...>/Bro_SSH.events.bif.bro)
 0.000000   MetaHookPre   LoadFile(0, .<...>/Bro_SSH.types.bif.bro)
+0.000000   MetaHookPre   LoadFile(0, .<...>/Bro_SSL.consts.bif.bro)
 0.000000   MetaHookPre   LoadFile(0, .<...>/Bro_SSL.events.bif.bro)
 0.000000   MetaHookPre   LoadFile(0, .<...>/Bro_SSL.functions.bif.bro)
 0.000000   MetaHookPre   LoadFile(0, .<...>/Bro_SSL.types.bif.bro)
@@ -2080,7 +2082,7 @@
 0.000000 | HookCallFunction Log::__create_stream(Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])
 0.000000 | HookCallFunction Log::__create_stream(X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])
 0.000000 | HookCallFunction Log::__create_stream(mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])
-0.000000 | HookCallFunction Log::__write(PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])
+0.000000 | HookCallFunction Log::__write(PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])
 0.000000 | HookCallFunction Log::add_default_filter(Broker::LOG)
 0.000000 | HookCallFunction Log::add_default_filter(Cluster::LOG)
 0.000000 | HookCallFunction Log::add_default_filter(Config::LOG)
@@ -2265,7 +2267,7 @@
 0.000000 | HookCallFunction Log::create_stream(Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])
 0.000000 | HookCallFunction Log::create_stream(X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])
 0.000000 | HookCallFunction Log::create_stream(mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])
-0.000000 | HookCallFunction Log::write(PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])
+0.000000 | HookCallFunction Log::write(PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])
 0.000000 | HookCallFunction NetControl::check_plugins()
 0.000000 | HookCallFunction NetControl::init()
 0.000000 | HookCallFunction Notice::want_pp()
@@ -2481,6 +2483,7 @@
 0.000000 | HookLoadFile  .<...>/Bro_SQLiteWriter.sqlite.bif.bro
 0.000000 | HookLoadFile  .<...>/Bro_SSH.events.bif.bro
 0.000000 | HookLoadFile  .<...>/Bro_SSH.types.bif.bro
+0.000000 | HookLoadFile  .<...>/Bro_SSL.consts.bif.bro
 0.000000 | HookLoadFile  .<...>/Bro_SSL.events.bif.bro
 0.000000 | HookLoadFile  .<...>/Bro_SSL.functions.bif.bro
 0.000000 | HookLoadFile  .<...>/Bro_SSL.types.bif.bro
@@ -2699,7 +2702,7 @@
 0.000000 | HookLoadFile  base<...>/x509
 0.000000 | HookLoadFile  base<...>/xmpp
 0.000000 | HookLogInit   packet_filter 1/1 {ts (time), node (string), filter (string), init (bool), success (bool)}
-0.000000 | HookLogWrite  packet_filter [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T]
+0.000000 | HookLogWrite  packet_filter [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T]
 0.000000 | HookQueueEvent NetControl::init()
 0.000000 | HookQueueEvent bro_init()
 0.000000 | HookQueueEvent filter_change_tracking()

testing/btest/scripts/base/protocols/ssl/dtls-no-dtls.test

@@ -0,0 +1,15 @@
+# This tests checks that non-dtls connections to which we attach don't trigger tons of errors.
+
+# @TEST-EXEC: bro -C -r $TRACES/dns-txt-multiple.trace %INPUT
+# @TEST-EXEC: btest-diff .stdout
+
+event bro_init()
+	{
+	const add_ports = { 53/udp };
+	Analyzer::register_for_ports(Analyzer::ANALYZER_DTLS, add_ports);
+	}
+
+event protocol_violation(c: connection, atype: Analyzer::Tag, aid: count, reason: string)
+	{
+	print c$id, atype, reason;
+	}