From 7e03233d55ebb1a4d7395eeff81dfd09f538e005 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 3 Jul 2020 11:16:35 -0700 Subject: [PATCH 1/3] Fix incorrect/missing Broker error status code numbers --- src/broker/Manager.cc | 62 +++++++------------------------------------ src/broker/comm.bif | 36 +++++++++++++++---------- 2 files changed, 31 insertions(+), 67 deletions(-) diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index a0c9797bd7..89f73edfaa 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -1290,61 +1290,17 @@ void Manager::ProcessError(broker::error err) if ( err.category() == caf::atom("broker") ) { - msg = caf::to_string(err.context()); + static auto enum_type = zeek::id::find_type("Broker::ErrorCode"); - switch ( static_cast(err.code()) ) { - case broker::ec::peer_incompatible: - ec = BifEnum::Broker::ErrorCode::PEER_INCOMPATIBLE; - break; - - case broker::ec::peer_invalid: - ec = BifEnum::Broker::ErrorCode::PEER_INVALID; - break; - - case broker::ec::peer_unavailable: - ec = BifEnum::Broker::ErrorCode::PEER_UNAVAILABLE; - break; - - case broker::ec::peer_timeout: - ec = BifEnum::Broker::ErrorCode::PEER_TIMEOUT; - break; - - case broker::ec::master_exists: - ec = BifEnum::Broker::ErrorCode::MASTER_EXISTS; - break; - - case broker::ec::no_such_master: - ec = BifEnum::Broker::ErrorCode::NO_SUCH_MASTER; - break; - - case broker::ec::no_such_key: - ec = BifEnum::Broker::ErrorCode::NO_SUCH_KEY; - break; - - case broker::ec::request_timeout: - ec = BifEnum::Broker::ErrorCode::REQUEST_TIMEOUT; - break; - - case broker::ec::type_clash: - ec = BifEnum::Broker::ErrorCode::TYPE_CLASH; - break; - - case broker::ec::invalid_data: - ec = BifEnum::Broker::ErrorCode::INVALID_DATA; - break; - - case broker::ec::backend_failure: - ec = BifEnum::Broker::ErrorCode::BACKEND_FAILURE; - break; - - case broker::ec::stale_data: - ec = BifEnum::Broker::ErrorCode::STALE_DATA; - break; - - case broker::ec::unspecified: // fall-through - default: + if ( enum_type->Lookup(err.code()) ) + ec = static_cast(err.code()); + else + { + reporter->Warning("Unknown Broker error code %u: mapped to unspecificed enum value ", err.code()); ec = BifEnum::Broker::ErrorCode::UNSPECIFIED; - } + } + + msg = caf::to_string(err.context()); } else { diff --git a/src/broker/comm.bif b/src/broker/comm.bif index f9baff387f..3e5b5e5dc2 100644 --- a/src/broker/comm.bif +++ b/src/broker/comm.bif @@ -24,20 +24,28 @@ event Broker::error%(code: ErrorCode, msg: string%); ## Enumerates the possible error types. enum ErrorCode %{ - UNSPECIFIED = 1, - PEER_INCOMPATIBLE = 2, - PEER_INVALID = 3, - PEER_UNAVAILABLE = 4, - PEER_TIMEOUT = 5, - MASTER_EXISTS = 6, - NO_SUCH_MASTER = 7, - NO_SUCH_KEY = 8, - REQUEST_TIMEOUT = 9, - TYPE_CLASH = 10, - INVALID_DATA = 11, - BACKEND_FAILURE = 12, - STALE_DATA = 13, - CAF_ERROR = 100, + NO_ERROR = 0, + UNSPECIFIED = 1, + PEER_INCOMPATIBLE = 2, + PEER_INVALID = 3, + PEER_UNAVAILABLE = 4, + PEER_DISCONNECT_DURING_HANDSHAKE = 5, + PEER_TIMEOUT = 6, + MASTER_EXISTS = 7, + NO_SUCH_MASTER = 8, + NO_SUCH_KEY = 9, + REQUEST_TIMEOUT = 10, + TYPE_CLASH = 11, + INVALID_DATA = 12, + BACKEND_FAILURE = 13, + STALE_DATA = 14, + CANNOT_OPEN_FILE = 15, + CANNOT_WRITE_FILE = 16, + INVALID_TOPIC_KEY = 17, + END_OF_FILE = 18, + INVALID_TAG = 19, + INVALID_STATUS = 20, + CAF_ERROR = 100, %} enum PeerStatus %{ From c84a51ac09e1db20823025b150d319c0fb02c343 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 16 Jul 2020 18:07:00 -0700 Subject: [PATCH 2/3] GH-837: emit Reporter errors for Broker errors Instead of only writing them in broker.log, which may be easy to overlook. --- scripts/base/frameworks/broker/log.zeek | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/base/frameworks/broker/log.zeek b/scripts/base/frameworks/broker/log.zeek index bd76684b74..84f9196a7c 100644 --- a/scripts/base/frameworks/broker/log.zeek +++ b/scripts/base/frameworks/broker/log.zeek @@ -71,10 +71,12 @@ event Broker::error(code: ErrorCode, msg: string) ev = subst_string(ev, "Broker::", ""); ev = subst_string(ev, "_", "-"); ev = to_lower(ev); - + Log::write(Broker::LOG, [$ts = network_time(), $ev = ev, $ty = ERROR, $message = msg]); + + Reporter::error(fmt("Broker error (%s): %s", code, msg)); } From 85fbdaf4293af21529b26013fa68ba72331bf757 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 16 Jul 2020 18:32:06 -0700 Subject: [PATCH 3/3] GH-837: Add test cases for mismatched Broker SSL configs --- auxil/broker | 2 +- .../bad_connect.broker.error | 1 + .../bad_connect_rev.broker.error | 1 + testing/btest/broker/ssl-mismatch.zeek | 71 +++++++++++++++++++ 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 testing/btest/Baseline/broker.ssl-mismatch/bad_connect.broker.error create mode 100644 testing/btest/Baseline/broker.ssl-mismatch/bad_connect_rev.broker.error create mode 100644 testing/btest/broker/ssl-mismatch.zeek diff --git a/auxil/broker b/auxil/broker index 529b67dd91..541f2ef02c 160000 --- a/auxil/broker +++ b/auxil/broker @@ -1 +1 @@ -Subproject commit 529b67dd91cd278e6585902467672a9f1687f975 +Subproject commit 541f2ef02ca48094fcf3d5f341779907f388151e diff --git a/testing/btest/Baseline/broker.ssl-mismatch/bad_connect.broker.error b/testing/btest/Baseline/broker.ssl-mismatch/bad_connect.broker.error new file mode 100644 index 0000000000..7a8d99ac5e --- /dev/null +++ b/testing/btest/Baseline/broker.ssl-mismatch/bad_connect.broker.error @@ -0,0 +1 @@ +Broker::PEER_UNAVAILABLE diff --git a/testing/btest/Baseline/broker.ssl-mismatch/bad_connect_rev.broker.error b/testing/btest/Baseline/broker.ssl-mismatch/bad_connect_rev.broker.error new file mode 100644 index 0000000000..7a8d99ac5e --- /dev/null +++ b/testing/btest/Baseline/broker.ssl-mismatch/bad_connect_rev.broker.error @@ -0,0 +1 @@ +Broker::PEER_UNAVAILABLE diff --git a/testing/btest/broker/ssl-mismatch.zeek b/testing/btest/broker/ssl-mismatch.zeek new file mode 100644 index 0000000000..60ab15462a --- /dev/null +++ b/testing/btest/broker/ssl-mismatch.zeek @@ -0,0 +1,71 @@ +# @TEST-PORT: BROKER_PORT +# +# @TEST-EXEC: btest-bg-run listen "zeek -b %INPUT connect=F Broker::disable_ssl=T" +# +# @TEST-EXEC: btest-bg-run good_connect "zeek -b %INPUT connect=T Broker::disable_ssl=T" +# @TEST-EXEC: $SCRIPTS/wait-for-file good_connect/listen_ready 20 || (btest-bg-wait -k 1 && false) +# +# @TEST-EXEC: btest-bg-run bad_connect "zeek -b %INPUT connect=T Broker::disable_ssl=F" +# @TEST-EXEC: $SCRIPTS/wait-for-file bad_connect/done 20 || (btest-bg-wait -k 1 && false) +# +# @TEST-EXEC: btest-bg-run last_connect "zeek -b %INPUT connect=T Broker::disable_ssl=T" +# +# @TEST-EXEC: btest-bg-wait 30 +# @TEST-EXEC: btest-diff bad_connect/broker.error +# +# And again, now reversing the SSL mismatch between client/server... +# +# @TEST-EXEC: btest-bg-run listen_rev "zeek -b %INPUT connect=F Broker::disable_ssl=F" +# +# @TEST-EXEC: btest-bg-run good_connect_rev "zeek -b %INPUT connect=T Broker::disable_ssl=F" +# @TEST-EXEC: $SCRIPTS/wait-for-file good_connect_rev/listen_ready 20 || (btest-bg-wait -k 1 && false) +# +# @TEST-EXEC: btest-bg-run bad_connect_rev "zeek -b %INPUT connect=T Broker::disable_ssl=T" +# @TEST-EXEC: $SCRIPTS/wait-for-file bad_connect_rev/done 20 || (btest-bg-wait -k 1 && false) +# +# @TEST-EXEC: btest-bg-run last_connect_rev "zeek -b %INPUT connect=T Broker::disable_ssl=F" +# +# @TEST-EXEC: btest-bg-wait 30 +# @TEST-EXEC: btest-diff bad_connect_rev/broker.error + +option connect = T; +global num_connections = 0; + +event zeek_init() + { + if ( connect ) + Broker::peer("127.0.0.1", to_port(getenv("BROKER_PORT"))); + else + Broker::listen("127.0.0.1", to_port(getenv("BROKER_PORT"))); + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print "peer added"; + ++num_connections; + + if ( connect ) + { + system("touch listen_ready"); + terminate(); + } + else if ( num_connections == 2 ) + terminate(); + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + print "peer lost"; + } + +event Broker::error(code: Broker::ErrorCode, msg: string) &priority=-10 + { + if ( connect ) + { + local f = open("broker.error"); + print f, code; + close(f); + system("touch done"); + terminate(); + } + }