Add `privacy.network.webRTCMultipleRoutesEnabled` to extensions API.

Add `privacy.network.webRTCMultipleRoutesEnabled` to extensions API.

Expose the preference for WebRTC's routing to extensions' privacy API. Disabling this preference prevents local IPs to be leaked when using WebRTC.

BUG=457629
Committed: https://crrev.com/654d477e070f5a11785883c9444c490867a104c9
Cr-Commit-Position: refs/heads/master@{#317066}

[msramek, 2015-02-19 10:28:36]

msramek@chromium.org changed reviewers:
+ kalman@chromium.org, mkwst@chromium.org

[msramek, 2015-02-19 10:28:37]

Hello again Mike and Benjamin,

could you have a look at this?

We're exposing the preference restricting WebRTC in order to prevent leaking
local IPs. I'm waiting for the WebRTC team to land their CL creating this
preference, but please review this asynchronously, so we can have the patch in
ASAP.

Thanks,
Martin

https://codereview.chromium.org/940893003/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/privacy.json (right):

https://codereview.chromium.org/940893003/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/privacy.json:23: "description": "If enabled, Chrome
will explore all possible routing options when using WebRTC. This preference's
value is a boolean, defaulting to <code>true</code>."
I would avoid awkward phrasing "You can only enable this preference if WebRTC
itself is enabled..."

- On Android, you can disable WebRTC through about://flags, but Clank doesn't
support extensions anyway
- On other OS, you can't disable it through about://flags
- WebRTC team is planning to remove the ENABLE_WEBRTC flag soon anyway

I think that it's not necessary to mention.

[Mike West, 2015-02-19 10:37:19]

https://codereview.chromium.org/940893003/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/privacy.json (right):

https://codereview.chromium.org/940893003/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/privacy.json:23: "description": "If enabled, Chrome
will explore all possible routing options when using WebRTC. This preference's
value is a boolean, defaulting to <code>true</code>."
On 2015/02/19 at 10:28:36, msramek wrote:
> I would avoid awkward phrasing "You can only enable this preference if WebRTC
itself is enabled..."
> 
> - On Android, you can disable WebRTC through about://flags, but Clank doesn't
support extensions anyway
> - On other OS, you can't disable it through about://flags
> - WebRTC team is planning to remove the ENABLE_WEBRTC flag soon anyway
> 
> I think that it's not necessary to mention.

I don't think that's necessary to mention, but I do think you should mention
something about the impact of the setting. That is, what does "explore all
possible routing options" mean? How does that have a privacy impact? If anything
it sounds like turning it _off_ would be the right thing to do from a privacy
perspective, but I think that's incorrect, isn't it?

[msramek, 2015-02-19 10:51:48]

https://codereview.chromium.org/940893003/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/privacy.json (right):

https://codereview.chromium.org/940893003/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/privacy.json:23: "description": "If enabled, Chrome
will explore all possible routing options when using WebRTC. This preference's
value is a boolean, defaulting to <code>true</code>."
On 2015/02/19 10:37:18, Mike West wrote:
> On 2015/02/19 at 10:28:36, msramek wrote:
> > I would avoid awkward phrasing "You can only enable this preference if
WebRTC
> itself is enabled..."
> > 
> > - On Android, you can disable WebRTC through about://flags, but Clank
doesn't
> support extensions anyway
> > - On other OS, you can't disable it through about://flags
> > - WebRTC team is planning to remove the ENABLE_WEBRTC flag soon anyway
> > 
> > I think that it's not necessary to mention.
> 
> I don't think that's necessary to mention, but I do think you should mention
> something about the impact of the setting. That is, what does "explore all
> possible routing options" mean? How does that have a privacy impact? If
anything
> it sounds like turning it _off_ would be the right thing to do from a privacy
> perspective, but I think that's incorrect, isn't it?

It's correct :) The flag is _on_ by default (needed for WebRTC's performance)
and we provide the possibility to turn it off for privacy-concerned users.

PTAL.

[Mike West, 2015-02-19 10:56:10]

Ok. LGTM.

[msramek, 2015-02-19 15:14:32]

New patchsets have been uploaded after l-g-t-m from mkwst@chromium.org

[not at google - send to devlin, 2015-02-19 16:10:22]

lgtm

https://codereview.chromium.org/940893003/diff/40001/chrome/test/data/extensi...
File chrome/test/data/extensions/api_test/preference/standard/test.js (right):

https://codereview.chromium.org/940893003/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/preference/standard/test.js:43: var
possibly_missing_preferences = [
An idle thought, I don't have much of an opinion here (especially since it's a
test), but I wonder if we should start using Set rather than Array. I always
found indexOf() an eyesore, has() is so much nicer.

[msramek, 2015-02-19 16:32:22]

New patchsets have been uploaded after l-g-t-m from kalman@chromium.org

[msramek, 2015-02-19 16:35:28]

https://codereview.chromium.org/940893003/diff/40001/chrome/test/data/extensi...
File chrome/test/data/extensions/api_test/preference/standard/test.js (right):

https://codereview.chromium.org/940893003/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/preference/standard/test.js:43: var
possibly_missing_preferences = [
On 2015/02/19 16:10:22, kalman wrote:
> An idle thought, I don't have much of an opinion here (especially since it's a
> test), but I wonder if we should start using Set rather than Array. I always
> found indexOf() an eyesore, has() is so much nicer.

Oh. Sure, it's nicer :)

Furthermore, this made me notice a missing semicolon! (JS usually doesn't care,
but still...)

[garyrothwell1977, 2015-02-19 16:36:53]

garyrothwell1977@gmail.com changed reviewers:
+ garyrothwell1977@gmail.com

[msramek, 2015-02-19 17:02:39]

The CQ bit was checked by msramek@chromium.org

[commit-bot: I haz the power, 2015-02-19 17:04:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/940893003/60001

[commit-bot: I haz the power, 2015-02-19 17:32:39]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-19 17:33:24]

Patchset 4 (id:??) landed as
https://crrev.com/654d477e070f5a11785883c9444c490867a104c9
Cr-Commit-Position: refs/heads/master@{#317066}