trevj Reporter
	Description • 9 years ago

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.33 Safari/537.36

Steps to reproduce:

I work on uProxy, a peer-to-peer VPN packaged as a browser add-on for Chrome and Firefox. We are unable to establish a peerconnection in Firefox 42 onwards.

It seems that mozRTCPeerConnection.createOffer never resolves. I've attached a minimal add-on that reproduces the problem; at the end is its index.js. This works fine in Firefox 41.

Could this be a bug accidentally introduced by the privacy-related createOffer/createAnswer hooks in 42?:
https://hacks.mozilla.org/2015/09/controlling-webrtc-peerconnections-with-an-extension/

```
const {Cu} = require("chrome");
Cu.import('resource://gre/modules/Services.jsm');

var hiddenWindow = Services.appShell.hiddenDOMWindow;
mozRTCPeerConnection = hiddenWindow.mozRTCPeerConnection;

var config = {
  iceServers: [{url: 'stun:stun.l.google.com:19302'}]
};

var offerer = new mozRTCPeerConnection(config, undefined);
offerer.createDataChannel('chan');
offerer.createOffer(function(offerSdp) {
  console.info('created offer: ' + offerSdp.sdp);
}, function(e) {
  console.error('could not create offer: ' + e.message);
});

```


Actual results:

Neither success nor failure callback is invoked.


Expected results:

An offer should be generated, and output to the console.

	Nils Ohlmeier [:drno]
	Comment 1 • 9 years ago

I just pasted your sample code into a jsfiddle here: http://jsfiddle.net/kcskot9h/

And I see the following result in the browser console:

RTCIceServer.url is deprecated! Use urls instead. <unknown>
created offer: v=0
o=mozilla...THIS_IS_SDPARTA-42.0a2 4733114827003998560 0 IN IP4 0.0.0.0
s=-
t=0 0
a=fingerprint:sha-256 22:77:F1:0E:A4:3A:81:CB:99:F4:6E:4B:24:39:53:A7:71:92:0B:D3:68:5F:A4:35:AA:D3:79:5E:AE:24:2D:CF
a=ice-options:trickle
a=msid-semantic:WMS *
m=application 9 DTLS/SCTP 5000
c=IN IP4 0.0.0.0
a=sendrecv
a=ice-pwd:1bc02f99f9eb1c4d1035b11b42844a0b
a=ice-ufrag:14abc4cd
a=mid:sdparta_0
a=sctpmap:5000 webrtc-datachannel 256
a=setup:actpass
a=ssrc:3845417755 cname:{5e9f9745-7de3-674b-9396-004b2c68aa5d}

So it seems to work for me.

One question: do you know if you have multi-process turned on (on the first page of the Preferences)? If so can you try turning it off?

Another request for trying to debug this: can you try to reproduce this problem with a clean fresh profile (to make sure that not some setting or add on is causing this)?

	trevj Reporter
	Comment 2 • 9 years ago

Hi Nils,

Many thanks for trying to reproduce -- I think there might be a misunderstanding, though: uProxy's a Firefox add-on and I'm only seeing this issue in an add-on context.

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 3 • 9 years ago

I get:

    TypeError: mm is null

here http://mxr.mozilla.org/mozilla-central/source/browser/modules/ContentWebRTC.jsm?rev=e87f635d3540&mark=112-112#94

so yes it looks like a regression from Bug 1189060. Good guess.

Hmm, no message manager on the hidden window? I need to investigate.

	trevj Reporter
	Comment 4 • 9 years ago

Horrible workaround for createOffer bug in add-ons in 42.

	trevj Reporter
	Comment 5 • 9 years ago

Jan,

Many thanks for looking at this. I hadn't spotted the TypeError.

I spent some time studying the changes between 41 and 42 and was able to come up with a workaround. I've attached an updated zip.

As you can see, it's pretty bad and of course I'd rather help with a proper fix. Is there some other mechanism we could use for ContentWebRTC -> webrtcUI communication, e.g. Services.obs?

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 6 • 9 years ago

This new permission code shouldn't run at all for add-ons (which are effectively privileged code). Patch coming up.

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 7 • 9 years ago

Bug 1207784 - skip permission hooks in createOffer when called from privileged code (add-ons).

	Martin Thomson [:mt:]
	Comment 8 • 9 years ago

Comment on attachment 8668595 [details]
MozReview Request: Bug 1207784 - skip permission hooks in createOffer when called from privileged code (add-ons).

https://reviewboard.mozilla.org/r/21005/#review18879

::: dom/media/PeerConnection.js:392
(Diff revision 1)
> +    this._chrome = Services.scriptSecurityManager.isSystemPrincipal(principal);

`this._isChrome` might be clearer.

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 9 • 9 years ago

whoops, missed nit.

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 10 • 9 years ago

Comment on attachment 8668595 [details]
MozReview Request: Bug 1207784 - skip permission hooks in createOffer when called from privileged code (add-ons).

Bug 1207784 - skip permission hooks in createOffer when called from privileged code (add-ons).

	Pulsebot
	Comment 11 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/c36e02a92481

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 12 • 9 years ago

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

	trevj Reporter
	Comment 13 • 9 years ago

Many thanks for the patch.

It looks like this should be in 44.0a1? I just downloaded it from https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/ and ran my original test case (createoffer.zip, attached) but I see the same problem. Is there something else I need to do in my add-on code?

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 14 • 9 years ago

Hmm, it works for me here using that (or a slightly modified version of that), and I see createOffer called without any of the previous symptoms. 

The patch simply removes the permission code when the caller is detected as privileged (e.g. chrome / add-on code).

How exactly are you testing it? Are you doing jpm xpi, and adding as manual add-on?

	trevj Reporter
	Comment 15 • 9 years ago

I've been using jpm run, e.g. jpm run -b ~/Downloads/firefox-44.0a1/firefox

I just packaged it as an XPI and see the same issue.

How did you modify the add-on? Could you please upload it?

	trevj Reporter
	Comment 16 • 9 years ago

Ah, I misunderstood -- I guess we're not using the same version of Firefox. From examining my omni.ja, though, your fix is in my build (last night's trunk, timestamped October 5).

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 17 • 9 years ago

FWIW I tried it with Nightly downloaded today, with your exact index.js, using "jpm xpi" and "Install Add-on From File...", and then hit the [Debug] button next to the add-on, and I get:

> createoffer:created offer: v=0
> o=mozilla...THIS_IS_SDPARTA-44.0a1 8325662961406882293 0 IN IP4 0.0.0.0
> s=-
> t=0 0
> (... etc.)
> index.js:17

From your last comment, do I understand right that it works for you as well now?

	trevj Reporter
	Comment 18 • 9 years ago

Thanks for testing! It sure sounds like we're running the same version but, no, it still does not work for me.

FWIW, this is my *exact* Firefox version:
https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/firefox-44.0a1.en-US.linux-x86_64.tar.bz2

With both "jpm run" and "jpm xpi" I get no offer (and still see "TypeError: mm is null" in the JavaScript console).

Sorry, it's probably something wrong with my setup but I'm running out of ideas.

	Martin Thomson [:mt:]
	Comment 19 • 9 years ago

trevj, that's a link to a moving target; it changes every day (I don't recall when exactly...)  It should be OK if you downloaded it today, but just to double-check, can you go to `about:` and get the build identifier?

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 20 • 9 years ago

Btw, I tested with 44.0a1 (2015-10-05) on OSX:

> ★ ~/moz/git/createoffer $ cat index.js 
> // with help from:
> //   https://github.com/muaz-khan/WebRTC-Experiment/blob/master/demos/client-side-datachannel.html#L238
> 
> const {Cu} = require("chrome");
> Cu.import('resource://gre/modules/Services.jsm');
> 
> var hiddenWindow = Services.appShell.hiddenDOMWindow;
> mozRTCPeerConnection = hiddenWindow.mozRTCPeerConnection;
> 
> var config = {
>   iceServers: [{url: 'stun:stun.l.google.com:19302'}]
> };
> 
> var offerer = new mozRTCPeerConnection(config, undefined);
> offerer.createDataChannel('chan');
> offerer.createOffer(function(offerSdp) {
>   console.info('created offer: ' + offerSdp.sdp);
> }, function(e) {
>   console.error('could not create offer: ' + e.message);
> });
> 
> ★ ~/moz/git/createoffer $ jpm run -b /Applications/FirefoxNightly.app/Contents/MacOS/firefox
> JPM [info] Starting jpm run on reproduces createOffer bug in FF42+
> Creating XPI
> JPM [info] XPI created at /var/folders/mf/7kfd8gkj3mlcv2_14yxmwh2r0000gn/T/@createoffer-0.0.1.xpi (48ms)
> Created XPI at /var/folders/mf/7kfd8gkj3mlcv2_14yxmwh2r0000gn/T/@createoffer-0.0.1.xpi
> JPM [info] Creating a new profile
> console.info: createoffer: created offer: v=0
> o=mozilla...THIS_IS_SDPARTA-44.0a1 2583180959484781026 0 IN IP4 0.0.0.0
> s=-
> t=0 0

	trevj Reporter
	Comment 21 • 9 years ago

Thanks for testing again. I just ran it on my OSX box and your fix works!

FWIW, I tested on my regular Linux desktop, a Docker container, and a third Linux box. Perhaps something is up with the Linux builds right now. I will keep trying on Linux and let you know if it doesn't start working soon.

	trevj Reporter
	Comment 22 • 9 years ago

BTW, @mt, my build identifier:
Build identifier: Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0

Identical, Linux aside, to the build identifier on my OSX box.

	trevj Reporter
	Comment 23 • 9 years ago

I guess the fix works -- many thanks again.

Since this addresses a serious regression (uProxy, as one example, is completely broken), I wanted to ask if it can be included in 42?

	Randell Jesup [:jesup] (needinfo me)
	Comment 24 • 9 years ago

Jib: please update the affected-by tracking entries

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 25 • 9 years ago

