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

ControlAddr should default to 0.0.0.0 if a target isn't localhost #793

Closed
yaxu opened this issue Feb 23, 2021 · 5 comments · Fixed by #1027
Closed

ControlAddr should default to 0.0.0.0 if a target isn't localhost #793

yaxu opened this issue Feb 23, 2021 · 5 comments · Fixed by #1027

Comments

@yaxu
Copy link
Member

yaxu commented Feb 23, 2021

Tidal now uses the control socket to send OSC out as well as receive OSC. This is so that superdirt knows where to send replies. However if superdirt is running on a remote host, this means that the default ControlAddr to listen on of localhost (127.0.0.1:6010) stops OSC from being sent.

The default should probably be to listen on all IPs, i.e. 0.0.0.0, if there is a target other than 127.0.0.1. However people should be able to set a specific IP if they want.

Relatedly, the oHandshake setting on Target is ignored. If it's set to True, maybe OSC should be sent out on an arbitrary IP.

@matthewkaney
Copy link
Contributor

I'd be interested in working out a fix for this. I'm curious, would it make sense to you for targets to listen for handshake messages on their own sockets (separate from the stream-wide control socket)? That seems like it would have the added benefit (in my view) of allowing targets to implement their own handshake protocols without conflicting with each other.

Is there a particular reason that handshake responses are handled by the control listener (other than it being a good existing starting point for building the handshake logic on)?

@yaxu
Copy link
Member Author

yaxu commented Feb 24, 2021

That'd be great! Yes I think you're right, it would be best to have a distinct socket for each target and control listener. It was indeed just a case of wanting to get something working quickly.

@matthewkaney
Copy link
Contributor

After thinking this over, I've changed my mind on separate ports. Thinking in terms of #797, I could imagine the control port becoming the sort of general-purpose OSC API for a stream. Perhaps the clock server functionality discussed in #681 could also be sent on that port as well? If targets had a way of specifying additional responses, then I think that OSC paths and the IP address/port of the sender gives you enough info for routing messages.

Happy to sketch some of these ideas out in a PR for consideration. I can work in small increments so it's easier to review.

One question: what was the original motivation for disabling the control port (via cCtrlListen)? I'm guessing it's a security concern? A solution to the 127.0.0.1 vs 0.0.0.0 problem listed above could just be to always open the port listening on all addresses (for handshakes etc), but not respond to /ctrl messages if the cCtrlListen flag is false.

@telephon
Copy link
Contributor

telephon commented Mar 1, 2021

Tidal now uses the control socket to send OSC out as well as receive OSC. This is so that superdirt knows where to send replies.

Since we have a handshake, tidal could also send the port and address which superdirt should send to.

@yaxu
Copy link
Member Author

yaxu commented Mar 1, 2021

Yes we could have a single port for everything. That means if we want to listen for OSC from more than one network we have to listen to all network s (0.0.0.0).. But if that is a problem for someone, they could apply firewall rules.

It would also mean that there would have to be communication to the different threads.. But we have MVars everywhere anyway. Probably by consolidating all this functionality - control messages, tempo changes, and tidal<>superdirt api messages, then I think things would be a lot simpler overall. It would add a bit of a bottleneck, but we could make sure any time-costly activity is in a different thread and isn't blocking anything else unnecessarily.

One question: what was the original motivation for disabling the control port (via cCtrlListen)?

I think partly for security reasons, but mostly in case there were multiple copies of tidal running. If things get consolidated I wouldn't worry about switching off /ctrl messages.

I think superdirt might listen on 0.0.0.0 by default now.. If so it probably makes sense for tidal to have the same default.

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 a pull request may close this issue.

3 participants