ungoogled-software / ungoogled-chromium-android

Android build for ungoogled-chromium
GNU General Public License v3.0
491 stars 43 forks source link

WebRTC policy in custom tabs #14

Closed Eloston closed 4 years ago

Eloston commented 4 years ago

Bromite is having some discussion about WebRTC policies in custom tabs (thanks @csagan5 for letting me know): https://github.com/bromite/bromite/issues/553#issuecomment-626196348. I believe custom tabs is an Android-only concept, hence I've enabled issues here to discuss it.

wchen342 commented 4 years ago

I will add the patch in next version. However @csagan5 I noticed the modified part is not wrapped in #if defined(OS_ANDROID), does that mean this is potentially not Android specific? If so then probably it need to be added to main repo.

csagan5 commented 4 years ago

I noticed the modified part is not wrapped in #if defined(OS_ANDROID), does that mean this is potentially not Android specific?

@wchen342 the only leak verified is of the LAN IP address, and it is not happening when using VPNs; ~this is not Android-specific, however~ there is some other discussion in https://github.com/bromite/bromite/issues/589 (feel free to participate) where a less invasive approach is proposed by @thestinger; I do not think I am going to change the patch though because:

Edit: @wchen342 it would be fine to wrap with #ifdefs

Eloston commented 4 years ago

If we are to move this into the main repo, I'd like to know how this will affect desktop platforms. Specifically, is this bug reproducible in a regular desktop tab? How about an incognito tab? I did a quick test by simply browsing to https://browserleaks.com/webrtc on Debian 81.0.4044.138 with the uBlock Origin's setting enabled, and did not see any leaks.

Also as @csagan5 noted, we need to be concerned about changing defaults. Notably, uBlock documents some breakages when toggling this feature: https://github.com/gorhill/uBlock/wiki/Prevent-WebRTC-from-leaking-local-IP-address#chromium-based-browsers. Potentially more applications are broken as well. In that case, we wouldn't want to change the default setting for ungoogled-chromium. EDIT: ungoogled-chromium-android is a bit of an exception since it's not as easy to configure settings, so it might be fine to change these defaults specifically for ungoogled-chromium-android.

csagan5 commented 4 years ago

this is not Android-specific

I was incorrect here: there is no equivalent of CustomContentTabs/SystemWebView on desktop so I don't think it's (easily) possible to reproduce it. If you can manage to get a tab without settings (e.g. using defaults), then you would be able to reproduce it. Until that it is safe to think of this as an Android-only issue, where (Android) app integrations leave this trail of security/privacy nightmares.

Also as @csagan5 noted, we need to be concerned about changing defaults.

To be precise: I am changing the defaults on Android to something which is possibly breaking, while @thestinger was suggesting to be more conservative (which is coincidentally also what you have in mind for the desktop use-case). It is however not possible to reproduce this issue on desktop, as far as I know.

it might be fine to change these defaults specifically for ungoogled-chromium-android.

This is what I am doing, mostly because it is not clear to me when/how the defaults are used.

A simple way to test breakage would be to use some WebRTC-enabled video conference app; if it does not work by default the user might have to change a flag.

wchen342 commented 4 years ago

I have included the patch from Bromite from 83.0.4131.61. Since the problem is not reproduced on desktop then I think the current method can be assumed safe since the change is Android-only.

Close for now unless new problem related to WebRTC/custom tabs emerges