Skip to content

Commit

Permalink
transcript hash is not no longer secure
Browse files Browse the repository at this point in the history
Co-authored-by: René Meusel <rene.meusel@nexenio.com>
  • Loading branch information
Hannes Rantzsch and reneme committed Feb 23, 2022
1 parent 91d05f0 commit c1e11f4
Show file tree
Hide file tree
Showing 14 changed files with 154 additions and 91 deletions.
4 changes: 2 additions & 2 deletions src/lib/tls/msg_finished.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,13 @@ bool Finished_12::verify(const Handshake_State& state,
Finished_13::Finished_13(Handshake_IO& io,
Handshake_State& state,
Cipher_State* cipher_state,
const secure_vector<uint8_t>& transcript_hash)
const Transcript_Hash& transcript_hash)
{
m_verification_data = cipher_state->finished_mac(transcript_hash);
state.hash().update(io.send(*this));
}

bool Finished_13::verify(Cipher_State* cipher_state, const secure_vector<uint8_t>& transcript_hash) const
bool Finished_13::verify(Cipher_State* cipher_state, const Transcript_Hash& transcript_hash) const
{
return cipher_state->verify_peer_finished_mac(transcript_hash, m_verification_data);
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/tls/tls13/tls_channel_impl_13.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class Channel_Impl_13 : public Channel_Impl

protected:
const Connection_Side m_side;
Transcript_Hash m_transcript_hash;
Transcript_Hash_State m_transcript_hash;
std::unique_ptr<Cipher_State> m_cipher_state;

private:
Expand Down
28 changes: 17 additions & 11 deletions src/lib/tls/tls13/tls_cipher_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,15 @@ std::unique_ptr<Cipher_State> Cipher_State::init_with_server_hello(
const Connection_Side side,
secure_vector<uint8_t>&& shared_secret,
const Ciphersuite& cipher,
const secure_vector<uint8_t>& transcript_hash)
const Transcript_Hash& transcript_hash)
{
auto cs = std::unique_ptr<Cipher_State>(new Cipher_State(side, cipher));
cs->advance_without_psk();
cs->advance_with_server_hello(std::move(shared_secret), transcript_hash);
return cs;
}

