diff --git a/CHANGES b/CHANGES index f2beb944fc..e0be717f10 100644 --- a/CHANGES +++ b/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 diff --git a/VERSION b/VERSION index 879e9b62f6..92df2bb961 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.6-200 +2.6-203 diff --git a/doc b/doc index 5aa921f0f6..6b81dd79eb 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 5aa921f0f6ce2931e446a11f2a10cffb7f0dbc09 +Subproject commit 6b81dd79ebef6372da961177852e4ea5c65dd5fa diff --git a/scripts/base/init-bare.bro b/scripts/base/init-bare.bro index 0c32cebcc5..e94efd07df 100644 --- a/scripts/base/init-bare.bro +++ b/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; diff --git a/src/analyzer/protocol/ssl/CMakeLists.txt b/src/analyzer/protocol/ssl/CMakeLists.txt index 14e41892c8..3193470635 100644 --- a/src/analyzer/protocol/ssl/CMakeLists.txt +++ b/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 @@ -16,7 +17,7 @@ bro_plugin_pac(tls-handshake.pac tls-handshake-protocol.pac tls-handshake-analyz ) bro_plugin_pac(ssl.pac ssl-dtls-analyzer.pac ssl-analyzer.pac ssl-dtls-protocol.pac ssl-protocol.pac ssl-defs.pac proc-client-hello.pac - proc-server-hello.pac + proc-server-hello.pac proc-certificate.pac ) bro_plugin_pac(dtls.pac ssl-dtls-analyzer.pac dtls-analyzer.pac ssl-dtls-protocol.pac dtls-protocol.pac ssl-defs.pac) diff --git a/src/analyzer/protocol/ssl/consts.bif b/src/analyzer/protocol/ssl/consts.bif new file mode 100644 index 0000000000..9dcbaa65d5 --- /dev/null +++ b/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; diff --git a/src/analyzer/protocol/ssl/dtls-protocol.pac b/src/analyzer/protocol/ssl/dtls-protocol.pac index 771aa267b3..70897a585c 100644 --- a/src/analyzer/protocol/ssl/dtls-protocol.pac +++ b/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: - bro_analyzer()->ProtocolViolation(fmt("Invalid version in DTLS connection. Packet reported version: %d", version)); + 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; } %} diff --git a/src/analyzer/protocol/ssl/dtls.pac b/src/analyzer/protocol/ssl/dtls.pac index b08dd61f8f..b2aa34d5c5 100644 --- a/src/analyzer/protocol/ssl/dtls.pac +++ b/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; diff --git a/testing/btest/Baseline/coverage.bare-load-baseline/canonified_loaded_scripts.log b/testing/btest/Baseline/coverage.bare-load-baseline/canonified_loaded_scripts.log index 4eeaa4b07b..bd24bf02aa 100644 --- a/testing/btest/Baseline/coverage.bare-load-baseline/canonified_loaded_scripts.log +++ b/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 diff --git a/testing/btest/Baseline/coverage.default-load-baseline/canonified_loaded_scripts.log b/testing/btest/Baseline/coverage.default-load-baseline/canonified_loaded_scripts.log index eaca1c489a..540910b350 100644 --- a/testing/btest/Baseline/coverage.default-load-baseline/canonified_loaded_scripts.log +++ b/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 diff --git a/testing/btest/Baseline/plugins.hooks/output b/testing/btest/Baseline/plugins.hooks/output index d4a84a5223..04908bed0b 100644 --- a/testing/btest/Baseline/plugins.hooks/output +++ b/testing/btest/Baseline/plugins.hooks/output @@ -277,7 +277,7 @@ 0.000000 MetaHookPost CallFunction(Log::__create_stream, , (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])) -> 0.000000 MetaHookPost CallFunction(Log::__create_stream, , (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])) -> 0.000000 MetaHookPost CallFunction(Log::__create_stream, , (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])) -> -0.000000 MetaHookPost CallFunction(Log::__write, , (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])) -> +0.000000 MetaHookPost CallFunction(Log::__write, , (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])) -> 0.000000 MetaHookPost CallFunction(Log::add_default_filter, , (Broker::LOG)) -> 0.000000 MetaHookPost CallFunction(Log::add_default_filter, , (Cluster::LOG)) -> 0.000000 MetaHookPost CallFunction(Log::add_default_filter, , (Config::LOG)) -> @@ -462,7 +462,7 @@ 0.000000 MetaHookPost CallFunction(Log::create_stream, , (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])) -> 0.000000 MetaHookPost CallFunction(Log::create_stream, , (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])) -> 0.000000 MetaHookPost CallFunction(Log::create_stream, , (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])) -> -0.000000 MetaHookPost CallFunction(Log::write, , (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])) -> +0.000000 MetaHookPost CallFunction(Log::write, , (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])) -> 0.000000 MetaHookPost CallFunction(NetControl::check_plugins, , ()) -> 0.000000 MetaHookPost CallFunction(NetControl::init, , ()) -> 0.000000 MetaHookPost CallFunction(Notice::want_pp, , ()) -> @@ -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, , (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])) 0.000000 MetaHookPre CallFunction(Log::__create_stream, , (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])) 0.000000 MetaHookPre CallFunction(Log::__create_stream, , (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])) -0.000000 MetaHookPre CallFunction(Log::__write, , (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])) +0.000000 MetaHookPre CallFunction(Log::__write, , (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])) 0.000000 MetaHookPre CallFunction(Log::add_default_filter, , (Broker::LOG)) 0.000000 MetaHookPre CallFunction(Log::add_default_filter, , (Cluster::LOG)) 0.000000 MetaHookPre CallFunction(Log::add_default_filter, , (Config::LOG)) @@ -1364,7 +1365,7 @@ 0.000000 MetaHookPre CallFunction(Log::create_stream, , (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])) 0.000000 MetaHookPre CallFunction(Log::create_stream, , (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])) 0.000000 MetaHookPre CallFunction(Log::create_stream, , (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])) -0.000000 MetaHookPre CallFunction(Log::write, , (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])) +0.000000 MetaHookPre CallFunction(Log::write, , (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])) 0.000000 MetaHookPre CallFunction(NetControl::check_plugins, , ()) 0.000000 MetaHookPre CallFunction(NetControl::init, , ()) 0.000000 MetaHookPre CallFunction(Notice::want_pp, , ()) @@ -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() diff --git a/testing/btest/Baseline/scripts.base.protocols.ssl.dtls-no-dtls/.stdout b/testing/btest/Baseline/scripts.base.protocols.ssl.dtls-no-dtls/.stdout new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testing/btest/scripts/base/protocols/ssl/dtls-no-dtls.test b/testing/btest/scripts/base/protocols/ssl/dtls-no-dtls.test new file mode 100644 index 0000000000..c8721529c9 --- /dev/null +++ b/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; + }