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.

9 files changed, 106 insertions, 38 deletions

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<byte> 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<Public_Key> key(cert.subject_public_key());
 
    std::pair<std::string, Signature_Format> 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<byte> 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<std::string, Signature_Format> 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<std::pair<std::string, std::string>>& 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<std::string, Signature_Format>
-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<byte> session_ticket() const;
 
       std::pair<std::string, Signature_Format>
-         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<std::string, Signature_Format>
          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<byte>& 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<std::string> 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<std::string> 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<std::string> 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));