1237299 - Relay candidates don't appear in my local candidates while in FF42 or 43 whether they appear in FF38

Status: RESOLVED FIXED

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

Description

[Xavier De Snoeck]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537.36

Steps to reproduce:

Connect to https://webrtc.github.io/samples/src/content/peerconnection/trickle-ice/ and enter the URL of my public TURN Server that listens on port 443 (turn:xxx.xxx.xxx:443?transport=tcp with username and credential).
I use this website from open Internet or Intranet behind firewall/proxy, each time from FF38 ESR or FF43.0.2.


Actual results:

Internet / Chrome 47 or 48 : I see host and relay local candidates
Intranet / Chrome 47 or 48 : I see host and relay local candidates
Intranet / FF 38 : I see host and relay local candidates
Internet / FF 43.0.2 : I see host and relay local candidates
BUT, Intranet / FF 43.0.2 : I see only host local candidates and never the relay ones !
What is the difference between the 38 and 43.0.2 that could cause problem inside the Intranet but works on Internet ?


Expected results:

I should see on FF 38 and 43.0.2 the host and relay candidates whether I am on open Internet or in the Intranet behind the proxy/firewall.

Why it works on FF38 ESR and not on FF 43.0.2 ?

Comment 1

[Byron Campen [:bwc]]

My first thought was that this was a regression from bug 949703, but it seems that this landed in 38.

Comment 2

[Byron Campen [:bwc]]

Can you attach a saved copy of about:webrtc?

Comment 3

[Xavier De Snoeck]

Attachment: The aboutWebrtc files for the different tests on both sides of the webrtc call

Here are the about:webrtc files for the following tests :

FF38 Intranet / FF43 Internet (that works) :
* aboutWebrtc-FF38-Intranet-with-FF43-Internet ==> on the FF38 Intranet side (see the local candidates with relay)
* aboutWebrtc-FF43-Internet-with-FF38-Intranet ==> on the FF43 Internet side (see the local candidates with relay)

FF43 Intranet / FF43 Internet (that doesn't work) :
* aboutWebrtc-FF43-Intranet-with-FF43-Internet ==> on the FF43 Intranet side (see no relay on local candidates !!!)
* aboutWebrtc-FF43-Internet-with-FF43-Intranet ==> on the FF43 Internet side (see the local candidates with relay)

Hope this helps

Xav

Comment 4

[Byron Campen [:bwc]]

So this does, in fact, seem to be related to bug 949703. We're hitting the error here:

https://dxr.mozilla.org/mozilla-central/source/media/mtransport/third_party/nICEr/src/net/nr_proxy_tunnel.c#299

This means we failed somewhere in here:

https://dxr.mozilla.org/mozilla-central/source/media/mtransport/nriceresolver.cpp#145

Unfortunately, the logging in that function is not so great.

Comment 5

[Byron Campen [:bwc]]

I've done a try push with better logging, binaries should end up here:

https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-40ace80a08efaa5b0c61aa85e561855701482c8f/

Once those are done building, try running with NSPR_LOG_MODULES=mtransport:6 as an environment variable. That should give us a better idea of what is going wrong.

Comment 6

[Xavier De Snoeck]

Attachment: aboutWebrtc logs for the test on Intranet with Nightly new build

Here is the log file of about:webrtc on the device with the Nightly build.

Hope this helps

Xav

Comment 7

[Byron Campen [:bwc]]

I'm going to need the console output with NSPR_LOG_MODULES=mtransport:6 as an environment variable; this new logging won't show up in about:webrtc.

Comment 8

[Nils Ohlmeier [:drno]]

My bet is that he is hitting this:
https://dxr.mozilla.org/mozilla-central/source/media/mtransport/nriceresolver.cpp#175

As nr_socket_proxy_tunnel_connect() never initializes the address family we probably get some random value for that.

What looks bad to me is that the resolver seems to insist on either IPv4 or IPv6, but doesn't allow to request both. But in case of the proxy tunnel I guess we don't really care if we use IPv4 or IPv6 to talk to the proxy. Haven't checked though where and how the actual socket gets create in nr_proxy_tunnel.

Comment 9

[Nils Ohlmeier [:drno]]

(In reply to Nils Ohlmeier [:drno] from comment #8)
> My bet is that he is hitting this:
> https://dxr.mozilla.org/mozilla-central/source/media/mtransport/
> nriceresolver.cpp#175

Xavier it would help if you could quickly test if your Intranet scenario still works with Firefox 41, but breaks with Firefox 42.

Comment 10

[Xavier De Snoeck]

Attachment: Log Nightly console with the environment variable set

Here is the console logs for Nightly with the variable environment set.
Hope this helps

Xav

Comment 11

[Xavier De Snoeck]

(In reply to Nils Ohlmeier [:drno] from comment #9)
> (In reply to Nils Ohlmeier [:drno] from comment #8)
> > My bet is that he is hitting this:
> > https://dxr.mozilla.org/mozilla-central/source/media/mtransport/
> > nriceresolver.cpp#175
> 
> Xavier it would help if you could quickly test if your Intranet scenario
> still works with Firefox 41, but breaks with Firefox 42.

I confirm that it works on FF 41 (41.0 or 41.0.2) but not on FF42.

Comment 12

[Nils Ohlmeier [:drno]]

Thanks for the confirmation Xavier. Can you please try if this build: http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-e5c35f59dbf0a98d98d3d02f76fda59919ae77ea/ fixes the problem for you?

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

Tentatively assigning to drno since he seems to know what's happening (and have a tentative solution).

44 is affected, but given current plans I doubt an uplift would be approved

Comment 14

[Xavier De Snoeck]

This new build of Nightly fixes the problem.
Can you tell me on which version the fix will be included ?
Thanks for your help.

Xav

Comment 15

[Nils Ohlmeier [:drno]]

Attachment: MozReview Request: Bug 1237299: addedd missing address family to DNS lookup for proxies

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

Comment 16

[Nils Ohlmeier [:drno]]

Xavier, the fix is definitely going to be in Firefox 46, which is currently Nightly. An uplift request to 45 (currently Dev-Edition) seems reasonable, but as usually only security patches are accepted into our Beta releases it seems rather unlikely that the fix will make it into 44.

Comment 17

[Xavier De Snoeck]

OK, thanks.
What is the planning for 45 or 46 versions ?

Comment 18

[Byron Campen [:bwc]]

Comment on attachment 8706817 [details]
MozReview Request: Bug 1237299: addedd missing address family to DNS lookup for proxies

https://reviewboard.mozilla.org/r/30493/#review27251

::: media/mtransport/third_party/nICEr/src/net/nr_proxy_tunnel.c:298
(Diff revision 1)
> +      ABORT(R_NOT_FOUND);

I would either ABORT(r), or maybe use R_INTERNAL, since this should never happen.

Comment 19

[Nils Ohlmeier [:drno]]

Comment on attachment 8706817 [details]
MozReview Request: Bug 1237299: addedd missing address family to DNS lookup for proxies

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30493/diff/1-2/

Comment 20

[Nils Ohlmeier [:drno]]

Check on try run results later.

Comment 21

[Nils Ohlmeier [:drno]]

(In reply to Xavier De Snoeck from comment #17)
> What is the planning for 45 or 46 versions ?

45 and 46 follow the usual release plan at Mozilla: https://wiki.mozilla.org/RapidRelease/Calendar#Future_branch_dates
So 45 will become the official release in March and 46 will become the release in April.

Comment 22

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d79ca05d7e8

Comment 23

[Nils Ohlmeier [:drno]]

Comment on attachment 8706817 [details]
MozReview Request: Bug 1237299: addedd missing address family to DNS lookup for proxies

Just asking for permission early to get it before the next uplift. I'm cool with letting this patch mature on Nightly before actually uplifting it.

Approval Request Comment
[Feature/regressing bug #]: Landing bug 797262 in Fx 42 introduced this new bug.

[User impact if declined]: User who are required to use a local HTTP proxy are no longer able to connect to their TURN server through the proxy and therefore might no longer be able to connect their WebRTC calls.

[Describe test coverage new/current, TreeHerder]: There is currently no test coverage for real DNS resolution of proxy names. Our unit tests use a fake DNS resolver which is not affected by this bug. So this is only covered by manual testing.

[Risks and why]: The risk is pretty low as the patch only properly initializes a value in a struct, which was left uninitialized since bug 797262 landed. But that value is properly initialized within the non-proxy code, which gets execute a lot more often then the code path in this patch, and that path hasn't caused any problems since landing of bug 797262.

[String/UUID change made/needed]: N/A

Comment 24

[Carsten Book [:Tomcat]]

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

Comment 26

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8706817 [details]
MozReview Request: Bug 1237299: addedd missing address family to DNS lookup for proxies

Sure, taking it. We have time to figure out potential regressions.

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a0ac106e1252