3 files changed, 63 insertions, 41 deletions

diff --git a/src/tls/tls_server.cpp b/src/tls/tls_server.cpp
index 585a80584..f4ae71b2e 100644
--- a/src/tls/tls_server.cpp
+++ b/src/tls/tls_server.cpp
@@ -281,6 +281,15 @@ void Server::process_handshake_msg(Handshake_Type type,
 
    if(type == CLIENT_HELLO || type == CLIENT_HELLO_SSLV2)
       {
+      if(!m_policy.allow_insecure_renegotiation() &&
+         !(m_secure_renegotiation.initial_handshake() || m_secure_renegotiation.supported()))
+         {
+         delete m_state;
+         m_state = nullptr;
+         send_alert(Alert(Alert::NO_RENEGOTIATION));
+         return;
+         }
+
       m_state->client_hello = new Client_Hello(contents, type);
 
       if(m_state->client_hello->sni_hostname() != "")
@@ -288,59 +297,54 @@ void Server::process_handshake_msg(Handshake_Type type,
 
       Protocol_Version client_version = m_state->client_hello->version();
 
-      if(!m_policy.acceptable_protocol_version(m_state->version()))
-         {
-         throw TLS_Exception(Alert::PROTOCOL_VERSION,
-                             "Client version is unacceptable by policy");
-         }
+      const Protocol_Version prev_version = m_reader.get_version();
+      const bool is_renegotiation = prev_version.valid();
 
-      if(!client_version.known_version())
+      if((is_renegotiation && client_version == prev_version) ||
+         (!is_renegotiation && client_version.known_version()))
          {
          /*
-         This isn't a version we know. This implies this is a later
-         version than any version defined as of 2012. Offer them
-         the latest version we know.
+         Common cases: new client hello with some known version, or a
+         renegotiation using the same version as previously
+         negotiated.
          */
-         m_state->set_version(Protocol_Version::TLS_V12);
+
+         m_state->set_version(client_version);
          }
-      else
+      else if(is_renegotiation && (client_version != prev_version))
          {
-         Protocol_Version prev_version = m_reader.get_version();
-
-         if(prev_version.valid() && client_version != prev_version)
+         /*
+         * If this is a renegotation, and the client has offered a
+         * later version than what it initially negotiated,
+         * negotiate the old version. This matches OpenSSL's
+         * behavior. If the client is offering a version earlier
+         * than what it initially negotiated, reject as a probable
+         * attack.
+         */
+         if(prev_version > client_version)
             {
-            /*
-            * If this is a renegotation, and the client has offered a
-            * later version than what it initially negotiated,
-            * negotiate the old version. This matches OpenSSL's
-            * behavior. If the client is offering a version earlier
-            * than what it initially negotiated, reject as a probable
-            * attack.
-            */
-            if(prev_version > client_version)
-               {
-               throw TLS_Exception(Alert::PROTOCOL_VERSION,
-                                   "Client negotiated " +
-                                   prev_version.to_string() +
-                                   " then renegotiated with " +
-                                   client_version.to_string());
-               }
-
-            m_state->set_version(prev_version);
+            throw TLS_Exception(Alert::PROTOCOL_VERSION,
+                                "Client negotiated " +
+                                prev_version.to_string() +
+                                " then renegotiated with " +
+                                client_version.to_string());
             }
          else
-            {
-            m_state->set_version(client_version);
-            }
+            m_state->set_version(prev_version);
+         }
+      else
+         {
+         /*
+         New negotiation using a version we don't know. Offer
+         them the best we currently know.
+         */
+         m_state->set_version(client_version.best_known_match());
          }
 
-      if(!m_policy.allow_insecure_renegotiation() &&
-         !(m_secure_renegotiation.initial_handshake() || m_secure_renegotiation.supported()))
+      if(!m_policy.acceptable_protocol_version(m_state->version()))
          {
-         delete m_state;
-         m_state = nullptr;
-         send_alert(Alert(Alert::NO_RENEGOTIATION));
-         return;
+         throw TLS_Exception(Alert::PROTOCOL_VERSION,
+                             "Client version is unacceptable by policy");
          }
 
       m_secure_renegotiation.update(m_state->client_hello);
diff --git a/src/tls/tls_version.cpp b/src/tls/tls_version.cpp
index 30e3855e7..f451da70e 100644
--- a/src/tls/tls_version.cpp
+++ b/src/tls/tls_version.cpp
@@ -46,6 +46,17 @@ bool Protocol_Version::operator>(const Protocol_Version& other) const
    return m_version > other.m_version;
    }
 
+Protocol_Version Protocol_Version::best_known_match() const
+   {
+   if(known_version())
+      return *this; // known version is its own best match
+
+   if(is_datagram_protocol())
+      return Protocol_Version::DTLS_V12;
+   else
+      return Protocol_Version::TLS_V12;
+   }
+
 bool Protocol_Version::known_version() const
    {
    // Don't include DTLS yet here as we can't actually process it
diff --git a/src/tls/tls_version.h b/src/tls/tls_version.h
index 4d1beb8cb..8112b2a11 100644
--- a/src/tls/tls_version.h
+++ b/src/tls/tls_version.h
@@ -72,6 +72,13 @@ class BOTAN_DLL Protocol_Version
       std::string to_string() const;
 
       /**
+      * If this version is known, return that. Otherwise return the
+      * best (most recent) version we know of.
+      * @return best matching protocol version
+      */
+      Protocol_Version best_known_match() const;
+
+      /**
       * @return true iff this is a DTLS version
       */
       bool is_datagram_protocol() const;