Adding View restriction

Adding finder for the chain to CC list.

Unfortunately I can't see  bug 648971 .

Why is shill running a web server?

Can we create iptables/ip6tables owner match rules to restrict web server access to a specific UID, or does it need to be accessed by chrome/chronos?

Shill is running a proxy server. 

https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/master/http_proxy.h
// The HTTPProxy class implements a simple web proxy that
// is bound to a specific interface and name server.  This
// allows us to specify which connection a URL should be
// fetched through, even though many connections
// could be active at the same time.
//
// This service is meant to be low-performance, since we
// do not want to divert resources from the rest of the
// connection manager.  As such, we serve one client request
// at a time.  This is probably okay since the use case is
// limited -- only portal detection, activation and Cashew
// are planned to be full-time users.

Hey henrika@, would you happen to be familiar with WebRTC's use of STUN/TURN (or know someone who is)? We received an exploit that uses a TURN request to a attacker-controlled server returning STUN_ERROR_TRY_ALTERNATE to trick Chrome into port scanning and connecting to a service listening on 127.0.0.1. By specifying a realm attribute in the response, they were able to get semi-controlled data (via the username field) sent to the local service to successfully exploit it.

I was wondering what kind of room we have to restrict what our TURN client is willing to connect to - at the very least, we can probably be pretty certain that a localhost alternate IP is illegal, right?

STUN_ERROR_TRY_ALTERNATE is indeed used used to exploit shill once the port is known. I should clarify that it's not used for port scan. The first plan was to use it for port scan as well, but making an external TURN connection for each port to be scanned was too slow.

Port scan does also use TURN though. It attempts a connection to each localhost port directly. Multiple servers can be listed in iceServers and that's used to scan in blocks of 1024 ports. Once WebRTC is done attempting a connection to all the ports, it will call pc.onicecandidate({candidate: null}). A successful connection to shill is kept alive for 2.5 seconds. So the delay of the callback leaks whether the block contained the shill port. The connection is kept alive regardless of what TURN sends, as long as the HTTP headers are not terminated. That's why the alternate server trick is not necessary for scan.

Blocking a redirect to localhost STUN_ERROR_TRY_ALTERNATE would certainly block the exploit. Not sure if that's doable. It's also worth double checking whether it's valid for STUN_ERROR_TRY_ALTERNATE to specify the realm attribute. Perhaps it should be specified by the alternate server instead?

henrika@ - Please have a look at whether we can introduce a restriction here. Thanks!

Re #8: I have not done any work on TURN, hence not really sure how my name popped up in this context.

hta@: can you find a more suitable owner?

TURN work is mainly done out of Kirkland, I think. Reassigning to pthatcher.

It would be very easy to prevent STUN_ERROR_TRY_ALTERNATE from redirecting to localhost. And I can't think of any legitimate scenario where this would happen.

I have a question, though: Why is STUN_ERROR_TRY_ALTERNATE even necessary for the exploit? Couldn't you just add the localhost address to iceServers?

STUN_ERROR_TRY_ALTERNATE is needed because in the exploit, the controlled data came from the username field, and auth info (including the username) is only sent if a realm was previously received from the server:

https://cs.chromium.org/chromium/src/third_party/webrtc/p2p/base/turnport.cc?l=1071

hash_ is set by the UpdateHash call in set_realm, triggered via https://cs.chromium.org/chromium/src/third_party/webrtc/p2p/base/turnport.cc?l=1215

We should probably still ban direct connections to localhost as well though!

Ah, right. I forgot about that.


We could probably ban STUN/TRUN connections to localhost altogether... But that could break someone's testbed. So if we do that, I'd say we should do it separately from addressing this exploit.

pthatcher: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

pthatcher: Uh oh! This issue still open and hasn't been updated in the last 28 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Increasing priority since this is a security exploit with a seemingly simple solution.

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/fb70b4503078503d7366f8cbe4928b2f437bd569

commit fb70b4503078503d7366f8cbe4928b2f437bd569
Author: deadbeef <deadbeef@webrtc.org>
Date: Mon Oct 24 20:15:59 2016

Preventing TURN redirects to loopback addresses.

This can be used for a certain security exploit, and doesn't have any
other practical applications we know of.

BUG= chromium:649118 

Review-Url: https://codereview.webrtc.org/2440043004
Cr-Commit-Position: refs/heads/master@{#14751}

[modify] https://crrev.com/fb70b4503078503d7366f8cbe4928b2f437bd569/webrtc/p2p/base/turnport.cc
[modify] https://crrev.com/fb70b4503078503d7366f8cbe4928b2f437bd569/webrtc/p2p/base/turnport_unittest.cc

Note: there's still one CL waiting to go in, fixing an "is loopback?" check.

Also, a question: In order to consider this "fixed", do we need to also block redirects to the machine's non-loopback addresses? This would certainly be possible, albeit a little more messy. But it seems less important than blocking redirects to loopback addresses, since a service listening on a non-loopback address is already vulnerable in some cases. Thoughts?

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/9922016ee44c70eb1c14f647cc4a5ade58a0018c

commit 9922016ee44c70eb1c14f647cc4a5ade58a0018c
Author: deadbeef <deadbeef@webrtc.org>
Date: Fri Oct 28 01:30:23 2016

Fix "IsLoopbackIp" to cover all loopback addresses; not just 127.0.0.1.

The loopback range is 127.0.0.0/8, which is everything from 127.0.0.0 to
127.255.255.255.

BUG= chromium:649118 

Review-Url: https://codereview.webrtc.org/2445933003
Cr-Commit-Position: refs/heads/master@{#14807}

[modify] https://crrev.com/9922016ee44c70eb1c14f647cc4a5ade58a0018c/webrtc/base/ipaddress.cc
[modify] https://crrev.com/9922016ee44c70eb1c14f647cc4a5ade58a0018c/webrtc/base/ipaddress_unittest.cc
[modify] https://crrev.com/9922016ee44c70eb1c14f647cc4a5ade58a0018c/webrtc/p2p/base/turnport_unittest.cc

Tentatively marking as fixed. Please reopen if you believe more redirects need to be blocked (see comment #22).

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot