websocket: Handle breaking from WebSocket::configure_analyzer()

...and various nits from the review.

scripts/base/protocols/websocket/main.zeek

@@ -53,7 +53,15 @@ export {
 	## Log policy hook.
 	global log_policy: Log::PolicyHook;
 
-	## Hook to allow interception of WebSocket analyzer configuration.
+	## Experimental: Hook to intercept WebSocket analyzer configuration.
+	##
+	## Breaking from this hook disables the WebSocket analyzer immediately.
+	## To modify the configuration of the analyzer, use the
+	## :zeek:see:`WebSocket::AnalyzerConfig` type.
+	##
+	## While this API allows quite some flexibility currently, should be
+	## considered experimental and may change in the future with or
+	## without a deprecation phase.
 	##
 	## c: The connection
 	##
@@ -77,9 +85,10 @@ function set_websocket(c: connection)
 	);
 	}
 
-function expected_accept_for(key: string): string {
+function expected_accept_for(key: string): string
+	{
 	return encode_base64(hexstr_to_bytestring(sha1_hash(key + HANDSHAKE_GUID)));
-}
+	}
 
 event http_header(c: connection, is_orig: bool, name: string, value: string)
 	{
@@ -205,9 +214,13 @@ event websocket_established(c: connection, aid: count) &priority=-5
 		config$server_extensions = ws$server_extensions;
 
 	# Give other scripts a chance to modify the analyzer configuration.
-	hook WebSocket::configure_analyzer(c, aid, config);
+	#
-
+	# Breaking from this hook disables the new WebSocket analyzer
-	WebSocket::__configure_analyzer(c, aid, config);
+	# completely instead of configuring it.
+	if ( hook WebSocket::configure_analyzer(c, aid, config) )
+		WebSocket::__configure_analyzer(c, aid, config);
+	else
+		disable_analyzer(c$id, aid);
 
 	ws$ts = network_time();
 	Log::write(LOG, ws);

src/analyzer/protocol/websocket/WebSocket.cc

@@ -22,7 +22,9 @@ WebSocket_Analyzer::WebSocket_Analyzer(Connection* conn) : analyzer::tcp::TCP_Ap
 void WebSocket_Analyzer::Init() {
     tcp::TCP_ApplicationAnalyzer::Init();
 
-    // This event calls back via Configure()
+    // This event gives scriptland a chance to log and configure the analyzer.
+    // The WebSocket analyzer ships with a handler that calls back into
+    // Configure(), via  WebSocket::__configure_analyzer().
     zeek::BifEvent::enqueue_websocket_established(this, Conn(), GetID());
 }
 

src/analyzer/protocol/websocket/events.bif

@@ -1,16 +1,10 @@
 ## Generated when a WebSocket handshake completed.
 ##
-## This is a bit artificial. It can be used to configure the WebSocket
-## analyzer if the HTTP headers contained protocol and extension headers.
-##
 ## c: The WebSocket connection.
 ##
 ## aid: The analyzer identifier of the WebSocket analyzer.
 ##
-## .. zeek:see:: WebSocket::__configure_analyzer
+## .. zeek:see:: WebSocket::__configure_analyzer WebSocket::configure_analyzer
-##
-## .. zeek:see:: WebSocket::configure_analyzer
-##
 event websocket_established%(c: connection, aid: count%);
 
 ## Generated for every WebSocket frame.

src/analyzer/protocol/websocket/functions.bif

@@ -31,7 +31,7 @@ function __configure_analyzer%(c: connection, aid: count, config: WebSocket::Ana
 
 	static const auto& config_type = zeek::id::find_type<zeek::RecordType>("WebSocket::AnalyzerConfig");
 
-    if ( config->GetType() != config_type )
+	if ( config->GetType() != config_type )
 		{
 		reporter->Warning("config has wrong type %s, expected %s",
 		                  config->GetType()->GetName().c_str(),

testing/btest/Baseline/scripts.base.protocols.websocket.wstunnel-ssh-configure-break/conn.log.cut

@@ -0,0 +1,4 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ts	uid	history	service
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	ShADadR	http
+XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	ShADadR	http

testing/btest/Baseline/scripts.base.protocols.websocket.wstunnel-ssh-configure-break/out

@@ -0,0 +1,5 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+WebSocket::configure_analyzer, CHhAvVGS1DHFjwGM9, 7
+disabling_analyzer, CHhAvVGS1DHFjwGM9, Analyzer::ANALYZER_WEBSOCKET, 7
+WebSocket::configure_analyzer, ClEkJM2Vm5giqnMf4h, 14
+disabling_analyzer, ClEkJM2Vm5giqnMf4h, Analyzer::ANALYZER_WEBSOCKET, 14

testing/btest/Baseline/scripts.base.protocols.websocket.wstunnel-ssh-configure-break/websocket.log

@@ -0,0 +1,12 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	websocket
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	host	uri	user_agent	subprotocol	client_protocols	server_extensions	client_extensions
+#types	time	string	addr	port	addr	port	string	string	string	string	vector[string]	vector[string]	vector[string]
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	42906	127.0.0.1	8888	localhost:8888	/v1/events	-	v1	v1,authorization.bearer.eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6IjAxOGQwNTVkLTQ5OTgtNzI5Zi04Yjg2LTMwZTBiZWEyZGE4ZiIsInAiOiJUY3AiLCJyIjoiMTk1LjIwMS4xNDguMjA5IiwicnAiOjIyfQ.jjTNJL12tQbAuhTB9p_geFXRkEHkxcvOS6zf76qDklQ	-	-
+XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	127.0.0.1	46796	127.0.0.1	8888	localhost:8888	/v1/events	-	v1	v1,authorization.bearer.eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6IjAxOGQwNTVkLTc4MWYtNzNiYi1hZDkwLTEzNjA5NzRjY2JmMyIsInAiOiJUY3AiLCJyIjoiMTk1LjIwMS4xNDguMjA5IiwicnAiOjIyfQ.2HQ4uC23p_OYIXnQWeSZCqdA3jc_lVVH7-T5xZDPrz4	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/scripts/base/protocols/websocket/wstunnel-ssh-configure-break.zeek

@@ -0,0 +1,33 @@
+# @TEST-DOC: Test that breaking from configure_analyzer() removes the attached analyzer.
+#
+# @TEST-EXEC: zeek -b -r $TRACES/websocket/wstunnel-ssh.pcap %INPUT >out 2>&1
+#
+# @TEST-EXEC: zeek-cut -m ts uid history service < conn.log > conn.log.cut
+
+# @TEST-EXEC: btest-diff out
+# @TEST-EXEC: btest-diff conn.log.cut
+# @TEST-EXEC: btest-diff websocket.log
+# @TEST-EXEC: test ! -f ssh.log
+# @TEST-EXEC: test ! -f analyzer.log
+
+@load base/protocols/conn
+@load base/protocols/http
+@load base/protocols/ssh
+@load base/protocols/websocket
+
+hook WebSocket::configure_analyzer(c: connection, aid: count, config: WebSocket::AnalyzerConfig)
+	{
+	print "WebSocket::configure_analyzer", c$uid, aid;
+	break;
+	}
+
+# These should never be raised
+event websocket_message(c: connection, is_orig: bool, opcode: count)
+	{
+	print "ERROR: websocket_message", c$uid, is_orig, "opcode", WebSocket::opcodes[opcode];
+	}
+
+hook Analyzer::disabling_analyzer(c: connection, atype: AllAnalyzers::Tag, aid: count)
+	{
+	print "disabling_analyzer", c$uid, atype, aid;
+	}