Merge pull request #981 from jagerman/quorumnet-backtalk-guard

Quorumnet backtalk guard

src/cryptonote_core/cryptonote_core.cpp

@@ -1665,7 +1665,7 @@ namespace cryptonote
 
     auto quorum_votes = m_quorum_cop.get_relayable_votes(height, hf_version, true);
     auto p2p_votes    = m_quorum_cop.get_relayable_votes(height, hf_version, false);
-    if (!quorum_votes.empty())
+    if (!quorum_votes.empty() && m_quorumnet_obj && m_service_node_keys)
       quorumnet_relay_obligation_votes(m_quorumnet_obj, quorum_votes);
 
     if (!p2p_votes.empty())

src/cryptonote_protocol/quorumnet.cpp

@@ -89,6 +89,11 @@ struct SNNWrapper {
     template <typename... Args>
     SNNWrapper(cryptonote::core &core, Args &&...args) :
         snn{std::forward<Args>(args)...}, core{core} {}
+
+    static SNNWrapper &from(void* obj) {
+        assert(obj);
+        return *reinterpret_cast<SNNWrapper*>(obj);
+    }
 };
 
 template <typename T>
@@ -477,7 +482,7 @@ quorum_vote_t deserialize_vote(const bt_value &v) {
 }
 
 void relay_obligation_votes(void *obj, const std::vector<service_nodes::quorum_vote_t> &votes) {
-    auto &snw = *reinterpret_cast<SNNWrapper *>(obj);
+    auto &snw = SNNWrapper::from(obj);
 
     auto my_keys_ptr = snw.core.get_service_node_keys();
     assert(my_keys_ptr);
@@ -520,7 +525,7 @@ void relay_obligation_votes(void *obj, const std::vector<service_nodes::quorum_v
 }
 
 void handle_obligation_vote(SNNetwork::message &m, void *self) {
-    auto &snw = *reinterpret_cast<SNNWrapper *>(self);
+    auto &snw = SNNWrapper::from(self);
 
     MDEBUG("Received a relayed obligation vote from " << as_hex(m.pubkey));
 
@@ -811,7 +816,7 @@ void process_blink_signatures(SNNWrapper &snw, const std::shared_ptr<blink_tx> &
 ///           submission will fail immediately if it does not).
 ///
 void handle_blink(SNNetwork::message &m, void *self) {
-    auto &snw = *reinterpret_cast<SNNWrapper *>(self);
+    auto &snw = SNNWrapper::from(self);
 
     // TODO: if someone sends an invalid tx (i.e. one that doesn't get to the distribution stage)
     // then put a timeout on that IP during which new submissions from them are dropped for a short
@@ -824,7 +829,9 @@ void handle_blink(SNNetwork::message &m, void *self) {
 
     MDEBUG("Received a blink tx from " << (m.sn ? "SN " : "non-SN ") << as_hex(m.pubkey));
 
-    assert(snw.core.get_service_node_keys());
+    auto keys = snw.core.get_service_node_keys();
+    assert(keys);
+    if (!keys) return;
 
     if (m.data.size() != 1) {
         MINFO("Rejecting blink message: expected one data entry not " << m.data.size());
@@ -1047,9 +1054,8 @@ void handle_blink(SNNetwork::message &m, void *self) {
     }
 
     auto hash_to_sign = btx.hash(approved);
-    auto &keys = *snw.core.get_service_node_keys();
     crypto::signature sig;
-    generate_signature(hash_to_sign, keys.pub, keys.key, sig);
+    generate_signature(hash_to_sign, keys->pub, keys->key, sig);
 
     // Now that we have the blink tx stored we can add our signature *and* any other pending
     // signatures we are holding onto, then blast the entire thing to our peers.
@@ -1099,7 +1105,7 @@ void copy_signature_values(std::list<pending_signature> &signatures, const bt_va
 ///
 /// Signatures will be forwarded if new; known signatures will be ignored.
 void handle_blink_signature(SNNetwork::message &m, void *self) {
-    auto &snw = *reinterpret_cast<SNNWrapper *>(self);
+    auto &snw = SNNWrapper::from(self);
 
     MDEBUG("Received a blink tx signature from SN " << as_hex(m.pubkey));
 
@@ -1282,7 +1288,7 @@ std::future<std::pair<cryptonote::blink_result, std::string>> send_blink(void *o
     if (!blink_tag) return future;
 
     try {
-        auto &snw = *reinterpret_cast<SNNWrapper *>(obj);
+        auto &snw = SNNWrapper::from(obj);
         uint64_t height = snw.core.get_current_blockchain_height();
         uint64_t checksum;
         auto quorums = get_blink_quorums(height, snw.core.get_service_node_list(), nullptr, &checksum);
@@ -1392,7 +1398,7 @@ void common_blink_response(uint64_t tag, cryptonote::blink_result res, std::stri
 ///
 /// It's possible for some nodes to accept and others to refuse, so we don't actually set the
 /// promise unless we get a nostart response from a majority of the remotes.
-void handle_blink_not_started(SNNetwork::message &m, void *self) {
+void handle_blink_not_started(SNNetwork::message &m, void *) {
     if (m.data.size() != 1) {
         MERROR("Bad blink not started response: expected one data entry not " << m.data.size());
         return;
@@ -1411,7 +1417,7 @@ void handle_blink_not_started(SNNetwork::message &m, void *self) {
 ///
 ///     ! - the tag as included in the submission
 ///
-void handle_blink_failure(SNNetwork::message &m, void *self) {
+void handle_blink_failure(SNNetwork::message &m, void *) {
     if (m.data.size() != 1) {
         MERROR("Blink failure message not understood: expected one data entry not " << m.data.size());
         return;
@@ -1435,7 +1441,7 @@ void handle_blink_failure(SNNetwork::message &m, void *self) {
 ///
 ///     ! - the tag as included in the submission
 ///
-void handle_blink_success(SNNetwork::message &m, void *self) {
+void handle_blink_success(SNNetwork::message &m, void *) {
     if (m.data.size() != 1) {
         MERROR("Blink success message not understood: expected one data entry not " << m.data.size());
         return;
@@ -1461,30 +1467,30 @@ void init_core_callbacks() {
     cryptonote::quorumnet_send_blink = send_blink;
 
     // Receives an obligation vote
-    SNNetwork::register_quorum_command("vote_ob", handle_obligation_vote);
+    SNNetwork::register_command("vote_ob", SNNetwork::command_type::quorum, handle_obligation_vote);
 
     // Receives a new blink tx submission from an external node, or forward from other quorum
     // members who received it from an external node.
-    SNNetwork::register_public_command("blink", handle_blink);
+    SNNetwork::register_command("blink", SNNetwork::command_type::public_, handle_blink);
 
     // Sends a message back to the blink initiator that the transaction was NOT relayed, either
     // because the height was invalid or the quorum checksum failed.  This is only sent by the entry
     // point service nodes into the quorum to let it know the tx verification has not started from
     // that node.  It does not necessarily indicate a failure unless all entry point attempts return
     // the same.
-    SNNetwork::register_quorum_command("bl_nostart", handle_blink_not_started);
+    SNNetwork::register_command("bl_nostart", SNNetwork::command_type::response, handle_blink_not_started);
 
     // Sends a message from the entry SNs back to the initiator that the Blink tx has been rejected:
     // that is, enough signed rejections have occured that the Blink tx cannot be accepted.
-    SNNetwork::register_quorum_command("bl_bad", handle_blink_failure);
+    SNNetwork::register_command("bl_bad", SNNetwork::command_type::response, handle_blink_failure);
 
     // Sends a message from the entry SNs back to the initiator that the Blink tx has been accepted
     // and validated and is being broadcast to the network.
-    SNNetwork::register_quorum_command("bl_good", handle_blink_success);
+    SNNetwork::register_command("bl_good", SNNetwork::command_type::response, handle_blink_success);
 
     // Receives blink tx signatures or rejections between quorum members (either original or
     // forwarded).  These are propagated by the receiver if new
-    SNNetwork::register_quorum_command("blink_sign", handle_blink_signature);
+    SNNetwork::register_command("blink_sign", SNNetwork::command_type::quorum, handle_blink_signature);
 }
 
 }

src/quorumnet/sn_network.cpp

@@ -237,20 +237,27 @@ msg_view_t view(const zmq::message_t &m) {
 constexpr const std::chrono::milliseconds SNNetwork::default_send_keep_alive;
 #endif
 
-std::unordered_map<std::string, std::pair<std::function<void(SNNetwork::message &message, void *data)>, bool>> SNNetwork::commands;
-bool SNNetwork::commands_mutable = true;
-void SNNetwork::register_quorum_command(std::string command, std::function<void(SNNetwork::message &message, void *data)> callback) {
-    if (!commands_mutable)
-        throw std::logic_error("Failed to register quorum command: command must be added before constructing a SNNetwork instance");
-
-    commands.emplace(std::move(command), std::make_pair(std::move(callback), false));
+static std::string command_type_string(SNNetwork::command_type t) {
+    return
+            t == SNNetwork::command_type::quorum ? "quorum" :
+            t == SNNetwork::command_type::public_ ? "public" :
+            t == SNNetwork::command_type::response ? "response" :
+            "unknown";
+}
+static std::ostream& operator<<(std::ostream& o, SNNetwork::command_type t) {
+    return o << command_type_string(t);
 }
 
-void SNNetwork::register_public_command(std::string command, std::function<void(SNNetwork::message &message, void *data)> callback) {
-    if (!commands_mutable)
-        throw std::logic_error("Failed to register public command: command must be added before constructing a SNNetwork instance");
+std::unordered_map<std::string, std::pair<void(*)(SNNetwork::message &message, void *data), SNNetwork::command_type>> SNNetwork::commands;
+bool SNNetwork::commands_mutable = true;
+void SNNetwork::register_command(std::string command, command_type cmd_type,
+        void(*callback)(SNNetwork::message &message, void *data)) {
+    assert(cmd_type >= SNNetwork::command_type::quorum && cmd_type <= SNNetwork::command_type::response);
 
-    commands.emplace(std::move(command), std::make_pair(std::move(callback), true));
+    if (!commands_mutable)
+        throw std::logic_error("Failed to register " + command_type_string(cmd_type) + " command: command must be added before constructing a SNNetwork instance");
+
+    commands.emplace(std::move(command), std::make_pair(callback, cmd_type));
 }
 
 std::atomic<int> next_id{1};
@@ -443,10 +450,16 @@ void SNNetwork::worker_thread(std::string worker_id) {
                 continue;
             }
 
-            const bool &public_cmd = cmdit->second.second;
-            if (!public_cmd && !msg.sn) {
+            auto cmd_type = cmdit->second.second;
+            const bool command_accepted = (
+                cmd_type == command_type::response ? msg.sn :
+                cmd_type == command_type::quorum ? msg.sn && is_service_node() :
+                cmd_type == command_type::public_ ? is_service_node() :
+                false);
+            if (!command_accepted) {
                 // If they aren't valid, tell them so that they can disconnect (and attempt to reconnect later with appropriate authentication)
-                SN_LOG(warn, worker_id << "/" << object_id << " received quorum-only command " << msg.command << " from non-SN remote " << as_hex(msg.pubkey) << "; replying with a BYE");
+                SN_LOG(warn, worker_id << "/" << object_id << " received disallowed " << cmd_type << " command " << msg.command <<
+                        " from " << (msg.sn ? "non-" : "") << "SN remote " << as_hex(msg.pubkey) << "; replying with a BYE");
                 send(msg.pubkey, "BYE", send_option::incoming{});
                 detail::send_control(get_control_socket(), "DISCONNECT", {{"pubkey",msg.pubkey}});
                 continue;

src/quorumnet/sn_network.h

@@ -124,6 +124,9 @@ public:
     /// if desired.
     void *data = nullptr;
 
+    /// Possible command types; see register_command
+    enum class command_type { quorum, public_, response };
+
 private:
     zmq::context_t context;
 
@@ -305,7 +308,7 @@ private:
     ///
     /// The value is the {callback, public} pair where `public` is true if unauthenticated
     /// connections may call this and false if only authenricated SNs may invoke the command.
-    static std::unordered_map<std::string, std::pair<std::function<void(SNNetwork::message &message, void *data)>, bool>> commands;
+    static std::unordered_map<std::string, std::pair<void(*)(SNNetwork::message &message, void *data), command_type>> commands;
     static bool commands_mutable;
 
     /// Starts up the proxy thread; called during construction
@@ -327,7 +330,6 @@ public:
      * @param allow_incoming called on incoming connections with the (verified) incoming connection
      * pubkey (32-byte binary string) to determine whether the given SN should be allowed to
      * connect.
-     * @param data - an opaque pointer to pass along to command callbacks
      * @param want_log 
      * @param log a function pointer (or non-capturing lambda) to call to get a std::ostream pointer
      * to send output to, or nullptr to suppress output.  Optional; if omitted the default returns
@@ -336,7 +338,8 @@ public:
      * std::thread::hardware_concurrency().  Note that threads are only started on demand (i.e. when
      * a request arrives when all existing threads are busy handling requests).
      */
-    SNNetwork(std::string pubkey, std::string privkey,
+    SNNetwork(std::string pubkey,
+            std::string privkey,
             const std::vector<std::string> &bind,
             LookupFunc peer_lookup,
             AllowFunc allow_connection,
@@ -362,6 +365,12 @@ public:
      */
     ~SNNetwork();
 
+    /**
+     * Returns true if we are running as a service node, which (currently) is synonymous with us
+     * being started in listening mode.
+     */
+    bool is_service_node() const { return (bool) listener; }
+
     /**
      * Try to initiate a connection to the given SN in anticipation of needing a connection in the
      * future.  If a connection is already established, the connection's idle timer will be reset
@@ -424,26 +433,27 @@ public:
     const std::string &get_privkey() const { return privkey; }
 
     /**
-     * Registers a quorum command that may be invoked by authenticated SN connections but not
-     * unauthenticated (non-SN) connections.
+     * Registers a command that may be invoked on a quorumnet connection.  The quorum command
+     * is one of three types:
+     *     - `SNNetwork::command_type::quorum` - for a command that is only permitted between
+     *       registered service nodes.  It will be discarded if received from a remote that is not
+     *       recognized as a service node, or if the local node is not a service node.
+     *     - `SNNetwork::command_type::public_` - for a command that is permitted from anyone, but
+     *       only if the local node is running as a service node.
+     *     - `SNNetwork::command_type::response` - for a command that can be issued by a service
+     *       node to the local client (whether or not running as a service node), typically issued
+     *       in response to a `public_` command issued by this service node.
      *
      * Commands may only be registered before any SNNetwork instance has been constructed.
      *
      * @param command - the command string to assign.  If it already exists it will be replaced.
      * @param callback - a callback that takes the message info and the opaque `data` pointer
+     * @param cmd_type - the command type, as described above.
      */
-    static void register_quorum_command(std::string command, std::function<void(SNNetwork::message &message, void *data)> callback);
-
-    /**
-     * Registers a network command that may be invoked by both authenticated SN connections and
-     * unauthenticated (non-SN) connections.
-     *
-     * Commands may only be registered before any SNNetwork instance has been constructed.
-     *
-     * @param command - the command string to assign.  If it already exists it will be replaced.
-     * @param callback - a callback that takes the message info and the opaque `data` pointer
-     */
-    static void register_public_command(std::string command, std::function<void(SNNetwork::message &message, void *data)> callback);
+    static void register_command(
+            std::string command,
+            command_type cmd_type,
+            void(*callback)(SNNetwork::message &message, void *data));
 };
 
 /// Namespace for options to the send() method