Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow connections between routers #112

Merged
merged 1 commit into from
Feb 19, 2024
Merged

Conversation

Yadunund
Copy link
Member

@Yadunund Yadunund commented Feb 16, 2024

Fix the default router config to listen on all connections established to the 7447 port.

Will add documentation on how to connect two routers, as part of the fix for #102

@Yadunund Yadunund requested a review from codebot February 16, 2024 10:22
Copy link
Collaborator

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hesitant about this one, from a security perspective. That is, with the current configuration, we are sure that nothing outside the current machine can possibly connect. If we enable this by default, then by default anyone can connect and gain access to the box.

Now, this situation isn't much different from DDS. But it still feels a little scary to do this by default. @codebot do you have any additional thoughts here?

@codebot
Copy link
Member

codebot commented Feb 16, 2024

I agree this opens up a plethora of security considerations, but I'm OK with it:

  • this is the default behavior of ROS 1 (accept inbound connections on any interface)
  • this is the default behavior of most other rmw_* in ROS 2, so I don't think people will be surprised.
  • this situation is (IMO) considerably better than the default DDS behavior of multicasting discovery packets; although our default config would open up a port for inbound connections, at least it's not also sending multicast packets around the LAN and automatically connecting to peers.

In general, I think that people expect that deploying ROS 2 into an untrusted / hostile environment requires lots of thinking about lots of network-configuration and security things, layered defenses, key management, etc.

I think the higher priority for rmw_zenoh is a "just works" out-of-the-box experience, including "I want to use rviz on my laptop to see data on my robot without having to learn about middleware config files".

Copy link
Member

@codebot codebot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; this causes the default behavior to align with ROS 1 and most other rmw_* in ROS 2.

@Yadunund Yadunund merged commit e77082f into rolling Feb 19, 2024
5 checks passed
@delete-merged-branch delete-merged-branch bot deleted the yadu/fix_router_listen_config branch February 19, 2024 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants