From b5dbd7942f8896367e65cbc8f58e9bfbce81d953 Mon Sep 17 00:00:00 2001
From: Nik Bougalis <nikb@bougalis.net>
Date: Thu, 21 Apr 2016 15:03:04 -0700
Subject: [PATCH] Correctly handle connections that fail security checks
 (RIPD-1114):

* Return error code 400 to the peer along with a descriptive message
* Release the slot and decrement IP connection counters.
---
 Builds/VisualStudio2015/RippleD.vcxproj       |  2 +
 .../VisualStudio2015/RippleD.vcxproj.filters  |  3 ++
 src/ripple/overlay/impl/OverlayImpl.cpp       | 48 ++++++++++++++++++-
 src/ripple/overlay/impl/OverlayImpl.h         |  5 ++
 src/ripple/server/SimpleWriter.h              | 20 ++++----
 5 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/Builds/VisualStudio2015/RippleD.vcxproj b/Builds/VisualStudio2015/RippleD.vcxproj
index a27349891..495589d1b 100644
--- a/Builds/VisualStudio2015/RippleD.vcxproj
+++ b/Builds/VisualStudio2015/RippleD.vcxproj
@@ -3471,6 +3471,8 @@
     </ClInclude>
     <ClInclude Include="..\..\src\ripple\server\Session.h">
     </ClInclude>
+    <ClInclude Include="..\..\src\ripple\server\SimpleWriter.h">
+    </ClInclude>
     <ClCompile Include="..\..\src\ripple\server\tests\Server_test.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
diff --git a/Builds/VisualStudio2015/RippleD.vcxproj.filters b/Builds/VisualStudio2015/RippleD.vcxproj.filters
index eebae5ed1..4b9fabe3a 100644
--- a/Builds/VisualStudio2015/RippleD.vcxproj.filters
+++ b/Builds/VisualStudio2015/RippleD.vcxproj.filters
@@ -3930,6 +3930,9 @@
     <ClInclude Include="..\..\src\ripple\server\Session.h">
       <Filter>ripple\server</Filter>
     </ClInclude>
+    <ClInclude Include="..\..\src\ripple\server\SimpleWriter.h">
+      <Filter>ripple\server</Filter>
+    </ClInclude>
     <ClCompile Include="..\..\src\ripple\server\tests\Server_test.cpp">
       <Filter>ripple\server\tests</Filter>
     </ClCompile>
diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp
index d87c538fe..d021723a3 100644
--- a/src/ripple/overlay/impl/OverlayImpl.cpp
+++ b/src/ripple/overlay/impl/OverlayImpl.cpp
@@ -26,6 +26,7 @@
 #include <ripple/basics/make_SSLContext.h>
 #include <ripple/protocol/JsonFields.h>
 #include <ripple/server/JsonWriter.h>
+#include <ripple/server/SimpleWriter.h>
 #include <ripple/overlay/Cluster.h>
 #include <ripple/overlay/impl/ConnectAttempt.h>
 #include <ripple/overlay/impl/OverlayImpl.h>
@@ -231,16 +232,30 @@ OverlayImpl::onHandoff (std::unique_ptr <beast::asio::ssl_bundle>&& ssl_bundle,
         }
     }
 
-    handoff.moved = true;
-
     auto hello = parseHello (true, request.headers, journal);
     if(! hello)
+    {
+        m_peerFinder->on_closed(slot);
+        handoff.moved = false;
+        handoff.response = makeErrorResponse (slot, request,
+            remote_endpoint.address(),
+            "Unable to parse HELLO message");
+        handoff.keep_alive = false;
         return handoff;
+    }
 
     auto sharedValue = makeSharedValue(
         ssl_bundle->stream.native_handle(), journal);
     if(! sharedValue)
