Skip to content

Commit c9dd2bf

Browse files
Groot2Publisher: add a constructor that takes the bind address (#1251)
The publisher always bound its two ZeroMQ sockets to tcp://*, so they were reachable from every network interface. The Groot2 protocol has no authentication, and a client that can reach the port can not only read the tree and its blackboards but also insert hooks that replace node results. Add Groot2Publisher(tree, server_port, bind_address), e.g. "127.0.0.1" to accept local connections only. The existing constructor delegates to it with "*", so its behavior and exported symbol are unchanged. Document the security implications in the class comment. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 5ca367f commit c9dd2bf

4 files changed

Lines changed: 59 additions & 5 deletions

File tree

‎include/behaviortree_cpp/loggers/groot2_publisher.h‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,31 @@ namespace BT
1515
*
1616
* An inter-process communication mechanism allows the two processes
1717
* to communicate through a TCP port. The user should provide the
18-
* port to be used in the constructor.
18+
* port to be used in the constructor. The publisher binds two ports:
19+
* server_port and server_port + 1.
20+
*
21+
* SECURITY: the protocol has no authentication. Any client that can reach
22+
* these ports can read the tree and its blackboards, and can insert hooks
23+
* (breakpoints) that replace the result of a node, changing how the tree
24+
* executes. Do not expose them to untrusted networks; use the bind_address
25+
* argument to restrict the interface they listen on.
1926
*/
2027
class Groot2Publisher : public StatusChangeLogger
2128
{
2229
using Position = Monitor::Hook::Position;
2330

2431
public:
32+
/// Listens on all network interfaces.
2533
Groot2Publisher(const BT::Tree& tree, unsigned server_port = 1667);
2634

35+
/**
36+
* @param bind_address the interface to listen on, in ZeroMQ tcp syntax:
37+
* an IPv4 address ("127.0.0.1" for local connections only), an
38+
* interface name ("eth0"), or "*" for all interfaces.
39+
*/
40+
Groot2Publisher(const BT::Tree& tree, unsigned server_port,
41+
const std::string& bind_address);
42+
2743
~Groot2Publisher() override;
2844

2945
Groot2Publisher(const Groot2Publisher& other) = delete;

‎src/loggers/groot2_publisher.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,11 @@ struct Groot2Publisher::PImpl
121121
};
122122

123123
Groot2Publisher::Groot2Publisher(const BT::Tree& tree, unsigned server_port)
124+
: Groot2Publisher(tree, server_port, "*")
125+
{}
126+
127+
Groot2Publisher::Groot2Publisher(const BT::Tree& tree, unsigned server_port,
128+
const std::string& bind_address)
124129
: StatusChangeLogger(), _p(std::make_unique<PImpl>())
125130
{
126131
_p->server_port = server_port;
@@ -154,8 +159,9 @@ Groot2Publisher::Groot2Publisher(const BT::Tree& tree, unsigned server_port)
154159
}
155160
}
156161
//-------------------------------
157-
_p->server_address = StrCat("tcp://*:", std::to_string(server_port));
158-
_p->publisher_address = StrCat("tcp://*:", std::to_string(server_port + 1));
162+
_p->server_address = StrCat("tcp://", bind_address, ":", std::to_string(server_port));
163+
_p->publisher_address =
164+
StrCat("tcp://", bind_address, ":", std::to_string(server_port + 1));
159165

160166
_p->server.bind(_p->server_address.c_str());
161167
_p->publisher.bind(_p->publisher_address.c_str());

‎tests/groot2_test_utils.hpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ struct PublisherAndPort
2020
unsigned port = 0;
2121
};
2222

23-
inline PublisherAndPort makePublisher(const BT::Tree& tree)
23+
inline PublisherAndPort makePublisher(const BT::Tree& tree,
24+
const std::string& bind_address = "*")
2425
{
2526
std::random_device random_device;
2627
std::uniform_int_distribution<unsigned> port_distribution(20000, 60000);
@@ -32,7 +33,7 @@ inline PublisherAndPort makePublisher(const BT::Tree& tree)
3233
const unsigned port = port_distribution(random_device) & ~1U;
3334
try
3435
{
35-
return { std::make_unique<BT::Groot2Publisher>(tree, port), port };
36+
return { std::make_unique<BT::Groot2Publisher>(tree, port, bind_address), port };
3637
}
3738
catch(const zmq::error_t& error)
3839
{

‎tests/gtest_groot2_publisher_integration.cpp‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,37 @@ TEST(Groot2PublisherIntegration, IncompleteHookRequestDoesNotTerminateServer)
173173
EXPECT_EXIT(incompleteHookScenario(), ::testing::ExitedWithCode(EXIT_SUCCESS), ".*");
174174
}
175175

176+
TEST(Groot2PublisherIntegration, BindToLoopbackAddress)
177+
{
178+
BT::BehaviorTreeFactory factory;
179+
auto tree = factory.createTreeFromText(R"(
180+
<root BTCPP_format="4">
181+
<BehaviorTree ID="MainTree">
182+
<AlwaysSuccess/>
183+
</BehaviorTree>
184+
</root>
185+
)");
186+
auto [publisher, port] = Groot2Test::makePublisher(tree, "127.0.0.1");
187+
Groot2Test::Client client(port);
188+
189+
auto reply = client.request(BT::Monitor::RequestType::FULLTREE);
190+
ASSERT_EQ(reply.size(), 2);
191+
EXPECT_NE(reply[1].to_string().find("MainTree"), std::string::npos);
192+
}
193+
194+
TEST(Groot2PublisherIntegration, InvalidBindAddressThrows)
195+
{
196+
BT::BehaviorTreeFactory factory;
197+
auto tree = factory.createTreeFromText(R"(
198+
<root BTCPP_format="4">
199+
<BehaviorTree ID="MainTree">
200+
<AlwaysSuccess/>
201+
</BehaviorTree>
202+
</root>
203+
)");
204+
EXPECT_THROW(BT::Groot2Publisher(tree, 1667, "not-an-interface"), zmq::error_t);
205+
}
206+
176207
TEST(Groot2PublisherIntegration, BlackboardDump_NoRootWithoutExternalBlackboard)
177208
{
178209
BT::BehaviorTreeFactory factory;

0 commit comments

Comments
 (0)