From 2f7225c5f56feab172978a0182ac27c20b02c080 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Wed, 6 Jan 2016 19:36:07 -0500 Subject: Check that TLS signature type is accepted by the policy. Previously the signature hashes and algos info was used to set the v1.2 signature_algorithms extension, but if the counterparty ignored the extension and sent something else, we wouldn't notice. --- src/lib/tls/msg_cert_verify.cpp | 6 +- src/lib/tls/msg_server_kex.cpp | 6 +- src/lib/tls/tls_client.cpp | 2 +- src/lib/tls/tls_handshake_state.cpp | 107 +++++++++++++++++++++++++++--------- src/lib/tls/tls_handshake_state.h | 8 ++- src/lib/tls/tls_messages.h | 6 +- src/lib/tls/tls_policy.cpp | 5 ++ src/lib/tls/tls_policy.h | 2 + src/lib/tls/tls_server.cpp | 2 +- 9 files changed, 106 insertions(+), 38 deletions(-) (limited to 'src/lib') diff --git a/src/lib/tls/msg_cert_verify.cpp b/src/lib/tls/msg_cert_verify.cpp index be6c8a069..0d157dc57 100644 --- a/src/lib/tls/msg_cert_verify.cpp +++ b/src/lib/tls/msg_cert_verify.cpp @@ -77,12 +77,14 @@ std::vector Certificate_Verify::serialize() const * Verify a Certificate Verify message */ bool Certificate_Verify::verify(const X509_Certificate& cert, - const Handshake_State& state) const + const Handshake_State& state, + const Policy& policy) const { std::unique_ptr key(cert.subject_public_key()); std::pair format = - state.understand_sig_format(*key.get(), m_hash_algo, m_sig_algo); + state.parse_sig_format(*key.get(), m_hash_algo, m_sig_algo, + true, policy); PK_Verifier verifier(*key, format.first, format.second); diff --git a/src/lib/tls/msg_server_kex.cpp b/src/lib/tls/msg_server_kex.cpp index 7389cb35b..98e3ad1f0 100644 --- a/src/lib/tls/msg_server_kex.cpp +++ b/src/lib/tls/msg_server_kex.cpp @@ -233,10 +233,12 @@ std::vector Server_Key_Exchange::serialize() const * Verify a Server Key Exchange message */ bool Server_Key_Exchange::verify(const Public_Key& server_key, - const Handshake_State& state) const + const Handshake_State& state, + const Policy& policy) const { std::pair format = - state.understand_sig_format(server_key, m_hash_algo, m_sig_algo); + state.parse_sig_format(server_key, m_hash_algo, m_sig_algo, + false, policy); PK_Verifier verifier(server_key, format.first, format.second); diff --git a/src/lib/tls/tls_client.cpp b/src/lib/tls/tls_client.cpp index 3a219ccff..301c77c6b 100644 --- a/src/lib/tls/tls_client.cpp +++ b/src/lib/tls/tls_client.cpp @@ -394,7 +394,7 @@ void Client::process_handshake_msg(const Handshake_State* active_state, { const Public_Key& server_key = state.get_server_public_Key(); - if(!state.server_kex()->verify(server_key, state)) + if(!state.server_kex()->verify(server_key, state, policy())) { throw TLS_Exception(Alert::DECRYPT_ERROR, "Bad signature on server key exchange"); diff --git a/src/lib/tls/tls_handshake_state.cpp b/src/lib/tls/tls_handshake_state.cpp index 3799c9e7d..67ba43265 100644 --- a/src/lib/tls/tls_handshake_state.cpp +++ b/src/lib/tls/tls_handshake_state.cpp @@ -1,6 +1,6 @@ /* * TLS Handshaking -* (C) 2004-2006,2011,2012,2015 Jack Lloyd +* (C) 2004-2006,2011,2012,2015,2016 Jack Lloyd * * Botan is released under the Simplified BSD License (see license.txt) */ @@ -449,58 +449,111 @@ Handshake_State::choose_sig_format(const Private_Key& key, throw Invalid_Argument(sig_algo + " is invalid/unknown for TLS signatures"); } +namespace { + +bool supported_algos_include( + const std::vector>& algos, + const std::string& key_type, + const std::string& hash_type) + { + for(auto&& algo : algos) + { + if(algo.first == hash_type && algo.second == key_type) + { + return true; + } + } + + return false; + } + +} + std::pair -Handshake_State::understand_sig_format(const Public_Key& key, - std::string hash_algo, - std::string sig_algo) const +Handshake_State::parse_sig_format(const Public_Key& key, + const std::string& input_hash_algo, + const std::string& input_sig_algo, + bool for_client_auth, + const Policy& policy) const { - const std::string algo_name = key.algo_name(); + const std::string key_type = key.algo_name(); - /* - FIXME: This should check what was sent against the client hello - preferences, or the certificate request, to ensure it was allowed - by those restrictions. + if(!policy.allowed_signature_method(key_type)) + { + throw TLS_Exception(Alert::HANDSHAKE_FAILURE, + "Rejecting " + key_type + " signature"); + } - Or not? - */ + std::string hash_algo; if(this->version().supports_negotiable_signature_algorithms()) { - if(hash_algo.empty()) + if(input_sig_algo != key_type) + throw Decoding_Error("Counterparty sent inconsistent key and sig types"); + + if(input_hash_algo == "") throw Decoding_Error("Counterparty did not send hash/sig IDS"); - if(sig_algo != algo_name) - throw Decoding_Error("Counterparty sent inconsistent key and sig types"); + hash_algo = input_hash_algo; + + if(for_client_auth && !cert_req()) + { + throw TLS_Exception(Alert::HANDSHAKE_FAILURE, + "No certificate verify set"); + } + + /* + Confirm the signature type we just received against the + supported_algos list that we sent; it better be there. + */ + + const auto supported_algos = + for_client_auth ? cert_req()->supported_algos() : + client_hello()->supported_algos(); + + if(!supported_algos_include(supported_algos, key_type, hash_algo)) + { + throw TLS_Exception(Alert::HANDSHAKE_FAILURE, + "TLS signature extension did not allow for " + + key_type + "/" + hash_algo + " signature"); + } } else { - if(!hash_algo.empty() || !sig_algo.empty()) + if(input_hash_algo != "" || input_sig_algo != "") throw Decoding_Error("Counterparty sent hash/sig IDs with old version"); - } - if(algo_name == "RSA") - { - if(!this->version().supports_negotiable_signature_algorithms()) + if(key_type == "RSA") { hash_algo = "Parallel(MD5,SHA-160)"; } + else if(key_type == "DSA" || key_type == "ECDSA") + { + hash_algo = "SHA-1"; + } + else + { + throw Invalid_Argument(key_type + " is invalid/unknown for TLS signatures"); + } + + /* + There is no check on the acceptability of a v1.0/v1.1 hash type, + since it's implicit with use of the protocol + */ + } + if(key_type == "RSA") + { const std::string padding = "EMSA3(" + hash_algo + ")"; return std::make_pair(padding, IEEE_1363); } - else if(algo_name == "DSA" || algo_name == "ECDSA") + else if(key_type == "DSA" || key_type == "ECDSA") { - if(!this->version().supports_negotiable_signature_algorithms()) - { - hash_algo = "SHA-1"; - } - const std::string padding = "EMSA1(" + hash_algo + ")"; - return std::make_pair(padding, DER_SEQUENCE); } - throw Invalid_Argument(algo_name + " is invalid/unknown for TLS signatures"); + throw Invalid_Argument(key_type + " is invalid/unknown for TLS signatures"); } } diff --git a/src/lib/tls/tls_handshake_state.h b/src/lib/tls/tls_handshake_state.h index 6260b090f..2943a8637 100644 --- a/src/lib/tls/tls_handshake_state.h +++ b/src/lib/tls/tls_handshake_state.h @@ -80,9 +80,11 @@ class Handshake_State std::vector session_ticket() const; std::pair - understand_sig_format(const Public_Key& key, - std::string hash_algo, - std::string sig_algo) const; + parse_sig_format(const Public_Key& key, + const std::string& hash_algo, + const std::string& sig_algo, + bool for_client_auth, + const Policy& policy) const; std::pair choose_sig_format(const Private_Key& key, diff --git a/src/lib/tls/tls_messages.h b/src/lib/tls/tls_messages.h index 00033826f..3bee89e13 100644 --- a/src/lib/tls/tls_messages.h +++ b/src/lib/tls/tls_messages.h @@ -395,7 +395,8 @@ class Certificate_Verify final : public Handshake_Message * @param state the handshake state */ bool verify(const X509_Certificate& cert, - const Handshake_State& state) const; + const Handshake_State& state, + const Policy& policy) const; Certificate_Verify(Handshake_IO& io, Handshake_State& state, @@ -463,7 +464,8 @@ class Server_Key_Exchange final : public Handshake_Message const std::vector& params() const { return m_params; } bool verify(const Public_Key& server_key, - const Handshake_State& state) const; + const Handshake_State& state, + const Policy& policy) const; // Only valid for certain kex types const Private_Key& server_kex_key() const; diff --git a/src/lib/tls/tls_policy.cpp b/src/lib/tls/tls_policy.cpp index 3100db50d..be4c61b16 100644 --- a/src/lib/tls/tls_policy.cpp +++ b/src/lib/tls/tls_policy.cpp @@ -84,6 +84,11 @@ std::vector Policy::allowed_signature_methods() const }; } +bool Policy::allowed_signature_method(const std::string& sig_method) const + { + return value_exists(allowed_signature_methods(), sig_method); + } + std::vector Policy::allowed_ecc_curves() const { return { diff --git a/src/lib/tls/tls_policy.h b/src/lib/tls/tls_policy.h index 6e4b442c8..67388b115 100644 --- a/src/lib/tls/tls_policy.h +++ b/src/lib/tls/tls_policy.h @@ -57,6 +57,8 @@ class BOTAN_DLL Policy */ virtual std::vector allowed_signature_methods() const; + bool allowed_signature_method(const std::string& sig_method) const; + /** * Return list of ECC curves we are willing to use in order of preference */ diff --git a/src/lib/tls/tls_server.cpp b/src/lib/tls/tls_server.cpp index 5ababe621..41b14ae08 100644 --- a/src/lib/tls/tls_server.cpp +++ b/src/lib/tls/tls_server.cpp @@ -618,7 +618,7 @@ void Server::process_handshake_msg(const Handshake_State* active_state, state.client_certs()->cert_chain(); const bool sig_valid = - state.client_verify()->verify(client_certs[0], state); + state.client_verify()->verify(client_certs[0], state, policy()); state.hash().update(state.handshake_io().format(contents, type)); -- cgit v1.2.3