Merge GH #645 TLS compressed points

13 files changed, 159 insertions, 9 deletions

diff --git a/src/lib/pubkey/ecdh/ecdh.h b/src/lib/pubkey/ecdh/ecdh.h
index 2f892436c..bdd9ea047 100644
--- a/src/lib/pubkey/ecdh/ecdh.h
+++ b/src/lib/pubkey/ecdh/ecdh.h
@@ -55,6 +55,12 @@ class BOTAN_DLL ECDH_PublicKey : public virtual EC_PublicKey
       std::vector<byte> public_value() const
          { return unlock(EC2OSP(public_point(), PointGFp::UNCOMPRESSED)); }
 
+      /**
+      * @return public point value
+      */
+      std::vector<byte> public_value(PointGFp::Compression_Type type) const
+         { return unlock(EC2OSP(public_point(), type)); }
+
    protected:
       ECDH_PublicKey();
    };
@@ -84,7 +90,10 @@ class BOTAN_DLL ECDH_PrivateKey : public ECDH_PublicKey,
          EC_PrivateKey(rng, domain, x) {}
 
       std::vector<byte> public_value() const override
-         { return ECDH_PublicKey::public_value(); }
+         { return ECDH_PublicKey::public_value(PointGFp::UNCOMPRESSED); }
+
+      std::vector<byte> public_value(PointGFp::Compression_Type type) const
+         { return ECDH_PublicKey::public_value(type); }
    };
 
 }
