diff --git a/src/bogo_shim/bogo_shim.cpp b/src/bogo_shim/bogo_shim.cpp index 77d0aa2d3cf..daa31b4d4be 100644 --- a/src/bogo_shim/bogo_shim.cpp +++ b/src/bogo_shim/bogo_shim.cpp @@ -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:" }, diff --git a/src/lib/tls/msg_server_hello.cpp b/src/lib/tls/msg_server_hello.cpp index f8354893345..c5fb5cfaf1c 100644 --- a/src/lib/tls/msg_server_hello.cpp +++ b/src/lib/tls/msg_server_hello.cpp @@ -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(client_hello.version(), io, hash, policy, cb, rng, reneg_info, client_hello, server_settings, next_protocol)) + m_impl(Message_Factory::create(client_hello.supported_versions(), io, hash, policy, cb, rng, reneg_info, client_hello, server_settings, next_protocol)) { } @@ -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(client_hello.version(), io, hash, policy, cb, rng, reneg_info, client_hello, resumed_session, offer_session_ticket, next_protocol)) + m_impl(Message_Factory::create(client_hello.supported_versions(), io, hash, policy, cb, rng, reneg_info, client_hello, resumed_session, offer_session_ticket, next_protocol)) { } diff --git a/src/lib/tls/tls12/tls_client_impl_12.cpp b/src/lib/tls/tls12/tls_client_impl_12.cpp index e72b96713d1..fd7cc3a0e09 100644 --- a/src/lib/tls/tls12/tls_client_impl_12.cpp +++ b/src/lib/tls/tls12/tls_client_impl_12.cpp @@ -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, @@ -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); diff --git a/src/lib/tls/tls_message_factory.h b/src/lib/tls/tls_message_factory.h index bb18222c4b7..15ef098a5ad 100644 --- a/src/lib/tls/tls_message_factory.h +++ b/src/lib/tls/tls_message_factory.h @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -101,13 +102,15 @@ std::unique_ptr create(const Protocol_Version &protocol_version, P case Protocol_Version::DTLS_V12: return std::make_unique(std::forward(parameters)...); default: - BOTAN_ASSERT(false, "unexpected protocol version"); + // TODO is this the right behavior? + throw TLS_Exception(Alert::PROTOCOL_VERSION, "unsupported protocol version"); } } template std::unique_ptr create(std::vector 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)) diff --git a/src/lib/tls/tls_version.cpp b/src/lib/tls/tls_version.cpp index 5ca9c576ad4..fc62d9e71a5 100644 --- a/src/lib/tls/tls_version.cpp +++ b/src/lib/tls/tls_version.cpp @@ -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 || diff --git a/src/lib/tls/tls_version.h b/src/lib/tls/tls_version.h index c581e7858b2..d6b98dd42d3 100644 --- a/src/lib/tls/tls_version.h +++ b/src/lib/tls/tls_version.h @@ -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