Merge remote-tracking branch 'origin/topic/jsiwek/gh-837-improve-broker-mismatch-errors'

* origin/topic/jsiwek/gh-837-improve-broker-mismatch-errors:
  GH-837: Add test cases for mismatched Broker SSL configs
  GH-837: emit Reporter errors for Broker errors
  Fix incorrect/missing Broker error status code numbers

CHANGES

@@ -1,4 +1,13 @@
 
+3.2.0-dev.906 | 2020-07-17 10:08:31 +0000
+
+  * GH-837: Add test cases for mismatched Broker SSL configs. (Jon Siwek, Corelight)
+
+  * GH-837: Emit reporter errors for Broker errors. (Jon Siwek,
+    Corelight)
+
+  * Fix incorrect/missing Broker error status code numbers. (Jon Siwek, Corelight)
+
 3.2.0-dev.902 | 2020-07-14 19:10:36 -0700
 
   * GH-1062: fix integer conversion errors related to Tag subtypes (Jon Siwek, Corelight)

VERSION

@@ -1 +1 @@
-3.2.0-dev.902
+3.2.0-dev.906

auxil/broker

@@ -1 +1 @@
-Subproject commit 529b67dd91cd278e6585902467672a9f1687f975
+Subproject commit 541f2ef02ca48094fcf3d5f341779907f388151e

scripts/base/frameworks/broker/log.zeek

@@ -76,5 +76,7 @@ event Broker::error(code: ErrorCode, msg: string)
 	           $ev = ev,
 	           $ty = ERROR,
 	           $message = msg]);
+
+	Reporter::error(fmt("Broker error (%s): %s", code, msg));
 	}
 

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<zeek::EnumType>("Broker::ErrorCode");
 
-		switch ( static_cast<broker::ec>(err.code()) ) {
+		if ( enum_type->Lookup(err.code()) )
-		case broker::ec::peer_incompatible:
+			ec = static_cast<BifEnum::Broker::ErrorCode>(err.code());
-			ec = BifEnum::Broker::ErrorCode::PEER_INCOMPATIBLE;
+		else
-			break;
+			{
-
+			reporter->Warning("Unknown Broker error code %u: mapped to unspecificed enum value ", err.code());
-		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:
 			ec = BifEnum::Broker::ErrorCode::UNSPECIFIED;
-		}
+			}
+
+		msg = caf::to_string(err.context());
 		}
 	else
 		{

src/broker/comm.bif

@@ -24,20 +24,28 @@ event Broker::error%(code: ErrorCode, msg: string%);
 
 ## Enumerates the possible error types.
 enum ErrorCode %{
-	UNSPECIFIED = 1,
+	NO_ERROR                         =   0,
-	PEER_INCOMPATIBLE = 2,
+	UNSPECIFIED                      =   1,
-	PEER_INVALID = 3,
+	PEER_INCOMPATIBLE                =   2,
-	PEER_UNAVAILABLE = 4,
+	PEER_INVALID                     =   3,
-	PEER_TIMEOUT = 5,
+	PEER_UNAVAILABLE                 =   4,
-	MASTER_EXISTS = 6,
+	PEER_DISCONNECT_DURING_HANDSHAKE =   5,
-	NO_SUCH_MASTER = 7,
+	PEER_TIMEOUT                     =   6,
-	NO_SUCH_KEY = 8,
+	MASTER_EXISTS                    =   7,
-	REQUEST_TIMEOUT = 9,
+	NO_SUCH_MASTER                   =   8,
-	TYPE_CLASH = 10,
+	NO_SUCH_KEY                      =   9,
-	INVALID_DATA = 11,
+	REQUEST_TIMEOUT                  =  10,
-	BACKEND_FAILURE = 12,
+	TYPE_CLASH                       =  11,
-	STALE_DATA = 13,
+	INVALID_DATA                     =  12,
-	CAF_ERROR = 100,
+	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 %{

testing/btest/Baseline/broker.ssl-mismatch/bad_connect.broker.error

@@ -0,0 +1 @@
+Broker::PEER_UNAVAILABLE

testing/btest/Baseline/broker.ssl-mismatch/bad_connect_rev.broker.error

@@ -0,0 +1 @@
+Broker::PEER_UNAVAILABLE

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();
+		}
+	}