1473840 - IPv6 candidates ignored even if not MAC-based or Teredo

Status: VERIFIED FIXED

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

Description

[alex]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180702164905

Steps to reproduce:

Just try to create a WebRTC PeerConnection and print the gathered ICE candidates.



Actual results:

IPv6 candidates are not showed, even if not MAC-based or Teredo ones, starting from Firefox-58. Firefox-57 gathers them correctly.


Expected results:

IPv6 candidates should be gathered.

The breaking change seems to be this patch: https://bugzilla.mozilla.org/show_bug.cgi?id=1408218

I believe the problem is in the nr_transport_addr_is_teredo function, which discards all v6 addresses starting with 2001. Teredo addresses start with 2001:0000.

Comment 1

[David Olah]

Hi Alex,

I want to reproduce this issue but I will need more details in order to be able to. Please provide step by step all your actions that generate this issue.

Comment 2

[alex]

I have an IPv6 address that starts with "2001:b07:". As such, it's not a Teredo one, as they all start with "2001:0000:".
When I try to set up a WebRTC PeerConnection, that v6 address is discarded.

You can reproduce the issue through this Javascript function, which prints all gathered candidates on the console:

function gather()  {
	const pc = new RTCPeerConnection();
	navigator.mediaDevices.getUserMedia({audio: true, video: false}).then(stream => {
		pc.onicecandidate = (ev => console.log('icecandidate event:', ev.candidate));  
		pc.addStream(stream);  
		pc.createOffer().then(offer => pc.setLocalDescription(offer));
	});
}

I do believe that this is due to this check done in media/mtransport/third_party/nICEr/src/net/transport_addr.c, in the  nr_transport_addr_is_teredo() function:
> if ((*addrTop & htonl(0xFFFF0000)) == htonl(0x20010000))

which should probably be like this:
> if ((*addrTop & htonl(0xFFFFFFFF)) == htonl(0x20010000))

Comment 3

[David Olah]

Would it be possible to attach a test case? Based on the fact that I cannot try to reproduce this issue based on the information received.

Also, maybe someone from Network component can take a look at this and give an advice. Valentin, can you please take a look at this?

Comment 4

[Nils Ohlmeier [:drno]]

Attachment: Bug 1473840: fixed Teredo IPv6 prefix detection.

Review commit: https://reviewboard.mozilla.org/r/257044/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/257044/

Comment 5

[Nils Ohlmeier [:drno]]

Alex, I just kicked off a try build, which means there should a binary for you to test in the IETF network soon. Alternatively I'll be arriving in Montreal on Monday and can test it myself.

Comment 6

[Nils Ohlmeier [:drno]]

And thanks a lot for tracking this down to the code line!

Comment 7

[alex]

Hi Nils,thank you for the quick reaction!
Please let me know how to download the binary and I will definitely test.

Comment 8

[Nils Ohlmeier [:drno]]

Linux 64bit test binary https://queue.taskcluster.net/v1/task/VbPLDg5vTv-2ge2woyZABw/runs/0/artifacts/public/build/target.tar.bz2
OSX https://queue.taskcluster.net/v1/task/P4kBvVhCQ8iJwpoMvNCz4w/runs/0/artifacts/public/build/target.dmg
Win 32 https://queue.taskcluster.net/v1/task/Qs_3Cu3xQ3GdUaY3s4tWwA/runs/0/artifacts/public/build/target.zip
Win 64 https://queue.taskcluster.net/v1/task/RnCSBWpCSCOUhWIHXGH1dw/runs/0/artifacts/public/build/target.zip

Comment 9

[alex]

I'v been told that IPv6 candidates from the IETF network are still ignored (I'm not in Montreal myself). You may want to check yourself when you get there. I'll test from my v6-enabled office network on Tuesday and report back.

Comment 10

[Michael Froman [:mjf]]

Comment on attachment 8992144 [details]
Bug 1473840: fixed Teredo IPv6 prefix detection.

https://reviewboard.mozilla.org/r/257044/#review264020

Looks good to me.

Comment 11

[Nils Ohlmeier [:drno]]

On the IETF 102 network it works with and without my patch on my MacBook Pro. Which makes me wonder if this only affects certain platforms?

Comment 12

[alex]

I can reproduce it consistently from my office network, both on my Linux laptop and on a Windows 7 machine. In both cases, the nightly build you linked fixed the issue.

I'll ask my colleagues to make more tests from the IETF network.

Comment 13

[alex]

Tested on the IETF meeting network with a MacBook Pro and the issue is there, so it's definitely not platform-dependent. I would go ahead and push the fix.

Thanks Nils!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db9d0bc7c8c9
fixed Teredo IPv6 prefix detection. r=mjf

Comment 15

[Nils Ohlmeier [:drno]]

Sat down with Paolo today and with the test build it appeared to be fixed for him as well.

Comment 16

[Nils Ohlmeier [:drno]]

Comment on attachment 8992144 [details]
Bug 1473840: fixed Teredo IPv6 prefix detection.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1408218

[User impact if declined]: WebRTC won't work over lost of (most?) IPv6 addresses.
[Is this code covered by automated tests?]: CI machines are not guaranteed to have IPv6, so this can't be tested in automation

[Has the fix been verified in Nightly?]: Manually verified by several people.

[Needs manual test from QE? If yes, steps to reproduce]:
- Use a Linux machine with IPv6 connectivity
- Open https://mozilla.github.io/webrtc-landing/stun_test.html
- Verify that IPv6 IP addresses show up at the bottom of the page in the lines which start with "ICE candidate"
 
[List of other uplifts needed for the feature/fix]: N/A

[Is the change risky?]: No

[Why is the change risky/not risky?]: It only prevents IPv6 addresses in WebRTC from getting filtered out.

[String changes made/needed]: N/A

Comment 17

[Eliza Balazs [:ebalazs_]]

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

Comment 18

[Liz Henry (:lizzard)]

David, looks like you are assigned to this in the verification roster, are you still planning to take a look?

Comment 19

[David Olah]

Yes Liz, I am still on it.

It's a bit complicated regarding the fact that our network doesn't support IPv6 connection here at the office. I tried to make it work with the IT guys but it is a hard process, so I am in touch with Nils on Slack but it get long.

Nils, the final conclusion is that this can be marked as Verified on Nightly?

Or can anyone else confirm that it works as expected now?

Comment 20

[alex]

I can confirm that Nightly is working as expected.

Comment 21

[David Olah]

Regarding Comment 20, and discussion with Nils on Slack, I will mark the issue as verified on Nightly.

Comment 22

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8992144 [details]
Bug 1473840: fixed Teredo IPv6 prefix detection.

Fix was verified on Nightly, Beta62+, ESR60.2+

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/75554fb128e2

Comment 24

[Andrei Vaida [:avaida]]

(In reply to alex from comment #20)
> I can confirm that Nightly is working as expected.

Alex, since our test environments don't support IPv6 at the moment, could you please check whether this works as expected on Beta 62 as well? You can find the latest beta build here: https://archive.mozilla.org/pub/devedition/candidates/62.0b12-candidates/build1/.

Comment 25

[alex]

I confirm that it works fine on the latest beta.

Comment 26

[Andrei Vaida [:avaida]]

(In reply to alex from comment #25)
> I confirm that it works fine on the latest beta.

Thanks, Alex! Updating status flags accordingly.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/aeb7ea1b5492