Comment on attachment 8668595 [details]
MozReview Request: Bug 1207784 - skip permission hooks in createOffer when called from privileged code (add-ons).

Approval Request Comment
[Feature/regressing bug #]: Bug 1189060
[User impact if declined]: WebRTC stops working in add-ons come 42.

[Describe test coverage new/current, TreeHerder]:
Landed on m-c. Verified fixed manually by me and reporter.

[Risks and why]: 
Very low risk, because it basically disables new code in the (WebRTC used from) add-on case.

Skips new hookable permission code added by Bug 1189060 when caller is privileged (taking instead the same code-path as B2G and media.navigator.permission.disabled=true today), because add-ons don't need permission, and because the permission code didn't work right for add-ons.

[String/UUID change made/needed]: none

	Sylvestre Ledru [:Sylvestre]
	Comment 26 • 9 years ago

Comment on attachment 8668595 [details]
MozReview Request: Bug 1207784 - skip permission hooks in createOffer when called from privileged code (add-ons).

We don't want to break addons. Taking it. Should be in 42 beta 5.

	Carsten Book [:Tomcat]
	Comment 27 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/319cc68b3658

	Carsten Book [:Tomcat]
	Comment 28 • 9 years ago

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

	trevj Reporter
	Comment 29 • 9 years ago

Sorry to re-open this...

I just tried 42 beta 5 in Linux and Windows and the fix isn't working: createOffer never resolves, and I see the mm TypeError.

This suggests that _isChrome is set to false. Could there be some issue with Services.scriptSecurityManager.isSystemPrincipal on these platforms? Is there any other way to determine the context?

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 30 • 9 years ago

I was discussing this in #addons earlier today. Turns out the hiddenWindow is XUL on OS X, but HTML on other systems (do add-on developers know about this?), which means it is not privileged on other systems.

The addon is calling new hiddenWindow.mozPeerConnection().createOffer(), and the way I chose to detect whether this add-on was privileged code or not was to test Cu.getWebIDLCallerPrincipal(), and it returns true on OSX and false on Windows (and I assume Linux).

I'm looking for help for a better way to detect this situation.

	trevj Reporter
	Comment 31 • 9 years ago

Thanks for the update. That is quite unfortunate. At least we caught it now...hoping there's a simple solution that works on all platforms.

	Boris Zbarsky [:bzbarsky]
	Comment 32 • 9 years ago

[Tracking Requested - why for this release]: Not fixed.

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 33 • 9 years ago

Bug 1207784 - skip permission hooks in createOffer when called from hiddenWindow (add-ons).

	Martin Thomson [:mt:]
	Comment 34 • 9 years ago

Comment on attachment 8675042 [details]
MozReview Request: Bug 1207784 - skip permission hooks in createOffer when called from hiddenWindow (add-ons).

https://reviewboard.mozilla.org/r/22313/#review19925

This looks OK.  The hiddenWindow is only in the parent, so I don't see any way to use this for sandbox escape.  Worst case I can think of is that you get access to RTCPeerConnection.

	Pulsebot
	Comment 35 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/62c613fe9297

	Carsten Book [:Tomcat]
	Comment 36 • 9 years ago

landed now the checkin-needed request (pulsebot will comment with the cset url) but wonder if this now need now approval for beta and aurora again when i read comment #29.

	Sylvestre Ledru [:Sylvestre]
	Comment 37 • 9 years ago

Indeed, jib, could you propose an uplift to aurora and beta?
Next time, please open a new bug for the followup actions.

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Assignee
	Comment 38 • 9 years ago

Comment on attachment 8675042 [details]
MozReview Request: Bug 1207784 - skip permission hooks in createOffer when called from hiddenWindow (add-ons).

Approval Request Comment
[Feature/regressing bug #]: Bug 1189060
[User impact if declined]: WebRTC stops working in add-ons come 42 on all platforms but OSX.

[Describe test coverage new/current, TreeHerder]:
Landed on m-c. Verified fixed manually.

[Risks and why]: 
Very low risk, because it just bypasses new code since release in the (WebRTC used from) hiddenWindow case.

Skips new hookable permission code added by Bug 1189060 when calling hiddenWindow.mozRTCPeerConnection (taking instead the same code-path as B2G and media.navigator.permission.disabled=true today), because add-ons that need to use hiddenWindow for access to RTCPeerConnection don't need permission, and because the permission code broke RTCPeerConnection in this case.

See also comment 34. The worst case of being able to get access to RTCPeerConnection, is no different than on release today.

[String/UUID change made/needed]: none

	Sylvestre Ledru [:Sylvestre]
	Comment 39 • 9 years ago

Comment on attachment 8675042 [details]
MozReview Request: Bug 1207784 - skip permission hooks in createOffer when called from hiddenWindow (add-ons).

Thanks. Should be in beta 8

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 40 • 9 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 41 • 9 years ago
bugherder uplift

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

	Carsten Book [:Tomcat]
	Comment 42 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/62c613fe9297

	Carsten Book [:Tomcat]
	Comment 43 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/62c613fe9297