1098314 - (CVE-2015-0834) TLS connection to a TURN server is not being secured

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[Alex]

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.

Comment 1

[Patrick McManus [:mcmanus]]

randell can you kind of triage this and point it in the right direction?

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

ekr: do you want this one, or if not who would you think would be best to put on it?

Comment 3

[Byron Campen [:bwc]]

We don't support TURN TLS. We probably have a bug that allows turns: URLs to be used.

Comment 4

[Eric Rescorla (:ekr)]

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.

Comment 5

[Maire Reavy [:mreavy]]

Jan-Ivar -- Can you take a look at this?

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: ignore turns: urls until we support TURN TLS.

Comment 7

[Al Billings [:abillings - ex-MoCo]]

What are the security implications of this? Is it just that we're sending login credentials in the clear?

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

(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).

Comment 9

[Daniel Veditz [:dveditz]]

If the TURN server is speaking TLS on port 443 why isn't the connection failing before we start to send anything?

Comment 10

[Eric Rescorla (:ekr)]

(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?

Comment 11

[Martin Thomson [:mt:]]

(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.

Comment 12

[Alex]

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.

Comment 13

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: ignore and warn on stuns: and turns: urls until we support STUN/TURN TLS. (2)

- Ignore stuns: urls as well.
- Add warning in web console.

Comment 14

[Martin Thomson [:mt:]]

(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.

Comment 15

[Alex]

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.

Comment 16

[Randell Jesup [:jesup] (needinfo me)]

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 17

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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+.

Comment 18

[Byron Campen [:bwc]]

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.

Comment 19

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: ignore and warn on stuns: and turns: urls until we support STUN/TURN TLS. (3) r=bwc

Incorporate feedback. Carrying forward r=bwc.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9e8ae3213d2

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f9e8ae3213d2

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Is this wontfix for Fx36 or should we get a beta approval request up?

Comment 23

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3b4908a629e8