From a6edbf8bcdfb326033056310369cfe41630a7145 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Thu, 22 Aug 2024 13:14:24 +0100 Subject: [PATCH] Fix parsing of version field in SSLv2 client hello It turns out that, for probably a long time, we have reported an incorrect version when parsing an SSLv2 client hello. We always reported this as SSLv2, no matter which version the client hello actually contained. This bug probably went unnoticed for a long time, as SSLv2 is essentially unused nowadays, and as this field does not show up in the default logs. This was found due to a baseline difference when writing the Spicy SSL analyzer. --- scripts/base/protocols/ssl/main.zeek | 8 +++++--- src/analyzer/protocol/ssl/ssl-protocol.pac | 12 ++++++------ .../Baseline/scripts.base.protocols.ssl.dpd/.stdout | 2 +- .../ssl-all.log | 2 +- testing/external/commit-hash.zeek-testing | 2 +- testing/external/commit-hash.zeek-testing-private | 2 +- 6 files changed, 15 insertions(+), 13 deletions(-) diff --git a/scripts/base/protocols/ssl/main.zeek b/scripts/base/protocols/ssl/main.zeek index f3c3f21c73..0a0c8d6f4a 100644 --- a/scripts/base/protocols/ssl/main.zeek +++ b/scripts/base/protocols/ssl/main.zeek @@ -283,9 +283,11 @@ event ssl_client_hello(c: connection, version: count, record_version: count, pos c$ssl$client_ticket_empty_session_seen = F; } - # add manually for SSLv2, since the handshake_message event is not raised, as there is no handshake protocol. - # We don't really have a direction in that case - if ( version == 2 ) + # add manually for SSLv2 client hello, since the handshake_message event is not raised, as there is no handshake protocol. + # We don't really have a direction in that case. + # SSLv2 client hello is signified by a record_layer version of 0, as the client-hello itself can indicate + # a higher supported maximum version + if ( record_version == 0 ) add_to_history(c, T, "c"); } diff --git a/src/analyzer/protocol/ssl/ssl-protocol.pac b/src/analyzer/protocol/ssl/ssl-protocol.pac index 5914bcaeda..9e1ea3b95c 100644 --- a/src/analyzer/protocol/ssl/ssl-protocol.pac +++ b/src/analyzer/protocol/ssl/ssl-protocol.pac @@ -8,24 +8,24 @@ type SSLRecord(is_orig: bool) = record { head2 : uint8; head3 : uint8; head4 : uint8; - rec : RecordText(this)[] &length=length, &requires(version,content_type,raw_tls_version); + rec : RecordText(this)[] &length=length, &requires(record_layer_version,content_type,raw_tls_version); } &length = length+5, &byteorder=bigendian, &let { - version : int = + record_layer_version : int = $context.connection.determine_ssl_record_layer(head0, head1, head2, head3, head4, is_orig); # unmodified tls record layer version of this packet. Do not use this if you are parsing SSLv2 - raw_tls_version: uint16 = case version of { + raw_tls_version: uint16 = case record_layer_version of { SSLv20 -> 0; default -> (head1<<8) | head2; } &requires(version); - content_type : int = case version of { + content_type : int = case record_layer_version of { SSLv20 -> head2+300; default -> head0; } &requires(version); - length : int = case version of { + length : int = case record_layer_version of { # fail analyzer if the packet cannot be recognized as TLS. UNKNOWN_VERSION -> 0; SSLv20 -> (((head0 & 0x7f) << 8) | head1) - 3; @@ -77,7 +77,7 @@ type V2ClientHello(rec: SSLRecord) = record { session_id : uint8[session_len]; challenge : bytestring &length = chal_len; } &length = 6 + csuit_len + session_len + chal_len, &let { - client_version : int = rec.version; + client_version : int = (rec.head3 << 8) | rec.head4; }; diff --git a/testing/btest/Baseline/scripts.base.protocols.ssl.dpd/.stdout b/testing/btest/Baseline/scripts.base.protocols.ssl.dpd/.stdout index 2e1e5a2282..76b1b068f3 100644 --- a/testing/btest/Baseline/scripts.base.protocols.ssl.dpd/.stdout +++ b/testing/btest/Baseline/scripts.base.protocols.ssl.dpd/.stdout @@ -2,7 +2,7 @@ Start test run Client hello, 192.168.4.149, 91.227.4.92, 2 Start test run -Client hello, 192.150.187.164, 194.127.84.106, 2 +Client hello, 192.150.187.164, 194.127.84.106, 769 Client hello, 192.150.187.164, 194.127.84.106, 769 Client hello, 192.150.187.164, 194.127.84.106, 769 Start test run diff --git a/testing/btest/Baseline/scripts.policy.protocols.ssl.ssl-log-ext/ssl-all.log b/testing/btest/Baseline/scripts.policy.protocols.ssl.ssl-log-ext/ssl-all.log index fc8d05ef2b..679d59b2fd 100644 --- a/testing/btest/Baseline/scripts.policy.protocols.ssl.ssl-log-ext/ssl-all.log +++ b/testing/btest/Baseline/scripts.policy.protocols.ssl.ssl-log-ext/ssl-all.log @@ -27,7 +27,7 @@ XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 192.168.18.50 56981 74.125.239.97 443 TLSv12 #open XXXX-XX-XX-XX-XX-XX #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p version cipher curve server_name resumed last_alert next_protocol established ssl_history cert_chain_fps client_cert_chain_fps sni_matches_cert server_version client_version client_ciphers ssl_client_exts ssl_server_exts ticket_lifetime_hint dh_param_size point_formats client_curves orig_alpn client_supported_versions server_supported_version psk_key_exchange_modes client_key_share_groups server_key_share_group client_comp_methods sigalgs hashalgs #types time string addr port addr port string string string string bool string string bool string vector[string] vector[string] bool count count vector[count] vector[count] vector[count] count count vector[count] vector[count] vector[string] vector[count] count vector[count] vector[count] count vector[count] vector[count] vector[count] -XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 192.150.187.164 58868 194.127.84.106 443 TLSv10 TLS_RSA_WITH_RC4_128_MD5 - - F - - T CsxnGIi ddd0218a34972ceab3d200b78959bd2b4c95eadf37399df35bfd68a5b658bc78,ba352de8d8faa0ecfdbeee560fa308fe192023d3b18d83a68845933bebf28360 (empty) - 769 2 57,56,53,51,50,4,5,47,22,19,65279,10,21,18,65278,9,100,98,3,6 - - - - - - - - - - - - (empty) - - +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 192.150.187.164 58868 194.127.84.106 443 TLSv10 TLS_RSA_WITH_RC4_128_MD5 - - F - - T CsxnGIi ddd0218a34972ceab3d200b78959bd2b4c95eadf37399df35bfd68a5b658bc78,ba352de8d8faa0ecfdbeee560fa308fe192023d3b18d83a68845933bebf28360 (empty) - 769 769 57,56,53,51,50,4,5,47,22,19,65279,10,21,18,65278,9,100,98,3,6 - - - - - - - - - - - - (empty) - - XXXXXXXXXX.XXXXXX ClEkJM2Vm5giqnMf4h 192.150.187.164 58869 194.127.84.106 443 TLSv10 TLS_RSA_WITH_RC4_128_MD5 - - F - - T CsxnGIi ddd0218a34972ceab3d200b78959bd2b4c95eadf37399df35bfd68a5b658bc78,ba352de8d8faa0ecfdbeee560fa308fe192023d3b18d83a68845933bebf28360 (empty) - 769 769 57,56,53,51,50,4,5,47,22,19,65279,10,21,18,65278,9,100,98,3,6 - - - - - - - - - - - - 0 - - XXXXXXXXXX.XXXXXX C4J4Th3PJpwUYZZ6gc 192.150.187.164 58870 194.127.84.106 443 TLSv10 TLS_RSA_WITH_RC4_128_MD5 - - F - - T CsxnGIi ddd0218a34972ceab3d200b78959bd2b4c95eadf37399df35bfd68a5b658bc78,ba352de8d8faa0ecfdbeee560fa308fe192023d3b18d83a68845933bebf28360 (empty) - 769 769 57,56,53,51,50,4,5,47,22,19,65279,10,21,18,65278,9,100,98,3,6 - - - - - - - - - - - - 0 - - #close XXXX-XX-XX-XX-XX-XX diff --git a/testing/external/commit-hash.zeek-testing b/testing/external/commit-hash.zeek-testing index c73f410174..e535afe051 100644 --- a/testing/external/commit-hash.zeek-testing +++ b/testing/external/commit-hash.zeek-testing @@ -1 +1 @@ -a1c74b74b8755dc4030dfd6034b2bdce23a07072 +df37cbcef57db5aeb09da8045deed0141d471507 diff --git a/testing/external/commit-hash.zeek-testing-private b/testing/external/commit-hash.zeek-testing-private index 005b970c2e..d7ac371590 100644 --- a/testing/external/commit-hash.zeek-testing-private +++ b/testing/external/commit-hash.zeek-testing-private @@ -1 +1 @@ -2a6b523ed423a550cc897cb2cb6a2d6e0cdaea22 +4e30d6b89edf12d99e1165fd5c5e193d1320e371