SSH: make banner parsing more robust

This change revamps SSH banner parsing.  The previous behavior was both
a bit too strict in some regards, and too permissive in other.

Specifically, clients are now required to send a line starting with
"SSH-" as the first line.  This is in line with the RFC, as well with
observed behavior. This also prevents the creation of `ssh.log` for
non-SSH traffic on port 22.

For the server side, we now accept text before the SSH banner. This
previously led to a protocol violation but is allowed by the spec.

New tests are added to cover these cases.

src/analyzer/protocol/ssh/events.bif

@@ -12,7 +12,7 @@
 ##    ssh_capabilities ssh2_server_host_key ssh1_server_host_key
 ##    ssh_server_host_key ssh_encrypted_packet ssh2_dh_server_params
 ##    ssh2_gss_error ssh2_ecc_key ssh2_ecc_init ssh2_dh_gex_init
-##    ssh2_gss_init ssh2_rsa_secret
+##    ssh2_gss_init ssh2_rsa_secret ssh_server_pre_banner_data
 event ssh_server_version%(c: connection, version: string%);
 
 ## An :abbr:`SSH (Secure Shell)` Protocol Version Exchange message
@@ -336,3 +336,14 @@ event ssh2_gss_init%(c: connection, is_orig: bool%);
 ##    ssh2_gss_error ssh2_ecc_key ssh2_ecc_init ssh2_dh_gex_init
 ##    ssh2_gss_init ssh2_rsa_secret
 event ssh2_rsa_secret%(c: connection, is_orig: bool%);
+
+## SSH servers can send textual data to the client before sending
+## a banner. The primary use case of this are error messages of TCP
+## wrappers.
+##
+## As this event happens before the SSH banner is exchanged, it is
+## possible that it contains data from different protocols; e.g. if
+## an SSH client connects to a non-SSH-server.
+##
+## .. zeek:see:: ssh_server_version
+event ssh_server_pre_banner_data%(c: connection, data: string%);

src/analyzer/protocol/ssh/ssh-analyzer.pac

@@ -68,9 +68,17 @@ refine flow SSH_Flow += {
 
 	function proc_ssh_version_server(msg: SSH_Version_Server): bool
 		%{
+		if ( ${msg.version}.length() > 0 )
+			{
 			zeek::BifEvent::enqueue_ssh_server_version(connection()->zeek_analyzer(),
 				connection()->zeek_analyzer()->Conn(),
 				to_stringval(${msg.version}));
+			}
+		else if ( ssh_server_pre_banner_data )
+			{
+				zeek::BifEvent::enqueue_ssh_server_pre_banner_data(connection()->zeek_analyzer(),
+				connection()->zeek_analyzer()->Conn(), to_stringval(${msg.nonversiondata}));
+			}
 		return true;
 		%}
 

src/analyzer/protocol/ssh/ssh-protocol.pac

@@ -30,15 +30,19 @@ type SSH_Version_Switch(is_orig: bool) = case is_orig of {
 	false -> server_version: SSH_Version_Server;
 };
 
+# SSH servers can have banners before their SSH version. Which... fun.
 type SSH_Version_Server = record {
-	version : bytestring &oneline;
+	version: RE/(SSH-.*)?/;
+	# only UTF-8 data. This doesn't catch all bad cases, but some
+	nonversiondata: RE/([^[\xC0-\xC1]|[\xF5-\xFF]])*/;
 } &let {
-	update_state   : bool = $context.connection.update_state(KEX_INIT, false);
+	update_state   : bool = $context.connection.update_state(KEX_INIT, false) &if(sizeof(version) > 0);
-	update_version : bool = $context.connection.update_version(version, false);
+	update_version : bool = $context.connection.update_version(version, false) &if(sizeof(version) > 0);
-};
+} &oneline;
 
+# SSH clients _always_ have to send a line starting with SSH- first.
 type SSH_Version_Client = record {
-	version : bytestring &oneline;
+	version : RE/SSH-.*/ &oneline;
 } &let {
 	update_state   : bool = $context.connection.update_state(KEX_INIT, true);
 	update_version : bool = $context.connection.update_version(version, true);

testing/btest/Baseline/scripts.base.protocols.ssh.http-port-22/analyzer.log

@@ -0,0 +1,11 @@
+### 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	analyzer
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	cause	analyzer_kind	analyzer_name	uid	fuid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	failure_reason	failure_data
+#types	time	string	string	string	string	string	addr	port	addr	port	string	string
+XXXXXXXXXX.XXXXXX	violation	protocol	SSH	CHhAvVGS1DHFjwGM9	-	10.0.0.1	51889	192.168.0.1	22	Binpac exception: binpac exception: string mismatch at <...>/ssh-protocol.pac:45: \x0aexpected pattern: "SSH-.*"\x0aactual data: "GET / HTTP/1.1"	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ssh.http-port-22/conn.log

@@ -0,0 +1,11 @@
+### 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	conn
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	service	duration	orig_bytes	resp_bytes	conn_state	local_orig	local_resp	missed_bytes	history	orig_pkts	orig_ip_bytes	resp_pkts	resp_ip_bytes	tunnel_parents	ip_proto
+#types	time	string	addr	port	addr	port	enum	string	interval	count	count	string	bool	bool	count	string	count	count	count	count	set[string]	count
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	10.0.0.1	51889	192.168.0.1	22	tcp	http	0.000260	18	12649	SF	T	T	0	ShADadFf	15	618	13	13169	-	6
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ssh.http-port-22/http.log

@@ -0,0 +1,11 @@
+### 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	http
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	trans_depth	method	host	uri	referrer	version	user_agent	origin	request_body_len	response_body_len	status_code	status_msg	info_code	info_msg	tags	username	password	proxied	orig_fuids	orig_filenames	orig_mime_types	resp_fuids	resp_filenames	resp_mime_types
+#types	time	string	addr	port	addr	port	count	string	string	string	string	string	string	string	count	count	count	string	count	string	set[enum]	string	string	set[string]	vector[string]	vector[string]	vector[string]	vector[string]	vector[string]	vector[string]
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	10.0.0.1	51889	192.168.0.1	22	1	GET	-	/	-	1.1	-	-	0	12632	200	OK	-	-	(empty)	-	-	-	-	-	-	FsaSIr11Ze8VUH5yPj	-	text/plain
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ssh.pre-banner/.stdout

@@ -0,0 +1,36 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ _____________
+< Hi stranger >
+ -------------
+   \             \
+    \             \_
+     \             \\
+      \             \\/\
+       \            _\\/
+        \         /   -\
+         \      /  oo   -\
+          \   /           \
+             |    ---\    -\
+             \--/     \     \
+                       |      -\
+                        \       -\         -------------\    /-\
+                         \        \-------/              ---/    \
+                          \                                  |\   \
+                           |                                 / |  |
+                           \                                |  \  |
+                            |                              /    \ |
+                            |                             /     \ |
+                             \                             \     \|
+                              -              /--------\    |      o
+                               \+   +---------          \   |
+                                |   |                   |   \
+                                |   |                    \   |
+                                |   |                    |   \
+                                |   |                     \   |
+                                 \  |                     |   |
+                                 |  |                      \  \
+                                 |  |                      |   |
+                                 +--+                       ---+
+Habit is habit, and not to be flung out of the window by any man, but coaxed
+down-stairs a step at a time.
+\x09\x09-- Mark Twain, "Pudd'nhead Wilson's Calendar

testing/btest/Baseline/scripts.base.protocols.ssh.pre-banner/conn.log

@@ -0,0 +1,11 @@
+### 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	conn
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	service	duration	orig_bytes	resp_bytes	conn_state	local_orig	local_resp	missed_bytes	history	orig_pkts	orig_ip_bytes	resp_pkts	resp_ip_bytes	tunnel_parents	ip_proto
+#types	time	string	addr	port	addr	port	enum	string	interval	count	count	string	bool	bool	count	string	count	count	count	count	set[string]	count
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	10.0.2.1	55343	10.0.2.10	22	tcp	ssh	0.201784	2869	4728	S1	T	T	0	ShADad	21	3973	15	5516	-	6
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ssh.pre-banner/ssh.log

@@ -0,0 +1,11 @@
+### 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	ssh
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	version	auth_success	auth_attempts	direction	client	server	cipher_alg	mac_alg	compression_alg	kex_alg	host_key_alg	host_key
+#types	time	string	addr	port	addr	port	count	bool	count	enum	string	string	string	string	string	string	string	string
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	10.0.2.1	55343	10.0.2.10	22	2	-	0	-	SSH-2.0-OpenSSH_9.7	SSH-2.0-OpenSSH_9.2p1 Debian-2+deb12u5	aes192-ctr	hmac-sha2-256	zlib@openssh.com	sntrup761x25519-sha512@openssh.com	ssh-ed25519	27:27:33:7a:1a:4f:46:b2:58:1c:04:c2:ad:6d:8a:86
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/scripts/base/protocols/ssh/http-port-22.test

@@ -0,0 +1,7 @@
+# Validate that a text-based protocol pn port 22 does not generate a ssh logfile.
+
+# @TEST-EXEC: zeek -r $TRACES/http/http-single-conn-22.pcap %INPUT
+# @TEST-EXEC: test ! -f ssh.log
+# @TEST-EXEC: btest-diff http.log
+# @TEST-EXEC: btest-diff conn.log
+# @TEST-EXEC: TEST_DIFF_CANONIFIER="$SCRIPTS/diff-remove-abspath | $SCRIPTS/diff-remove-timestamps" btest-diff analyzer.log

testing/btest/scripts/base/protocols/ssh/pre-banner.test

@@ -0,0 +1,11 @@
+# This tests a trace that has data before the banner.
+
+# @TEST-EXEC: zeek -r $TRACES/ssh/server-pre-banner-data.pcap %INPUT
+# @TEST-EXEC: btest-diff ssh.log
+# @TEST-EXEC: btest-diff conn.log
+# @TEST-EXEC: btest-diff .stdout
+
+event ssh_server_pre_banner_data(c: connection, data: string)
+	{
+	print data;
+	}

testing/external/commit-hash.zeek-testing-private

@@ -1 +1 @@
-296a3b2bfd36a74c8aa22f175cea4c00a9f4d079
+2fa4bd6a18c376c64629a6d5679c230423f60913