Full support for renegotiation including RFC 5746 extensions for

client and server. Server side can handle SCSV values as well,
client always sends the extension instead.

Handle an empty SNI extension coming back from the server - this is
used to indicate that it understood the name. Also add better checking
for extensions by passing in what the supposed size of the extension
is.

Only send the secure negotiation extension in the server hello if the
client indicated support for it.

8 files changed, 133 insertions, 42 deletions

diff --git a/src/tls/hello.cpp b/src/tls/hello.cpp
index c2593ba4c..5390b6071 100644
--- a/src/tls/hello.cpp
+++ b/src/tls/hello.cpp
@@ -10,8 +10,7 @@
 #include <botan/internal/tls_session_key.h>
 #include <botan/internal/tls_extensions.h>
 #include <botan/tls_record.h>
-
-#include <stdio.h>
+#include <botan/internal/stl_util.h>
 
 namespace Botan {
 
@@ -79,7 +78,7 @@ Client_Hello::Client_Hello(Record_Writer& writer,
    comp_methods(policy.compression()),
    requested_hostname(hostname),
    requested_srp_id(srp_identifier),
-   has_secure_renegotiation(false),
+   has_secure_renegotiation(true),
    renegotiation_info_bits(reneg_info)
    {
    send(writer, hash);
@@ -161,8 +160,8 @@ void Client_Hello::deserialize_sslv2(const MemoryRegion<byte>& buf)
    c_random.resize(challenge_len);
    copy_mem(&c_random[0], &buf[9+cipher_spec_len+sess_id_len], challenge_len);
 
-   // FIXME: might be a ciphersuite value
-   has_secure_renegotiation = false;
+   has_secure_renegotiation =
+      value_exists(suites, static_cast<u16bit>(TLS_EMPTY_RENEGOTIATION_INFO_SCSV));
    }
 
 /*
@@ -206,6 +205,26 @@ void Client_Hello::deserialize(const MemoryRegion<byte>& buf)
          renegotiation_info_bits = reneg->renegotiation_info();
          }
       }
+
+   if(value_exists(suites, static_cast<u16bit>(TLS_EMPTY_RENEGOTIATION_INFO_SCSV)))
+      {
+      /*
+      * Clients are allowed to send both the extension and the SCSV
+      * though it is not recommended. If it did, require that the
+      * extension value be empty.
+      */
+      if(has_secure_renegotiation)
+         {
+         if(!renegotiation_info_bits.empty())
+            {
+            throw TLS_Exception(HANDSHAKE_FAILURE,
+                                "Client send SCSV and non-empty extension");
+            }
+         }
+
+      has_secure_renegotiation = true;
+      renegotiation_info_bits.clear();
+      }
    }
 
 /*
@@ -226,6 +245,7 @@ Server_Hello::Server_Hello(Record_Writer& writer,
                            TLS_Handshake_Hash& hash,
                            const TLS_Policy& policy,
                            RandomNumberGenerator& rng,
+                           bool client_has_secure_renegotiation,
                            const MemoryRegion<byte>& reneg_info,
                            const std::vector<X509_Certificate>& certs,
                            const Client_Hello& c_hello,
@@ -234,7 +254,7 @@ Server_Hello::Server_Hello(Record_Writer& writer,
    s_version(ver),
    sess_id(session_id),
    s_random(rng.random_vec(32)),
-   has_secure_renegotiation(false),
+   has_secure_renegotiation(client_has_secure_renegotiation),
    renegotiation_info_bits(reneg_info)
    {
    bool have_rsa = false, have_dsa = false;
@@ -266,6 +286,7 @@ Server_Hello::Server_Hello(Record_Writer& writer,
 Server_Hello::Server_Hello(Record_Writer& writer,
                            TLS_Handshake_Hash& hash,
                            RandomNumberGenerator& rng,
+                           bool client_has_secure_renegotiation,
                            const MemoryRegion<byte>& reneg_info,
                            const MemoryRegion<byte>& session_id,
                            u16bit ciphersuite,
@@ -276,7 +297,7 @@ Server_Hello::Server_Hello(Record_Writer& writer,
    s_random(rng.random_vec(32)),
    suite(ciphersuite),
    comp_method(compression),
-   has_secure_renegotiation(false),
+   has_secure_renegotiation(client_has_secure_renegotiation),
    renegotiation_info_bits(reneg_info)
    {
    send(writer, hash);
@@ -300,9 +321,12 @@ MemoryVector<byte> Server_Hello::serialize() const
 
    buf.push_back(comp_method);
 
-   TLS_Extensions extensions;
-
-   extensions.push_back(new Renegotation_Extension(renegotiation_info_bits));
+   if(has_secure_renegotiation)
+      {
+      TLS_Extensions extensions;
+      extensions.push_back(new Renegotation_Extension(renegotiation_info_bits));
+      buf += extensions.serialize();
+      }
 
    return buf;
    }
@@ -341,8 +365,6 @@ void Server_Hello::deserialize(const MemoryRegion<byte>& buf)
       {
       TLS_Extension* extn = extensions.at(i);
 
-      printf("Extension from server %d\n", extn->type());
-
       if(Renegotation_Extension* reneg = dynamic_cast<Renegotation_Extension*>(extn))
          {
          // checked by TLS_Client / TLS_Server as they know the handshake state
diff --git a/src/tls/tls_channel.cpp b/src/tls/tls_channel.cpp
index 144ca659f..7d4bdc744 100644
--- a/src/tls/tls_channel.cpp
+++ b/src/tls/tls_channel.cpp
@@ -10,8 +10,6 @@
 #include <botan/internal/tls_state.h>
 #include <botan/loadstor.h>
 
-#include <stdio.h>
-
 namespace Botan {
 
 TLS_Channel::TLS_Channel(std::tr1::function<void (const byte[], size_t)> socket_output_fn,
@@ -200,14 +198,51 @@ void TLS_Channel::alert(Alert_Level alert_level, Alert_Type alert_code)
 
 void TLS_Channel::Secure_Renegotiation_State::update(Client_Hello* client_hello)
    {
+   if(initial_handshake)
+      {
+      secure_renegotiation = client_hello->secure_renegotiation();
+      }
+   else
+      {
+      if(secure_renegotiation != client_hello->secure_renegotiation())
+         throw TLS_Exception(HANDSHAKE_FAILURE,
+                             "Client changed its mind about secure negotiation");
+      }
 
+   if(client_hello->secure_renegotiation())
+      {
+      const MemoryVector<byte>& data = client_hello->renegotiation_info();
+
+      if(initial_handshake)
+         {
+         if(!data.empty())
+            throw TLS_Exception(HANDSHAKE_FAILURE,
+                                "Client sent renegotiation data on initial handshake");
+         }
+      else
+         {
+         if(data != for_client_hello())
+            throw TLS_Exception(HANDSHAKE_FAILURE,
+                                "Client sent bad renegotiation data");
+         }
+      }
    }
 
 void TLS_Channel::Secure_Renegotiation_State::update(Server_Hello* server_hello)
    {
-   secure_renegotiation = server_hello->secure_renegotiation();
-
-   printf("server hello says sec reneg: %d\n", secure_renegotiation);
+   if(initial_handshake)
+      {
+      /* If the client offered but server rejected, then this toggles
+      *  secure_renegotiation to off
+      */
+      secure_renegotiation = server_hello->secure_renegotiation();
+      }
+   else
+      {
+      if(secure_renegotiation != server_hello->secure_renegotiation())
+         throw TLS_Exception(HANDSHAKE_FAILURE,
+                             "Server changed its mind about secure negotiation");
+      }
 
    if(secure_renegotiation)
       {
diff --git a/src/tls/tls_client.cpp b/src/tls/tls_client.cpp
index a97ac65b3..8c7c4188e 100644
--- a/src/tls/tls_client.cpp
+++ b/src/tls/tls_client.cpp
@@ -12,8 +12,6 @@
 #include <botan/dsa.h>
 #include <botan/dh.h>
 
-#include <stdio.h>
-
 namespace Botan {
 
 /*
@@ -89,8 +87,6 @@ void TLS_Client::process_handshake_msg(Handshake_Type type,
 
    if(type == HELLO_REQUEST)
       {
-      printf("got a hello request\n");
-
       Hello_Request hello_request(contents);
 
       // Ignore request entirely if we are currently negotiating a handshake
diff --git a/src/tls/tls_extensions.cpp b/src/tls/tls_extensions.cpp
index 1e8060821..1b3858a5a 100644
--- a/src/tls/tls_extensions.cpp
+++ b/src/tls/tls_extensions.cpp
@@ -8,11 +8,27 @@
 #include <botan/internal/tls_extensions.h>
 #include <botan/internal/tls_reader.h>
 
-#include <stdio.h>
-
 namespace Botan {
 
-TLS_Extensions::TLS_Extensions(class TLS_Data_Reader& reader)
+namespace {
+
+TLS_Extension* make_extension(TLS_Data_Reader& reader,
+                              u16bit extension_code,
+                              u16bit extension_size)
+   {
+   if(extension_code == TLSEXT_SERVER_NAME_INDICATION)
+      return new Server_Name_Indicator(reader, extension_size);
+   else if(extension_code == TLSEXT_SRP_IDENTIFIER)
+      return new SRP_Identifier(reader, extension_size);
+   else if(extension_code == TLSEXT_SAFE_RENEGOTIATION)
+      return new Renegotation_Extension(reader, extension_size);
+   else
+      return 0; // not known
+   }
+
+}
+
+TLS_Extensions::TLS_Extensions(TLS_Data_Reader& reader)
    {
    if(reader.has_remaining())
       {
@@ -26,17 +42,14 @@ TLS_Extensions::TLS_Extensions(class TLS_Data_Reader& reader)
          const u16bit extension_code = reader.get_u16bit();
          const u16bit extension_size = reader.get_u16bit();
 
-         if(extension_code == TLSEXT_SERVER_NAME_INDICATION)
-            extensions.push_back(new Server_Name_Indicator(reader));
-         else if(extension_code == TLSEXT_SRP_IDENTIFIER)
-            extensions.push_back(new SRP_Identifier(reader));
-         else if(extension_code == TLSEXT_SAFE_RENEGOTIATION)
-            extensions.push_back(new Renegotation_Extension(reader));
+         TLS_Extension* extn = make_extension(reader,
+                                              extension_code,
+                                              extension_size);
+
+         if(extn)
+            extensions.push_back(extn);
          else // unknown/unhandled extension
-            {
-            printf("Unknown extension code %d\n", extension_code);
             reader.discard_next(extension_size);
-            }
          }
       }
    }
@@ -82,10 +95,20 @@ TLS_Extensions::~TLS_Extensions()
    extensions.clear();
    }
 
-Server_Name_Indicator::Server_Name_Indicator(TLS_Data_Reader& reader)
+Server_Name_Indicator::Server_Name_Indicator(TLS_Data_Reader& reader,
+                                             u16bit extension_size)
    {
+   /*
+   * This is used by the server to confirm that it knew the name
+   */
+   if(extension_size == 0)
+      return;
+
    u16bit name_bytes = reader.get_u16bit();
 
+   if(name_bytes + 2 != extension_size)
+      throw Decoding_Error("Bad encoding of SNI extension");
+
    while(name_bytes)
       {
       byte name_type = reader.get_byte();
@@ -124,9 +147,13 @@ MemoryVector<byte> Server_Name_Indicator::serialize() const
    return buf;
    }
 
-SRP_Identifier::SRP_Identifier(TLS_Data_Reader& reader)
+SRP_Identifier::SRP_Identifier(TLS_Data_Reader& reader,
+                               u16bit extension_size)
    {
    srp_identifier = reader.get_string(1, 1, 255);
+
+   if(srp_identifier.size() + 1 != extension_size)
+      throw Decoding_Error("Bad encoding for SRP identifier extension");
    }
 
 MemoryVector<byte> SRP_Identifier::serialize() const
@@ -141,9 +168,13 @@ MemoryVector<byte> SRP_Identifier::serialize() const
    return buf;
    }
 
-Renegotation_Extension::Renegotation_Extension(TLS_Data_Reader& reader)
+Renegotation_Extension::Renegotation_Extension(TLS_Data_Reader& reader,
+                                               u16bit extension_size)
    {
    reneg_data = reader.get_range<byte>(1, 0, 255);
+
+   if(reneg_data.size() + 1 != extension_size)
+      throw Decoding_Error("Bad encoding for secure renegotiation extn");
    }
 
 MemoryVector<byte> Renegotation_Extension::serialize() const
diff --git a/src/tls/tls_extensions.h b/src/tls/tls_extensions.h
index aa2349cf1..bbf4aa851 100644
--- a/src/tls/tls_extensions.h
+++ b/src/tls/tls_extensions.h
@@ -43,7 +43,8 @@ class Server_Name_Indicator : public TLS_Extension
       Server_Name_Indicator(const std::string& host_name) :
          sni_host_name(host_name) {}
 
-      Server_Name_Indicator(TLS_Data_Reader& reader);
+      Server_Name_Indicator(TLS_Data_Reader& reader,
+                            u16bit extension_size);
 
       std::string host_name() const { return sni_host_name; }
 
@@ -66,7 +67,8 @@ class SRP_Identifier : public TLS_Extension
       SRP_Identifier(const std::string& identifier) :
          srp_identifier(identifier) {}
 
-      SRP_Identifier(TLS_Data_Reader& reader);
+      SRP_Identifier(TLS_Data_Reader& reader,
+                     u16bit extension_size);
 
       std::string identifier() const { return srp_identifier; }
 
@@ -91,7 +93,8 @@ class Renegotation_Extension : public TLS_Extension
       Renegotation_Extension(const MemoryRegion<byte>& bits) :
          reneg_data(bits) {}
 
-      Renegotation_Extension(TLS_Data_Reader& reader);
+      Renegotation_Extension(TLS_Data_Reader& reader,
+                             u16bit extension_size);
 
       const MemoryVector<byte>& renegotiation_info() const
          { return reneg_data; }
diff --git a/src/tls/tls_messages.h b/src/tls/tls_messages.h
index 3f02903fe..cb56ead6c 100644
--- a/src/tls/tls_messages.h
+++ b/src/tls/tls_messages.h
@@ -135,6 +135,7 @@ class Server_Hello : public HandshakeMessage
                    TLS_Handshake_Hash& hash,
                    const TLS_Policy& policies,
                    RandomNumberGenerator& rng,
+                   bool client_has_secure_renegotiation,
                    const MemoryRegion<byte>& reneg_info,
                    const std::vector<X509_Certificate>& certs,
                    const Client_Hello& other,
@@ -144,6 +145,7 @@ class Server_Hello : public HandshakeMessage
       Server_Hello(Record_Writer& writer,
                    TLS_Handshake_Hash& hash,
                    RandomNumberGenerator& rng,
+                   bool client_has_secure_renegotiation,
                    const MemoryRegion<byte>& reneg_info,
                    const MemoryRegion<byte>& session_id,
                    u16bit ciphersuite,
diff --git a/src/tls/tls_reader.h b/src/tls/tls_reader.h
index bc2dffb58..ef36912d3 100644
--- a/src/tls/tls_reader.h
+++ b/src/tls/tls_reader.h
@@ -154,7 +154,10 @@ class TLS_Data_Reader
       void assert_at_least(size_t n) const
          {
          if(buf.size() - offset < n)
+            {
+            abort();
             throw Decoding_Error("TLS_Data_Reader: Corrupt packet");
+            }
          }
 
       const MemoryRegion<byte>& buf;
diff --git a/src/tls/tls_server.cpp b/src/tls/tls_server.cpp
index 427e699af..645e207e4 100644
--- a/src/tls/tls_server.cpp
+++ b/src/tls/tls_server.cpp
@@ -12,8 +12,6 @@
 #include <botan/rsa.h>
 #include <botan/dh.h>
 
-#include <stdio.h>
-
 namespace Botan {
 
 namespace {
@@ -118,7 +116,6 @@ void TLS_Server::renegotiate()
    state = new Handshake_State;
    state->set_expected_next(CLIENT_HELLO);
    Hello_Request hello_req(writer);
-   printf("sent new hello request\n");
    }
 
 /*
@@ -188,6 +185,7 @@ void TLS_Server::process_handshake_msg(Handshake_Type type,
             writer,
             state->hash,
             rng,
+            secure_renegotiation.supported(),
             secure_renegotiation.for_server_hello(),
             session_info.session_id(),
             session_info.ciphersuite(),
@@ -219,6 +217,7 @@ void TLS_Server::process_handshake_msg(Handshake_Type type,
             state->hash,
             policy,
             rng,
+            secure_renegotiation.supported(),
             secure_renegotiation.for_server_hello(),
             cert_chain,
             *(state->client_hello),