From e6e30240ecd4ff1a6fba57ede0efb73731834d8e Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Wed, 16 Jun 2021 14:57:30 +0200 Subject: [PATCH 1/2] Reformat function in SSH base script. --- scripts/base/protocols/ssh/main.zeek | 90 ++++++++++++++-------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/scripts/base/protocols/ssh/main.zeek b/scripts/base/protocols/ssh/main.zeek index f2b5d52e5c..963cff5697 100644 --- a/scripts/base/protocols/ssh/main.zeek +++ b/scripts/base/protocols/ssh/main.zeek @@ -156,51 +156,51 @@ function set_session(c: connection) } function set_version(c: connection, version: string) - { - if ( c$ssh?$server && c$ssh?$client && |c$ssh$client| > 4 && |c$ssh$server| > 4 ) - { - if ( c$ssh$client[4] == "1" && c$ssh$server[4] == "2" ) - { - # SSH199 vs SSH2 -> 2 - if ( ( |c$ssh$client| > 7 ) && ( c$ssh$client[6] == "9" ) && ( c$ssh$client[7] == "9" ) ) - c$ssh$version = 2; - # SSH1 vs SSH2 -> Undefined - else - c$ssh$version = 0; - } - else if ( c$ssh$client[4] == "2" && c$ssh$server[4] == "1" ) - { - # SSH2 vs SSH199 -> 2 - if ( ( |c$ssh$server| > 7 ) && ( c$ssh$server[6] == "9" ) && ( c$ssh$server[7] == "9" ) ) - c$ssh$version = 2; - else - # SSH2 vs SSH1 -> Undefined - c$ssh$version = 0; - } - else if ( c$ssh$client[4] == "1" && c$ssh$server[4] == "1" ) - { - # SSH1 vs SSH199 -> 1 - if ( ( |c$ssh$server| > 7 ) && ( c$ssh$server[6] == "9" ) && ( c$ssh$server[7] == "9" ) ) - { - # SSH199 vs SSH199 - if (( |c$ssh$client| > 7 ) && ( c$ssh$client[6] == "9" ) && ( c$ssh$client[7] == "9" )) - c$ssh$version = 2; - else - c$ssh$version = 1; - } - else - { - # SSH1 vs SSH1 -> 1 - c$ssh$version = 1; - } - } - # SSH2 vs SSH2 - else if (c$ssh$client[4] == "2" && c$ssh$server[4] == "2" ) - { - c$ssh$version = 2; - } - } - } + { + if ( c$ssh?$server && c$ssh?$client && |c$ssh$client| > 4 && |c$ssh$server| > 4 ) + { + if ( c$ssh$client[4] == "1" && c$ssh$server[4] == "2" ) + { + # SSH199 vs SSH2 -> 2 + if ( ( |c$ssh$client| > 7 ) && ( c$ssh$client[6] == "9" ) && ( c$ssh$client[7] == "9" ) ) + c$ssh$version = 2; + # SSH1 vs SSH2 -> Undefined + else + c$ssh$version = 0; + } + else if ( c$ssh$client[4] == "2" && c$ssh$server[4] == "1" ) + { + # SSH2 vs SSH199 -> 2 + if ( ( |c$ssh$server| > 7 ) && ( c$ssh$server[6] == "9" ) && ( c$ssh$server[7] == "9" ) ) + c$ssh$version = 2; + else + # SSH2 vs SSH1 -> Undefined + c$ssh$version = 0; + } + else if ( c$ssh$client[4] == "1" && c$ssh$server[4] == "1" ) + { + # SSH1 vs SSH199 -> 1 + if ( ( |c$ssh$server| > 7 ) && ( c$ssh$server[6] == "9" ) && ( c$ssh$server[7] == "9" ) ) + { + # SSH199 vs SSH199 + if (( |c$ssh$client| > 7 ) && ( c$ssh$client[6] == "9" ) && ( c$ssh$client[7] == "9" )) + c$ssh$version = 2; + else + c$ssh$version = 1; + } + else + { + # SSH1 vs SSH1 -> 1 + c$ssh$version = 1; + } + } + # SSH2 vs SSH2 + else if (c$ssh$client[4] == "2" && c$ssh$server[4] == "2" ) + { + c$ssh$version = 2; + } + } + } event ssh_server_version(c: connection, version: string) { From daa9537f924a2717b7bec509b099d5c4ba9081df Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Wed, 16 Jun 2021 09:01:19 +0200 Subject: [PATCH 2/2] Change SSH version field to be `&optional`. In 3769ed6c668 we added handling for SSH version 1.99 which unsed a SSH version of 0 to indicate weird cases where no version could be determined. This patch is a fixup for that patch. Instead of using a magic version of 0 we now use an `&optional` version value. If no SSH version can be extracted the version will be unset; additionally a `conn_weird` event will be raised. Closes #1590. --- scripts/base/protocols/ssh/main.zeek | 37 ++++-- .../output | 16 +++ .../base/protocols/ssh/set_version.zeek | 117 ++++++++++++++++++ 3 files changed, 162 insertions(+), 8 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.protocols.ssh.set_version/output create mode 100644 testing/btest/scripts/base/protocols/ssh/set_version.zeek diff --git a/scripts/base/protocols/ssh/main.zeek b/scripts/base/protocols/ssh/main.zeek index 963cff5697..50e474305e 100644 --- a/scripts/base/protocols/ssh/main.zeek +++ b/scripts/base/protocols/ssh/main.zeek @@ -20,8 +20,12 @@ export { uid: string &log; ## The connection's 4-tuple of endpoint addresses/ports. id: conn_id &log; - ## SSH major version (1 or 2) - version: count &log; + ## SSH major version (1, 2, or unset). The version can be unset if the + ## client and server version strings ## unset, malformed or incompatible + ## so no common version can be extracted. If no version can be extracted + ## even though both client and server versions are set a `conn_weird` + ## event will be raised. + version: count &log &optional; ## Authentication result (T=success, F=failure, unset=unknown) auth_success: bool &log &optional; ## The number of authentication attemps we observed. There's always @@ -155,9 +159,20 @@ function set_session(c: connection) } } -function set_version(c: connection, version: string) +function set_version(c: connection) { - if ( c$ssh?$server && c$ssh?$client && |c$ssh$client| > 4 && |c$ssh$server| > 4 ) + # We always either set the version field to a concrete value, or unset it. + delete c$ssh$version; + + # If either the client or server string is unset we cannot compute a + # version and return early. We do not raise a weird in this case as we + # might arrive here while having only seen one side of the handshake. + const has_server = c$ssh?$server && |c$ssh$server| > 0; + const has_client = c$ssh?$client && |c$ssh$client| > 0; + if ( ! ( has_server && has_client ) ) + return; + + if ( |c$ssh$client| > 4 && |c$ssh$server| > 4 ) { if ( c$ssh$client[4] == "1" && c$ssh$server[4] == "2" ) { @@ -166,7 +181,8 @@ function set_version(c: connection, version: string) c$ssh$version = 2; # SSH1 vs SSH2 -> Undefined else - c$ssh$version = 0; + Reporter::conn_weird("SSH protocol version mismatch", c, fmt("%s vs %s", c$ssh$server, c$ssh$client)); + return; } else if ( c$ssh$client[4] == "2" && c$ssh$server[4] == "1" ) { @@ -175,7 +191,8 @@ function set_version(c: connection, version: string) c$ssh$version = 2; else # SSH2 vs SSH1 -> Undefined - c$ssh$version = 0; + Reporter::conn_weird("SSH protocol version mismatch", c, fmt("%s vs %s", c$ssh$server, c$ssh$client)); + return; } else if ( c$ssh$client[4] == "1" && c$ssh$server[4] == "1" ) { @@ -199,21 +216,25 @@ function set_version(c: connection, version: string) { c$ssh$version = 2; } + + return; } + + Reporter::conn_weird("cannot determine SSH protocol version used", c, fmt("%s vs %s", c$ssh$server, c$ssh$client)); } event ssh_server_version(c: connection, version: string) { set_session(c); c$ssh$server = version; - set_version(c, version); + set_version(c); } event ssh_client_version(c: connection, version: string) { set_session(c); c$ssh$client = version; - set_version(c, version); + set_version(c); } event ssh_auth_attempted(c: connection, authenticated: bool) &priority=5 diff --git a/testing/btest/Baseline/scripts.base.protocols.ssh.set_version/output b/testing/btest/Baseline/scripts.base.protocols.ssh.set_version/output new file mode 100644 index 0000000000..79c8c831f5 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.ssh.set_version/output @@ -0,0 +1,16 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +SSH1 vs SSH1, 1 +SSH199 vs SSH1, 1 +SSH2 vs SSH2, 2 +SSH199 vs SSH2, 2 +unset vs unset, F +client unset, F +server unset, F +incomplete server version, F +incomplete client version, F +SSH1 vs SSH2, F +SSH2 vs SSH1, F +conn_weird:, cannot determine SSH protocol version used, [orig_h=127.0.0.1, orig_p=40/tcp, resp_h=127.0.0.1, resp_p=40/tcp], SSH vs SSH-1.5-OpenSSH_6.2, +conn_weird:, cannot determine SSH protocol version used, [orig_h=127.0.0.1, orig_p=40/tcp, resp_h=127.0.0.1, resp_p=40/tcp], SSH-1.5-OpenSSH_6.2 vs SSH, +conn_weird:, SSH protocol version mismatch, [orig_h=127.0.0.1, orig_p=40/tcp, resp_h=127.0.0.1, resp_p=40/tcp], SSH-1.5-OpenSSH_6.2 vs SSH-2.0-OpenSSH_5.9, +conn_weird:, SSH protocol version mismatch, [orig_h=127.0.0.1, orig_p=40/tcp, resp_h=127.0.0.1, resp_p=40/tcp], SSH-2.0-OpenSSH_5.9 vs SSH-1.5-OpenSSH_6.2, diff --git a/testing/btest/scripts/base/protocols/ssh/set_version.zeek b/testing/btest/scripts/base/protocols/ssh/set_version.zeek new file mode 100644 index 0000000000..493e2d7f14 --- /dev/null +++ b/testing/btest/scripts/base/protocols/ssh/set_version.zeek @@ -0,0 +1,117 @@ +# @TEST-EXEC: zeek -b %INPUT >output +# @TEST-EXEC: btest-diff output + +@load base/bif/event.bif.zeek +@load base/protocols/ssh + +module SSH; + +# Creates a mock connection. This connection is good enough for e.g., +# `SSH::set_version`, but not in line with what Zeek considers active +# connections. +function make_conn(server: string, client: string): connection + { + local c: connection; + c$uid = "uid"; + + local id: conn_id; + id$orig_h = 127.0.0.1; + id$resp_h = 127.0.0.1; + id$orig_p = 40/tcp; + id$resp_p = 40/tcp; + c$id = id; + + local ssh: SSH::Info; + ssh$ts = network_time(); + ssh$server = server; + ssh$client = client; + c$ssh = ssh; + + SSH::set_session(c); + + delete c$ssh$version; + return c; + } + +# While `SSH::set_version` triggers a `conn_weird` we are dealing with mock +# connections which since they are injected are always considered expired by +# Zeek. +event expired_conn_weird(name: string, id: conn_id, uid: string, addl: string, source: string) + { + print "conn_weird:", name, id, addl, source; + } + +const v1 = "SSH-1.5-OpenSSH_6.2"; +const v199 = "SSH-1.99-OpenSSH_3.1p1"; +const v2 = "SSH-2.0-OpenSSH_5.9"; + +event zeek_init() + { + local c: connection; + + # Good cases. + { + # SSH1 vs SSH1 -> 1. + c = make_conn(v1, v1); + SSH::set_version(c); + print "SSH1 vs SSH1", c$ssh$version; + + # SSH199 vs SSH1 -> 1. + c = make_conn(v1, v199); + SSH::set_version(c); + print "SSH199 vs SSH1", c$ssh$version; # 1. + + # SSH2 vs SSH2 -> 2. + c = make_conn(v2, v2); + SSH::set_version(c); + print "SSH2 vs SSH2", c$ssh$version; # 2. + + # SSH199 vs SSH2 -> 2. + c = make_conn(v2, v199); + SSH::set_version(c); + print "SSH199 vs SSH2", c$ssh$version; # 2. + } + + # Error cases. + { + # Unset vs unset -> unset. + c = make_conn("", ""); + c$ssh$version = 42; + SSH::set_version(c); + print "unset vs unset", c$ssh?$version; # Unset. + + # Client unset. + c = make_conn(v2, ""); + c$ssh$version = 42; + SSH::set_version(c); + print "client unset", c$ssh?$version; # Unset. + + # Server unset. + c = make_conn("", v2); + c$ssh$version = 42; + SSH::set_version(c); + print "server unset", c$ssh?$version; # Unset. + + # Unable to extract full server version. + c = make_conn("SSH", v1); + c$ssh$version = 42; + SSH::set_version(c); + print "incomplete server version", c$ssh?$version; + + # Unable to extract full client version. + c = make_conn(v1, "SSH"); + c$ssh$version = 42; + SSH::set_version(c); + print "incomplete client version", c$ssh?$version; + + # SSH1 vs SSH2. + c = make_conn(v1, v2); + SSH::set_version(c); + print "SSH1 vs SSH2", c$ssh?$version; # Unset. + + # SSH2 vs SSH1. + c = make_conn(v2, v1); + SSH::set_version(c); + print "SSH2 vs SSH1", c$ssh?$version; # Unset. + } + }