Fix bug causing TLS client to sometimes reject DHE server kex

Re-encoding the server key exchange meant that any leading zeros
in the values for DHE (or SRP) would be stripped out. This would
cause the signature check to fail.

3 files changed, 20 insertions, 35 deletions

diff --git a/doc/news.rst b/doc/news.rst
index aa40fe2a9..2622c66bd 100644
--- a/doc/news.rst
+++ b/doc/news.rst
@@ -22,6 +22,10 @@ Version 1.11.25, Not Yet Released
   TLS ciphersuite with an empty identity hint. ECDHE_PSK and DHE_PSK
   suites were not affected.
 
+* Fixed a bug that would cause the TLS client to occasionally reject a
+  valid server key exchange message as having an invalid signature.
+  This only affected DHE ciphersuites.
+
 * Support for negotiating use of SHA-224 in TLS has been disabled in the
   default policy.
 
diff --git a/src/lib/tls/msg_server_kex.cpp b/src/lib/tls/msg_server_kex.cpp
index 50caf3288..0c3b5c704 100644
--- a/src/lib/tls/msg_server_kex.cpp
+++ b/src/lib/tls/msg_server_kex.cpp
@@ -148,15 +148,14 @@ Server_Key_Exchange::Server_Key_Exchange(const std::vector<byte>& buf,
    TLS_Data_Reader reader("ServerKeyExchange", buf);
 
    /*
-   * We really are just serializing things back to what they were
-   * before, but unfortunately to know where the signature is we need
-   * to be able to parse the whole thing anyway.
+   * Here we are deserializing enough to find out what offset the
+   * signature is at. All processing is done when the Client Key Exchange
+   * is prepared.
    */
 
    if(kex_algo == "PSK" || kex_algo == "DHE_PSK" || kex_algo == "ECDHE_PSK")
       {
-      const std::string identity_hint = reader.get_string(2, 0, 65535);
-      append_tls_length_value(m_params, identity_hint, 2);
+      reader.get_string(2, 0, 65535); // identity hint
       }
 
    if(kex_algo == "DH" || kex_algo == "DHE_PSK")
@@ -165,49 +164,29 @@ Server_Key_Exchange::Server_Key_Exchange(const std::vector<byte>& buf,
 
       for(size_t i = 0; i != 3; ++i)
          {
-         BigInt v = BigInt::decode(reader.get_range<byte>(2, 1, 65535));
-         append_tls_length_value(m_params, BigInt::encode(v), 2);
+         reader.get_range<byte>(2, 1, 65535);
          }
       }
    else if(kex_algo == "ECDH" || kex_algo == "ECDHE_PSK")
       {
-      const byte curve_type = reader.get_byte();
-
-      if(curve_type != 3)
-         throw Decoding_Error("Server_Key_Exchange: Server sent non-named ECC curve");
-
-      const u16bit curve_id = reader.get_u16bit();
-
-      const std::string name = Supported_Elliptic_Curves::curve_id_to_name(curve_id);
-
-      std::vector<byte> ecdh_key = reader.get_range<byte>(1, 1, 255);
-
-      if(name == "")
-         throw Decoding_Error("Server_Key_Exchange: Server sent unknown named curve " +
-                              std::to_string(curve_id));
-
-      m_params.push_back(curve_type);
-      m_params.push_back(get_byte(0, curve_id));
-      m_params.push_back(get_byte(1, curve_id));
-      append_tls_length_value(m_params, ecdh_key, 1);
+      reader.get_byte(); // curve type
+      reader.get_u16bit(); // curve id
+      reader.get_range<byte>(1, 1, 255); // public key
       }
    else if(kex_algo == "SRP_SHA")
       {
       // 2 bigints (N,g) then salt, then server B
 
-      const BigInt N = BigInt::decode(reader.get_range<byte>(2, 1, 65535));
-      const BigInt g = BigInt::decode(reader.get_range<byte>(2, 1, 65535));
-      std::vector<byte> salt = reader.get_range<byte>(1, 1, 255);
-      const BigInt B = BigInt::decode(reader.get_range<byte>(2, 1, 65535));
-
-      append_tls_length_value(m_params, BigInt::encode(N), 2);
-      append_tls_length_value(m_params, BigInt::encode(g), 2);
-      append_tls_length_value(m_params, salt, 1);
-      append_tls_length_value(m_params, BigInt::encode(B), 2);
+      reader.get_range<byte>(2, 1, 65535);
+      reader.get_range<byte>(2, 1, 65535);
+      reader.get_range<byte>(1, 1, 255);
+      reader.get_range<byte>(2, 1, 65535);
       }
    else if(kex_algo != "PSK")
       throw Decoding_Error("Server_Key_Exchange: Unsupported kex type " + kex_algo);
 
+   m_params.assign(buf.data(), buf.data() + reader.read_so_far());
+
    if(sig_algo != "")
       {
       if(version.supports_negotiable_signature_algorithms())
diff --git a/src/lib/tls/tls_reader.h b/src/lib/tls/tls_reader.h
index c2aef3163..63a59625f 100644
--- a/src/lib/tls/tls_reader.h
+++ b/src/lib/tls/tls_reader.h
@@ -34,6 +34,8 @@ class TLS_Data_Reader
             throw decode_error("Extra bytes at end of message");
          }
 
+      size_t read_so_far() const { return m_offset; }
+
       size_t remaining_bytes() const { return m_buf.size() - m_offset; }
 
       bool has_remaining() const { return (remaining_bytes() > 0); }