void Cipher_State::advance_with_server_finished(const secure_vector<uint8_t>& transcript_hash)
void Cipher_State::advance_with_server_finished(const Transcript_Hash& transcript_hash)
{
BOTAN_ASSERT_NOMSG(m_state == State::HandshakeTraffic);

Expand All @@ -122,7 +122,7 @@ void Cipher_State::advance_with_server_finished(const secure_vector<uint8_t>& tr
m_state = State::ApplicationTraffic;
}

void Cipher_State::advance_with_client_finished(const secure_vector<uint8_t>& transcript_hash)
void Cipher_State::advance_with_client_finished(const Transcript_Hash& transcript_hash)
{
BOTAN_ASSERT_NOMSG(m_state == State::ApplicationTraffic);

Expand Down Expand Up @@ -190,7 +190,7 @@ size_t Cipher_State::encrypt_output_length(const size_t input_length) const
return m_encrypt->output_length(input_length);
}

std::vector<uint8_t> Cipher_State::finished_mac(const secure_vector<uint8_t>& transcript_hash) const
std::vector<uint8_t> Cipher_State::finished_mac(const Transcript_Hash& transcript_hash) const
{
BOTAN_ASSERT_NOMSG(m_state == State::HandshakeTraffic);

Expand All @@ -200,7 +200,7 @@ std::vector<uint8_t> Cipher_State::finished_mac(const secure_vector<uint8_t>& tr
return hmac.final_stdvec();
}

bool Cipher_State::verify_peer_finished_mac(const secure_vector<uint8_t>& transcript_hash,
bool Cipher_State::verify_peer_finished_mac(const Transcript_Hash& transcript_hash,
const std::vector<uint8_t>& peer_mac) const
{
BOTAN_ASSERT_NOMSG(m_state == State::HandshakeTraffic);
Expand All @@ -211,7 +211,7 @@ bool Cipher_State::verify_peer_finished_mac(const secure_vector<uint8_t>& transc
return hmac.verify_mac(peer_mac);
}

secure_vector<uint8_t> Cipher_State::psk(const secure_vector<uint8_t>& nonce) const
secure_vector<uint8_t> Cipher_State::psk(const std::vector<uint8_t>& nonce) const
{
BOTAN_ASSERT_NOMSG(m_state == State::Completed);

Expand Down Expand Up @@ -267,13 +267,13 @@ void Cipher_State::advance_without_psk()
BOTAN_ASSERT_NOMSG(m_state == State::Uninitialized);

const auto early_secret = hkdf_extract(secure_vector<uint8_t>(m_hash->output_length(), 0x00));
m_salt = derive_secret(early_secret, "derived", m_hash->process(""));
m_salt = derive_secret(early_secret, "derived", empty_hash());

m_state = State::EarlyTraffic;
}

void Cipher_State::advance_with_server_hello(secure_vector<uint8_t>&& shared_secret,
const secure_vector<uint8_t>& transcript_hash)
const Transcript_Hash& transcript_hash)
{
BOTAN_ASSERT_NOMSG(m_state == State::EarlyTraffic);

Expand All @@ -284,7 +284,7 @@ void Cipher_State::advance_with_server_hello(secure_vector<uint8_t>&& shared_sec
derive_secret(handshake_secret, "s hs traffic", transcript_hash),
true);

m_salt = derive_secret(handshake_secret, "derived", m_hash->process(""));
m_salt = derive_secret(handshake_secret, "derived", empty_hash());

m_state = State::HandshakeTraffic;
}
Expand Down Expand Up @@ -329,7 +329,7 @@ secure_vector<uint8_t> Cipher_State::hkdf_extract(secure_vector<uint8_t>&& ikm)
secure_vector<uint8_t> Cipher_State::hkdf_expand_label(
const secure_vector<uint8_t>& secret,
std::string label,
const secure_vector<uint8_t>& context,
const std::vector<uint8_t>& context,
const size_t length) const
{
// assemble (serialized) HkdfLabel
Expand Down Expand Up @@ -360,7 +360,13 @@ secure_vector<uint8_t> Cipher_State::hkdf_expand_label(
secure_vector<uint8_t> Cipher_State::derive_secret(
const secure_vector<uint8_t>& secret,
std::string label,
const secure_vector<uint8_t>& messages_hash) const
const Transcript_Hash& messages_hash) const
{
return hkdf_expand_label(secret, label, messages_hash, m_hash->output_length());
}

std::vector<uint8_t> Cipher_State::empty_hash() const
{
m_hash->update("");
return m_hash->final_stdvec();
}
22 changes: 13 additions & 9 deletions src/lib/tls/tls13/tls_cipher_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include <botan/secmem.h>
#include <botan/tls_magic.h>

#include <botan/internal/tls_transcript_hash_13.h>

namespace Botan {

class AEAD_Mode;
Expand Down Expand Up @@ -71,17 +73,17 @@ class BOTAN_TEST_API Cipher_State
const Connection_Side side,
secure_vector<uint8_t>&& shared_secret,
const Ciphersuite& cipher,
const secure_vector<uint8_t>& transcript_hash);
const Transcript_Hash& transcript_hash);

/**
* Transition internal secrets/keys for transporting application data.
*/
void advance_with_server_finished(const secure_vector<uint8_t>& transcript_hash);
void advance_with_server_finished(const Transcript_Hash& transcript_hash);

/**
* Transition to the final internal state allowing to create resumptions.
*/
void advance_with_client_finished(const secure_vector<uint8_t>& transcript_hash);
void advance_with_client_finished(const Transcript_Hash& transcript_hash);

/**
* Encrypt a TLS record fragment (RFC 8446 5.2 -- TLSInnerPlaintext) using the
Expand Down Expand Up @@ -111,18 +113,18 @@ class BOTAN_TEST_API Cipher_State
/**
* Calculate the MAC for a TLS "Finished" handshake message (RFC 8446 4.4.4)
*/
std::vector<uint8_t> finished_mac(const secure_vector<uint8_t>& transcript_hash) const;
std::vector<uint8_t> finished_mac(const Transcript_Hash& transcript_hash) const;

/**
* Validate a MAC received in a TLS "Finished" handshake message (RFC 8446 4.4.4)
*/
bool verify_peer_finished_mac(const secure_vector<uint8_t>& transcript_hash,
bool verify_peer_finished_mac(const Transcript_Hash& transcript_hash,
const std::vector<uint8_t>& peer_mac) const;

/**
* Calculate the PSK for the given nonce (RFC 8446 4.6.1)
*/
secure_vector<uint8_t> psk(const secure_vector<uint8_t>& nonce) const;
secure_vector<uint8_t> psk(const std::vector<uint8_t>& nonce) const;

/**
* Indicates whether the appropriate secrets to encrypt/decrypt
Expand All @@ -143,7 +145,7 @@ class BOTAN_TEST_API Cipher_State
void advance_without_psk();

void advance_with_server_hello(secure_vector<uint8_t>&& shared_secret,
const secure_vector<uint8_t>& transcript_hash);
const Transcript_Hash& transcript_hash);

std::vector<uint8_t> current_nonce(const uint64_t seq_no,
const secure_vector<uint8_t>& iv) const;
Expand All @@ -163,7 +165,7 @@ class BOTAN_TEST_API Cipher_State
secure_vector<uint8_t> hkdf_expand_label(
const secure_vector<uint8_t>& secret,
std::string label,
const secure_vector<uint8_t>& context,
const std::vector<uint8_t>& context,
const size_t length) const;

/**
Expand All @@ -172,7 +174,9 @@ class BOTAN_TEST_API Cipher_State
secure_vector<uint8_t> derive_secret(
const secure_vector<uint8_t>& secret,
std::string label,
const secure_vector<uint8_t>& messages_hash) const;
const Transcript_Hash& messages_hash) const;

std::vector<uint8_t> empty_hash() const;

private:
enum class State
Expand Down
57 changes: 53 additions & 4 deletions src/lib/tls/tls13/tls_client_impl_13.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,61 @@ void Client_Impl_13::process_handshake_msg(Handshake_Message_13 message)
}, m_handshake_state.received(std::move(message)));
}

void Client_Impl_13::handle(const Server_Hello_13& server_hello_msg)
void Client_Impl_13::handle(const Server_Hello_13& sh)
{
if(sh.legacy_version() != Protocol_Version::TLS_V12)
{
BOTAN_UNUSED(server_hello_msg);
throw Not_Implemented("NYI");
// RFC 8446 4.1.3:
// In TLS 1.3, the TLS server indicates
// its version using the "supported_versions" extension
// (Section 4.2.1), and the legacy_version field MUST be set to
// 0x0303, which is the version number for TLS 1.2.
throw TLS_Exception(Alert::PROTOCOL_VERSION, "legacy_version must be set to 1.2 in TLS 1.3");
}

if(auto requested = sh.random_signals_downgrade())
{
if(requested.value() == Protocol_Version::TLS_V11)
{ throw TLS_Exception(Alert::PROTOCOL_VERSION, "TLS 1.1 is not supported"); }
if(requested.value() == Protocol_Version::TLS_V12)
{ throw Not_Implemented("downgrade is nyi"); }
}

if(sh.random_signals_hello_retry_request())
{
throw Not_Implemented("hello retry is nyi");
}

if(!m_handshake_state.client_hello().offered_suite(sh.ciphersuite()))
{
throw TLS_Exception(Alert::ILLEGAL_PARAMETER, "Ciphersuite was not offered");
}

auto cipher = Ciphersuite::by_id(sh.ciphersuite());
BOTAN_ASSERT_NOMSG(cipher.has_value()); // should work, since we offered this suite

if(!sh.extensions().has<Key_Share>())
{
// TODO
throw Unexpected_Message("keyshare ext not found!");
}

BOTAN_ASSERT_NOMSG(m_handshake_state.client_hello().extensions().has<Key_Share>());
auto my_keyshare = m_handshake_state.client_hello().extensions().get<Key_Share>();
auto shared_secret = my_keyshare->exchange(sh.extensions().get<Key_Share>(), policy(), callbacks(), rng());

m_transcript_hash.set_algorithm(cipher.value().prf_algo());

m_cipher_state = Cipher_State::init_with_server_hello(m_side,
std::move(shared_secret),
cipher.value(),
m_transcript_hash.current());

callbacks().tls_examine_extensions(m_handshake_state.server_hello().extensions(), SERVER);

// m_handshake_state.set_expected_next(ENCRYPTED_EXTENSIONS); // TODO expect CCS (middlebox compat)
}

void Client_Impl_13::handle(const Encrypted_Extensions& encrypted_extensions_msg)
{
BOTAN_UNUSED(encrypted_extensions_msg);
Expand Down Expand Up @@ -100,7 +149,7 @@ void Client_Impl_13::handle(const Finished& finished_msg)
// state.server_hello(new Server_Hello_13(contents));
// auto sh = state.server_hello_13();

// if(sh->legacy_version() != Protocol_Version::TLS_V12)
// if(sh.legacy_version() != Protocol_Version::TLS_V12)
// {
// // RFC 8446 4.1.3:
// // In TLS 1.3, the TLS server indicates
Expand Down
4 changes: 2 additions & 2 deletions src/lib/tls/tls13/tls_handshake_layer_13.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ void Handshake_Layer::copy_data(const std::vector<uint8_t>& data_from_peer)
}

Handshake_Layer::ReadResult<Handshake_Message_13> Handshake_Layer::next_message(const Policy& policy,
Transcript_Hash& transcript_hash)
Transcript_Hash_State& transcript_hash)
{
TLS::TLS_Data_Reader reader("handshake message", m_read_buffer);

Expand Down Expand Up @@ -82,7 +82,7 @@ Handshake_Message_13 Handshake_Layer::parse_message(
}

std::vector<uint8_t> Handshake_Layer::prepare_message(const Handshake_Message_13_Ref message,
Transcript_Hash& transcript_hash)
Transcript_Hash_State& transcript_hash)
{
auto [type, serialized] = std::visit([](auto msg)
{
Expand Down
7 changes: 2 additions & 5 deletions src/lib/tls/tls13/tls_handshake_layer_13.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@

namespace Botan::TLS {

class Handshake_Message;
class Transcript_Hash;

using BytesNeeded = size_t;

/**
Expand Down Expand Up @@ -56,9 +53,9 @@ class BOTAN_TEST_API Handshake_Layer
*
* @param policy the TLS policy
*/
ReadResult<Handshake_Message_13> next_message(const Policy& policy, Transcript_Hash& transcript_hash);
ReadResult<Handshake_Message_13> next_message(const Policy& policy, Transcript_Hash_State& transcript_hash);

std::vector<uint8_t> prepare_message(const Handshake_Message_13_Ref message, Transcript_Hash& transcript_hash);
std::vector<uint8_t> prepare_message(const Handshake_Message_13_Ref message, Transcript_Hash_State& transcript_hash);

// TODO: add interfaces and checks for conditions in 8446 5.1

Expand Down
14 changes: 7 additions & 7 deletions src/lib/tls/tls13/tls_transcript_hash_13.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,19 @@

namespace Botan::TLS {

Transcript_Hash::Transcript_Hash(const std::string &algo_spec)
Transcript_Hash_State::Transcript_Hash_State(const std::string &algo_spec)
{
set_algorithm(algo_spec);
}

Transcript_Hash::Transcript_Hash(const Transcript_Hash& other)
Transcript_Hash_State::Transcript_Hash_State(const Transcript_Hash_State& other)
: m_hash((other.m_hash != nullptr) ? other.m_hash->copy_state() : nullptr)
, m_unprocessed_transcript(other.m_unprocessed_transcript)
, m_current(other.m_current)
, m_previous(other.m_previous)
{}

void Transcript_Hash::update(const std::vector<uint8_t>& serialized_message)
void Transcript_Hash_State::update(const std::vector<uint8_t>& serialized_message)
{
if(m_hash != nullptr)
{
Expand All @@ -40,19 +40,19 @@ void Transcript_Hash::update(const std::vector<uint8_t>& serialized_message)
}
}

const std::vector<uint8_t>& Transcript_Hash::current() const
const Transcript_Hash& Transcript_Hash_State::current() const
{
BOTAN_STATE_CHECK(!m_current.empty());
return m_current;
}

const std::vector<uint8_t>& Transcript_Hash::previous() const
const Transcript_Hash& Transcript_Hash_State::previous() const
{
BOTAN_STATE_CHECK(!m_previous.empty());
return m_previous;
}

void Transcript_Hash::set_algorithm(const std::string& algo_spec)
void Transcript_Hash_State::set_algorithm(const std::string& algo_spec)
{
BOTAN_STATE_CHECK(m_hash == nullptr || m_hash->name() == algo_spec);
if(m_hash != nullptr)
Expand All @@ -66,7 +66,7 @@ void Transcript_Hash::set_algorithm(const std::string& algo_spec)
}
}

Transcript_Hash Transcript_Hash::clone() const
Transcript_Hash_State Transcript_Hash_State::clone() const
{
return *this;
}
Expand Down
Loading

0 comments on commit c1e11f4

Please sign in to comment.