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

Harden Jamulus server for unwanted connections #3107

Open
ann0see opened this issue Jul 20, 2023 · 6 comments
Open

Harden Jamulus server for unwanted connections #3107

ann0see opened this issue Jul 20, 2023 · 6 comments
Labels
feature request Feature request

Comments

@ann0see
Copy link
Member

ann0see commented Jul 20, 2023

Even though #2935 is already old, I think we should still consider hardening the "connection" process.
@dtinth proposed a "waiting room" approach, which we kind of already have (we already have something like this:

bIsIdentified ( false ),
).

I have another suggestion for a fix.

Problems with checking for valid audio packet is that we need to know 2 things in order to successfully decode a packet: (1) which decoder to use and (2) whether sequence numbers are used. These information is sent out-of-band and not in Opus audio packet, so we need to know their NETW_TRANSPORT_PROPS before we can succesfully decode an audio packet. We can also brute-force all combinations but efficiency is in question.

So here’s a new proposal:

Upon server receiving a packet from an unknown source, put them in a waiting room (do not assign a channel ID yet, dropping all audio packets in the meantime) and send it PROTMESSID_REQ_NETW_TRANSPORT_PROPS. It turns out that a Jamulus client can respond to that message before it has a channel ID assigned. Once an acknowledgement (or a reply, or a well-formed reply) is received, move them from waiting room to jamming room (where all messages are processed as usual).

Originally posted by @dtinth in #2935 (comment)

@mcfnord
Copy link
Contributor

mcfnord commented Jul 21, 2023

Is this code change intended to thwart the connection floods we've seen? I believe those floods aren't possible when a server's firewall blocks UDP packets from random sources. Would this code change benefit servers that block random UDP packets?

@ann0see
Copy link
Member Author

ann0see commented Jul 21, 2023

Is this code change intended to thwart the connection floods we've seen?

There's no code yet, but yes, it should make the software a bit stricter.

I believe those floods aren't possible when a server's firewall blocks UDP packets from random sources.

The problem is: how would you conclude that some source is "random"? Having a firewall surely helps.
My server even uses a blacklist - but that's a protection on a lower level and mainly protects other services.

@ann0see
Copy link
Member Author

ann0see commented Jul 21, 2023

It would be good to see if we can even reproduce the observed behaviour on modern servers. Maybe bIsIdentified is enough

@mcfnord
Copy link
Contributor

mcfnord commented Jul 21, 2023

A source is random when it doesn't rely on the directory server to open a connection prior to use. I believe this prevents random connections outside the directory protocol from reaching a server. So my servers should be immune to this particular attack. I could open UDP and make a server susceptible to that type of attack. Of course, private servers (more accurately called unlisted servers) must open UDP to anyone.

@ann0see
Copy link
Member Author

ann0see commented Jul 21, 2023

Ok. But as you said that doesn't hold true for unlisted servers or direct connections. So that's only a small "solution" if you even want to call it that.

@mcfnord
Copy link
Contributor

mcfnord commented Jul 21, 2023

Thanks for watching out for users in all scenarios. I do limit my scope to developing the directory-based public network.

@pljones pljones added the feature request Feature request label Aug 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request
Projects
Status: Triage
Development

No branches or pull requests

3 participants