Address some issues with PR 492

Adds copyright notices for Juraj Somorovsky and Christian Mainka of Hackmanit
for the changes in 7c7fcecbe6a and 6d327f879c

Add Policy::check_peer_key_acceptable which lets the app set an arbitrary
callback for examining keys - both the end entity signature keys from
certificates and the peer PFS public keys. Default impl checks that the
algorithm size matches the min keylength. This centralizes this logic
and lets the application do interesting things.

Adds a policy for ECDSA group size checks.

Increases default policy minimums to 2048 RSA and 256 ECC.
(Maybe I'm an optimist after all.)

14 files changed, 120 insertions, 58 deletions

diff --git a/src/lib/tls/msg_cert_verify.cpp b/src/lib/tls/msg_cert_verify.cpp
index 2598255eb..6b59e703f 100644
--- a/src/lib/tls/msg_cert_verify.cpp
+++ b/src/lib/tls/msg_cert_verify.cpp
@@ -82,6 +82,8 @@ bool Certificate_Verify::verify(const X509_Certificate& cert,
    {
    std::unique_ptr<Public_Key> key(cert.subject_public_key());
 
+   policy.check_peer_key_acceptable(*key);
+
    std::pair<std::string, Signature_Format> format =
       state.parse_sig_format(*key.get(), m_hash_algo, m_sig_algo,
                              true, policy);
diff --git a/src/lib/tls/msg_certificate.cpp b/src/lib/tls/msg_certificate.cpp
index a83d32d11..10ee7c95f 100644
--- a/src/lib/tls/msg_certificate.cpp
+++ b/src/lib/tls/msg_certificate.cpp
@@ -31,7 +31,7 @@ Certificate::Certificate(Handshake_IO& io,
 /**
 * Deserialize a Certificate message
 */
-Certificate::Certificate(const std::vector<byte>& buf, const Policy &policy)
+Certificate::Certificate(const std::vector<byte>& buf, const Policy& /*policy_currently_unused*/)
    {
    if(buf.size() < 3)
       throw Decoding_Error("Certificate: Message malformed");
@@ -54,35 +54,7 @@ Certificate::Certificate(const std::vector<byte>& buf, const Policy &policy)
          throw Decoding_Error("Certificate: Message malformed");
 
       DataSource_Memory cert_buf(&certs[3], cert_size);
-      X509_Certificate cert(cert_buf);
-      
-	  std::unique_ptr<Public_Key> cert_pub_key(cert.subject_public_key());
-          
-      const std::string algo_name = cert_pub_key->algo_name();
-      const size_t keylength = cert_pub_key->max_input_bits();
-      if(algo_name == "RSA")
-         {
-         const size_t expected_keylength = policy.minimum_rsa_bits();
-         if(keylength < expected_keylength)
-            throw TLS_Exception(Alert::INSUFFICIENT_SECURITY,
-                                "The peer sent RSA certificate of " +
-                                std::to_string(keylength) +
-                                " bits, policy requires at least " +
-                                std::to_string(expected_keylength));
-         }
-      else if(algo_name == "ECDH")
-         {
-         const size_t expected_keylength = policy.minimum_ecdh_group_size();
-         if(keylength < expected_keylength)
-            throw TLS_Exception(Alert::INSUFFICIENT_SECURITY,
-                                "The peer sent ECDH certificate of " +
-                                std::to_string(keylength) +
-                                " bits, policy requires at least " +
-                                std::to_string(expected_keylength));
-          
-         }
-      
-      m_certs.push_back(cert);
+      m_certs.push_back(X509_Certificate(cert_buf));
 
       certs += cert_size + 3;
       }
diff --git a/src/lib/tls/msg_client_kex.cpp b/src/lib/tls/msg_client_kex.cpp
index 192c774b8..bc4d33d52 100644
--- a/src/lib/tls/msg_client_kex.cpp
+++ b/src/lib/tls/msg_client_kex.cpp
@@ -110,6 +110,8 @@ Client_Key_Exchange::Client_Key_Exchange(Handshake_IO& io,
 
          DH_PublicKey counterparty_key(group, Y);
 
+         policy.check_peer_key_acceptable(counterparty_key);
+
          DH_PrivateKey priv_key(rng, group);
 
          PK_Key_Agreement ka(priv_key, "Raw");
@@ -153,6 +155,8 @@ Client_Key_Exchange::Client_Key_Exchange(Handshake_IO& io,
 
          ECDH_PublicKey counterparty_key(group, OS2ECP(ecdh_key, group.get_curve()));
 
+         policy.check_peer_key_acceptable(counterparty_key);
+         
          ECDH_PrivateKey priv_key(rng, group);
 
          PK_Key_Agreement ka(priv_key, "Raw");
diff --git a/src/lib/tls/msg_server_kex.cpp b/src/lib/tls/msg_server_kex.cpp
index 99f0d0f09..10581fe45 100644
--- a/src/lib/tls/msg_server_kex.cpp
+++ b/src/lib/tls/msg_server_kex.cpp
@@ -147,7 +147,6 @@ Server_Key_Exchange::Server_Key_Exchange(Handshake_IO& io,
 Server_Key_Exchange::Server_Key_Exchange(const std::vector<byte>& buf,
                                          const std::string& kex_algo,
                                          const std::string& sig_algo,
-                                         const Policy& policy,
                                          Protocol_Version version)
    {
    TLS_Data_Reader reader("ServerKeyExchange", buf);
@@ -166,18 +165,11 @@ Server_Key_Exchange::Server_Key_Exchange(const std::vector<byte>& buf,
    if(kex_algo == "DH" || kex_algo == "DHE_PSK")
       {
       // 3 bigints, DH p, g, Y
-      std::vector<byte> p = reader.get_range<byte>(2, 1, 65535);
-      reader.get_range<byte>(2, 1, 65535);
-      reader.get_range<byte>(2, 1, 65535);
-      
-      // protection against the LOGJAM attack
-      int key_size = p.size() * 8;
-      if(key_size < policy.minimum_dh_group_size())
-         throw TLS_Exception(Alert::INSUFFICIENT_SECURITY,
-                                "Server sent DH group of " +
-                                std::to_string(key_size) +
-                                " bits, policy requires at least " +
-                                std::to_string(policy.minimum_dh_group_size()));
+
+      for(size_t i = 0; i != 3; ++i)
+         {
+         reader.get_range<byte>(2, 1, 65535);
+         }
       }
    else if(kex_algo == "ECDH" || kex_algo == "ECDHE_PSK")
       {
@@ -244,6 +236,8 @@ bool Server_Key_Exchange::verify(const Public_Key& server_key,
                                  const Handshake_State& state,
                                  const Policy& policy) const
    {
+   policy.check_peer_key_acceptable(server_key);
+
    std::pair<std::string, Signature_Format> format =
       state.parse_sig_format(server_key, m_hash_algo, m_sig_algo,
                              false, policy);
diff --git a/src/lib/tls/tls_ciphersuite.cpp b/src/lib/tls/tls_ciphersuite.cpp
index 1526c1059..0b575a6ca 100644
--- a/src/lib/tls/tls_ciphersuite.cpp
+++ b/src/lib/tls/tls_ciphersuite.cpp
@@ -102,8 +102,9 @@ bool Ciphersuite::ecc_ciphersuite() const
 
 bool Ciphersuite::cbc_ciphersuite() const
    {
-   return (cipher_algo() == "3DES" || cipher_algo() == "AES-128" || cipher_algo() == "AES-256"
-        || cipher_algo() == "Camellia-128" || cipher_algo() == "Camellia-256");
+   return (cipher_algo() == "3DES" || cipher_algo() == "SEED" ||
+           cipher_algo() == "AES-128" || cipher_algo() == "AES-256" ||
+           cipher_algo() == "Camellia-128" || cipher_algo() == "Camellia-256");
    }
 
 namespace {
diff --git a/src/lib/tls/tls_client.cpp b/src/lib/tls/tls_client.cpp
index 13dde99c4..bf7ccdf8c 100644
--- a/src/lib/tls/tls_client.cpp
+++ b/src/lib/tls/tls_client.cpp
@@ -387,7 +387,6 @@ void Client::process_handshake_msg(const Handshake_State* active_state,
          new Server_Key_Exchange(contents,
                                  state.ciphersuite().kex_algo(),
                                  state.ciphersuite().sig_algo(),
-                                 policy(),
                                  state.version())
          );
 
diff --git a/src/lib/tls/tls_extensions.cpp b/src/lib/tls/tls_extensions.cpp
index 49b7355ab..3dceb505a 100644
--- a/src/lib/tls/tls_extensions.cpp
+++ b/src/lib/tls/tls_extensions.cpp
@@ -1,6 +1,7 @@
 /*
 * TLS Extensions
 * (C) 2011,2012,2015,2016 Jack Lloyd
+*     2016 Juraj Somorovsky
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
diff --git a/src/lib/tls/tls_extensions.h b/src/lib/tls/tls_extensions.h
index e273ae096..28c49f084 100644
--- a/src/lib/tls/tls_extensions.h
+++ b/src/lib/tls/tls_extensions.h
@@ -1,6 +1,7 @@
 /*
 * TLS Extensions
 * (C) 2011,2012,2016 Jack Lloyd
+*     2016 Juraj Somorovsky
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
diff --git a/src/lib/tls/tls_messages.h b/src/lib/tls/tls_messages.h
index a5ababc0e..c6a65b658 100644
--- a/src/lib/tls/tls_messages.h
+++ b/src/lib/tls/tls_messages.h
@@ -499,7 +499,6 @@ class Server_Key_Exchange final : public Handshake_Message
       Server_Key_Exchange(const std::vector<byte>& buf,
                           const std::string& kex_alg,
                           const std::string& sig_alg,
-                          const Policy& policy,
                           Protocol_Version version);
 
       ~Server_Key_Exchange();
diff --git a/src/lib/tls/tls_policy.cpp b/src/lib/tls/tls_policy.cpp
index fdc6ba862..592d4f572 100644
--- a/src/lib/tls/tls_policy.cpp
+++ b/src/lib/tls/tls_policy.cpp
@@ -1,6 +1,7 @@
 /*
 * Policies for TLS
 * (C) 2004-2010,2012,2015,2016 Jack Lloyd
+*     2016 Christian Mainka
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -119,23 +120,73 @@ std::string Policy::choose_curve(const std::vector<std::string>& curve_names) co
 
 std::string Policy::dh_group() const
    {
+   // We offer 2048 bit DH because we can
    return "modp/ietf/2048";
    }
 
 size_t Policy::minimum_dh_group_size() const
    {
+   // Many servers still send 1024 bit
    return 1024;
    }
 
-size_t Policy::minimum_ecdh_group_size() const
+size_t Policy::minimum_ecdsa_group_size() const
    {
-   return 159;
+   // Here we are at the mercy of whatever the CA signed, but most certs should be 256 bit by now
+   return 256;
    }
 
+size_t Policy::minimum_ecdh_group_size() const
+   {
+   // P-256 is smallest curve currently supplrted for TLS key exchange (after 1.11.29)
+   return 256;
+   }
 
 size_t Policy::minimum_rsa_bits() const
    {
-   return 1000; // Not 1024, since some certificates use, e.g., only 1023 bits
+   /* Default assumption is all end-entity certificates should
+      be at least 2048 bits these days.
+
+      If you are connecting to arbitrary servers on the Internet
+      (ie as a web browser or SMTP client) you'll probably have to reduce this
+      to 1024 bits, or perhaps even lower.
+   */
+   return 2048;
+   }
+
+void Policy::check_peer_key_acceptable(const Public_Key& public_key) const
+   {
+   const std::string algo_name = public_key.algo_name();
+
+   // FIXME this is not really the right way to do this 
+   size_t keylength = public_key.max_input_bits();
+   size_t expected_keylength = 0;
+
+   if(algo_name == "RSA")
+      {
+      expected_keylength = minimum_rsa_bits();
+      keylength += 1; // fixup for use of max_input_bits above
+      }
+   else if(algo_name == "DH")
+      {
+      expected_keylength = minimum_dh_group_size();
+      }
+   else if(algo_name == "ECDH")
+      {
+      expected_keylength = minimum_ecdh_group_size();
+      }
+   else if(algo_name == "ECDSA")
+      {
+      expected_keylength = minimum_ecdsa_group_size();
+      }
+   // else some other algo, so leave expected_keylength as zero and the check is a no-op
+
+   if(keylength < expected_keylength)
+      throw TLS_Exception(Alert::INSUFFICIENT_SECURITY,
+                          "Peer sent " + 
+                           std::to_string(keylength) + " bit " + algo_name + " key"
+                           ", policy requires at least " +
+                           std::to_string(expected_keylength));
    }
 
 /*
@@ -158,13 +209,13 @@ bool Policy::send_fallback_scsv(Protocol_Version version) const
 
 bool Policy::acceptable_protocol_version(Protocol_Version version) const
    {
-        // Uses boolean optimization:
-        // First check the current version (left part), then if it is allowed 
-        // (right part)
-        // checks are ordered according to their probability
-        return (
-           ( ( version == Protocol_Version::TLS_V10)  && allow_tls10()  ) ||
+   // Uses boolean optimization:
+   // First check the current version (left part), then if it is allowed 
+   // (right part)
+   // checks are ordered according to their probability
+   return (
            ( ( version == Protocol_Version::TLS_V12)  && allow_tls12()  ) ||
+           ( ( version == Protocol_Version::TLS_V10)  && allow_tls10()  ) ||
            ( ( version == Protocol_Version::TLS_V11)  && allow_tls11()  ) ||
            ( ( version == Protocol_Version::DTLS_V12) && allow_dtls12() ) ||
            ( ( version == Protocol_Version::DTLS_V10) && allow_dtls10() )
diff --git a/src/lib/tls/tls_policy.h b/src/lib/tls/tls_policy.h
index 3a09a1747..76e80ddde 100644
--- a/src/lib/tls/tls_policy.h
+++ b/src/lib/tls/tls_policy.h
@@ -131,18 +131,47 @@ class BOTAN_DLL Policy
 
       /**
       * Return the minimum DH group size we're willing to use
+      * Default is currently 1024 (insecure), should be 2048
       */
       virtual size_t minimum_dh_group_size() const;
       
       /**
+      * For ECDSA authenticated ciphersuites, the smallest key size the
+      * client will accept.
+      * This policy is currently only enforced on the server by the client.
+      */
+      virtual size_t minimum_ecdsa_group_size() const;
+      
+      /**
       * Return the minimum ECDH group size we're willing to use
+      * for key exchange
+      *
+      * Default 256, allowing P-256 and larger
+      * P-256 is the smallest curve we will negotiate
       */
       virtual size_t minimum_ecdh_group_size() const;
       
       /**
-      * Return the minimum RSA bit size we're willing to use
+      * Return the minimum bit size we're willing to accept for RSA
+      * key exchange or server signatures.
+      *
+      * It does not place any requirements on the size of any RSA signature(s)
+      * which were used to check the server certificate. This is only
+      * concerned with the server's public key.
+      *
+      * Default is 2048 which is smallest RSA key size still secure
+      * for medium term security.
       */
       virtual size_t minimum_rsa_bits() const;
+
+      /**
+      * Throw an exception if you don't like the peer's key.
+      * Default impl checks the key size against minimum_rsa_bits, minimum_ecdsa_group_size,
+      * or minimum_ecdh_group_size depending on the key's type.
+      * Override if you'd like to perform some other kind of test on
+      * (or logging of) the peer's keys.
+      */
+      virtual void check_peer_key_acceptable(const Public_Key& public_key) const;
       
       /**
       * If this function returns false, unknown SRP/PSK identifiers
@@ -352,6 +381,12 @@ class BOTAN_DLL Text_Policy : public Policy
       std::string dh_group() const override
          { return get_str("dh_group", Policy::dh_group()); }
 
+      size_t minimum_ecdh_group_size() const override
+         { return get_len("minimum_ecdh_group_size", Policy::minimum_ecdh_group_size()); }
+
+      size_t minimum_ecdsa_group_size() const override
+         { return get_len("minimum_ecdsa_group_size", Policy::minimum_ecdsa_group_size()); }
+
       size_t minimum_dh_group_size() const override
          { return get_len("minimum_dh_group_size", Policy::minimum_dh_group_size()); }
 
diff --git a/src/lib/tls/tls_record.cpp b/src/lib/tls/tls_record.cpp
index 16dfecc6a..c273a1546 100644
--- a/src/lib/tls/tls_record.cpp
+++ b/src/lib/tls/tls_record.cpp
@@ -1,6 +1,7 @@
 /*
 * TLS Record Handling
 * (C) 2012,2013,2014,2015 Jack Lloyd
+* (C) 2016 Juraj Somorovsky
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
diff --git a/src/lib/tls/tls_session.h b/src/lib/tls/tls_session.h
index 15e79b811..643b79ac6 100644
--- a/src/lib/tls/tls_session.h
+++ b/src/lib/tls/tls_session.h
@@ -184,7 +184,7 @@ class BOTAN_DLL Session
       const Server_Information& server_info() const { return m_server_info; }
 
    private:
-      enum { TLS_SESSION_PARAM_STRUCT_VERSION = 20160103 };
+      enum { TLS_SESSION_PARAM_STRUCT_VERSION = 20160812};
 
       std::chrono::system_clock::time_point m_start_time;
 
diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp
index 91eca0d5a..1dbc20273 100644
--- a/src/tests/unit_tls.cpp
+++ b/src/tests/unit_tls.cpp
@@ -666,6 +666,8 @@ class Test_Policy : public Botan::TLS::Text_Policy
 
       size_t dtls_initial_timeout() const override { return 1; }
       size_t dtls_maximum_timeout() const override { return 8; }
+
+      size_t minimum_rsa_bits() const { return 1024; }
    };