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

Network refactoring for remoteconsole #1157

Merged
merged 4 commits into from
Jan 26, 2023
Merged

Network refactoring for remoteconsole #1157

merged 4 commits into from
Jan 26, 2023

Conversation

moritz-h
Copy link
Member

@moritz-h moritz-h commented Jan 24, 2023

Summary of Changes

  • Use a single zmq socket for all connections (ZMQ_ROUTER) instead of the previously used two step connection startup. This simplifies the code, removes unnecessary threads and makes MegaMol a lot more firewall friendly, because no additional randomly chosen ports are used.

@moritz-h moritz-h requested review from geringsj and reinago January 24, 2023 17:50
@moritz-h moritz-h marked this pull request as ready for review January 24, 2023 17:50
@geringsj
Copy link
Contributor

While we are at it, we could also fix Issue #1114 and change the default port for host to something other than 33333

@moritz-h
Copy link
Member Author

#1114 is not about changing the port, but changing the network socket to listen on any incoming connection instead of just on localhost. So setting host address to tcp://*:33333 instead of tcp://127.0.0.1:33333. This would make it way more convenient when the remote console just works. On the other hand this is a probably a pretty bad default state from security perspective. Opening a remote port, accepting any incoming connection (without any form of authentication) and allowing arbitrary lua code execution is probably not far from a security nightmare.
Maybe instead we just add the setting to the default config file, that you can just edit the config directly instead of must finding out the correct command every time. A little more work would be to add some basic password authentication to the remote console or some other security solution.
Maybe we should bring this up in the next team meeting as it is the classical convenience vs security design decision.

@moritz-h moritz-h merged commit 63bc538 into master Jan 26, 2023
@moritz-h moritz-h deleted the remoteconsole branch January 26, 2023 16:03
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.

2 participants