diff --git a/CHANGES b/CHANGES index 795c53823b..5dfcb490c9 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,20 @@ +7.2.0-dev.387 | 2025-03-19 15:28:08 +0000 + + * SSH: make banner parsing more robust (Johanna Amann, Corelight) + + 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. + 7.2.0-dev.382 | 2025-03-18 11:52:03 -0700 * Add analyzer registration from VLAN to VNTAG (Tim Wojtulewicz, Corelight) diff --git a/VERSION b/VERSION index 65145143a2..afe2781706 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.2.0-dev.382 +7.2.0-dev.387 diff --git a/src/analyzer/protocol/ssh/events.bif b/src/analyzer/protocol/ssh/events.bif index f075581689..9a206cf820 100644 --- a/src/analyzer/protocol/ssh/events.bif +++ b/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,18 @@ 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. +## +## c: The connection. +## +## data: The pre-banner data. +## +## .. zeek:see:: ssh_server_version +event ssh_server_pre_banner_data%(c: connection, data: string%); diff --git a/src/analyzer/protocol/ssh/ssh-analyzer.pac b/src/analyzer/protocol/ssh/ssh-analyzer.pac index 34b6f4b407..a322a9658c 100644 --- a/src/analyzer/protocol/ssh/ssh-analyzer.pac +++ b/src/analyzer/protocol/ssh/ssh-analyzer.pac @@ -58,20 +58,28 @@ const char* fingerprint_md5(const unsigned char* d) %} refine flow SSH_Flow += { - function proc_ssh_version(msg: SSH_Version): bool + function proc_ssh_version_client(msg: SSH_Version_Client): bool %{ - if ( ssh_client_version && ${msg.is_orig } ) - { + if ( ssh_client_version ) zeek::BifEvent::enqueue_ssh_client_version(connection()->zeek_analyzer(), connection()->zeek_analyzer()->Conn(), to_stringval(${msg.version})); - } - else if ( ssh_server_version ) + return true; + %} + + function proc_ssh_version_server(msg: SSH_Version_Server): bool + %{ + if ( ssh_server_version && ${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; %} @@ -267,8 +275,12 @@ refine flow SSH_Flow += { %} }; -refine typeattr SSH_Version += &let { - proc: bool = $context.flow.proc_ssh_version(this); +refine typeattr SSH_Version_Client += &let { + proc: bool = $context.flow.proc_ssh_version_client(this); +}; + +refine typeattr SSH_Version_Server += &let { + proc: bool = $context.flow.proc_ssh_version_server(this); }; refine typeattr SSH2_KEXINIT += &let { diff --git a/src/analyzer/protocol/ssh/ssh-protocol.pac b/src/analyzer/protocol/ssh/ssh-protocol.pac index c152f29b93..f0f1f37d79 100644 --- a/src/analyzer/protocol/ssh/ssh-protocol.pac +++ b/src/analyzer/protocol/ssh/ssh-protocol.pac @@ -20,16 +20,32 @@ proc: bool = $context.connection.inc_encrypted_byte_count_in_current_segment(); }; type SSH_PDU(is_orig: bool) = case $context.connection.get_state(is_orig) of { - VERSION_EXCHANGE -> version : SSH_Version(is_orig); + VERSION_EXCHANGE -> version : SSH_Version_Switch(is_orig); ENCRYPTED -> encrypted : EncryptedByte(is_orig); default -> kex : SSH_Key_Exchange(is_orig); } &byteorder=bigendian; -type SSH_Version(is_orig: bool) = record { - version : bytestring &oneline; +type SSH_Version_Switch(is_orig: bool) = case is_orig of { + true -> client_version : SSH_Version_Client; + false -> server_version: SSH_Version_Server; +}; + +# SSH servers can have banners before their SSH version. Which... fun. +type SSH_Version_Server = record { + 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, is_orig); - update_version : bool = $context.connection.update_version(version, is_orig); + update_state : bool = $context.connection.update_state(KEX_INIT, false) &if(sizeof(version) > 0); + 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 : RE/SSH-.*/ &oneline; +} &let { + update_state : bool = $context.connection.update_state(KEX_INIT, true); + update_version : bool = $context.connection.update_version(version, true); }; type SSH_Key_Exchange(is_orig: bool) = record { diff --git a/testing/btest/Baseline/scripts.base.protocols.ssh.http-port-22/analyzer.log b/testing/btest/Baseline/scripts.base.protocols.ssh.http-port-22/analyzer.log new file mode 100644 index 0000000000..cf3e6e1aca --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/scripts.base.protocols.ssh.http-port-22/conn.log b/testing/btest/Baseline/scripts.base.protocols.ssh.http-port-22/conn.log new file mode 100644 index 0000000000..6745ee0868 --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/scripts.base.protocols.ssh.http-port-22/http.log b/testing/btest/Baseline/scripts.base.protocols.ssh.http-port-22/http.log new file mode 100644 index 0000000000..13af18bbf4 --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/scripts.base.protocols.ssh.pre-banner/.stdout b/testing/btest/Baseline/scripts.base.protocols.ssh.pre-banner/.stdout new file mode 100644 index 0000000000..cbab7d28c8 --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/scripts.base.protocols.ssh.pre-banner/conn.log b/testing/btest/Baseline/scripts.base.protocols.ssh.pre-banner/conn.log new file mode 100644 index 0000000000..45467980dc --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/scripts.base.protocols.ssh.pre-banner/ssh.log b/testing/btest/Baseline/scripts.base.protocols.ssh.pre-banner/ssh.log new file mode 100644 index 0000000000..73274a934a --- /dev/null +++ b/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 diff --git a/testing/btest/Traces/http/http-single-conn-22.pcap b/testing/btest/Traces/http/http-single-conn-22.pcap new file mode 100644 index 0000000000..a450fefde2 Binary files /dev/null and b/testing/btest/Traces/http/http-single-conn-22.pcap differ diff --git a/testing/btest/Traces/ssh/server-pre-banner-data.pcap b/testing/btest/Traces/ssh/server-pre-banner-data.pcap new file mode 100644 index 0000000000..3b74e2f511 Binary files /dev/null and b/testing/btest/Traces/ssh/server-pre-banner-data.pcap differ diff --git a/testing/btest/scripts/base/protocols/ssh/http-port-22.test b/testing/btest/scripts/base/protocols/ssh/http-port-22.test new file mode 100644 index 0000000000..a2e6df25b9 --- /dev/null +++ b/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 diff --git a/testing/btest/scripts/base/protocols/ssh/pre-banner.test b/testing/btest/scripts/base/protocols/ssh/pre-banner.test new file mode 100644 index 0000000000..80f3d81a04 --- /dev/null +++ b/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; + } diff --git a/testing/external/commit-hash.zeek-testing-private b/testing/external/commit-hash.zeek-testing-private index 48b931a063..e7dc949572 100644 --- a/testing/external/commit-hash.zeek-testing-private +++ b/testing/external/commit-hash.zeek-testing-private @@ -1 +1 @@ -296a3b2bfd36a74c8aa22f175cea4c00a9f4d079 +d20f3027e30434d340f1d3b45b5f86c84e5c74e0