diff --git a/include/behaviortree_cpp/loggers/groot2_publisher.h b/include/behaviortree_cpp/loggers/groot2_publisher.h index c87ae9444..1e8595382 100644 --- a/include/behaviortree_cpp/loggers/groot2_publisher.h +++ b/include/behaviortree_cpp/loggers/groot2_publisher.h @@ -15,15 +15,31 @@ namespace BT * * An inter-process communication mechanism allows the two processes * to communicate through a TCP port. The user should provide the - * port to be used in the constructor. + * port to be used in the constructor. The publisher binds two ports: + * server_port and server_port + 1. + * + * SECURITY: the protocol has no authentication. Any client that can reach + * these ports can read the tree and its blackboards, and can insert hooks + * (breakpoints) that replace the result of a node, changing how the tree + * executes. Do not expose them to untrusted networks; use the bind_address + * argument to restrict the interface they listen on. */ class Groot2Publisher : public StatusChangeLogger { using Position = Monitor::Hook::Position; public: + /// Listens on all network interfaces. Groot2Publisher(const BT::Tree& tree, unsigned server_port = 1667); + /** + * @param bind_address the interface to listen on, in ZeroMQ tcp syntax: + * an IPv4 address ("127.0.0.1" for local connections only), an + * interface name ("eth0"), or "*" for all interfaces. + */ + Groot2Publisher(const BT::Tree& tree, unsigned server_port, + const std::string& bind_address); + ~Groot2Publisher() override; Groot2Publisher(const Groot2Publisher& other) = delete; diff --git a/src/loggers/groot2_publisher.cpp b/src/loggers/groot2_publisher.cpp index af740e756..4b8a363cc 100644 --- a/src/loggers/groot2_publisher.cpp +++ b/src/loggers/groot2_publisher.cpp @@ -121,6 +121,11 @@ struct Groot2Publisher::PImpl }; Groot2Publisher::Groot2Publisher(const BT::Tree& tree, unsigned server_port) + : Groot2Publisher(tree, server_port, "*") +{} + +Groot2Publisher::Groot2Publisher(const BT::Tree& tree, unsigned server_port, + const std::string& bind_address) : StatusChangeLogger(), _p(std::make_unique()) { _p->server_port = server_port; @@ -154,8 +159,9 @@ Groot2Publisher::Groot2Publisher(const BT::Tree& tree, unsigned server_port) } } //------------------------------- - _p->server_address = StrCat("tcp://*:", std::to_string(server_port)); - _p->publisher_address = StrCat("tcp://*:", std::to_string(server_port + 1)); + _p->server_address = StrCat("tcp://", bind_address, ":", std::to_string(server_port)); + _p->publisher_address = + StrCat("tcp://", bind_address, ":", std::to_string(server_port + 1)); _p->server.bind(_p->server_address.c_str()); _p->publisher.bind(_p->publisher_address.c_str()); diff --git a/tests/groot2_test_utils.hpp b/tests/groot2_test_utils.hpp index e63cdb1e0..10a63342f 100644 --- a/tests/groot2_test_utils.hpp +++ b/tests/groot2_test_utils.hpp @@ -20,7 +20,8 @@ struct PublisherAndPort unsigned port = 0; }; -inline PublisherAndPort makePublisher(const BT::Tree& tree) +inline PublisherAndPort makePublisher(const BT::Tree& tree, + const std::string& bind_address = "*") { std::random_device random_device; std::uniform_int_distribution port_distribution(20000, 60000); @@ -32,7 +33,7 @@ inline PublisherAndPort makePublisher(const BT::Tree& tree) const unsigned port = port_distribution(random_device) & ~1U; try { - return { std::make_unique(tree, port), port }; + return { std::make_unique(tree, port, bind_address), port }; } catch(const zmq::error_t& error) { diff --git a/tests/gtest_groot2_publisher_integration.cpp b/tests/gtest_groot2_publisher_integration.cpp index 41b1c4625..8c5e70512 100644 --- a/tests/gtest_groot2_publisher_integration.cpp +++ b/tests/gtest_groot2_publisher_integration.cpp @@ -173,6 +173,37 @@ TEST(Groot2PublisherIntegration, IncompleteHookRequestDoesNotTerminateServer) EXPECT_EXIT(incompleteHookScenario(), ::testing::ExitedWithCode(EXIT_SUCCESS), ".*"); } +TEST(Groot2PublisherIntegration, BindToLoopbackAddress) +{ + BT::BehaviorTreeFactory factory; + auto tree = factory.createTreeFromText(R"( + + + + + +)"); + auto [publisher, port] = Groot2Test::makePublisher(tree, "127.0.0.1"); + Groot2Test::Client client(port); + + auto reply = client.request(BT::Monitor::RequestType::FULLTREE); + ASSERT_EQ(reply.size(), 2); + EXPECT_NE(reply[1].to_string().find("MainTree"), std::string::npos); +} + +TEST(Groot2PublisherIntegration, InvalidBindAddressThrows) +{ + BT::BehaviorTreeFactory factory; + auto tree = factory.createTreeFromText(R"( + + + + + +)"); + EXPECT_THROW(BT::Groot2Publisher(tree, 1667, "not-an-interface"), zmq::error_t); +} + TEST(Groot2PublisherIntegration, BlackboardDump_NoRootWithoutExternalBlackboard) { BT::BehaviorTreeFactory factory;