+    {
+        m_peerFinder->on_closed(slot);
+        handoff.moved = false;
+        handoff.response = makeErrorResponse (slot, request,
+            remote_endpoint.address(),
+            "Incorrect security cookie (possible MITM detected)");
+        handoff.keep_alive = false;
         return handoff;
+    }
 
     auto publicKey = verifyHello (*hello,
         *sharedValue,
@@ -248,7 +263,15 @@ OverlayImpl::onHandoff (std::unique_ptr <beast::asio::ssl_bundle>&& ssl_bundle,
         beast::IPAddressConversion::from_asio(
             remote_endpoint), journal, app_);
     if(! publicKey)
+    {
+        m_peerFinder->on_closed(slot);
+        handoff.moved = false;
+        handoff.response = makeErrorResponse (slot, request,
+            remote_endpoint.address(),
+            "Unable to verify HELLO message");
+        handoff.keep_alive = false;
         return handoff;
+    }
 
     auto const result = m_peerFinder->activate (slot, *publicKey,
         static_cast<bool>(app_.cluster().member(*publicKey)));
@@ -348,6 +371,27 @@ OverlayImpl::makeRedirectResponse (PeerFinder::Slot::ptr const& slot,
     return response;
 }
 
+std::shared_ptr<Writer>
+OverlayImpl::makeErrorResponse (PeerFinder::Slot::ptr const& slot,
+    http_request_type const& request,
+    address_type remote_address,
+    std::string msg)
+{
+    beast::deprecated_http::message m;
+    m.version(std::make_pair(request.version / 10, request.version % 10));
+    m.request(false);
+    m.status(400);
+    m.reason("Bad Request");
+    m.headers.insert("Remote-Address", remote_address.to_string());
+
+    auto response = std::make_shared<SimpleWriter> (std::move(m));
+
+    if (!msg.empty())
+        response->body (msg);
+
+    return response;
+}
+
 //------------------------------------------------------------------------------
 
 void
diff --git a/src/ripple/overlay/impl/OverlayImpl.h b/src/ripple/overlay/impl/OverlayImpl.h
index e551e4eb1..64ea17d8f 100644
--- a/src/ripple/overlay/impl/OverlayImpl.h
+++ b/src/ripple/overlay/impl/OverlayImpl.h
@@ -276,6 +276,11 @@ private:
     makeRedirectResponse (PeerFinder::Slot::ptr const& slot,
         http_request_type const& request, address_type remote_address);
 
+    std::shared_ptr<Writer>
+    makeErrorResponse (PeerFinder::Slot::ptr const& slot,
+        http_request_type const& request, address_type remote_address,
+        std::string msg);
+
     bool
     processRequest (http_request_type const& req,
         Handoff& handoff);
diff --git a/src/ripple/server/SimpleWriter.h b/src/ripple/server/SimpleWriter.h
index 25e21df74..1a364f507 100644
--- a/src/ripple/server/SimpleWriter.h
+++ b/src/ripple/server/SimpleWriter.h
@@ -21,30 +21,29 @@
 #define RIPPLE_SERVER_SIMPLEWRITER_H_INCLUDED
 
 #include <ripple/server/Writer.h>
-#include <beast/asio/streambuf.h>
-#include <beast/http/message.h>
+#include <ripple/beast/deprecated_http.h>
+#include <beast/streambuf.hpp>
 #include <utility>
 
 namespace ripple {
-namespace HTTP {
 
 /** Writer that sends a simple HTTP response with a message body. */
 class SimpleWriter : public Writer
 {
 private:
-    beast::http::message message_;
-    beast::asio::streambuf streambuf_;
+    beast::deprecated_http::message message_;
+    beast::streambuf streambuf_;
     std::string body_;
     bool prepared_ = false;
 
 public:
     explicit
-    SimpleWriter(beast::http::message&& message)
-        : message_(std::forward<beast::http::message>(message))
+    SimpleWriter(beast::deprecated_http::message&& message)
+        : message_(std::forward<beast::deprecated_http::message>(message))
     {
     }
 
-    beast::http::message&
+    beast::deprecated_http::message&
     message()
     {
         return message_;
@@ -95,14 +94,13 @@ private:
     {
         prepared_ = true;
         message_.headers.erase("Content-Length");
-        message_.headers.append("Content-Length",
+        message_.headers.insert("Content-Length",
             std::to_string(body_.size()));
         write(streambuf_, message_);
-        write(streambuf_, body_;
+        beast::http::detail::write(streambuf_, body_);
     }
 };
 
-}
 }
 
 #endif