1603790 - Peer reflex candidate is not showing up as redacted when hostname obfuscation enabled

Status: RESOLVED FIXED

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

Description

[Dan Minor [:dminor]]

We should only add a hostname to mSignaledAddresses if it is not obfuscated. Prior to Bug 1567201 this was handled by an early exit in MediaTransportHandlerSTS::AddIceCandidate if the address was obfuscated. With the changes in Bug 1567201, that early exit went away and we're now adding all addresses to mSignaledAddresses which breaks hiding prflx and srflx addresses.

Comment 1

[Dan Minor [:dminor]]

Attachment: Bug 1603790 - Only add unobfuscated addresses to mSignaledAddresses; r=mjf!

We should only add a hostname to mSignaledAddresses if it is not obfuscated.
Prior to Bug 1567201 this was handled by an early exit in
MediaTransportHandlerSTS::AddIceCandidate if the address was obfuscated.
With the changes in Bug 1567201, that early exit went away and we're now adding
all addresses to mSignaledAddresses which breaks hiding prflx and srflx
addresses. Unfortunately, this is not something that easily unit testable.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ace0f986eba
Only add unobfuscated addresses to mSignaledAddresses; r=mjf

Comment 3

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/3ace0f986eba

Comment 4

[Dan Minor [:dminor]]

Comment on attachment 9115817 [details]
Bug 1603790 - Only add unobfuscated addresses to mSignaledAddresses; r=mjf!

Beta/Release Uplift Approval Request

• User impact if declined: Firefox will reveal some IP addresses that should be kept hidden, which could be used for browser fingerprinting.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a very small change, it just adds an additional check prior to saving ip addresses to a list of addresses that are safe to reveal.
• String changes made/needed: None

Comment 5

[Julien Cristau [:jcristau]]

Comment on attachment 9115817 [details]
Bug 1603790 - Only add unobfuscated addresses to mSignaledAddresses; r=mjf!

webrtc fix for 72.0b9

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d09f48790fa5

Comment 7

[Anca Soncutean, Desktop QA]

I've notices this issue while testing Fx - Fx interoperability between OSes, and the real IP is displayed inside the Ice Stats, although it should be hidden. I will verified again this issue on Beta 72.0b9, where the fixed will be landed and come back with a comment.

Comment 8

[Anca Soncutean, Desktop QA]

(In reply to Anca Soncutean [:Anca], Desktop Release QA from comment #7)

I've notices this issue while testing Fx - Fx interoperability between OSes, and the real IP is displayed inside the Ice Stats, although it should be hidden. I will verified again this issue on Beta 72.0b9, where the fixed will be landed and come back with a comment.

Retested and verified in Fx 72.0b9 (Win 7 x86 - macOS 10.15, Win 10 x64 - Ubuntu 18.04 x64, macOS 10.15 - Win 10 x64) with whereby.com and file.pizza.