KEP-1606 - don't forward messages with invalid timestamps (#347)

pbft/pbft.cpp

@@ -300,16 +300,42 @@ pbft::setup_request_operation(const bzn_envelope& request_env, const bzn::hash_t
     return op;
 }
 
+void
+pbft::send_error_response(const bzn_envelope& request_env, const std::shared_ptr<session_base>& session
+    , const std::string& msg) const
+{
+    database_msg req;
+    if (session && req.ParseFromString(request_env.database_msg()))
+    {
+        database_response resp;
+        *resp.mutable_header() = req.header();
+        resp.mutable_error()->set_message(msg);
+
+        session->send_message(std::make_shared<std::string>(this->wrap_message(resp).SerializeAsString()));
+    }
+}
+
 void
 pbft::handle_request(const bzn_envelope& request_env, const std::shared_ptr<session_base>& session)
 {
+    if (request_env.timestamp() < (this->now() - MAX_REQUEST_AGE_MS) || request_env.timestamp() > (this->now() + MAX_REQUEST_AGE_MS))
+    {
+        this->send_error_response(request_env, session, TIMESTAMP_ERROR_MSG);
+
+        LOG(info) << "Rejecting request because it is outside allowable timestamp range: "
+                << request_env.ShortDebugString();
+        return;
+    }
+
     // Allowing the maximum size to simply be bzn::MAX_VALUE_SIZE does not take into account the overhead that the
     // primary peer will add to the message when it re-broadcasts the request to the other peers in the swarm. This
     // additional overhead has been experimentally determined, on MacOS, to be 245 bytes. For now we shall use the magic
     // number overhead size of 512 when we limit
     const size_t OVERHEAD_SIZE{512};
     if (!request_env.database_msg().empty() && request_env.database_msg().size() >= (bzn::MAX_VALUE_SIZE - OVERHEAD_SIZE))
     {
+        this->send_error_response(request_env, session, TOO_LARGE_ERROR_MSG);
+
         LOG(warning) << "Rejecting request because it is too large [" << request_env.database_msg().size() << " bytes]";
         return;
     }
@@ -332,20 +358,6 @@ pbft::handle_request(const bzn_envelope& request_env, const std::shared_ptr<sess
         return;
     }
 
-
-
-
-
-
-
-
-    if (request_env.timestamp() < (this->now() - MAX_REQUEST_AGE_MS) || request_env.timestamp() > (this->now() + MAX_REQUEST_AGE_MS))
-    {
-        // TODO: send error message to client
-        LOG(info) << "Rejecting request because it is outside allowable timestamp range: " << request_env.ShortDebugString();
-        return;
-    }
-
     // keep track of what requests we've seen based on timestamp and only send preprepares once
     if (this->already_seen_request(request_env, hash))
     {
@@ -890,30 +902,39 @@ pbft::get_primary(std::optional<uint64_t> view) const
     return this->current_peers()[view.value_or(this->view.value()) % this->current_peers().size()];
 }
 
+bzn_envelope
+pbft::wrap_message(bzn_envelope& env) const
+{
+    env.set_sender(this->uuid);
+    env.set_timestamp(this->now());
+    env.set_swarm_id(this->options->get_swarm_id());
+    this->crypto->sign(env);
+
+    return env;
+}
+
 bzn_envelope
 pbft::wrap_message(const pbft_msg& msg) const
 {
     bzn_envelope result;
     result.set_pbft(msg.SerializeAsString());
-    result.set_sender(this->uuid);
-    result.set_timestamp(this->now());
-    result.set_swarm_id(this->options->get_swarm_id());
-    this->crypto->sign(result);
-
-    return result;
+    return this->wrap_message(result);
 }
 
 bzn_envelope
 pbft::wrap_message(const pbft_membership_msg& msg) const
 {
     bzn_envelope result;
     result.set_pbft_membership(msg.SerializeAsString());
-    result.set_sender(this->uuid);
-    result.set_timestamp(this->now());
-    result.set_swarm_id(this->options->get_swarm_id());
-    this->crypto->sign(result);
-
-    return result;
+    return this->wrap_message(result);
+}
+
+bzn_envelope
+pbft::wrap_message(const database_response& msg) const
+{
+    bzn_envelope result;
+    result.set_database_response(msg.SerializeAsString());
+    return this->wrap_message(result);
 }
 
 const bzn::uuid_t&

pbft/pbft.hpp

@@ -52,6 +52,8 @@ namespace
     const std::string LAST_VIEW_SENT_KEY{"last_view_sent"};
     const std::string VALID_VIEWCHANGE_MESSAGES_FOR_VIEW_KEY{"valid_viewchange_messages_for_view"};
     const std::string SAVED_NEWVIEW_KEY{"saved_newview"};
+    const std::string TIMESTAMP_ERROR_MSG{"INVALID TIMESTAMP"};
+    const std::string TOO_LARGE_ERROR_MSG{"REQUEST TOO LARGE"};
 }
 
 
@@ -173,8 +175,10 @@ namespace bzn
         void handle_bzn_message(const bzn_envelope& msg, std::shared_ptr<bzn::session_base> session);
         void handle_membership_message(const bzn_envelope& msg, std::shared_ptr<bzn::session_base> session = nullptr);
 
+        bzn_envelope wrap_message(bzn_envelope& env) const;
         bzn_envelope wrap_message(const pbft_msg& message) const;
         bzn_envelope wrap_message(const pbft_membership_msg&) const;
+        bzn_envelope wrap_message(const database_response&) const;
 
         void async_signed_broadcast(const pbft_msg& msg);
         void async_signed_broadcast(const pbft_membership_msg& message);
@@ -188,6 +192,10 @@ namespace bzn
 
         void broadcast(const bzn_envelope& message);
 
+        void send_error_response(const bzn_envelope& request_env, const std::shared_ptr<session_base>& session
+            , const std::string& msg) const;
+
+
         void handle_audit_heartbeat_timeout(const boost::system::error_code& ec);
         void handle_new_config_timeout(const boost::system::error_code& ec);
         void handle_join_retry_timeout(const boost::system::error_code& ec);