Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Discussing potential security issues #314

Closed
corrados opened this issue May 31, 2020 · 40 comments
Closed

Discussing potential security issues #314

corrados opened this issue May 31, 2020 · 40 comments

Comments

@corrados
Copy link
Contributor

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
Copy link
Collaborator

pljones commented May 31, 2020

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
Copy link
Contributor

Snayler commented May 31, 2020

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link

lefty665 commented Jun 1, 2020

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
Copy link
Contributor Author

corrados commented Jun 1, 2020

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
Copy link
Contributor Author

corrados commented Jun 1, 2020

#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
Copy link
Collaborator

pljones commented Jun 1, 2020

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
Copy link
Contributor

Snayler commented Jun 1, 2020

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
Copy link
Contributor

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
Copy link
Contributor Author

corrados commented Jun 1, 2020

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
Copy link

lefty665 commented Jun 5, 2020

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?

@corrados
Copy link
Contributor Author

corrados commented Jun 5, 2020

What exploitation are you referring to?

@gilgongo
Copy link
Member

gilgongo commented Jun 6, 2020

I think the closest we've got to an explanation of the sort of thing meant by "exploitation" is this comment.

@corrados
Copy link
Contributor Author

corrados commented Jun 6, 2020

Ok, from that comment: The first step in hacking a computer is figuring out its address, the URL.
With the Jamulus server list, all the IP addresses and port numbers are published anyway. So an attacker simply starts the Jamulus software and gets all the IP addresses anyway.

@gilgongo
Copy link
Member

gilgongo commented Jun 6, 2020

OK. So then the attacker needs to have an exploit based on a vulnerability in Jamulus that they wish to use against those IP addresses. And we know of no such vulnerability yet.

I guess the other scenario is private servers: @lefty665 seems to imply that an attacker might scan the Internet with Jamulus connection attempts, an activity which (because Jamulus doesn't use ICMP ping) routers are defenceless against once the Jamulus port is open. This seems... unlikely to me. But it's not my area :-)

@lefty665
Copy link

Is it your suggestion that because of Jamulus's heavy use of pings and distribution of URL lists that there is no way to protect users from exploitation?

@corrados
Copy link
Contributor Author

Well, let's say it in other words: There are many Jamulus servers. If a client wants to connect to one of them, it needs the IP address of that server. If we want to give the client the possibility to connect to each of them, we need to provide im with all the IP addresses. Since it does not make sense to connect to a server which is too far away, a good inidicator is the ping to the server. Therefore we ping all the servers in the list and the user can pick one suitable.
Even if you encrypt the IP addresses and the ping messages, that does not help since at one point in the client, the IP address of the server which he wants to connect must be decrypted. Otherwise the client cannot connect to the server. So if a hacker wants to get the IP addresses, he simply changes the Jamulus client code to get all the IP addresses easily.
There are even external tools available for showing the server list, see: http://jamulus.softins.co.uk

@corrados
Copy link
Contributor Author

corrados commented Jun 13, 2020

We have a new feature request which requests that we get "clickable" links in the chat: #360 What do our security experts say to this request? A hacker might log into a server shortly and send malicious links to all connected clients. Is this a real scenario to be considered or do you see no security issues with that?

@WolfganP
Copy link

As far is the link is not auto-redirecting and the user needs to explicitly trust and click, I see no issue with this feature (in fact that same link may be distributed by other means).
It should also be clear that the link was posted by an specific user and doesn't come from the system/jamulus app (ie in case soeone tries to impersonate the system saying "click here for updated manual / connecting instructions / donating to the author / ..." or some bait like that :-)

@corrados
Copy link
Contributor Author

It is a HTML link. So you can hide the actual address/URL.
The name shown in the chat is the name which is set in the client profile settings. The names are listed in the server list. A hacker could get the name of a client from the list, set the same name and log into the server for a very short time and set a link under that name so that the other clients can think that the link comes from the already connected client and not from the hacker. Of course, the probability that such a thing actually happens is very very low, I guess...

