From 4ff9396ad45fab0ce04c625551a8696a0ed600b7 Mon Sep 17 00:00:00 2001 From: Alexey Volokitin Date: Thu, 23 May 2024 15:22:46 +0300 Subject: [PATCH] logging of updating keys(CLIENT_TRAFFIC_SECRET_N and SERVER_TRAFFIC_SECRET_N where N>0) --- src/lib/tls/tls13/tls_channel_impl_13.cpp | 4 ++-- src/lib/tls/tls13/tls_cipher_state.cpp | 22 ++++++++++++++++++++-- src/lib/tls/tls13/tls_cipher_state.h | 7 +++++-- src/lib/tls/tls_policy.cpp | 1 + src/tests/test_tls_cipher_state.cpp | 6 +++--- 5 files changed, 31 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..e9e89e73ddf 100644 --- a/src/lib/tls/tls13/tls_cipher_state.cpp +++ b/src/lib/tls/tls13/tls_cipher_state.cpp @@ -444,6 +444,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_server_traffic_secret_number(0), + m_client_traffic_secret_number(0), m_ticket_nonce(0) {} Cipher_State::~Cipher_State() = default; @@ -593,20 +595,36 @@ 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()); + std::stringstream label; + if(m_connection_side == Connection_Side::Server) { + label << "CLIENT_TRAFFIC_SECRET_" << ++m_client_traffic_secret_number; + } else { + label << "SERVER_TRAFFIC_SECRET_" << ++m_server_traffic_secret_number; + } + logger.maybe_log_secret(label.str(), 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()); + std::stringstream label; + if(m_connection_side == Connection_Side::Server) { + label << "SERVER_TRAFFIC_SECRET_" << ++m_server_traffic_secret_number; + } else { + label << "CLIENT_TRAFFIC_SECRET_" << ++m_client_traffic_secret_number; + } + logger.maybe_log_secret(label.str(), 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..18112a49830 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_server_traffic_secret_number; + uint32_t m_client_traffic_secret_number; + 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..c99ed2699dd 100644 --- a/src/tests/test_tls_cipher_state.cpp +++ b/src/tests/test_tls_cipher_state.cpp @@ -463,9 +463,9 @@ 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* logger, Connection_Side, Test::Result& result) { + cs->update_read_keys(*logger); + cs->update_write_keys(*logger); result.confirm("can encrypt application traffic", cs->can_encrypt_application_traffic()); }),