diff options
| -rw-r--r-- | src/lib/tls/msg_client_hello.cpp | 40 | ||||
| -rw-r--r-- | src/lib/tls/tls_extensions.cpp | 21 | ||||
| -rw-r--r-- | src/lib/tls/tls_extensions.h | 9 | ||||
| -rw-r--r-- | src/lib/tls/tls_messages.h | 14 | ||||
| -rw-r--r-- | src/lib/tls/tls_server.cpp | 36 |
5 files changed, 89 insertions, 31 deletions
diff --git a/src/lib/tls/msg_client_hello.cpp b/src/lib/tls/msg_client_hello.cpp index 69f9a5e11..56e226a40 100644 --- a/src/lib/tls/msg_client_hello.cpp +++ b/src/lib/tls/msg_client_hello.cpp @@ -80,25 +80,27 @@ Client_Hello::Client_Hello(Handshake_IO& io, client_settings.srp_identifier() != "")), m_comp_methods(policy.compression()) { + BOTAN_ASSERT(policy.acceptable_protocol_version(client_settings.protocol_version()), + "Our policy accepts the version we are offering"); + + /* + * Place all empty extensions in front to avoid a bug in some sytems + * which reject hellos when the last extension in the list is empty. + */ m_extensions.add(new Extended_Master_Secret); + m_extensions.add(new Session_Ticket()); + if(policy.negotiate_encrypt_then_mac()) + m_extensions.add(new Encrypt_then_MAC); + m_extensions.add(new Renegotiation_Extension(reneg_info)); m_extensions.add(new Server_Name_Indicator(client_settings.hostname())); - m_extensions.add(new Session_Ticket()); - m_extensions.add(new Supported_Elliptic_Curves(policy.allowed_ecc_curves())); - if(m_version.supports_negotiable_signature_algorithms()) - m_extensions.add(new Signature_Algorithms(policy.allowed_signature_hashes(), - policy.allowed_signature_methods())); + if(reneg_info.empty() && !next_protocols.empty()) + m_extensions.add(new Application_Layer_Protocol_Notification(next_protocols)); if(m_version.is_datagram_protocol()) m_extensions.add(new SRTP_Protection_Profiles(policy.srtp_profiles())); - if(reneg_info.empty() && !next_protocols.empty()) - m_extensions.add(new Application_Layer_Protocol_Notification(next_protocols)); - - if(policy.negotiate_encrypt_then_mac()) - m_extensions.add(new Encrypt_then_MAC); - #if defined(BOTAN_HAS_SRP6) m_extensions.add(new SRP_Identifier(client_settings.srp_identifier())); #else @@ -108,8 +110,11 @@ Client_Hello::Client_Hello(Handshake_IO& io, } #endif - BOTAN_ASSERT(policy.acceptable_protocol_version(client_settings.protocol_version()), - "Our policy accepts the version we are offering"); + m_extensions.add(new Supported_Elliptic_Curves(policy.allowed_ecc_curves())); + + if(m_version.supports_negotiable_signature_algorithms()) + m_extensions.add(new Signature_Algorithms(policy.allowed_signature_hashes(), + policy.allowed_signature_methods())); if(policy.send_fallback_scsv(client_settings.protocol_version())) m_suites.push_back(TLS_FALLBACK_SCSV); @@ -256,6 +261,15 @@ Client_Hello::Client_Hello(const std::vector<byte>& buf) m_extensions.add(new Renegotiation_Extension()); } } + + // Parsing complete, now any additional decoding checks + + if(m_version.supports_negotiable_signature_algorithms() == false) + { + if(m_extensions.has<Signature_Algorithms>()) + throw TLS_Exception(Alert::HANDSHAKE_FAILURE, + "Client sent signature_algorithms extension in version that doesn't support it"); + } } bool Client_Hello::sent_fallback_scsv() const diff --git a/src/lib/tls/tls_extensions.cpp b/src/lib/tls/tls_extensions.cpp index 3dceb505a..e38e4ccdc 100644 --- a/src/lib/tls/tls_extensions.cpp +++ b/src/lib/tls/tls_extensions.cpp @@ -463,16 +463,27 @@ Signature_Algorithms::Signature_Algorithms(TLS_Data_Reader& reader, while(len) { - const std::string hash_code = hash_algo_name(reader.get_byte()); - const std::string sig_code = sig_algo_name(reader.get_byte()); - + const byte hash_code = reader.get_byte(); + const byte sig_code = reader.get_byte(); len -= 2; + if(sig_code == 0) + { + /* + RFC 5247 7.4.1.4.1 explicitly prohibits anonymous (0) signature code in + the client hello. ("It MUST NOT appear in this extension.") + */ + throw TLS_Exception(Alert::DECODE_ERROR, "Client sent ANON signature"); + } + + const std::string hash_name = hash_algo_name(hash_code); + const std::string sig_name = sig_algo_name(sig_code); + // If not something we know, ignore it completely - if(hash_code.empty() || sig_code.empty()) + if(hash_name.empty() || sig_name.empty()) continue; - m_supported_algos.push_back(std::make_pair(hash_code, sig_code)); + m_supported_algos.push_back(std::make_pair(hash_name, sig_name)); } } diff --git a/src/lib/tls/tls_extensions.h b/src/lib/tls/tls_extensions.h index dc69eec36..4bd564a85 100644 --- a/src/lib/tls/tls_extensions.h +++ b/src/lib/tls/tls_extensions.h @@ -274,8 +274,9 @@ class Signature_Algorithms final : public Extension static std::string sig_algo_name(byte code); static byte sig_algo_code(const std::string& name); - std::vector<std::pair<std::string, std::string> > - supported_signature_algorthms() const + // [(hash,sig),(hash,sig),...] + const std::vector<std::pair<std::string, std::string>>& + supported_signature_algorthms() const { return m_supported_algos; } @@ -287,13 +288,13 @@ class Signature_Algorithms final : public Extension Signature_Algorithms(const std::vector<std::string>& hashes, const std::vector<std::string>& sig_algos); - explicit Signature_Algorithms(const std::vector<std::pair<std::string, std::string> >& algos) : + explicit Signature_Algorithms(const std::vector<std::pair<std::string, std::string>>& algos) : m_supported_algos(algos) {} Signature_Algorithms(TLS_Data_Reader& reader, u16bit extension_size); private: - std::vector<std::pair<std::string, std::string> > m_supported_algos; + std::vector<std::pair<std::string, std::string>> m_supported_algos; }; /** diff --git a/src/lib/tls/tls_messages.h b/src/lib/tls/tls_messages.h index 8ccb2fbff..cf35053f2 100644 --- a/src/lib/tls/tls_messages.h +++ b/src/lib/tls/tls_messages.h @@ -19,6 +19,7 @@ #include <botan/x509cert.h> #include <vector> #include <string> +#include <set> namespace Botan { @@ -105,6 +106,14 @@ class Client_Hello final : public Handshake_Message return std::vector<std::pair<std::string, std::string>>(); } + std::set<std::string> supported_sig_algos() const + { + std::set<std::string> sig; + for(auto&& hash_and_sig : supported_algos()) + sig.insert(hash_and_sig.second); + return sig; + } + std::vector<std::string> supported_ecc_curves() const { if(Supported_Elliptic_Curves* ecc = m_extensions.get<Supported_Elliptic_Curves>()) @@ -167,6 +176,11 @@ class Client_Hello final : public Handshake_Message return m_extensions.has<Encrypt_then_MAC>(); } + bool sent_signature_algorithms() const + { + return m_extensions.has<Signature_Algorithms>(); + } + std::vector<std::string> next_protocols() const { if(auto alpn = m_extensions.get<Application_Layer_Protocol_Notification>()) diff --git a/src/lib/tls/tls_server.cpp b/src/lib/tls/tls_server.cpp index 40aa18d27..1676ef659 100644 --- a/src/lib/tls/tls_server.cpp +++ b/src/lib/tls/tls_server.cpp @@ -154,11 +154,11 @@ u16bit choose_ciphersuite( Protocol_Version version, Credentials_Manager& creds, const std::map<std::string, std::vector<X509_Certificate> >& cert_chains, - const Client_Hello* client_hello) + const Client_Hello& client_hello) { const bool our_choice = policy.server_uses_own_ciphersuite_preferences(); - const bool have_srp = creds.attempt_srp("tls-server", client_hello->sni_hostname()); - const std::vector<u16bit> client_suites = client_hello->ciphersuites(); + const bool have_srp = creds.attempt_srp("tls-server", client_hello.sni_hostname()); + const std::vector<u16bit> client_suites = client_hello.ciphersuites(); const std::vector<u16bit> server_suites = policy.ciphersuite_list(version, have_srp); if(server_suites.empty()) @@ -166,7 +166,11 @@ u16bit choose_ciphersuite( "Policy forbids us from negotiating any ciphersuite"); const bool have_shared_ecc_curve = - (policy.choose_curve(client_hello->supported_ecc_curves()) != ""); + (policy.choose_curve(client_hello.supported_ecc_curves()) != ""); + + /* + Walk down one list in preference order + */ std::vector<u16bit> pref_list = server_suites; std::vector<u16bit> other_list = client_suites; @@ -174,19 +178,33 @@ u16bit choose_ciphersuite( if(!our_choice) std::swap(pref_list, other_list); + const std::set<std::string> client_sig_algos = client_hello.supported_sig_algos(); + for(auto suite_id : pref_list) { if(!value_exists(other_list, suite_id)) continue; - Ciphersuite suite = Ciphersuite::by_id(suite_id); + const Ciphersuite suite = Ciphersuite::by_id(suite_id); - if(!have_shared_ecc_curve && suite.ecc_ciphersuite()) + if(suite.valid() == false) continue; - if(suite.sig_algo() != "" && cert_chains.count(suite.sig_algo()) == 0) + if(suite.ecc_ciphersuite() && have_shared_ecc_curve == false) continue; + // For non-anon ciphersuites + if(suite.sig_algo() != "") + { + // Do we have any certificates for this sig? + if(cert_chains.count(suite.sig_algo()) == 0) + continue; + + // Client reques + if(!client_sig_algos.empty() && client_sig_algos.count(suite.sig_algo()) == 0) + continue; + } + #if defined(BOTAN_HAS_SRP6) /* The client may offer SRP cipher suites in the hello message but @@ -196,7 +214,7 @@ u16bit choose_ciphersuite( client hello message. - RFC 5054 section 2.5.1.2 */ - if(suite.kex_algo() == "SRP_SHA" && client_hello->srp_identifier() == "") + if(suite.kex_algo() == "SRP_SHA" && client_hello.srp_identifier() == "") throw TLS_Exception(Alert::UNKNOWN_PSK_IDENTITY, "Client wanted SRP but did not send username"); #endif @@ -747,7 +765,7 @@ void Server::session_create(Server_Handshake_State& pending_state, pending_state.version(), m_creds, cert_chains, - pending_state.client_hello()), + *pending_state.client_hello()), choose_compression(policy(), pending_state.client_hello()->compression_methods()), have_session_ticket_key); |