From caa04f440c13e96a4df8b4fc0346ab5463516557 Mon Sep 17 00:00:00 2001 From: Alexey Volokitin Date: Thu, 23 May 2024 15:38:11 +0300 Subject: [PATCH] logging of updating keys(CLIENT_TRAFFIC_SECRET_N and SERVER_TRAFFIC_SECRET_N where N>0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: René Meusel --- src/lib/tls/tls13/tls_channel_impl_13.cpp | 4 +-- src/lib/tls/tls13/tls_cipher_state.cpp | 17 ++++++++++-- src/lib/tls/tls13/tls_cipher_state.h | 7 +++-- src/lib/tls/tls_policy.cpp | 1 + src/tests/test_tls_cipher_state.cpp | 33 ++++++++++++++++++++--- 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/lib/tls/tls13/tls_channel_impl_13.cpp b/src/lib/tls/tls13/tls_channel_impl_13.cpp index c0bb8dd788c..243f1a23e1c 100644 --- a/src/lib/tls/tls13/tls_channel_impl_13.cpp +++ b/src/lib/tls/tls13/tls_channel_impl_13.cpp @@ -198,7 +198,7 @@ void Channel_Impl_13::handle(const Key_Update& key_update) { throw Unexpected_Message("Unexpected additional post-handshake message data found in record"); } - m_cipher_state->update_read_keys(); + m_cipher_state->update_read_keys(*this); // TODO: introduce some kind of rate limit of key updates, otherwise we // might be forced into an endless loop of key updates. @@ -319,7 +319,7 @@ void Channel_Impl_13::update_traffic_keys(bool request_peer_update) { BOTAN_STATE_CHECK(is_handshake_complete()); BOTAN_ASSERT_NONNULL(m_cipher_state); send_post_handshake_message(Key_Update(request_peer_update)); - m_cipher_state->update_write_keys(); + m_cipher_state->update_write_keys(*this); } void Channel_Impl_13::send_record(Record_Type record_type, const std::vector& record) { diff --git a/src/lib/tls/tls13/tls_cipher_state.cpp b/src/lib/tls/tls13/tls_cipher_state.cpp index 2284f3a3c1c..a5c91c29fab 100644 --- a/src/lib/tls/tls13/tls_cipher_state.cpp +++ b/src/lib/tls/tls13/tls_cipher_state.cpp @@ -98,6 +98,7 @@ #include #include +#include #include #include #include @@ -444,6 +445,8 @@ Cipher_State::Cipher_State(Connection_Side whoami, std::string_view hash_functio m_salt(m_hash->output_length(), 0x00), m_write_seq_no(0), m_read_seq_no(0), + m_write_key_update_count(0), + m_read_key_update_count(0), m_ticket_nonce(0) {} Cipher_State::~Cipher_State() = default; @@ -593,20 +596,30 @@ std::vector Cipher_State::empty_hash() const { return m_hash->final_stdvec(); } -void Cipher_State::update_read_keys() { +void Cipher_State::update_read_keys(const Secret_Logger& logger) { BOTAN_ASSERT_NOMSG(m_state == State::ServerApplicationTraffic || m_state == State::Completed); m_read_application_traffic_secret = hkdf_expand_label(m_read_application_traffic_secret, "traffic upd", {}, m_hash->output_length()); + const auto secret_label = fmt("{}_TRAFFIC_SECRET_{}", + m_connection_side == Connection_Side::Server ? "CLIENT" : "SERVER", + ++m_read_key_update_count); + logger.maybe_log_secret(secret_label, m_read_application_traffic_secret); + derive_read_traffic_key(m_read_application_traffic_secret); } -void Cipher_State::update_write_keys() { +void Cipher_State::update_write_keys(const Secret_Logger& logger) { BOTAN_ASSERT_NOMSG(m_state == State::ServerApplicationTraffic || m_state == State::Completed); m_write_application_traffic_secret = hkdf_expand_label(m_write_application_traffic_secret, "traffic upd", {}, m_hash->output_length()); + const auto secret_label = fmt("{}_TRAFFIC_SECRET_{}", + m_connection_side == Connection_Side::Server ? "SERVER" : "CLIENT", + ++m_write_key_update_count); + logger.maybe_log_secret(secret_label, m_write_application_traffic_secret); + derive_write_traffic_key(m_write_application_traffic_secret); } diff --git a/src/lib/tls/tls13/tls_cipher_state.h b/src/lib/tls/tls13/tls_cipher_state.h index 9a4d09feaa8..c14955a8c11 100644 --- a/src/lib/tls/tls13/tls_cipher_state.h +++ b/src/lib/tls/tls13/tls_cipher_state.h @@ -237,7 +237,7 @@ class BOTAN_TEST_API Cipher_State { * Note that this must not be called before the connection is ready for * application traffic. */ - void update_read_keys(); + void update_read_keys(const Secret_Logger& channel); /** * Updates the key material used for encrypting data @@ -246,7 +246,7 @@ class BOTAN_TEST_API Cipher_State { * Note that this must not be called before the connection is ready for * application traffic. */ - void update_write_keys(); + void update_write_keys(const Secret_Logger& channel); /** * Remove handshake/traffic secrets for decrypting data from peer @@ -328,6 +328,9 @@ class BOTAN_TEST_API Cipher_State { uint64_t m_write_seq_no; uint64_t m_read_seq_no; + uint32_t m_write_key_update_count; + uint32_t m_read_key_update_count; + uint16_t m_ticket_nonce; secure_vector m_finished_key; diff --git a/src/lib/tls/tls_policy.cpp b/src/lib/tls/tls_policy.cpp index 55f979a547f..2e2c4d05580 100644 --- a/src/lib/tls/tls_policy.cpp +++ b/src/lib/tls/tls_policy.cpp @@ -622,6 +622,7 @@ void Policy::print(std::ostream& o) const { print_bool(o, "allow_tls12", allow_tls12()); print_bool(o, "allow_tls13", allow_tls13()); print_bool(o, "allow_dtls12", allow_dtls12()); + print_bool(o, "allow_ssl_key_log_file", allow_ssl_key_log_file()); print_vec(o, "ciphers", allowed_ciphers()); print_vec(o, "macs", allowed_macs()); print_vec(o, "signature_hashes", allowed_signature_hashes()); diff --git a/src/tests/test_tls_cipher_state.cpp b/src/tests/test_tls_cipher_state.cpp index 7c05409f1f6..b1b81d7a76b 100644 --- a/src/tests/test_tls_cipher_state.cpp +++ b/src/tests/test_tls_cipher_state.cpp @@ -144,6 +144,16 @@ std::vector test_secret_derivation_rfc8448_rtt1() { "a1 1a f9 f0 55 31 f8 56 ad 47 11 6b 45 a9 50 32" "82 04 b4 f4 4b fb 6b 3a 4b 4f 1f 3f cb 63 16 43"); + // application traffic secret (1) for the client (not in RFC 8448) + const auto updated_client_traffic_secret = Botan::hex_decode( + "fc df cc 72 72 5a ae e4 8b f6 4e 4f d8 b7 49 cd" + "bd ba b3 9d 90 da 0b 26 e2 24 5c a6 ea 16 72 07"); + + // application traffic secret (1) for the server (not in RFC 8448) + const auto updated_server_traffic_secret = Botan::hex_decode( + "51 92 1b 8a a3 00 19 76 eb 40 1d 0a 43 19 a8 51" + "64 16 a6 c5 60 01 a3 57 e5 d1 62 03 1e 84 f9 16"); + // encrypted with server_handshake_traffic_secret const auto encrypted_extensions = RFC8448_TestData("encrypted_extensions", @@ -463,9 +473,26 @@ std::vector test_secret_derivation_rfc8448_rtt1() { }), CHECK_both("key update", - [&](Cipher_State* cs, Journaling_Secret_Logger*, Connection_Side, Test::Result& result) { - cs->update_read_keys(); - cs->update_write_keys(); + [&](Cipher_State* cs, Journaling_Secret_Logger* sl, Connection_Side side, Test::Result& result) { + const auto read_label = + side == Connection_Side::Client ? "SERVER_TRAFFIC_SECRET_1" : "CLIENT_TRAFFIC_SECRET_1"; + const auto write_label = + side == Connection_Side::Client ? "CLIENT_TRAFFIC_SECRET_1" : "SERVER_TRAFFIC_SECRET_1"; + + cs->update_read_keys(*sl); + result.test_eq("read secret update is here", sl->secrets.size(), 6); + result.require("has new read traffic secret", sl->secrets.contains(read_label)); + + cs->update_write_keys(*sl); + result.test_eq("write secret update is here", sl->secrets.size(), 7); + result.require("has new write traffic secret", sl->secrets.contains(write_label)); + + result.test_eq("client traffic secret (1)", + sl->secrets.at("CLIENT_TRAFFIC_SECRET_1"), + updated_client_traffic_secret); + result.test_eq("server traffic secret (1)", + sl->secrets.at("SERVER_TRAFFIC_SECRET_1"), + updated_server_traffic_secret); result.confirm("can encrypt application traffic", cs->can_encrypt_application_traffic()); }),