From ebea26a0654d764ea9daeb38a81def96efd7bfa0 Mon Sep 17 00:00:00 2001 From: Florian Wilkens Date: Thu, 24 Jun 2021 17:05:34 +0200 Subject: [PATCH] analyzer/ssl: several improvements - use better data structures for secret and key material storage - add documentation to the new methods in the analyzer --- src/analyzer/protocol/ssl/SSL.cc | 61 +++++++++++------ src/analyzer/protocol/ssl/SSL.h | 90 +++++++++++++++++++++++-- src/analyzer/protocol/ssl/functions.bif | 4 +- 3 files changed, 126 insertions(+), 29 deletions(-) diff --git a/src/analyzer/protocol/ssl/SSL.cc b/src/analyzer/protocol/ssl/SSL.cc index 24897c0693..55c0f74324 100644 --- a/src/analyzer/protocol/ssl/SSL.cc +++ b/src/analyzer/protocol/ssl/SSL.cc @@ -50,9 +50,6 @@ SSL_Analyzer::SSL_Analyzer(Connection* c) had_gap = false; c_seq = 0; s_seq = 0; - // FIXME: should this be initialized to nullptr? - secret = nullptr; - keys = nullptr; pia = nullptr; } @@ -136,14 +133,27 @@ void SSL_Analyzer::Undelivered(uint64_t seq, int len, bool orig) interp->NewGap(orig, len); } -void SSL_Analyzer::SetSecret(const u_char* data, int len) +void SSL_Analyzer::SetSecret(zeek::StringVal* secret) { - secret = make_intrusive(len, (const char*)data); + SetSecret(secret->Len(), secret->Bytes()); } -void SSL_Analyzer::SetKeys(const u_char* data, int len) +void SSL_Analyzer::SetSecret(size_t len, const u_char* data) { - keys = make_intrusive(len, (const char*)data); + secret.clear(); + secret.append((const char*)data, len); + } + +void SSL_Analyzer::SetKeys(zeek::StringVal* keys) + { + SetKeys(keys->Len(), keys->Bytes()); + } + +void SSL_Analyzer::SetKeys(size_t len, const u_char* data) + { + keys.clear(); + keys.reserve(len); + std::copy(data, data + len, std::back_inserter(keys)); } bool SSL_Analyzer::TLS12_PRF(const std::string& secret, const std::string& label, @@ -152,7 +162,7 @@ bool SSL_Analyzer::TLS12_PRF(const std::string& secret, const std::string& label #ifdef OPENSSL_HAVE_KDF_H // alloc buffers EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_TLS1_PRF, NULL); - size_t seed_len = label.length() + rnd1_len + rnd2_len; + size_t seed_len = label.size() + rnd1_len + rnd2_len; std::string seed{}; seed.reserve(seed_len); @@ -166,9 +176,9 @@ bool SSL_Analyzer::TLS12_PRF(const std::string& secret, const std::string& label // FIXME: sha384 should not be hardcoded if (EVP_PKEY_CTX_set_tls1_prf_md(pctx, EVP_sha384()) <= 0) goto abort; /* Error */ - if (EVP_PKEY_CTX_set1_tls1_prf_secret(pctx, secret.data(), secret.length()) <= 0) + if (EVP_PKEY_CTX_set1_tls1_prf_secret(pctx, secret.data(), secret.size()) <= 0) goto abort; /* Error */ - if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed.data(), seed.length()) <= 0) + if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed.data(), seed.size()) <= 0) goto abort; /* Error */ if (EVP_PKEY_derive(pctx, out, &out_len) <= 0) goto abort; /* Error */ @@ -195,16 +205,16 @@ bool SSL_Analyzer::TryDecryptApplicationData(int len, const u_char* data, bool i } // Neither secret or key present: abort - if ( ( secret == nullptr || secret->Len() == 0 ) && ( keys == nullptr || keys->Len() == 0 ) ) + if ( secret.size() == 0 && keys.size() == 0 ) { DBG_LOG(DBG_ANALYZER, "Could not decrypt packet due to missing keys/secret.\n"); // FIXME: change util function to return a printably std::string for DBG_LOG - //print_hex("->client_random:", handshake_interp->client_random().data(), handshake_interp->client_random().length()); + //print_hex("->client_random:", handshake_interp->client_random().data(), handshake_interp->client_random().size()); return false; } // Secret present, but no keys derived yet: derive keys - if ( secret != nullptr && secret->Len() != 0 && ( keys == nullptr || keys->Len() == 0 ) ) + if ( secret.size() != 0 && keys.size() == 0 ) { #ifdef OPENSSL_HAVE_KDF_H DBG_LOG(DBG_ANALYZER, "Deriving TLS keys for connection foo"); @@ -218,7 +228,7 @@ bool SSL_Analyzer::TryDecryptApplicationData(int len, const u_char* data, bool i memcpy(crand, &(ts), 4); memcpy(crand + 4, c_rnd.data(), c_rnd.length()); - auto res = TLS12_PRF(secret->ToStdString(), "key expansion", + auto res = TLS12_PRF(secret, "key expansion", (char*)s_rnd.data(), s_rnd.length(), crand, sizeof(crand), keybuf, sizeof(keybuf)); if ( !res ) { @@ -227,29 +237,36 @@ bool SSL_Analyzer::TryDecryptApplicationData(int len, const u_char* data, bool i } // save derived keys - SetKeys(keybuf, sizeof(keybuf)); + SetKeys(sizeof(keybuf), keybuf); #else DBG_LOG(DBG_ANALYZER, "Cannot derive TLS keys as Zeek was compiled without "); #endif } // Keys present: decrypt TLS application data - if ( keys != nullptr && keys->Len() == 72 ) + if ( keys.size() == 72 ) { // FIXME: could also print keys or conn id here DBG_LOG(DBG_ANALYZER, "Decrypting application data"); - // session keys & AEAD data + + // client write_key u_char c_wk[32]; + // server write_key u_char s_wk[32]; + // client IV u_char c_iv[4]; + // server IV u_char s_iv[4]; + + // AEAD nonce & tag u_char s_aead_nonce[12]; u_char s_aead_tag[13]; - memcpy(c_wk, keys->Bytes(), 32); - memcpy(s_wk, &(keys->Bytes()[32]), 32); - memcpy(c_iv, &(keys->Bytes()[64]), 4); - memcpy(s_iv, &(keys->Bytes()[68]), 4); + // FIXME: there should be a better way to do these copies + memcpy(c_wk, keys.data(), 32); + memcpy(s_wk, keys.data() + 32, 32); + memcpy(c_iv, keys.data() + 64, 4); + memcpy(s_iv, keys.data() + 68, 4); // FIXME: should we change types here? u_char* encrypted = (u_char*)data; @@ -301,7 +318,7 @@ bool SSL_Analyzer::TryDecryptApplicationData(int len, const u_char* data, bool i EVP_DecryptUpdate(ctx, decrypted, &decrypted_len, (const u_char*) encrypted, encrypted_len); int res = 0; - if ( ! (res = EVP_DecryptFinal(ctx, NULL, &res)) ) + if ( ! ( res = EVP_DecryptFinal(ctx, NULL, &res) ) ) { DBG_LOG(DBG_ANALYZER, "Decryption failed with return code: %d. Invalid key?\n", res); EVP_CIPHER_CTX_free(ctx); diff --git a/src/analyzer/protocol/ssl/SSL.h b/src/analyzer/protocol/ssl/SSL.h index e4df7a5b55..6a5abe2ce7 100644 --- a/src/analyzer/protocol/ssl/SSL.h +++ b/src/analyzer/protocol/ssl/SSL.h @@ -34,12 +34,92 @@ public: static analyzer::Analyzer* Instantiate(Connection* conn) { return new SSL_Analyzer(conn); } - // Key material for decryption - void SetSecret(const u_char* data, int len); - void SetKeys(const u_char* data, int len); + /** + * Set the secret that should be used to derive keys for the + * connection. (For TLS 1.2 this is the pre-master secret) + * + * @param secret The secret to set + */ + void SetSecret(StringVal* secret); + /** + * Set the secret that should be used to derive keys for the + * connection. (For TLS 1.2 this is the pre-master secret) + * + * @param len Length of the secret bytes + * + * @param data Pointer to the secret bytes + */ + void SetSecret(size_t len, const u_char* data); + + /** + * Set the decryption keys that should be used to decrypt + * TLS application data in the connection. + * + * @param keys The key buffer as derived via TLS PRF (for + * AES_GCM this should be 72 bytes in length) + */ + void SetKeys(StringVal* keys); + + /** + * Set the decryption keys that should be used to decrypt + * TLS application data in the connection. + * + * @param len Length of the key buffer (for AES_GCM this should + * be 72) + * + * @param data Pointer to the key buffer as derived via TLS PRF + */ + void SetKeys(size_t len, const u_char* data); + + /** + * Try to decrypt TLS application data from a packet. Requires secret or keys to be set prior + * + * @param len Length of the encrypted bytes to decrypt + * + * @param data Pointer to the encrypted bytes to decrypt + * + * @param is_orig Direction of the connection + * + * @param content_type Content type as given in the TLS packet + * + * @param raw_tls_version Raw TLS version as given in the TLS packets + */ bool TryDecryptApplicationData(int len, const u_char* data, bool is_orig, uint8_t content_type, uint16_t raw_tls_version); + + /** + * TLS 1.2 pseudo random function (PRF) used to expand the pre-master secret and derive keys. + * The seed is obtained by concatinating rnd1 and rnd2 + * + * @param secret Secret as defined in the TLS RFC + * + * @param label Label as defined in the TLS RFC + * + * @param rnd1 Pointer to the first part of the seed + * + * @param rnd1_len Length of the first part of the seed + * + * @param rnd2 Pointer to the second part of the seed + * + * @param rnd2_len Length of the second part of the seed + * + * @param out Pointer to the derived bytes + * + * @param out_len Length indicating how many bytes should be derived + * + * @return True, if the operation completed successfully, false otherwise + */ bool TLS12_PRF(const std::string& secret, const std::string& label, const char* rnd1, size_t rnd1_len, const char* rnd2, size_t rnd2_len, u_char* out, size_t out_len); + + /** + * Forward decrypted TLS application data to child analyzers + * + * @param len Length of the data to forward + * + * @param data Pointer to the data to forward + * + * @param is_orig Direction of the connection + */ void ForwardDecryptedData(int len, const u_char* data, bool is_orig); protected: @@ -50,8 +130,8 @@ protected: // FIXME: should this be moved into the connection? int c_seq; int s_seq; - StringValPtr secret; - StringValPtr keys; + std::string secret; + std::vector keys; zeek::analyzer::pia::PIA_TCP *pia; }; diff --git a/src/analyzer/protocol/ssl/functions.bif b/src/analyzer/protocol/ssl/functions.bif index b04bf19b3f..19661ad4f3 100644 --- a/src/analyzer/protocol/ssl/functions.bif +++ b/src/analyzer/protocol/ssl/functions.bif @@ -22,7 +22,7 @@ function set_secret%(c: connection, secret: string%): bool analyzer::Analyzer* sa = c->FindAnalyzer("SSL"); if ( sa ) { - static_cast(sa)->SetSecret(secret->Bytes(), secret->Len()); + static_cast(sa)->SetSecret(secret); return zeek::val_mgr->True(); } return zeek::val_mgr->False(); @@ -33,7 +33,7 @@ function set_keys%(c: connection, keys: string%): bool analyzer::Analyzer* sa = c->FindAnalyzer("SSL"); if ( sa ) { - static_cast(sa)->SetKeys(keys->Bytes(), keys->Len()); + static_cast(sa)->SetKeys(keys); return zeek::val_mgr->True(); } return zeek::val_mgr->False();