Discussing potential security issues

[corrados]

The number of Jamulus servers in the server list grew significantly in the last three month. If the Jamulus software has security issues, a lot more people are affected now as it was in the past. Therefore I open this Issue to start a discussion about potential security issues.

Since Jamulus opens a network port and listens for UDP packets, this is the entry point for possible attacks. Basically Jamulus detects if the received UDP packet is a protocol message and parses it. If it detects that it is no protocol message and the size of the packet is correct, it puts it directly into the OPUS decoder. Assuming that the OPUS decoder is stable, the most critical part is the protocol parser. To test the parser with random messages, a test bench is already implemented in Jamulus: https://github.com/corrados/jamulus/blob/master/src/testbench.h. More advanced tests would be to use tools like this: https://lcamtuf.coredump.cx/afl

I read posts about considering encryption or sign the network packets. But actually I do not see a need for that. Jamulus is an open system so encryption of the audio packets is not necessary in my opinion since someone who wants to hear the stream can simply connect to the server. Signing will also not work since an arbitrary client can connect to any available server.

Since I am no security expert, with this Issue I just want to collect your opinions on that matter. If this discussion leads to specific implementation need, a new Issue shall created for it. But this Issue shall only be for the discussion (therefore I added the "documentation" label to it).

[pljones]

This was going to be vaguely coherent but...

Whilst privacy is one of many forms of security it shouldn't be the main focus of anything required for Jamulus, as Jamulus does not publish personal ("person"-related) information without user consent. Cryptographic technologies mostly relate to privacy ("tampering" isn't always a privacy issue but it's usually taken as one when cryptographic solutions are suggested). Like you, I'd discount them. (They're also expensive in CPU cycles.)

Currently I find it "hard" to connect to certain servers (e.g. mojolounge and nylonmojo that open port 22120). Whilst I get some protocol messages (I get the list of other participants, for example), I don't get audio protocol messages (and may well not get others). I think others may have similar difficulty with my server (some people do okay...). I'm guessing this relates to firewalls - so I think they do seem to be an area that needs consideration here. They don't all work the same way, some aren't able to do bi-directional port forwarding, sometimes the source port on a packet is re-written by the forwarding, sometimes it seems the firewall port-forwarding gets ... lost on the way back (there's no connection...).

I think it's great you have the test suite. If I were you, I'd hate myself and reject (gently...) any pull request that didn't keep it up to date. Such is life... (He says as he raises three changes against a large chunk of code with no test coverage at all.)

Writing a virus needs to be worthwhile. There still aren't enough Jamulus users to make the big time ;) - unless someone's really annoyed a virus writer... I'd not be overly concerned about code injection points.

I've resolutely had "other people" to worry about these things throughout my career - I'm not sure if "fortunately" is the right word. It's a big area...

[Snayler]

Currently I find it "hard" to connect to certain servers (e.g. mojolounge and nylonmojo that open port 22120).

That happens to me too if I have the small network buffers enabled. As soon as I disable it, the audio comes through. Must be some incompatibility between the option and older versions.

[atsampson]

If anyone wants to experiment further with fuzzing, I've pushed my (temporary) changes to make Jamulus accept fuzzer input on this branch. AFL gets good coverage of the protocol parser when seeded with a set of messages generated by CTestbench; #302 was the only issue I found. But there would also be value in testing the client logic with coverage-directed fuzzing as well as CTestbench's random messages.

It would be helpful to add a note to the README (or the GitHub security policy) saying how you'd prefer people to report security problems - many projects don't want them filed in their public bugtracker so they have a chance to fix them before they're made public.

[atsampson]

In terms of privacy, one problem with the current Jamulus protocol is that the server includes the IP address of each user as part of the ChannelInfo sent to other connected users. IP addresses are considered PII under EU law, so this is potentially a GDPR concern for server operators.

As far as I can see, iIpAddr is only used as a fallback for the displayed name if the user hasn't set an alias (with the last byte being obscured on the client side!), so it should be pretty straightforward to improve this without breaking backwards compatibility: have the server send a dummy address instead of the real one, and complain at the user if they try to connect to a server before setting their alias.

[lefty665]

atsampson "have the server send a dummy address instead of the real one, and complain at the user if they try to connect to a server before setting their alias." or make the user name a required field. The truncated URL is an exposure.

[corrados]

IP addresses are considered PII under EU law, so this is potentially a GDPR concern for server operators [...] (with the last byte being obscured on the client side!) [...] The truncated URL is an exposure.

Thanks for bringing that up. When I initially implemented it, I simply did it the same way as Ninjam. But if you see an issue with that, a simple quick fix would be to just show an empty name at the fader instead of the IP address. That would not solve that the IP address is still in the protocol but if it is just in the protocol, you would need special tools to make it visible and I am not sure if this is already treated as a GDPR issue. BTW: The server list protocol message contains all IP addresses of all listed servers. For this we need a working IP address, otherwise the list would not work at all. You could also argument with the GDPR for all the server operators as well. Not an easy topic at all...

[corrados]

#302 was the only issue I found [...] many projects don't want them filed in their public bugtracker so they have a chance to fix them before they're made public.

Ok, let's talk about this, too. atsampson found a security issue in the Jamulus protocol and fixed it. The new code is in the Git repo on master but there is currently no official release version available. My plan is to make a new release asap but right now I am not at home and do not have the possibility to make a release. In about a week we will have a fixed official version.
Anyway, these things will happen in the future again since no complex software is bug free. The problem with Jamulus is that everybody can create a server and there is no chance to contact them to inform them about security issues. So even if I create a new official release immediately, it will take years until all server operators use versions which are higher than the fixed version.
One possible solution would be that the Central Server queries the version number of the server which wants to register and if he detects that the version number is too low, it refuses the registration. If the vulnerable server does not show up in the server list, the risk of being hacked is very low.

[pljones]

Perhaps as similar message to "Central Server full" - "Vulnerabilty, upgrade required, minimum version X.XX". (Of course, depending on the central server with which the server is registering having been upgraded...)

[Snayler]

That would also solve some problems with incompatibilities between newer features and older server versions, like enabling the small network buffers and receiving no sound from older server versions.

[atsampson]

I should say that I don't think #302 is serious enough to be worth rushing out a new release - it's an out-of-bounds read where the value is checked before use, so the worst thing you can do with it is to crash a server, and there are already plenty of ways of being obnoxious on a public server (e.g. forgetting the words to Comfortably Numb, to which I plead guilty).

you would need special tools to make it visible and I am not sure if this is already treated as a GDPR issue.

It would be - an attacker would have no difficulty making a one-line modification to the Jamulus source (or running tcpdump, etc.). It's best practice not to store or send personal information at all if it's not required to get the job done.

Think of it as being analogous to the changes that IRC servers have made to improve privacy over the last few years - they used to send the user's IP address as part of their user information, but these days they send an anonymised ID (which is hard to get right given how small the IPv4 address space is) or nothing at all.

(In the case of servers, the user's address is required while they're connected - the server has to know it to be able to send replies to them. But Jamulus doesn't log the address or store it after the connection's over, so it's already doing the right thing by default there.)

[corrados]

It's best practice not to store or send personal information at all if it's not required to get the job done.

I just created a new Issue: #316
Please discuss things related to privacy issues with the IP address display in that Issue.

[lefty665]

Jamulus makes heavy use of pings. They are proprietary which bypasses a common router option to prevent responses to pings from the web. It seems likely Jamulus pings are small enough that encryption/decryption would not incur much overhead. Would this be worthwhile considering to reduce opportunities for exploitation?