Merge GH #673 X25519 TLS key exchange

9 files changed, 113 insertions, 44 deletions

diff --git a/doc/manual/tls.rst b/doc/manual/tls.rst
index 7ef61e63d..20546eb0b 100644
--- a/doc/manual/tls.rst
+++ b/doc/manual/tls.rst
@@ -590,7 +590,7 @@ policy settings from a file.
      Return a list of ECC curves we are willing to use, in order of preference.
 
      Default: "brainpool512r1", "secp521r1", "brainpool384r1",
-     "secp384r1", "brainpool256r1", "secp256r1"
+     "secp384r1", "brainpool256r1", "secp256r1", "x25519"
 
      No other values are currently defined.
 
diff --git a/doc/todo.rst b/doc/todo.rst
index 6420c1937..38b19e1f0 100644
--- a/doc/todo.rst
+++ b/doc/todo.rst
@@ -60,7 +60,6 @@ TLS
 * Make DTLS support optional at build time
 * Make TLS v1.0 and v1.1 optional at build time
 * Make finite field DH optional at build time
-* Curve25519 key exchange
 * NEWHOPE (CECPQ1) key exchange (GH #613)
 * TLS OCSP stapling (RFC 6066)
 * Authentication using TOFU (sqlite3 storage)
diff --git a/src/lib/pubkey/curve25519/curve25519.cpp b/src/lib/pubkey/curve25519/curve25519.cpp
index 02ee516de..4a072e648 100644
--- a/src/lib/pubkey/curve25519/curve25519.cpp
+++ b/src/lib/pubkey/curve25519/curve25519.cpp
@@ -29,10 +29,13 @@ secure_vector<byte> curve25519(const secure_vector<byte>& secret,
    return out;
    }
 
-secure_vector<byte> curve25519_basepoint(const secure_vector<byte>& secret)
+std::vector<byte> curve25519_basepoint(const secure_vector<byte>& secret)
    {
    const byte basepoint[32] = { 9 };
-   return curve25519(secret, basepoint);
+   std::vector<byte> out(32);
+   const int rc = curve25519_donna(out.data(), secret.data(), basepoint);
+   BOTAN_ASSERT_EQUAL(rc, 0, "Return value of curve25519_donna is ok");
+   return out;
    }
 
 }
diff --git a/src/lib/pubkey/curve25519/curve25519.h b/src/lib/pubkey/curve25519/curve25519.h
index 476db80d1..03d274e0b 100644
--- a/src/lib/pubkey/curve25519/curve25519.h
+++ b/src/lib/pubkey/curve25519/curve25519.h
@@ -27,7 +27,7 @@ class BOTAN_DLL Curve25519_PublicKey : public virtual Public_Key
 
       std::vector<byte> x509_subject_public_key() const override;
 
-      std::vector<byte> public_value() const { return unlock(m_public); }
+      std::vector<byte> public_value() const { return m_public; }
 
       /**
       * Create a Curve25519 Public Key.
@@ -39,13 +39,20 @@ class BOTAN_DLL Curve25519_PublicKey : public virtual Public_Key
 
       /**
       * Create a Curve25519 Public Key.
-      * @param pub DER encoded public key bits
+      * @param pub 32-byte raw public key
       */
-      explicit Curve25519_PublicKey(const secure_vector<byte>& pub) : m_public(pub) {}
+      explicit Curve25519_PublicKey(const std::vector<byte>& pub) : m_public(pub) {}
+
+      /**
+      * Create a Curve25519 Public Key.
+      * @param pub 32-byte raw public key
+      */
+      explicit Curve25519_PublicKey(const secure_vector<byte>& pub) :
+         m_public(pub.begin(), pub.end()) {}
 
    protected:
       Curve25519_PublicKey() {}
-      secure_vector<byte> m_public;
+      std::vector<byte> m_public;
    };
 
 class BOTAN_DLL Curve25519_PrivateKey : public Curve25519_PublicKey,
diff --git a/src/lib/tls/msg_client_kex.cpp b/src/lib/tls/msg_client_kex.cpp
index 02ebcc2c8..81b01a704 100644
--- a/src/lib/tls/msg_client_kex.cpp
+++ b/src/lib/tls/msg_client_kex.cpp
@@ -20,8 +20,12 @@
 #include <botan/ecdh.h>
 #include <botan/rsa.h>
 
+#if defined(BOTAN_HAS_CURVE_25519)
+  #include <botan/curve25519.h>
+#endif
+
 #if defined(BOTAN_HAS_SRP6)
-#include <botan/srp6.h>
+  #include <botan/srp6.h>
 #endif
 
 namespace Botan {
@@ -138,31 +142,53 @@ Client_Key_Exchange::Client_Key_Exchange(Handshake_IO& io,
 
          const u16bit curve_id = reader.get_u16bit();
 
-         const std::string name = Supported_Elliptic_Curves::curve_id_to_name(curve_id);
+         const std::string curve_name = Supported_Elliptic_Curves::curve_id_to_name(curve_id);
 
-         if(name == "")
+         if(curve_name == "")
             throw Decoding_Error("Server sent unknown named curve " + std::to_string(curve_id));
 
-         if(!policy.allowed_ecc_curve(name))
+         if(!policy.allowed_ecc_curve(curve_name))
             {
             throw TLS_Exception(Alert::HANDSHAKE_FAILURE,
                                 "Server sent ECC curve prohibited by policy");
             }
 
-         EC_Group group(name);
-
-         std::vector<byte> ecdh_key = reader.get_range<byte>(1, 1, 255);
+         const std::vector<byte> ecdh_key = reader.get_range<byte>(1, 1, 255);
 
-         ECDH_PublicKey counterparty_key(group, OS2ECP(ecdh_key, group.get_curve()));
+         std::vector<byte> our_ecdh_public;
+         secure_vector<byte> ecdh_secret;
 
-         policy.check_peer_key_acceptable(counterparty_key);
-         
-         ECDH_PrivateKey priv_key(rng, group);
-
-         PK_Key_Agreement ka(priv_key, rng, "Raw");
+         if(curve_name == "x25519")
+            {
+#if defined(BOTAN_HAS_CURVE_25519)
+            if(ecdh_key.size() != 32)
+               throw TLS_Exception(Alert::HANDSHAKE_FAILURE, "Invalid X25519 key size");
+
+            Curve25519_PublicKey counterparty_key(ecdh_key);
+            policy.check_peer_key_acceptable(counterparty_key);
+            Curve25519_PrivateKey priv_key(rng);
+            PK_Key_Agreement ka(priv_key, rng, "Raw");
+            ecdh_secret = ka.derive_key(0, counterparty_key.public_value()).bits_of();
+#else
+            throw Internal_Error("Negotiated X25519 somehow, but it is disabled");
+#endif
 
-         secure_vector<byte> ecdh_secret =
-            ka.derive_key(0, counterparty_key.public_value()).bits_of();
+            // X25519 is always compressed but sent as "uncompressed" in TLS
+            our_ecdh_public = priv_key.public_value();
+            }
+         else
+            {
+            EC_Group group(curve_name);
+            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, rng, "Raw");
+            ecdh_secret = ka.derive_key(0, counterparty_key.public_value()).bits_of();
+
+            // follow server's preference for point compression
+            our_ecdh_public = priv_key.public_value(
+               state.server_hello()->prefers_compressed_ec_points() ? PointGFp::COMPRESSED : PointGFp::UNCOMPRESSED);
+            }
 
          if(kex_algo == "ECDH")
             m_pre_master = ecdh_secret;
@@ -172,10 +198,7 @@ Client_Key_Exchange::Client_Key_Exchange(Handshake_IO& io,
             append_tls_length_value(m_pre_master, psk.bits_of(), 2);
             }
 
-         // follow server's preference for point compression
-         append_tls_length_value(m_key_material,
-               priv_key.public_value(state.server_hello()->prefers_compressed_ec_points() ?
-               PointGFp::COMPRESSED : PointGFp::UNCOMPRESSED ), 1);
+         append_tls_length_value(m_key_material, our_ecdh_public, 1);
          }
 #if defined(BOTAN_HAS_SRP6)
       else if(kex_algo == "SRP_SHA")
diff --git a/src/lib/tls/msg_server_kex.cpp b/src/lib/tls/msg_server_kex.cpp
index 325e5d1b0..4515ea450 100644
--- a/src/lib/tls/msg_server_kex.cpp
+++ b/src/lib/tls/msg_server_kex.cpp
@@ -17,8 +17,12 @@
 #include <botan/dh.h>
 #include <botan/ecdh.h>
 
+#if defined(BOTAN_HAS_CURVE_25519)
+  #include <botan/curve25519.h>
+#endif
+
 #if defined(BOTAN_HAS_SRP6)
-#include <botan/srp6.h>
+  #include <botan/srp6.h>
 #endif
 
 namespace Botan {
@@ -69,28 +73,40 @@ Server_Key_Exchange::Server_Key_Exchange(Handshake_IO& io,
          throw TLS_Exception(Alert::HANDSHAKE_FAILURE,
                              "Could not agree on an ECC curve with the client");
 
-      EC_Group ec_group(curve_name);
+      const uint16_t named_curve_id = Supported_Elliptic_Curves::name_to_curve_id(curve_name);
+      if(named_curve_id == 0)
+         throw Internal_Error("TLS does not support ECC with " + curve_name);
 
-      std::unique_ptr<ECDH_PrivateKey> ecdh(new ECDH_PrivateKey(rng, ec_group));
+      std::vector<byte> ecdh_public_val;
 
-      const std::string ecdh_domain_oid = ecdh->domain().get_oid();
-      const std::string domain = OIDS::lookup(OID(ecdh_domain_oid));
+      if(curve_name == "x25519")
+         {
+#if defined(BOTAN_HAS_CURVE_25519)
+         std::unique_ptr<Curve25519_PrivateKey> x25519(new Curve25519_PrivateKey(rng));
+         ecdh_public_val = x25519->public_value();
+         m_kex_key.reset(x25519.release());
+#else
+         throw Internal_Error("Negotiated X25519 somehow, but it is disabled");
+#endif
+         }
+      else
+         {
+         EC_Group ec_group(curve_name);
+         std::unique_ptr<ECDH_PrivateKey> ecdh(new ECDH_PrivateKey(rng, ec_group));
 
-      if(domain == "")
-         throw Internal_Error("Could not find name of ECDH domain " + ecdh_domain_oid);
+         // follow client's preference for point compression
+         ecdh_public_val = ecdh->public_value(
+            state.client_hello()->prefers_compressed_ec_points() ?
+            PointGFp::COMPRESSED : PointGFp::UNCOMPRESSED);
 
-      const u16bit named_curve_id = Supported_Elliptic_Curves::name_to_curve_id(domain);
+         m_kex_key.reset(ecdh.release());
+         }
 
       m_params.push_back(3); // named curve
       m_params.push_back(get_byte(0, named_curve_id));
       m_params.push_back(get_byte(1, named_curve_id));
 
-      // follow client's preference for point compression
-      append_tls_length_value(m_params,
-            ecdh->public_value(state.client_hello()->prefers_compressed_ec_points() ?
-            PointGFp::COMPRESSED : PointGFp::UNCOMPRESSED), 1);
-
-      m_kex_key.reset(ecdh.release());
+      append_tls_length_value(m_params, ecdh_public_val, 1);
       }
 #if defined(BOTAN_HAS_SRP6)
    else if(kex_algo == "SRP_SHA")
diff --git a/src/lib/tls/tls_extensions.cpp b/src/lib/tls/tls_extensions.cpp
index df265d915..a2db1faaf 100644
--- a/src/lib/tls/tls_extensions.cpp
+++ b/src/lib/tls/tls_extensions.cpp
@@ -293,6 +293,12 @@ std::string Supported_Elliptic_Curves::curve_id_to_name(u16bit id)
          return "brainpool384r1";
       case 28:
          return "brainpool512r1";
+
+#if defined(BOTAN_HAS_CURVE_25519)
+      case 29:
+         return "x25519";
+#endif
+
       default:
          return ""; // something we don't know or support
       }
@@ -313,7 +319,13 @@ u16bit Supported_Elliptic_Curves::name_to_curve_id(const std::string& name)
    if(name == "brainpool512r1")
       return 28;
 
-   throw Invalid_Argument("name_to_curve_id unknown name " + name);
+#if defined(BOTAN_HAS_CURVE_25519)
+   if(name == "x25519")
+      return 29;
+#endif
+
+   // Unknown/unavailable EC curves are ignored
+   return 0;
    }
 
 std::vector<byte> Supported_Elliptic_Curves::serialize() const
@@ -323,8 +335,12 @@ std::vector<byte> Supported_Elliptic_Curves::serialize() const
    for(size_t i = 0; i != m_curves.size(); ++i)
       {
       const u16bit id = name_to_curve_id(m_curves[i]);
-      buf.push_back(get_byte(0, id));
-      buf.push_back(get_byte(1, id));
+
+      if(id > 0)
+         {
+         buf.push_back(get_byte(0, id));
+         buf.push_back(get_byte(1, id));
+         }
       }
 
    buf[0] = get_byte(0, static_cast<u16bit>(buf.size()-2));
diff --git a/src/lib/tls/tls_policy.cpp b/src/lib/tls/tls_policy.cpp
index 53ef8e46d..6ee1e0eac 100644
--- a/src/lib/tls/tls_policy.cpp
+++ b/src/lib/tls/tls_policy.cpp
@@ -96,6 +96,7 @@ std::vector<std::string> Policy::allowed_ecc_curves() const
       "secp384r1",
       "brainpool256r1",
       "secp256r1",
+      "x25519",
       };
    }
 
diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp
index 45d75dab2..9cc6d4793 100644
--- a/src/tests/unit_tls.cpp
+++ b/src/tests/unit_tls.cpp
@@ -910,6 +910,10 @@ class TLS_Unit_Tests : public Test
          test_modern_versions(results, *creds, "ECDH", "AES-128/GCM");
          test_modern_versions(results, *creds, "ECDH", "AES-128/GCM", "AEAD",
                               { { "use_ecc_point_compression", "true" } });
+         test_modern_versions(results, *creds, "ECDH", "AES-128/GCM", "AEAD",
+                              { { "ecc_curves", "secp384r1" } });
+         test_modern_versions(results, *creds, "ECDH", "AES-128/GCM", "AEAD",
+                              { { "ecc_curves", "x25519" } });
 
          std::unique_ptr<Botan::Credentials_Manager> creds_with_client_cert(create_creds(rng, true));
          test_modern_versions(results, *creds_with_client_cert, "ECDH", "AES-256/GCM");