diff --git a/src/lib/tls/msg_client_hello.cpp b/src/lib/tls/msg_client_hello.cpp
index b493fd3ee..36335e7ce 100644
--- a/src/lib/tls/msg_client_hello.cpp
+++ b/src/lib/tls/msg_client_hello.cpp
@@ -98,8 +98,12 @@ Client_Hello::Client_Hello(Handshake_IO& io,
    if(reneg_info.empty() && !next_protocols.empty())
       m_extensions.add(new Application_Layer_Protocol_Notification(next_protocols));
 
+   if(m_version.supports_negotiable_signature_algorithms())
+      m_extensions.add(new Signature_Algorithms(policy.allowed_signature_hashes(),
+                                                policy.allowed_signature_methods()));
+
    if(m_version.is_datagram_protocol())
-     m_extensions.add(new SRTP_Protection_Profiles(policy.srtp_profiles()));
+      m_extensions.add(new SRTP_Protection_Profiles(policy.srtp_profiles()));
 
 #if defined(BOTAN_HAS_SRP6)
    m_extensions.add(new SRP_Identifier(client_settings.srp_identifier()));
@@ -112,6 +116,11 @@ Client_Hello::Client_Hello(Handshake_IO& io,
 
    m_extensions.add(new Supported_Elliptic_Curves(policy.allowed_ecc_curves()));
 
+   if(!policy.allowed_ecc_curves().empty() && policy.use_ecc_point_compression())
+   {
+      m_extensions.add(new Supported_Point_Formats());
+   }
+
    if(m_version.supports_negotiable_signature_algorithms())
       m_extensions.add(new Signature_Algorithms(policy.allowed_signature_hashes(),
                                                 policy.allowed_signature_methods()));
@@ -156,6 +165,11 @@ Client_Hello::Client_Hello(Handshake_IO& io,
    m_extensions.add(new Session_Ticket(session.session_ticket()));
    m_extensions.add(new Supported_Elliptic_Curves(policy.allowed_ecc_curves()));
 
+   if(!policy.allowed_ecc_curves().empty() && policy.use_ecc_point_compression())
+   {
+      m_extensions.add(new Supported_Point_Formats());
+   }
+
    if(m_version.supports_negotiable_signature_algorithms())
       m_extensions.add(new Signature_Algorithms(policy.allowed_signature_hashes(),
                                                 policy.allowed_signature_methods()));
diff --git a/src/lib/tls/msg_client_kex.cpp b/src/lib/tls/msg_client_kex.cpp
index bc4d33d52..0eceadb3b 100644
--- a/src/lib/tls/msg_client_kex.cpp
+++ b/src/lib/tls/msg_client_kex.cpp
@@ -172,7 +172,10 @@ Client_Key_Exchange::Client_Key_Exchange(Handshake_IO& io,
             append_tls_length_value(m_pre_master, psk.bits_of(), 2);
             }
 
-         append_tls_length_value(m_key_material, priv_key.public_value(), 1);
+         // 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);
          }
 #if defined(BOTAN_HAS_SRP6)
       else if(kex_algo == "SRP_SHA")
diff --git a/src/lib/tls/msg_server_hello.cpp b/src/lib/tls/msg_server_hello.cpp
index ebe8fb085..4f95a5c9d 100644
--- a/src/lib/tls/msg_server_hello.cpp
+++ b/src/lib/tls/msg_server_hello.cpp
@@ -35,12 +35,18 @@ Server_Hello::Server_Hello(Handshake_IO& io,
    if(client_hello.supports_extended_master_secret())
       m_extensions.add(new Extended_Master_Secret);
 
+   Ciphersuite c = Ciphersuite::by_id(m_ciphersuite);
+
    if(client_hello.supports_encrypt_then_mac() && policy.negotiate_encrypt_then_mac())
       {
-      Ciphersuite c = Ciphersuite::by_id(m_ciphersuite);
       if(c.cbc_ciphersuite())
          m_extensions.add(new Encrypt_then_MAC);
       }
+
+   if(c.ecc_ciphersuite() && policy.use_ecc_point_compression())
+      {
+      m_extensions.add(new Supported_Point_Formats());
+      }
       
    if(client_hello.secure_renegotiation())
       m_extensions.add(new Renegotiation_Extension(reneg_info));
diff --git a/src/lib/tls/msg_server_kex.cpp b/src/lib/tls/msg_server_kex.cpp
index 10581fe45..33b980ba9 100644
--- a/src/lib/tls/msg_server_kex.cpp
+++ b/src/lib/tls/msg_server_kex.cpp
@@ -85,7 +85,10 @@ Server_Key_Exchange::Server_Key_Exchange(Handshake_IO& io,
       m_params.push_back(get_byte(0, named_curve_id));
       m_params.push_back(get_byte(1, named_curve_id));
 
-      append_tls_length_value(m_params, ecdh->public_value(), 1);
+      // 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());
       }
diff --git a/src/lib/tls/tls_extensions.cpp b/src/lib/tls/tls_extensions.cpp
index e38e4ccdc..df265d915 100644
--- a/src/lib/tls/tls_extensions.cpp
+++ b/src/lib/tls/tls_extensions.cpp
@@ -33,6 +33,9 @@ Extension* make_extension(TLS_Data_Reader& reader,
       case TLSEXT_USABLE_ELLIPTIC_CURVES:
          return new Supported_Elliptic_Curves(reader, size);
 
+      case TLSEXT_EC_POINT_FORMATS:
+         return new Supported_Point_Formats(reader, size);
+
       case TLSEXT_SAFE_RENEGOTIATION:
          return new Renegotiation_Extension(reader, size);
 
@@ -353,6 +356,43 @@ Supported_Elliptic_Curves::Supported_Elliptic_Curves(TLS_Data_Reader& reader,
       }
    }
 
+std::vector<byte> Supported_Point_Formats::serialize() const
+   {
+   // if we send this extension, we prefer compressed points,
+   // otherwise we don't send it (which is equal to supporting only uncompressed)
+   // if this extension is sent, it MUST include uncompressed (RFC 4492, section 5.1)
+   return std::vector<byte>{2, ANSIX962_COMPRESSED_PRIME, UNCOMPRESSED};
+   }
+
+Supported_Point_Formats::Supported_Point_Formats(TLS_Data_Reader& reader,
+                                                 u16bit extension_size)
+   {
+   byte len = reader.get_byte();
+
+   if(len + 1 != extension_size)
+      throw Decoding_Error("Inconsistent length field in supported point formats list");
+
+   for(size_t i = 0; i != len; ++i)
+      {
+      byte format = reader.get_byte();
+
+      if(format == UNCOMPRESSED)
+         {
+         m_prefers_compressed = false;
+         reader.discard_next(len-i-1);
+         return;
+         }
+      else if(format == ANSIX962_COMPRESSED_PRIME)
+         {
+         m_prefers_compressed = true;
+         reader.discard_next(len-i-1);
+         return;
+         }
+
+      // ignore ANSIX962_COMPRESSED_CHAR2, we don't support these curves
+      }
+   }
+
 std::string Signature_Algorithms::hash_algo_name(byte code)
    {
    switch(code)
diff --git a/src/lib/tls/tls_extensions.h b/src/lib/tls/tls_extensions.h
index c270bf23a..d69e40a60 100644
--- a/src/lib/tls/tls_extensions.h
+++ b/src/lib/tls/tls_extensions.h
@@ -258,6 +258,38 @@ class Supported_Elliptic_Curves final : public Extension
    };
 
 /**
+* Supported Point Formats Extension (RFC 4492)
+*/
+class Supported_Point_Formats final : public Extension
+   {
+   public:
+      enum ECPointFormat : byte {
+         UNCOMPRESSED = 0,
+         ANSIX962_COMPRESSED_PRIME = 1,
+         ANSIX962_COMPRESSED_CHAR2 = 2, // don't support these curves
+      };
+
+      static Handshake_Extension_Type static_type()
+         { return TLSEXT_EC_POINT_FORMATS; }
+
+      Handshake_Extension_Type type() const override { return static_type(); }
+
+      std::vector<byte> serialize() const override;
+
+      explicit Supported_Point_Formats() : m_prefers_compressed(true) {}
+
+      Supported_Point_Formats(TLS_Data_Reader& reader,
+                              u16bit extension_size);
+
+      bool empty() const override { return false; }
+
+      bool prefers_compressed() { return m_prefers_compressed; }
+
+   private:
+      bool m_prefers_compressed = false;
+   };
+
+/**
 * Signature Algorithms Extension for TLS 1.2 (RFC 5246)
 */
 class Signature_Algorithms final : public Extension
diff --git a/src/lib/tls/tls_messages.h b/src/lib/tls/tls_messages.h
index 698dbc99f..25228c865 100644
--- a/src/lib/tls/tls_messages.h
+++ b/src/lib/tls/tls_messages.h
@@ -121,6 +121,15 @@ class BOTAN_DLL Client_Hello final : public Handshake_Message
          return std::vector<std::string>();
          }
 
+      bool prefers_compressed_ec_points() const
+         {
+         if(Supported_Point_Formats* ecc_formats = m_extensions.get<Supported_Point_Formats>())
+            {
+            return ecc_formats->prefers_compressed();
+            }
+         return false;
+         }
+
       std::string sni_hostname() const
          {
          if(Server_Name_Indicator* sni = m_extensions.get<Server_Name_Indicator>())
@@ -328,6 +337,15 @@ class BOTAN_DLL Server_Hello final : public Handshake_Message
       std::set<Handshake_Extension_Type> extension_types() const
          { return m_extensions.extension_types(); }
 
+      bool prefers_compressed_ec_points() const
+         {
+         if(auto ecc_formats = m_extensions.get<Supported_Point_Formats>())
+            {
+            return ecc_formats->prefers_compressed();
+            }
+         return false;
+         }
+
       Server_Hello(Handshake_IO& io,
                    Handshake_Hash& hash,
                    const Policy& policy,
diff --git a/src/lib/tls/tls_policy.cpp b/src/lib/tls/tls_policy.cpp
index 592d4f572..9646aa320 100644
--- a/src/lib/tls/tls_policy.cpp
+++ b/src/lib/tls/tls_policy.cpp
@@ -104,6 +104,11 @@ bool Policy::allowed_ecc_curve(const std::string& curve) const
    return value_exists(allowed_ecc_curves(), curve);
    }
 
+bool Policy::use_ecc_point_compression() const
+   {
+   return false;
+   }
+
 /*
 * Choose an ECC curve to use
 */
diff --git a/src/lib/tls/tls_policy.h b/src/lib/tls/tls_policy.h
index 76e80ddde..47ac51685 100644
--- a/src/lib/tls/tls_policy.h
+++ b/src/lib/tls/tls_policy.h
@@ -67,6 +67,11 @@ class BOTAN_DLL Policy
       bool allowed_ecc_curve(const std::string& curve) const;
 
       /**
+      * Request that ECC curve points are sent compressed
+      */
+      virtual bool use_ecc_point_compression() const;
+
+      /**
       * Returns a list of compression algorithms we are willing to use,
       * in order of preference. Allowed values any value of
       * Compression_Method.
@@ -348,6 +353,9 @@ class BOTAN_DLL Text_Policy : public Policy
       std::vector<std::string> allowed_ecc_curves() const override
          { return get_list("ecc_curves", Policy::allowed_ecc_curves()); }
       
+      bool use_ecc_point_compression() const override
+         { return get_bool("use_ecc_point_compression", Policy::use_ecc_point_compression()); }
+
       bool allow_tls10() const override
          { return get_bool("allow_tls10", Policy::allow_tls10()); }
       
diff --git a/src/tests/data/tls/client_hello.vec b/src/tests/data/tls/client_hello.vec
index d629e3f6e..aa8c03258 100644
--- a/src/tests/data/tls/client_hello.vec
+++ b/src/tests/data/tls/client_hello.vec
@@ -13,13 +13,13 @@ Exception =
 # with extensions: point formats, ec curves, session ticket, signature algorithms, heartbeat (point formats and heartbeat not supported, empty renegotiation generated)
 Buffer = 0303871e18983024eaee1be8ae6607d5ecad941d33fd7fc1d8554a9e1fbfda8d30880000aac030c02cc028c024c014c00a00a500a300a1009f006b006a0069006800390038003700360088008700860085c032c02ec02ac026c00fc005009d003d00350084c02fc02bc027c023c013c00900a400a200a0009e00670040003f003e0033003200310030009a0099009800970045004400430042c031c02dc029c025c00ec004009c003c002f00960041c011c007c00cc00200050004c012c008001600130010000dc00dc003000a00ff01000055000b000403000102000a001c001a00170019001c001b0018001a0016000e000d000b000c0009000a00230000000d0020001e060106020603050105020503040104020403030103020303020102020203000f000101
 Protocol = 0303
-AdditionalData = 000A000D0023FF01 
+AdditionalData = 000A000B000D0023FF01
 Exception = 
 
 # with extensions: point formats, ec curves, session ticket, signature algorithms, heartbeat, Encrypt-then-MAC, Extended Master Secret (point formats and heartbeat not supported, empty renegotiation generated)
 Buffer = 0303e00da23523058b5dc9c445d97b2bb6315b019e97838ac4f16c23b2cb031b6a490000e2c0afc0adc030c02cc028c024c014c00ac0a3c09f00a500a300a1009f006b006a006900680039003800370036cca9cca8c077c073ccaa00c400c300c200c10088008700860085c032c02ec02ac026c00fc005c079c075c0a1c09d009d003d003500c00084c0aec0acc02fc02bc027c023c013c009c0a2c09e00a400a200a0009e00670040003f003e0033003200310030c076c07200be00bd00bc00bb009a0099009800970045004400430042c031c02dc029c025c00ec004c078c074c0a0c09c009c003c002f00ba009600410007c012c008001600130010000dc00dc003000a00ff0100005f000b000403000102000a001c001a00170019001c001b0018001a0016000e000d000b000c0009000a00230000000d00220020060106020603050105020503040104020403030103020303020102020203eded000f0001010016000000170000
 Protocol = 0303
-AdditionalData = 000A000D001600170023FF01 
+AdditionalData = 000A000B000D001600170023FF01
 Exception = 
 
 # empty
diff --git a/src/tests/data/tls/server_hello.vec b/src/tests/data/tls/server_hello.vec
index 64ec40b80..f3bf889cb 100644
--- a/src/tests/data/tls/server_hello.vec
+++ b/src/tests/data/tls/server_hello.vec
@@ -9,14 +9,14 @@
 Buffer = 0303ffea0bcfba564a4ce177c6a444b0ebdff5629b277293c618c1125f231e8628dd00c030000016ff01000100000b00040300010200230000000f000101
 Protocol = 0303
 Ciphersuite = C030
-AdditionalData = 0023FF01
+AdditionalData = 000B0023FF01
 Exception = 
 
 # correct, with session ticket, extended master secret, and renegotiation info
 Buffer = 03019f9cafa88664d9095f85dd64a39e5dd5c09f5a4a5362938af3718ee4e818af6a00c03000001aff01000100000b00040300010200230000000f00010100170000
 Protocol = 0301
 Ciphersuite = C030
-AdditionalData = 00170023FF01
+AdditionalData = 000B00170023FF01
 Exception = 
 
 # incorrect, corrupted
diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp
index de54c9747..5ae24ba81 100644
--- a/src/tests/unit_tls.cpp
+++ b/src/tests/unit_tls.cpp
@@ -868,6 +868,18 @@ class TLS_Unit_Tests : public Test
          results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy, server_policy));
          results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy, server_policy));
          
+         policy.set("use_ecc_point_compression", "true");
+         policy.set("key_exchange_methods", "ECDH");
+         policy.set("ciphers", "AES-128");
+         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
+         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
+
+         server_policy.set("use_ecc_point_compression", "true");
+         server_policy.set("key_exchange_methods", "ECDH");
+         server_policy.set("ciphers", "AES-128");
+         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy, server_policy));
+         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy, server_policy));
+
          return results;
          }