@gilgongo
Copy link
Member

Of course, the probability that such a thing actually happens is very very low, I guess.

I would imagine this also applies to other exploits.

BTW the anonymous nature of Jamulus may also be an advantage here, since it makes no claim of trust (no logins or emails for users). So when it comes to things like social engineering exploits, it's also pretty safe I would think. At least, I'm pretty sure that if I changed my name to Lars Ulrich, most people would not believe I was really the drummer for Metallica (even though I play better than him, obviously).

@lefty665
Copy link

corrados @ Jun 13, 2020 I appreciate how Jamulus works and that it uses efficient techniques. In a benign world I would not have concerns. I saw in the previous chat that spawned this thread that there were EU regs on disclosure of IP addresses. Is Jamulus square with both the intent and letter of the regs?

gilgongo @ Jun 14, 2020 You are incredibly naive, do you come from an Apple background?

A message of appreciation. Last night I was in a Jamulus jam that included participants from Italy, South America, Canada, and both east and west coasts of the US (the server was on the US east coast with fat pipes). Impressive for a lightweight app, and it attracts good musicians. Thank you.

@gilgongo
Copy link
Member

Interesting point about GDPR, but not in relation to IP addresses. If the Jamulus user has a legal means of using IP addresses to uncover personal information (so they'd have to be an ISP, or the police perhaps) then they need to treat the addresses as personal data, but that doesn't concern us.

What might concern us is the fact that since Jamulus displays the "names" of users on the server, and the server operator can't ensure users' consent to show these. So a server operator might possibly be on the wrong side of GDPR for not first obtaining consent.

This was in fact one of the reasons why I said the current CC licence toggle should be replaced or modified with the ability for the server operator to put up any (or no) consent information they wanted. #81 (comment)

@pljones
Copy link
Collaborator

pljones commented Jun 15, 2020

A lot of this is falling back to privacy rather than security. You cannot rely on privacy to provide security - it's essentially a "security through obscurity" attempt if you do. The IP address was a flaw in that it would allow a quick port scan across all hosts on the truncated address to find the potentially exposed client.


Just on the privacy topic, though, it does need to be made clear -- but you cannot ensure users will choose to understand -- that, by connecting to a Jamulus server, certain information about them is made visible to anyone else who has access to that same server.

It should also make clear who is likely to have access to that same server.

This would include any other person actually connected to the server as well as the Jamulus server operator and anyone to whom the server operator has made the server details available. It would potentially also include the Jamulus server platform supplier, if that were a separate party.

It would include anyone aware of any central server with which the server is registered.

If that central server were one of the "well known" ones, then it would be pretty much anyone with internet access.

Given that the most common state is that last one - i.e. a connected user's details are, in essence, made public to the internet, then anything less public I don't think needs to be of concern here (yet). So what is of interest next is identifying for

  • what details?
  • is informed consent required to make these details public?
  • if so, how best to inform the user to best assist their understanding?
  • if so, how best to require consent in an manner that would provide legal standing for the server operator?

Back to security. That's the "But what then?" So there's a client with "this exact IP / port". What can someone use that information for? That's where code reviews here can help - "hey, this allows code injection if you send a malformed audio packet with pan set to -1" or whatever.

@gilgongo
Copy link
Member

(We're getting a bit off the software security topic here but I don't suppose that matters.)

what details?

The only information that Jamulus collects that's relevant to GDPR in terms of personally identifiable information is the "name" they choose to display, and their IP address. I've explained above how the issue of the IP address is not our concern though, so it's just the "name."

is informed consent required to make these details public?

Yes, because in theory at least they could be identified by their "name" (but see my last point).

if so, how best to inform the user to best assist their understanding?

A message that explains their "name" is displayed publicly for the purposes of differentiating them from other musicians on the server. It could also say their IP address will be displayed to server operators as part of Jamulus's necessary operation, just so they know.

if so, how best to require consent in an manner that would provide legal standing for the server operator?

As I've said in the past after talking to somebody who works in this field, I think we should allow server operators to supply their own wording for a consent dialogue, and allow them to have none at all. It's up to them. If the Jamulus project gets involved in advising users about legal issues, then bad things might happen to the Jamulus project as a consequence. The current imposition of a CC licence option goes too far in my opinion because of that.

@pljones
Copy link
Collaborator

pljones commented Jun 15, 2020

As I've said in the past after talking to somebody who works in this field, I think we should allow server operators to supply their own wording for a consent dialogue, and allow them to have none at all. It's up to them. If the Jamulus project gets involved in advising users about legal issues, then bad things might happen to the Jamulus project as a consequence.

Can you make it the server operator's responsibility? Does the GPL 2.0's disclaimer cover this adequately and acceptably? (The server operator should understand that by running the server they're accepting the terms of the software license. I think that's reasonable.)


The current imposition of a CC licence option goes too far in my opinion because of that.

The CC-NC-SA-4i license does not aim to address this and is aimed at a user connecting to the server, not the server operator. It's about intellectual property rather than personal information. It's based on a creator choosing to identify themselves as the creator of a work, of course, so there is implicit consent, I suppose. It also explicitly grants consent to anyone to then make use of that work subject to the licence.

I agree that a server operator should be able to pick a licence, too. It should be up to them. (You could use the welcome message with a "By continued use you accept the following terms" and hope you get away with that...)

@gilgongo
Copy link
Member

My main point is whether (or what) any licence covers (CC or otherwise) is not for us to have any opinion on. And whether server operators should understand any legal issues is similarly not for us to say. Jamulus doesn't operate in any particular legal jurisdiction and we must not offer any advice because we're not qualified to do so.

In terms of what we can do though, we should (in my opinion) have a function that server operators can use that displays some text and prevents users from joining the server until they have have agreed to it. The content of that function should be up to them. We could I suppose help them by supplying suggestions (if we disclaim their efficacy).

To be clear: the scenario to avoid is one in which a server operator is sued for something, and then drags the Jamulus project into it because we told them that some wording was "correct".

(This is why I have also reminded anyone involved in the project on these public forums not to make any assertion about what server operators should legally do in their jurisdiction.)

@pljones
Copy link
Collaborator

pljones commented Jun 15, 2020

The licence used to publish the software is relevant as that is the legal basis under which any claim against any author of the software would proceed - on the basis the server operator or client user had agreed to it.

That licence should be complete and adequate for the issues you're discussing.

@gilgongo
Copy link
Member

gilgongo commented Jun 15, 2020

Ah, the Jamulus software licence is a completely separate matter. Sorry for any confusion. I'm not talking about that.

I'm talking about the agreement that may or may not be necessary to play music in a public space on the Internet. Hence copyrights, GDPR, etc. If rights holders spank a server operator for allowing their music to be performed under, say a CC licence, then that's bad for us if we are seen as imposing that licence on the operator.

@pljones
Copy link
Collaborator

pljones commented Jun 15, 2020

And it's that software licence that's the only important matter between the authors of Jamulus and the users of Jamulus (server or client). What licence a server operator chooses to display or not display (i.e. whether they choose to use the CC-NC-SA 4i supplier or not), isn't of concern.

Yes, a separate feature to extend the ability for a server operator to restrict access to a server until some condition is met could be considered - but that's distinct.

@gilgongo
Copy link
Member

gilgongo commented Jun 15, 2020

And it's that software licence that's the only important matter between the authors of Jamulus and the users of Jamulus

Yes, of course. And I have no opinion on that.

EDIT: I've opened a ticket #367 as per @pljones suggestion.

@pljones
Copy link
Collaborator

pljones commented Jun 15, 2020

As it's a new feature request, rather than a discussion of potential security issues, it's probably best moved into its own issue.

@atsampson
Copy link
Contributor

We have a new feature request which requests that we get "clickable" links in the chat: [...] A hacker might log into a server shortly and send malicious links to all connected clients.

The way chat clients tend to address that is by ensuring the link target is visible - either directly or in a tooltip - and by restricting what kinds of links are made clickable. Some of the things that can go wrong:

  • Allowing links to file: or other schemas that aren't handled safely by a web browser (e.g. links to local executables in Pidgin or in Zoom) - you could only allow http: and https:.
  • Allowing links to localhost or local network addresses (e.g. to cause the user to perform some action on their broadband router or another local service when they click the link) - browsers have some protection against this these days, and making the link target visible helps.
  • Sending a link with an IDN that looks like a legitimate domain name (e.g.) - you could just disallow IDNs entirely.
  • Command injection or quoting faults in how the browser gets invoked - Qt should get this right anyway.

@corrados
Copy link
Contributor Author

Thanks, I have referenced your comment in #360.

@atsampson
Copy link
Contributor

[from #370]

The Jamulus software contacts some server and queries some information. Then it maybe downloads a file and executes it. I can see multiple ways for a hacker to use this to modify informations and inject unwanted code.

Yes, automatic updates are really hard to do safely. If you can get a OS packaging system to deal with this for you, then that's a much better bet than implementing it yourself.

The Update Framework is a best-practice design for software update systems that quite a few projects now use. Section 1.5 of the spec lists some of the attacks they're concerned about.

@ann0see
Copy link
Member

ann0see commented Jan 17, 2021

@corrados I just scanned the Code via codacy and came across an error:

"Check buffer boundaries if used in a loop including recursive loops (CWE-120, CWE-20)."

in src/signalhandler.cpp line 85

Not sure, but this might be a security issue.

hoffie added a commit to hoffie/jamulus that referenced this issue Jan 24, 2021
Previously, only https:// URLs were whitelisted as part of a security
hardening to prevent problematic handlers such as file:// (jamulussoftware#314, jamulussoftware#360).

This PR adds http:// to the whitelist. This is helpful as some resources
are still not available using encrypted https:// connections.

Fixes jamulussoftware#879.

Signed-off-by: Christian Hoffmann <[email protected]>
@hoffie
Copy link
Member

hoffie commented Jan 24, 2021

Some further ideas to look into:

  • UDP spoofing: UDP packets can be spoofed. This could possibly be used to disturb the signal of a Jamulus user by injecting malicious packets (i.e. causing a Denial of Service for him).
  • Reflection/amplification attacks: UDP services are commonly misued for reflection attacks in order to hide DDoS attacks and multiply their effect. This basically works with any UDP service that accepts an arbitrary packet and responds to it, possibly with a much larger response packet. In the past, NTP and DNS have been abused like this.
  • Resource exhaustion: It would probably take only very little resources for an attacker to occupy all slots of one, more or even all public Jamulus servers. For private servers, some form of authentication could mitigate that. For public servers, I cannot think of an easy way to work around this.

Note that these are just generic ideas which could be investigated. I haven't looked into them in detail. Things like some kind of handshake (similar to TCP) might partially or completely mitigate that.

@gene96817
Copy link

The handling of UDP attacks should be at the OS level. We shouldn't have to be creating a Jamulus specific defense.

hoffie added a commit to hoffie/jamulus that referenced this issue Jan 25, 2021
Previously, only https:// URLs were whitelisted as part of a security
hardening to prevent problematic handlers such as file:// (jamulussoftware#314, jamulussoftware#360).

This PR adds http:// to the whitelist. This is helpful as some resources
are still not available using encrypted https:// connections.

Fixes jamulussoftware#879.

Signed-off-by: Christian Hoffmann <[email protected]>
@pljones
Copy link
Collaborator

pljones commented Jan 25, 2021

The handling of UDP attacks should be at the OS level. We shouldn't have to be creating a Jamulus specific defense.

Much of what @hoffie mentions is application layer attack, so yes, if the risk is high enough to warrant it, it would be a Jamulus-specific defence required. Using UDP shouldn't be an excuse for enabling attack vectors.

@ann0see ann0see closed this as completed Jan 28, 2021
@jamulussoftware jamulussoftware locked and limited conversation to collaborators Jan 28, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants