Closed Bug 1098314 (CVE-2015-0834) Opened 6 years ago Closed 5 years ago

TLS connection to a TURN server is not being secured

Categories

(Core :: WebRTC: Networking, defect, major)

33 Branch
x86_64
Windows 7
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox33 --- wontfix
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- fixed
firefox37 --- fixed
firefox-esr31 --- wontfix
b2g-v2.0 --- wontfix
b2g-v2.1 --- wontfix
b2g-v2.2 --- fixed

People

(Reporter: mozillaorg, Assigned: jib)

Details

(Keywords: sec-low, Whiteboard: [adv-main36+])

Attachments

(1 file, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0
Build ID: 20141106120505

Steps to reproduce:

Not sure about the Core's branch version. I'm using Firefox 33.1

0. Make sure your PC is firewalled, so you're able to reach ports 80 and 443 only.
1. Open http://sipml5.org/call.htm#
2. Specify credentials for a SIP server
3. Click on Expert mode button
4. Check "Enable RTCWeb Breaker"
5. Put appropriate in "WebSocket Server URL"
6. Specify ICE Servers:
[{url: 'turns:myturnserver.domain.com:443?transport=tcp',credential: 'testpass',username: 'testuser'}]
7. Save and close the expert tab
8. Log into the SIP server and make a call


Actual results:

Login credentials sent to TURN server unencrypted.

Here is a TURN server log excerpt:
83037: IPv4. tcp or tls connected to: xx.xx.238.165:29129
83037: session 000000000000001401: user <>: incoming packet message processed, error 401: Unauthorised
...
83037: IPv4. Local relay addr: 172.30.1.191:49765
83037: session 000000000000001401: new, username=<testuser>, lifetime=3600
83037: session 000000000000001401: user <testuser>: incoming packet ALLOCATE processed, success
....


Expected results:

Traffic to TURN server should be encrypted.

Here is how the TURN's log should look like:

83210: IPv4. tcp or tls connected to: xx.xx.238.165:20493
83210: session 000000000000001430: user <>: incoming packet message processed, error 401: Unauthorised
83210: IPv4. Local relay addr: 172.30.1.191:53728
83210: session 000000000000001430: new, username=<testuser>, lifetime=600, cipher=ECDHE-RSA-AES256-SHA, method=TLSv1.2 (TLSv1.2)
83210: session 000000000000001430: user <testuser>: incoming packet ALLOCATE processed, success
...

Same settings under Chrome browser work correctly.
Severity: normal → major
randell can you kind of triage this and point it in the right direction?
Group: network-core-security
Component: Networking: WebSockets → WebRTC: Networking
Group: core-security
ekr: do you want this one, or if not who would you think would be best to put on it?
Flags: needinfo?(ekr)
We don't support TURN TLS. We probably have a bug that allows turns: URLs to be used.
We don't currently do TURN-TLS at all, so the immediate problem is that we
accept "turns:" URIs and treat them as TURN over TCP. See:

http://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp?from=PeerConnectionImpl.cpp&case=true#462

So this is probably a JIB thing.
Jan-Ivar -- Can you take a look at this?
Assignee: nobody → jib
Status: UNCONFIRMED → NEW
Ever confirmed: true
What are the security implications of this? Is it just that we're sending login credentials in the clear?
(In reply to Al Billings [:abillings] from comment #7)
> What are the security implications of this? Is it just that we're sending
> login credentials in the clear?

It exposes credentials that can be used to access the TURN service (TURN costs money to run (bandwidth, colo, etc).
If the TURN server is speaking TLS on port 443 why isn't the connection failing before we start to send anything?
Keywords: sec-moderate
(In reply to Daniel Veditz [:dveditz] from comment #9)
> If the TURN server is speaking TLS on port 443 why isn't the connection
> failing before we start to send anything?

Two points:

1. An active attacker can of course simulate being a TURN server
and complete the connection.

2. Turn uses a challenge-response handshake so the attacker would
need to mount a dictionary attack on them or something, assuming
they are low entropy.

Also, I looked at JIB's patch. Shouldn't you also screen out stuns?
Flags: needinfo?(ekr)
Flags: needinfo?(jib)
(In reply to Randell Jesup [:jesup] from comment #8)
> (In reply to Al Billings [:abillings] from comment #7)
> > What are the security implications of this? Is it just that we're sending
> > login credentials in the clear?
> 
> It exposes credentials that can be used to access the TURN service (TURN
> costs money to run (bandwidth, colo, etc).

Let's be really clear about the actual exposure: it exposes a hashed version of the same.  The time required to preimage a hash over what is usually a high-entropy value is high. And these credentials, while purportedly "long-term", have to change frequently on the web for other reasons (i.e., we hand them out to our users).

Therefore, I don't consider this to be a security-bug-worthy problem.  Make this public.  The only real risk here is embarrassment to Mozilla.
Martin, FF sends login and password in clear text. An attacker may sniff for them and later use them to connect to a TURN server.
- Ignore stuns: urls as well.
- Add warning in web console.
Attachment #8524281 - Attachment is obsolete: true
Attachment #8524281 - Flags: review?(ekr)
Flags: needinfo?(jib)
Attachment #8529141 - Flags: review?(ekr)
(In reply to Alex from comment #12)
> Martin, FF sends login and password in clear text. An attacker may sniff for
> them and later use them to connect to a TURN server.

If it does, I'd be surprised.  TURN uses long term credentials (http://tools.ietf.org/html/rfc5389#section-15.4), which are first run through MD5, then finally HMAC-SHA-1.  The only risk that cleartext presents there is on low entropy secrets (human-selected passwords).  And I haven't seen much evidence that those are used.
Yeah, after double checking, I admit - my fault. The password is not being sent in clear. Only username and realm are being sent in clear.
From the above discussion, moving to sec-low, and we can probably un-hide, as Martin suggests.  This is more of a functionality bug.
Comment on attachment 8529141 [details] [diff] [review]
ignore and warn on stuns: and turns: urls until we support STUN/TURN TLS. (2)

Byron, I think this is your area as well. Can you take a look? I'll take first available r+.
Attachment #8529141 - Flags: review?(docfaraday)
Comment on attachment 8529141 [details] [diff] [review]
ignore and warn on stuns: and turns: urls until we support STUN/TURN TLS. (2)

Review of attachment 8529141 [details] [diff] [review]:
-----------------------------------------------------------------

This seems to have rotted, but it is easy enough to review anyhow. Just one nit.

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ +461,5 @@
>        port = (isStuns || isTurns)? 5349 : 3478;
>  
> +    if (isTurns || isStuns) {
> +      continue; // TODO: Support TURNS and STUNS (Bug 1056934)
> +    }

I would do this right after we init |isTurns| and |isStuns|. It is a lot easier to rule out side-effects that way.
Attachment #8529141 - Flags: review?(docfaraday) → review+
Incorporate feedback. Carrying forward r=bwc.
Attachment #8529141 - Attachment is obsolete: true
Attachment #8529141 - Flags: review?(ekr)
Attachment #8535790 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/f9e8ae3213d2
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Group: network-core-security
Is this wontfix for Fx36 or should we get a beta approval request up?
Comment on attachment 8535790 [details] [diff] [review]
ignore and warn on stuns: and turns: urls until we support STUN/TURN TLS. (3) r=bwc

Can't hurt.

Approval Request Comment
[Feature/regressing bug #]: TURN, STUN support
[User impact if declined]:
This bug causes some security degradation by using STUN/TURN when STUNS/TURNS is requested. The degradation is not considered critical, since per comment 14, actual TURN credentials sent are still MD5ed and HMAC-SHA-1ed, and per comment 10, an active attacker simulating a TURN server would need to mount a dictionary attack on the challenge-response handshake in the response-time.

[Describe test coverage new/current, TBPL]:
Landed on m-c which is now aurora. We're removing functionality which is hard to test for, though there are tests that prove we don't choke on the parsing change.

[Risks and why]: 
Extremely low risk due to simplicity of patch. It merely warns on and passes on any turns/stuns servers found while walking the iceServers array, as it should, whereas before it would interpret them as turn/stun. 

[String/UUID change made/needed]: none
Flags: needinfo?(jib)
Attachment #8535790 - Flags: approval-mozilla-beta?
Attachment #8535790 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [adv-main36+]
Alias: CVE-2015-0834
Group: core-security
You need to log in before you can comment on or make changes to this bug.