Skip to content

Commit

Permalink
fixes for bogo TLS 1.2
Browse files Browse the repository at this point in the history
  • Loading branch information
Hannes Rantzsch committed Jan 6, 2022
1 parent 46f2e60 commit 937f8c9
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/bogo_shim/bogo_shim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,12 @@ std::string map_to_bogo_error(const std::string& e)
{ "Server changed its mind about extended master secret", ":RENEGOTIATION_EMS_MISMATCH:" },
{ "Server changed its mind about secure renegotiation", ":RENEGOTIATION_MISMATCH:" },
{ "Server changed version after renegotiation", ":WRONG_SSL_VERSION:" },
{ "Server downgraded version after renegotiation", ":WRONG_SSL_VERSION:" },
{ "Server policy prohibits renegotiation", ":NO_RENEGOTIATION:" },
{ "Server replied using a ciphersuite not allowed in version it offered", ":WRONG_CIPHER_RETURNED:" },
{ "Server replied with DTLS-SRTP alg we did not send", ":BAD_SRTP_PROTECTION_PROFILE_LIST:" },
{ "Server replied with ciphersuite we didn't send", ":WRONG_CIPHER_RETURNED:" },
{ "Server replied with later version than client offered", ":UNSUPPORTED_PROTOCOL:" },
{ "Server version Unknown 18.52 is unacceptable by policy", ":UNSUPPORTED_PROTOCOL:" }, // bogus version from "ServerBogusVersion"
{ "Server version SSL v3 is unacceptable by policy", ":UNSUPPORTED_PROTOCOL:" }, // "NoSSL3-Client-Unsolicited"
{ "Server replied with non-null compression method", ":UNSUPPORTED_COMPRESSION_ALGORITHM:" },
{ "Server replied with some unknown ciphersuite", ":UNKNOWN_CIPHER_RETURNED:" },
{ "Server replied with unsupported extensions: 0", ":UNEXPECTED_EXTENSION:" },
Expand Down
4 changes: 2 additions & 2 deletions src/lib/tls/msg_server_hello.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Server_Hello::Server_Hello(Handshake_IO& io,
const Client_Hello& client_hello,
const Server_Hello::Settings& server_settings,
const std::string next_protocol) :
m_impl(Message_Factory::create<Server_Hello_Impl>(client_hello.version(), io, hash, policy, cb, rng, reneg_info, client_hello, server_settings, next_protocol))
m_impl(Message_Factory::create<Server_Hello_Impl>(client_hello.supported_versions(), io, hash, policy, cb, rng, reneg_info, client_hello, server_settings, next_protocol))
{
}

Expand All @@ -48,7 +48,7 @@ Server_Hello::Server_Hello(Handshake_IO& io,
Session& resumed_session,
bool offer_session_ticket,
const std::string& next_protocol) :
m_impl(Message_Factory::create<Server_Hello_Impl>(client_hello.version(), io, hash, policy, cb, rng, reneg_info, client_hello, resumed_session, offer_session_ticket, next_protocol))
m_impl(Message_Factory::create<Server_Hello_Impl>(client_hello.supported_versions(), io, hash, policy, cb, rng, reneg_info, client_hello, resumed_session, offer_session_ticket, next_protocol))
{
}

Expand Down
14 changes: 7 additions & 7 deletions src/lib/tls/tls12/tls_client_impl_12.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,13 @@ void Client_Impl_12::process_handshake_msg(const Handshake_State* active_state,
{
state.server_hello(new Server_Hello(contents));

if(!state.server_hello()->version().valid())
{
throw TLS_Exception(Alert::PROTOCOL_VERSION,
"Server version " + state.server_hello()->version().to_string() +
" is unacceptable by policy");
}

if(!state.client_hello()->offered_suite(state.server_hello()->ciphersuite()))
{
throw TLS_Exception(Alert::HANDSHAKE_FAILURE,
Expand Down Expand Up @@ -403,13 +410,6 @@ void Client_Impl_12::process_handshake_msg(const Handshake_State* active_state,
"Server attempting to negotiate SSLv3 which is not supported");
}

if(!policy().acceptable_protocol_version(state.version()))
{
throw TLS_Exception(Alert::PROTOCOL_VERSION,
"Server version " + state.version().to_string() +
" is unacceptable by policy");
}

if(state.ciphersuite().signature_used() || state.ciphersuite().kex_method() == Kex_Algo::STATIC_RSA)
{
state.set_expected_next(CERTIFICATE);
Expand Down
5 changes: 4 additions & 1 deletion src/lib/tls/tls_message_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <botan/tls_messages.h>
#include <botan/tls_version.h>
#include <botan/tls_exceptn.h>
#include <botan/internal/stl_util.h>

#include <exception>
Expand Down Expand Up @@ -101,13 +102,15 @@ std::unique_ptr<MessageBaseT> create(const Protocol_Version &protocol_version, P
case Protocol_Version::DTLS_V12:
return std::make_unique<typename impl_t::v12>(std::forward<ParamTs>(parameters)...);
default:
BOTAN_ASSERT(false, "unexpected protocol version");
// TODO is this the right behavior?
throw TLS_Exception(Alert::PROTOCOL_VERSION, "unsupported protocol version");
}
}

template <typename MessageBaseT, typename... ParamTs>
std::unique_ptr<MessageBaseT> create(std::vector<Protocol_Version> supported_versions, ParamTs&&... parameters)
{
// TODO: this will not work for DTLS
#if defined(BOTAN_HAS_TLS_13)
const auto protocol_version =
value_exists(supported_versions, Protocol_Version(Protocol_Version::TLS_V13))
Expand Down
21 changes: 21 additions & 0 deletions src/lib/tls/tls_version.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,27 @@ bool Protocol_Version::operator>(const Protocol_Version& other) const
return m_version > other.m_version;
}

bool Protocol_Version::valid() const
{
const uint8_t maj = major_version();
const uint8_t min = minor_version();

if(maj == 3 && min <= 4)
// 3.0: SSLv3
// 3.1: TLS 1.0
// 3.2: TLS 1.1
// 3.3: TLS 1.2
// 3.4: TLS 1.3
return true;

if(maj == 254 && (min == 253 || min == 255))
// 254.253: DTLS 1.2
// 254.255: DTLS 1.0
return true;

return false;
}

bool Protocol_Version::known_version() const
{
return (m_version == Protocol_Version::TLS_V12 ||
Expand Down
2 changes: 1 addition & 1 deletion src/lib/tls/tls_version.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class BOTAN_PUBLIC_API(2,0) Protocol_Version final
/**
* @return true if this is a valid protocol version
*/
bool valid() const { return (m_version != 0); }
bool valid() const;

/**
* @return true if this is a protocol version we know about
Expand Down

0 comments on commit 937f8c9

Please sign in to comment.