From bea3075c1fcc54dfad7db0f345a2c027d15e3f32 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Mon, 14 Dec 2020 19:51:05 +0000 Subject: [PATCH] TLS analyzer: change logic to track TLS 1.3 connection establishment This commit changes the logic that is used to tracks connection establishment - and moves it from scriptland into the core. TLS 1.3 connection establishment is much more finnicky for us than the establishment of earlier versions - since we cannot rely on the CCS message anymore (which is meaningless and not sent in a lot of cases). With this commit, the ssl_encrypted_data message gets raised for encrypted TLS 1.3 handshake messages - which is much more correct than the behavior before that just interpreted them as plaintext messages. I will refine this a bit more - at the moment the connection established event happens a bit too early - earlier than TLS 1.3 connections actually can be estasblished. Part of GH-1323 --- scripts/base/protocols/ssl/main.zeek | 43 +---------------- src/analyzer/protocol/ssl/SSL.cc | 5 ++ src/analyzer/protocol/ssl/SSL.h | 2 + src/analyzer/protocol/ssl/ssl-protocol.pac | 47 ++++++++++++++++++- .../protocol/ssl/tls-handshake-protocol.pac | 15 +++++- 5 files changed, 68 insertions(+), 44 deletions(-) diff --git a/scripts/base/protocols/ssl/main.zeek b/scripts/base/protocols/ssl/main.zeek index bfbd32158c..1b8c32397b 100644 --- a/scripts/base/protocols/ssl/main.zeek +++ b/scripts/base/protocols/ssl/main.zeek @@ -46,12 +46,6 @@ export { ## by the client. This value is used to determine if a session ## is being resumed. It's not logged. client_key_exchange_seen: bool &default=F; - ## Count to track if the server already sent an application data - ## packet for TLS 1.3. Used to track when a session was established. - server_appdata: count &default=0; - ## Flag to track if the client already sent an application data - ## packet for TLS 1.3. Used to track when a session was established. - client_appdata: bool &default=F; ## Last alert that was seen during the connection. last_alert: string &log &optional; @@ -370,41 +364,8 @@ event ssl_plaintext_data(c: connection, is_orig: bool, record_version: count, co if ( ! c$ssl?$version || c$ssl$established || content_type != APPLICATION_DATA ) return; - if ( c$ssl$version_num/0xFF != 0x7F && c$ssl$version_num != TLSv13 ) - { - local wi = Weird::Info($ts=network_time(), $name="ssl_early_application_data", $uid=c$uid, $id=c$id); - Weird::weird(wi); - return; - } - - if ( is_orig ) - { - c$ssl$client_appdata = T; - return; - } - - if ( c$ssl$client_appdata && c$ssl$server_appdata == 0 ) - { - # something went wrong in the handshake here - we can't say if it was established. Just abort. - return; - } - else if ( ! c$ssl$client_appdata && c$ssl$server_appdata == 0 ) - { - c$ssl$server_appdata = 1; - return; - } - else if ( c$ssl$client_appdata && c$ssl$server_appdata == 1 ) - { - # wait for one more packet before we believe it was established. This one could be an encrypted alert. - c$ssl$server_appdata = 2; - return; - } - else if ( c$ssl$client_appdata && c$ssl$server_appdata == 2 ) - { - set_ssl_established(c); - event ssl_established(c); - return; - } + local wi = Weird::Info($ts=network_time(), $name="ssl_early_application_data", $uid=c$uid, $id=c$id); + Weird::weird(wi); } event protocol_violation(c: connection, atype: Analyzer::Tag, aid: count, diff --git a/src/analyzer/protocol/ssl/SSL.cc b/src/analyzer/protocol/ssl/SSL.cc index 980b2a556d..718ec92b69 100644 --- a/src/analyzer/protocol/ssl/SSL.cc +++ b/src/analyzer/protocol/ssl/SSL.cc @@ -48,6 +48,11 @@ void SSL_Analyzer::StartEncryption() interp->setEstablished(); } +uint16_t SSL_Analyzer::GetNegotiatedVersion() const + { + return handshake_interp->chosen_version(); + } + void SSL_Analyzer::DeliverStream(int len, const u_char* data, bool orig) { analyzer::tcp::TCP_ApplicationAnalyzer::DeliverStream(len, data, orig); diff --git a/src/analyzer/protocol/ssl/SSL.h b/src/analyzer/protocol/ssl/SSL.h index 50fabe1327..fda43d2726 100644 --- a/src/analyzer/protocol/ssl/SSL.h +++ b/src/analyzer/protocol/ssl/SSL.h @@ -24,6 +24,8 @@ public: // Tell the analyzer that encryption has started. void StartEncryption(); + // Get the TLS version that the server chose. 0 if not yet known. + uint16_t GetNegotiatedVersion() const; // Overriden from analyzer::tcp::TCP_ApplicationAnalyzer. void EndpointEOF(bool is_orig) override; diff --git a/src/analyzer/protocol/ssl/ssl-protocol.pac b/src/analyzer/protocol/ssl/ssl-protocol.pac index 26735c1f97..57b7f192c6 100644 --- a/src/analyzer/protocol/ssl/ssl-protocol.pac +++ b/src/analyzer/protocol/ssl/ssl-protocol.pac @@ -30,10 +30,10 @@ type SSLRecord(is_orig: bool) = record { UNKNOWN_VERSION -> 0; SSLv20 -> (((head0 & 0x7f) << 8) | head1) - 3; default -> (head3 << 8) | head4; - }; + } &requires(version); }; -type RecordText(rec: SSLRecord) = case $context.connection.state(rec.is_orig) of { +type RecordText(rec: SSLRecord) = case $context.connection.determine_state(rec.is_orig, rec.content_type) of { STATE_ENCRYPTED -> ciphertext : CiphertextRecord(rec); default @@ -137,6 +137,49 @@ type SSLPDU(is_orig: bool) = record { refine connection SSL_Conn += { + ## So - this falls a bit under the envelope of dirty hack - but I don't + ## really have a better idea. This function determines if a packet should + ## be handled as an encrypted or as a plaintext packet. + ## + ## For TLS 1.2 and below - this is relatively straightforward. Everything + ## that arrives before CCS (Change Cipher Spec) is a plaintext record. And + ## everything that arrives after CCS will be encrypted. + ## + ## TLS 1.3, however, messes this up a bunch. Some clients still choose to + ## send a CCS message. The message, however, is pretty much meaningless + ## from a protocol perspective - and just ignored by the other side. Also - + ## it is not necessary to send it and some implementations just don't. + ## + ## So - what we do here is that we enable the encrypted flag when we get + ## the first application data in a connection that negotiated TLS 1.3. + ## + ## This is correct insofar as the packet will be encrypted. We sadly loose + ## a bit of context here - we can't really say when we get the first packet + ## that uses the final cryptographic key material - and will contain content + ## data. We just don't have that information available in TLS 1.3 anymore. + function determine_state(is_orig: bool, content_type: int) : int + %{ + int current_state = state(is_orig); + if ( current_state == STATE_ENCRYPTED || content_type != APPLICATION_DATA ) + return current_state; + + // state = STATE_CLEAR && content_type == APPLICATION_DATA + uint16_t negotiated_version = zeek_analyzer()->GetNegotiatedVersion(); + + // in theory, we should check for TLS13 or draft-TLS13 instead of doing the reverse. + // But - people use weird version numbers. And all of those weird version numbers are + // some sort of TLS1.3. So - let's do it this way round instead. + if ( negotiated_version != SSLv20 && negotiated_version != SSLv30 && negotiated_version != TLSv10 && negotiated_version != TLSv11 && negotiated_version != TLSv12 ) + { + // well, it seems like this is a TLS 1.3 (or equivalent) applicatio data packet. Let's enable encryption + // and handle it as encrypted. + startEncryption(is_orig); + return STATE_ENCRYPTED; + } + + return current_state; // has to be STATE_CLEAR + %} + function determine_ssl_record_layer(head0 : uint8, head1 : uint8, head2 : uint8, head3: uint8, head4: uint8, is_orig: bool) : int %{ diff --git a/src/analyzer/protocol/ssl/tls-handshake-protocol.pac b/src/analyzer/protocol/ssl/tls-handshake-protocol.pac index 3fcf1c595c..e37a0c512a 100644 --- a/src/analyzer/protocol/ssl/tls-handshake-protocol.pac +++ b/src/analyzer/protocol/ssl/tls-handshake-protocol.pac @@ -795,10 +795,14 @@ type SupportedVersions(rec: HandshakeRecord) = record { versions: uint16[] &until($input.length() == 0); } &length=length+1; +# If the server sends it, this is the authorative version. Set it. type OneSupportedVersion(rec: HandshakeRecord) = record { version: uint16; +} &let { + version_set : bool = $context.connection.set_version(version); }; + type PSKKeyExchangeModes(rec: HandshakeRecord) = record { length: uint8; modes: uint8[] &until($input.length() == 0); @@ -944,6 +948,7 @@ refine connection Handshake_Conn += { %init{ chosen_cipher_ = NO_CHOSEN_CIPHER; chosen_version_ = UNKNOWN_VERSION; + record_version_ = 0; %} @@ -955,10 +960,18 @@ refine connection Handshake_Conn += { return true; %} - function chosen_version() : int %{ return chosen_version_; %} + function chosen_version() : uint16 %{ return chosen_version_; %} + # This function is called several times in certain circumstances. + # If it is called twice, it is first called due to the supported_versions + # field in the server hello - and then again due to the outer version in + # the server hello. So - once we have a version here, let's just stick + # with it. function set_version(version: uint16) : bool %{ + if ( chosen_version_ != UNKNOWN_VERSION ) + return false; + chosen_version_ = version; return true; %}