Closed Bug 1237299 Opened 5 years ago Closed 5 years ago

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

Categories

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

43 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox42 --- wontfix
firefox43 --- wontfix
firefox44 --- wontfix
firefox45 --- fixed
firefox46 --- fixed
Blocking Flags:

People

(Reporter: xavier.desnoeck, Assigned: drno)

References

Details

(Keywords: regression)

Attachments

(4 files)

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 ?
Has Regression Range: --- → no
Component: Untriaged → WebRTC: Networking
Keywords: regression
Product: Firefox → Core
My first thought was that this was a regression from bug 949703, but it seems that this landed in 38.
Can you attach a saved copy of about:webrtc?
Flags: needinfo?(xavier.desnoeck)
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
Flags: needinfo?(xavier.desnoeck)
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.
I've done a try push with better logging, binaries should end up here:

https://archive.mozilla.org/pub/firefox/try-builds/bcampen@mozilla.com-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.
Here is the log file of about:webrtc on the device with the Nightly build.

Hope this helps

Xav
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.
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.
See Also: → 1233652
(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.
Flags: needinfo?(xavier.desnoeck)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Here is the console logs for Nightly with the variable environment set.
Hope this helps

Xav
Flags: needinfo?(xavier.desnoeck)
(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.
Thanks for the confirmation Xavier. Can you please try if this build: http://archive.mozilla.org/pub/firefox/try-builds/drno@ohlmeier.org-e5c35f59dbf0a98d98d3d02f76fda59919ae77ea/ fixes the problem for you?
Flags: needinfo?(xavier.desnoeck)
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
Assignee: nobody → drno
backlog: --- → webrtc/webaudio+
Rank: 12
Priority: -- → P1
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
Flags: needinfo?(xavier.desnoeck)
Attachment #8706817 - Flags: review?(docfaraday)
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.
OK, thanks.
What is the planning for 45 or 46 versions ?
Attachment #8706817 - Flags: review?(docfaraday) → review+
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 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/
Check on try run results later.
Flags: needinfo?(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.
Has Regression Range: no → yes
Depends on: 797262
Flags: needinfo?(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
Attachment #8706817 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/3d79ca05d7e8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
See Also: → 1240131
Duplicate of this bug: 1240131
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.
Attachment #8706817 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.