TLS Server should respect client signature_algorithms. Stricter TLS hello decoding.

If the client sent a signature_algorithms extension, we should negotiate a ciphersuite in
the shared union of the ciphersuite list and the extension, instead of ignoring it.
Found by Juraj Somorovsky GH #619

The TLS v1.2 spec says that clients should only send the signature_algorithms
extension in a hello for that version. Enforce that when decoding client hellos
to prevent this extension from confusing a v1.0 negotiation.

TLS v1.2 spec says ANON signature type is prohibited in the signature_algorithms extension
in the client hello. Prohibit it.

Reorder the TLS extensions in the client hello so there is no chance an empty extension is
the last extension in the list. Some implementations apparently reject such hellos, even
(perhaps especially) when they do not recognize the extension, this bug was mentioned on
the ietf-tls mailing list a while back.

1 files changed, 27 insertions, 13 deletions

diff --git a/src/lib/tls/msg_client_hello.cpp b/src/lib/tls/msg_client_hello.cpp
index 69f9a5e11..56e226a40 100644
--- a/src/lib/tls/msg_client_hello.cpp
+++ b/src/lib/tls/msg_client_hello.cpp
@@ -80,25 +80,27 @@ Client_Hello::Client_Hello(Handshake_IO& io,
                                     client_settings.srp_identifier() != "")),
    m_comp_methods(policy.compression())
    {
+   BOTAN_ASSERT(policy.acceptable_protocol_version(client_settings.protocol_version()),
+                "Our policy accepts the version we are offering");
+
+   /*
+   * Place all empty extensions in front to avoid a bug in some sytems
+   * which reject hellos when the last extension in the list is empty.
+   */
    m_extensions.add(new Extended_Master_Secret);
+   m_extensions.add(new Session_Ticket());
+   if(policy.negotiate_encrypt_then_mac())
+      m_extensions.add(new Encrypt_then_MAC);
+
    m_extensions.add(new Renegotiation_Extension(reneg_info));
    m_extensions.add(new Server_Name_Indicator(client_settings.hostname()));
-   m_extensions.add(new Session_Ticket());
-   m_extensions.add(new Supported_Elliptic_Curves(policy.allowed_ecc_curves()));
 
-   if(m_version.supports_negotiable_signature_algorithms())
-      m_extensions.add(new Signature_Algorithms(policy.allowed_signature_hashes(),
-                                                policy.allowed_signature_methods()));
+   if(reneg_info.empty() && !next_protocols.empty())
+      m_extensions.add(new Application_Layer_Protocol_Notification(next_protocols));
 
    if(m_version.is_datagram_protocol())
      m_extensions.add(new SRTP_Protection_Profiles(policy.srtp_profiles()));
 
-   if(reneg_info.empty() && !next_protocols.empty())
-      m_extensions.add(new Application_Layer_Protocol_Notification(next_protocols));
-   
-   if(policy.negotiate_encrypt_then_mac())
-      m_extensions.add(new Encrypt_then_MAC);
-
 #if defined(BOTAN_HAS_SRP6)
    m_extensions.add(new SRP_Identifier(client_settings.srp_identifier()));
 #else
@@ -108,8 +110,11 @@ Client_Hello::Client_Hello(Handshake_IO& io,
       }
 #endif
 
-   BOTAN_ASSERT(policy.acceptable_protocol_version(client_settings.protocol_version()),
-                "Our policy accepts the version we are offering");
+   m_extensions.add(new Supported_Elliptic_Curves(policy.allowed_ecc_curves()));
+
+   if(m_version.supports_negotiable_signature_algorithms())
+      m_extensions.add(new Signature_Algorithms(policy.allowed_signature_hashes(),
+                                                policy.allowed_signature_methods()));
 
    if(policy.send_fallback_scsv(client_settings.protocol_version()))
       m_suites.push_back(TLS_FALLBACK_SCSV);
@@ -256,6 +261,15 @@ Client_Hello::Client_Hello(const std::vector<byte>& buf)
          m_extensions.add(new Renegotiation_Extension());
          }
       }
+
+   // Parsing complete, now any additional decoding checks
+
+   if(m_version.supports_negotiable_signature_algorithms() == false)
+      {
+      if(m_extensions.has<Signature_Algorithms>())
+         throw TLS_Exception(Alert::HANDSHAKE_FAILURE,
+                             "Client sent signature_algorithms extension in version that doesn't support it");
+      }
    }
 
 bool Client_Hello::sent_fallback_scsv() const