1207784 - RTCPeerConnection in add-ons broke in 42

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[trevj]

Attachment: createoffer.zip

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.

Comment 1

[Nils Ohlmeier [:drno]]

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)?

Comment 2

[trevj]

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.

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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.

Comment 4

[trevj]

Attachment: createoffer-horrible-workaround.zip

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

Comment 5

[trevj]

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?

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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

Comment 7

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: 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).

Comment 8

[Martin Thomson [:mt:]]

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.

Comment 9

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

whoops, missed nit.

Comment 10

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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).

Comment 11

[Pulsebot]

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

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 13

[trevj]

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?

Comment 14

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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?

Comment 15

[trevj]

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?

Comment 16

[trevj]

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).

Comment 17

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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?

Comment 18

[trevj]

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.

Comment 19

[Martin Thomson [:mt:]]

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?

Comment 20

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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

Comment 21

[trevj]

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.

Comment 22

[trevj]

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.

Comment 23

[trevj]

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?

Comment 24

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

Jib: please update the affected-by tracking entries

Comment 25

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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

Comment 26

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 27

[Carsten Book [:Tomcat]]

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

Comment 28

[Carsten Book [:Tomcat]]

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

Comment 29

[trevj]

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?

Comment 30

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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.

Comment 31

[trevj]

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.

Comment 32

[Boris Zbarsky [:bzbarsky]]

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

Comment 33

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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

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

Comment 34

[Martin Thomson [:mt:]]

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.

Comment 35

[Pulsebot]

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

Comment 36

[Carsten Book [:Tomcat]]

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.

Comment 37

[Sylvestre Ledru [:Sylvestre]]

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

Comment 38

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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

Comment 39

[Sylvestre Ledru [:Sylvestre]]

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

Comment 40

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 41

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 42

[Carsten Book [:Tomcat]]

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

Comment 43

[Carsten Book [:Tomcat]]

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