Add TLS::Policy::minimum_dh_group_size, default 1024. Send an

insufficient_security alert if the server tries to give us a DH group
smaller than that. Also check to make sure the key isn't obviously
bogus (<=1 || >= p-1), though as the key is purely ephemeral it
doesn't seem like a small subgroup attack would provide much advantage
anyway.

5 files changed, 30 insertions, 2 deletions

diff --git a/src/tls/c_kex.cpp b/src/tls/c_kex.cpp
index a173b18ad..54c5af5c3 100644
--- a/src/tls/c_kex.cpp
+++ b/src/tls/c_kex.cpp
@@ -49,6 +49,7 @@ secure_vector<byte> strip_leading_zeros(const secure_vector<byte>& input)
 */
 Client_Key_Exchange::Client_Key_Exchange(Record_Writer& writer,
                                          Handshake_State* state,
+                                         const Policy& policy,
                                          Credentials_Manager& creds,
                                          const std::vector<X509_Certificate>& peer_certs,
                                          const std::string& hostname,
@@ -111,6 +112,23 @@ Client_Key_Exchange::Client_Key_Exchange(Record_Writer& writer,
          if(reader.remaining_bytes())
             throw Decoding_Error("Bad params size for DH key exchange");
 
+         if(p.bits() < policy.minimum_dh_group_size())
+            throw TLS_Exception(Alert::INSUFFICIENT_SECURITY,
+                                "Server sent DH group of " +
+                                std::to_string(p.bits()) +
+                                " bits, policy requires at least " +
+                                std::to_string(policy.minimum_dh_group_size()));
+
+         /*
+         * A basic check for key validity. As we do not know q here we
+         * cannot check that Y is in the right subgroup. However since
+         * our key is ephemeral there does not seem to be any
+         * advantage to bogus keys anyway.
+         */
+         if(Y <= 1 || Y >= p - 1)
+            throw TLS_Exception(Alert::INSUFFICIENT_SECURITY,
+                                "Server sent bad DH key for DHE exchange");
+
          DL_Group group(p, g);
 
          if(!group.verify_group(rng, true))
@@ -118,8 +136,6 @@ Client_Key_Exchange::Client_Key_Exchange(Record_Writer& writer,
 
          DH_PublicKey counterparty_key(group, Y);
 
-         // FIXME Check that public key is residue?
-
          DH_PrivateKey priv_key(rng, group);
 
          PK_Key_Agreement ka(priv_key, "Raw");
diff --git a/src/tls/tls_client.cpp b/src/tls/tls_client.cpp
index 0231b9c53..7dc0c224e 100644
--- a/src/tls/tls_client.cpp
+++ b/src/tls/tls_client.cpp
@@ -368,6 +368,7 @@ void Client::process_handshake_msg(Handshake_Type type,
       m_state->client_kex =
          new Client_Key_Exchange(m_writer,
                                  m_state,
+                                 m_policy,
                                  m_creds,
                                  m_peer_certs,
                                  m_hostname,
diff --git a/src/tls/tls_messages.h b/src/tls/tls_messages.h
index 0bca2f44c..2e8bf9ba3 100644
--- a/src/tls/tls_messages.h
+++ b/src/tls/tls_messages.h
@@ -246,6 +246,7 @@ class Client_Key_Exchange : public Handshake_Message
 
       Client_Key_Exchange(Record_Writer& output,
                           Handshake_State* state,
+                          const Policy& policy,
                           Credentials_Manager& creds,
                           const std::vector<X509_Certificate>& peer_certs,
                           const std::string& hostname,
diff --git a/src/tls/tls_policy.cpp b/src/tls/tls_policy.cpp
index 8dd56a7ff..87f8b5a14 100644
--- a/src/tls/tls_policy.cpp
+++ b/src/tls/tls_policy.cpp
@@ -100,6 +100,11 @@ DL_Group Policy::dh_group() const
    return DL_Group("modp/ietf/2048");
    }
 
+size_t Policy::minimum_dh_group_size() const
+   {
+   return 1024;
+   }
+
 /*
 * Return allowed compression algorithms
 */
diff --git a/src/tls/tls_policy.h b/src/tls/tls_policy.h
index 7678ecfd1..c47c45605 100644
--- a/src/tls/tls_policy.h
+++ b/src/tls/tls_policy.h
@@ -85,6 +85,11 @@ class BOTAN_DLL Policy
       virtual DL_Group dh_group() const;
 
       /**
+      * Return the minimum DH group size we're willing to use
+      */
+      virtual size_t minimum_dh_group_size() const;
+
+      /**
       * If this function returns false, unknown SRP/PSK identifiers
       * will be rejected with an unknown_psk_identifier alert as soon
       * as the non-existence is identified. Otherwise, a false