Repository: randombit/botan (cpp)

Issue:
DTLS server can be used as network traffic reflector/amplifier for DDoS attacks
Botan DTLS server in versions 2.9.0 and 2.8.0 (and probably earlier ones) can be used to perform Distributed Denial of Service attacks as network amplifier with spoofing of the source IP address.

After receiving DTLS v1.2 Client Hello message (197 bytes long) Botan DTLS server responds with 6 DTLS v1.2 packets:
1. DTLS Alert (warning, unrecognized_name) - 15 bytes,
2. Server Hello - 110 bytes,
3. Certificate - 1022 bytes,
4. Server Key Exchange - 1085 bytes,
5. Server Key Exchange - 1083 bytes,
6. Server Hello Done - 25 bytes.

Together all response packets are 3340 bytes, which gives 1595% amplification factor (calculated as: output/input - 1).
Server responds to IP address from first packet without getting any concurrent response from client, so this request can be used for Distributed Denial of Service.

In case the destination (spoofed) client sends ICMP Host Unreachable to any responses, another set of packets is sent:
1. DTLS Alert (Warning, Unrecognized name) - 15 bytes,
2. Server Hello - 110 bytes,
3. Certificate - 1022 bytes,
4. DTLS Alert (Fatal, Internal Error) - 15 bytes.

Together all response packets are 1162 bytes, which gives 490% amplification factor, which is still enough to perform DDoS attacks.

Script that can be used to reproduce this issue was delivered to Jack Lloyd via encrypted email.

Pull request:
Add support for HelloVerifyRequest on server side
Closes #1833 

Patch:
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
@@ -313,6 +313,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);
 


Test patch:
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");