diff --git a/CHANGES b/CHANGES index f61d3bf3a2..1975d9e120 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,16 @@ +4.1.0-dev.731 | 2021-06-17 10:40:58 +0100 + + * Change SSH version field to be `&optional`. + + In version 3.3.0-dev.537 we added handling for SSH version 1.99 which + used a SSH version of 0 to indicate weird cases where no version could be + determined. + + This patch is a fixup for that patch. We now use an `&optional` version value. + If no SSH version can be eixtracted the version will be unset; additionally a + `conn_weird` event will be raised. See GH-1590. (Benjamin Bannier, Corelight) + 4.1.0-dev.727 | 2021-06-14 16:19:34 -0700 * Bump Highwayhash submodule to pull in fix for FreeBSD (Christian Kreibich, Corelight) diff --git a/NEWS b/NEWS index b8fbdf1f6f..b67fb63c43 100644 --- a/NEWS +++ b/NEWS @@ -75,6 +75,10 @@ Changed Functionality analyzers in the future that better align with the packet analysis framework than they do with session analysis. + +- The version field in ssh.log is now optional and will not be set if we cannot + determine the version that was negotiated by the client and server. + Removed Functionality --------------------- diff --git a/VERSION b/VERSION index c46d2d6716..f0842b3ea5 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -4.1.0-dev.727 +4.1.0-dev.731 diff --git a/scripts/base/protocols/ssh/main.zeek b/scripts/base/protocols/ssh/main.zeek index f2b5d52e5c..1dbc1bcfcc 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 are 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 weird + ## will be generated. + 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,65 +159,82 @@ 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; - } - } - } +function set_version(c: connection) + { + # 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" ) + { + # 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 + Reporter::conn_weird("SSH_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" ) + { + # 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 + Reporter::conn_weird("SSH_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" ) + { + # 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; + } + + return; + } + + Reporter::conn_weird("SSH_cannot_determine_version", 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..9daff7bc9f --- /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:, SSH_cannot_determine_version, [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:, SSH_cannot_determine_version, [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_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_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. + } + }