Merge GH #1999 Support DTLS HelloVerifyRequest on server side

12 files changed, 132 insertions, 13 deletions

diff --git a/doc/api_ref/credentials_manager.rst b/doc/api_ref/credentials_manager.rst
index bc18c5d8e..1f1963f82 100644
--- a/doc/api_ref/credentials_manager.rst
+++ b/doc/api_ref/credentials_manager.rst
@@ -129,9 +129,26 @@ authentication.
     is to generate a random key the first time ``psk`` is called to
     retrieve the session ticket key, cache it for later use in the
     ``Credentials_Manager``, and simply let it be thrown away when the
-    process terminates.
-
-    See :rfc:`4507` for more information about TLS session tickets.
+    process terminates. See :rfc:`4507` for more information about TLS
+    session tickets.
+
+    A similar special case exists for DTLS cookie verification. In
+    this case *type* will be "tls-server" and *context* is
+    "dtls-cookie-secret". If no key is returned, then DTLS cookies are
+    not used. Similar to the session ticket key, the DTLS cookie
+    secret can be chosen during server startup and rotated at any time
+    with no ill effect.
+
+    .. warning::
+
+       If DTLS cookies are not used then the server is prone to be
+       abused as a DoS amplifier, where the attacker sends a
+       relatively small client hello in a UDP packet with a forged
+       return address, and then the server replies to the victim with
+       several messages that are larger. This not only hides the
+       attackers address from the victim, but increases their
+       effective bandwidth. This is not an issue when using DTLS over
+       SCTP or TCP.
 
 .. cpp:function:: std::string psk_identity_hint(const std::string& type, \
                                                 const std::string& context)
diff --git a/src/bogo_shim/bogo_shim.cpp b/src/bogo_shim/bogo_shim.cpp
index 64fedf57f..ec4af181e 100644
--- a/src/bogo_shim/bogo_shim.cpp
+++ b/src/bogo_shim/bogo_shim.cpp
@@ -1170,6 +1170,11 @@ class Shim_Credentials final : public Botan::Credentials_Manager
                return Botan::SymmetricKey("ABCDEF0123456789");
             }
 
+         if(type == "tls-server" && context == "dtls-cookie-secret")
+            {
+            return Botan::SymmetricKey("F00FB00FD00F100F700F");
+            }
+
          if(identity != m_psk_identity)
             throw Shim_Exception("Unexpected PSK identity");
          return m_psk;
diff --git a/src/lib/tls/msg_client_hello.cpp b/src/lib/tls/msg_client_hello.cpp
index a3e1206df..149f3f0d4 100644
--- a/src/lib/tls/msg_client_hello.cpp
+++ b/src/lib/tls/msg_client_hello.cpp
@@ -259,6 +259,25 @@ std::vector<uint8_t> Client_Hello::serialize() const
    return buf;
    }
 
+std::vector<uint8_t> Client_Hello::cookie_input_data() const
+   {
+   std::vector<uint8_t> buf;
+
+   buf.push_back(m_version.major_version());
+   buf.push_back(m_version.minor_version());
+   buf += m_random;
+
+   append_tls_length_value(buf, m_session_id, 1);
+
+   append_tls_length_value(buf, m_suites, 2);
+   append_tls_length_value(buf, m_comp_methods, 1);
+
+   // Here we don't serialize the extensions since the client extensions
+   // may contain values we don't know how to serialize back.
+
+   return buf;
+   }
+
 /*
 * Read a counterparty client hello
 */
diff --git a/src/lib/tls/msg_hello_verify.cpp b/src/lib/tls/msg_hello_verify.cpp
index 648ca1a4e..bc93af9d6 100644
--- a/src/lib/tls/msg_hello_verify.cpp
+++ b/src/lib/tls/msg_hello_verify.cpp
@@ -35,7 +35,7 @@ Hello_Verify_Request::Hello_Verify_Request(const std::vector<uint8_t>& client_he
                                            const std::string& client_identity,
                                            const SymmetricKey& secret_key)
    {
-   std::unique_ptr<MessageAuthenticationCode> hmac(MessageAuthenticationCode::create("HMAC(SHA-256)"));
+   std::unique_ptr<MessageAuthenticationCode> hmac = MessageAuthenticationCode::create_or_throw("HMAC(SHA-256)");
    hmac->set_key(secret_key);
 
    hmac->update_be(static_cast<uint64_t>(client_hello_bits.size()));
@@ -43,7 +43,8 @@ Hello_Verify_Request::Hello_Verify_Request(const std::vector<uint8_t>& client_he
    hmac->update_be(static_cast<uint64_t>(client_identity.size()));
    hmac->update(client_identity);
 
-   m_cookie = unlock(hmac->final());
+   m_cookie.resize(hmac->output_length());
+   hmac->final(m_cookie.data());
    }
 
 std::vector<uint8_t> Hello_Verify_Request::serialize() const
diff --git a/src/lib/tls/tls_callbacks.cpp b/src/lib/tls/tls_callbacks.cpp
index bcd3aa39b..18868e0ef 100644
--- a/src/lib/tls/tls_callbacks.cpp
+++ b/src/lib/tls/tls_callbacks.cpp
@@ -33,6 +33,11 @@ std::string TLS::Callbacks::tls_server_choose_app_protocol(const std::vector<std
    return "";
    }
 
+std::string TLS::Callbacks::tls_peer_network_identity()
+   {
+   return "";
+   }
+
 void TLS::Callbacks::tls_modify_extensions(Extensions&, Connection_Side)
    {
    }
diff --git a/src/lib/tls/tls_callbacks.h b/src/lib/tls/tls_callbacks.h
index 325ed884b..c9ba070c0 100644
--- a/src/lib/tls/tls_callbacks.h
+++ b/src/lib/tls/tls_callbacks.h
@@ -314,6 +314,18 @@ class BOTAN_PUBLIC_API(2,0) Callbacks
        virtual std::string tls_decode_group_param(Group_Params group_param);
 
        /**
+       * Optional callback: return peer network identity
+       *
+       * There is no expected or specified format. The only expectation is this
+       * function will return a unique value. For example returning the peer
+       * host IP and port.
+       *
+       * This is used to bind the DTLS cookie to a particular network identity.
+       * It is only called if the dtls-cookie-secret PSK is also defined.
+       */
+       virtual std::string tls_peer_network_identity();
+
+       /**
        * Optional callback: error logging. (not currently called)
        * @param err An error message related to this connection.
        */
diff --git a/src/lib/tls/tls_client.cpp b/src/lib/tls/tls_client.cpp
index 7779481bc..12c95595d 100644
--- a/src/lib/tls/tls_client.cpp
+++ b/src/lib/tls/tls_client.cpp
@@ -279,7 +279,6 @@ void Client::process_handshake_msg(const Handshake_State* active_state,
       state.set_expected_next(HELLO_VERIFY_REQUEST); // might get it again
 
       Hello_Verify_Request hello_verify_request(contents);
-
       state.hello_verify_request(hello_verify_request);
       }
    else if(type == SERVER_HELLO)
diff --git a/src/lib/tls/tls_handshake_io.cpp b/src/lib/tls/tls_handshake_io.cpp
index 7ac868612..62f3bebb8 100644
--- a/src/lib/tls/tls_handshake_io.cpp
+++ b/src/lib/tls/tls_handshake_io.cpp
@@ -379,7 +379,6 @@ Datagram_Handshake_IO::format(const std::vector<uint8_t>& msg,
    return format_w_seq(msg, type, m_in_message_seq - 1);
    }
 
-
 std::vector<uint8_t>
 Datagram_Handshake_IO::send(const Handshake_Message& msg)
    {
@@ -392,6 +391,13 @@ Datagram_Handshake_IO::send(const Handshake_Message& msg)
       m_send_hs(epoch, CHANGE_CIPHER_SPEC, msg_bits);
       return std::vector<uint8_t>(); // not included in handshake hashes
       }
+   else if(msg_type == HELLO_VERIFY_REQUEST)
+      {
+      // This message is not included in the handshake hashes
+      send_message(m_out_message_seq, epoch, msg_type, msg_bits);
+      m_out_message_seq += 1;
+      return std::vector<uint8_t>();
+      }
 
    // Note: not saving CCS, instead we know it was there due to change in epoch
    m_flights.rbegin()->push_back(m_out_message_seq);
diff --git a/src/lib/tls/tls_handshake_state.cpp b/src/lib/tls/tls_handshake_state.cpp
index 8bc603a43..8261cefbc 100644
--- a/src/lib/tls/tls_handshake_state.cpp
+++ b/src/lib/tls/tls_handshake_state.cpp
@@ -203,8 +203,16 @@ void Handshake_State::hello_verify_request(const Hello_Verify_Request& hello_ver
 
 void Handshake_State::client_hello(Client_Hello* client_hello)
    {
-   m_client_hello.reset(client_hello);
-   note_message(*m_client_hello);
+   if(client_hello == nullptr)
+      {
+      m_client_hello.reset();
+      hash().reset();
+      }
+   else
+      {
+      m_client_hello.reset(client_hello);
+      note_message(*m_client_hello);
+      }
    }
 
 void Handshake_State::server_hello(Server_Hello* server_hello)
@@ -304,9 +312,11 @@ void Handshake_State::confirm_transition_to(Handshake_Type handshake_msg)
    const bool ok = (m_hand_expecting_mask & mask) != 0; // overlap?
 
    if(!ok)
+      {
       throw Unexpected_Message("Unexpected state transition in handshake, expected " +
                                handshake_mask_to_string(m_hand_expecting_mask, '|') +
                                " received " + handshake_mask_to_string(m_hand_received_mask, '+'));
+      }
 
    /* We don't know what to expect next, so force a call to
       set_expected_next; if it doesn't happen, the next transition
diff --git a/src/lib/tls/tls_messages.h b/src/lib/tls/tls_messages.h
index 7c35bff87..e67b82888 100644
--- a/src/lib/tls/tls_messages.h
+++ b/src/lib/tls/tls_messages.h
@@ -53,7 +53,7 @@ class BOTAN_UNSTABLE_API Hello_Verify_Request final : public Handshake_Message
       std::vector<uint8_t> serialize() const override;
       Handshake_Type type() const override { return HELLO_VERIFY_REQUEST; }
 
-      std::vector<uint8_t> cookie() const { return m_cookie; }
+      const std::vector<uint8_t>& cookie() const { return m_cookie; }
 
       explicit Hello_Verify_Request(const std::vector<uint8_t>& buf);
 
@@ -146,6 +146,10 @@ class BOTAN_UNSTABLE_API Client_Hello final : public Handshake_Message
 
       void update_hello_cookie(const Hello_Verify_Request& hello_verify);
 
+      const std::vector<uint8_t>& cookie() const { return m_hello_cookie; }
+
+      std::vector<uint8_t> cookie_input_data() const;
+
       std::set<Handshake_Extension_Type> extension_types() const
          { return m_extensions.extension_types(); }
 
diff --git a/src/lib/tls/tls_server.cpp b/src/lib/tls/tls_server.cpp
index d2474c225..c0c5e1a5c 100644
--- a/src/lib/tls/tls_server.cpp
+++ b/src/lib/tls/tls_server.cpp
@@ -493,8 +493,9 @@ void Server::process_client_hello_msg(const Handshake_State* active_state,
 
    pending_state.client_hello(new Client_Hello(contents));
    const Protocol_Version client_offer = pending_state.client_hello()->version();
+   const bool datagram = client_offer.is_datagram_protocol();
 
-   if(client_offer.is_datagram_protocol())
+   if(datagram)
       {
       if(client_offer.major_version() == 0xFF)
          throw TLS_Exception(Alert::PROTOCOL_VERSION, "Client offered DTLS version with major version 0xFF");
@@ -507,19 +508,54 @@ void Server::process_client_hello_msg(const Handshake_State* active_state,
          throw TLS_Exception(Alert::PROTOCOL_VERSION, "SSLv3 is not supported");
       }
 
+   /*
+   * BoGo test suite expects that we will send the hello verify with a record
+   * version matching the version that is eventually negotiated. This is wrong
+   * but harmless, so go with it. Also doing the version negotiation step first
+   * allows to immediately close the connection with an alert if the client has
+   * offered a version that we are not going to negotiate anyway, instead of
+   * making them first do the cookie exchange and then telling them no.
+   *
+   * There is no issue with amplification here, since the alert is just 2 bytes.
+   */
    const Protocol_Version negotiated_version =
       select_version(policy(), client_offer,
                      active_state ? active_state->version() : Protocol_Version(),
                      pending_state.client_hello()->sent_fallback_scsv(),
                      pending_state.client_hello()->supported_versions());
 
+   pending_state.set_version(negotiated_version);
+
    const auto compression_methods = pending_state.client_hello()->compression_methods();
    if(!value_exists(compression_methods, uint8_t(0)))
       throw TLS_Exception(Alert::ILLEGAL_PARAMETER, "Client did not offer NULL compression");
 
-   secure_renegotiation_check(pending_state.client_hello());
+   if(initial_handshake && datagram)
+      {
+      SymmetricKey cookie_secret;
 
-   pending_state.set_version(negotiated_version);
+      try
+         {
+         cookie_secret = m_creds.psk("tls-server", "dtls-cookie-secret", "");
+         }
+      catch(...) {}
+
+      if(cookie_secret.size() > 0)
+         {
+         const std::string client_identity = callbacks().tls_peer_network_identity();
+         Hello_Verify_Request verify(pending_state.client_hello()->cookie_input_data(), client_identity, cookie_secret);
+
+         if(pending_state.client_hello()->cookie() != verify.cookie())
+            {
+            pending_state.handshake_io().send(verify);
+            pending_state.client_hello(nullptr);
+            pending_state.set_expected_next(CLIENT_HELLO);
+            return;
+            }
+         }
+      }
+
+   secure_renegotiation_check(pending_state.client_hello());
 
    callbacks().tls_examine_extensions(pending_state.client_hello()->extensions(), CLIENT);
 
diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp
index 34c25e9ef..c3355b118 100644
--- a/src/tests/unit_tls.cpp
+++ b/src/tests/unit_tls.cpp
@@ -176,6 +176,11 @@ class Credentials_Manager_Test final : public Botan::Credentials_Manager
             return Botan::SymmetricKey("AABBCCDDEEFF012345678012345678");
             }
 
+         if(type == "tls-server" && context == "dtls-cookie-secret")
+            {
+            return Botan::SymmetricKey("4AEA5EAD279CADEB537A594DA0E9DE3A");
+            }
+
          if(context == "server.example.com" && type == "tls-client")
             {
             return Botan::SymmetricKey("20B602D1475F2DF888FCB60D2AE03AFD");