Last Comment Bug 692677 - Relax same-origin XHR restrictions for privileged applications
: Relax same-origin XHR restrictions for privileged applications
Status: RESOLVED FIXED
[qa-]
: dev-doc-needed
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla16
Assigned To: Philipp von Weitershausen [:philikon]
:
:
Mentors:
Depends on: 749372 761227 761479 763814 765468 768804 788369 811432
Blocks: webapi b2g-product-phone
  Show dependency treegraph
 
Reported: 2011-10-06 17:57 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2015-10-18 12:00 PDT (History)
38 users (show)
ptheriault: sec‑review+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
2.2?
?
backlog


Attachments
wip 1 (6.98 KB, patch)
2012-06-01 17:18 PDT, Philipp von Weitershausen [:philikon]
jonas: feedback+
Details | Diff | Splinter Review
wip 2 (12.26 KB, patch)
2012-06-03 13:57 PDT, Philipp von Weitershausen [:philikon]
jonas: feedback+
Details | Diff | Splinter Review
v1 (27.26 KB, patch)
2012-06-04 19:20 PDT, Philipp von Weitershausen [:philikon]
jonas: review+
Details | Diff | Splinter Review
v1.1 (27.29 KB, patch)
2012-06-06 18:42 PDT, Philipp von Weitershausen [:philikon]
no flags Details | Diff | Splinter Review
Clicker (15 bytes, patch)
2015-10-18 11:58 PDT, super420840
no flags Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-06 17:57:23 PDT
Let's bring back |netscape.security.PrivilegeManager.enablePrivilege('UniversalBrowserRead');| hahaha!

But seriously, we need a way to enable this.  There are many use cases.  Probably the most compelling case for the purposes of this bug is a client-side-only, Google Reader-style news aggregator.  Yes, CORS is a partial solution for XHR, but it's naive to assume all aggregated servers will support it.  The economics of the situation are wrong too: only the aggregator app suffers by not being able to compete with native apps.  The app can't pressure servers into better CORS support unless it has a lot of users, but the app can't get users unless news servers support CORS.  A better market is setting up a privilege barrier for cross-origin access in the news aggregator.  Then the happy little world of CORS-compliant servers and apps that don't need explicit cross-origin privileges would theoretically have a better UX than the crufty old world, and possibly become more popular.

This is a pretty interesting design problem.  We certainly want sites to explicitly request this privilege.  (Popping up a doorhanger saying "Do you want to allow cross-origin access?" whenever a site generates a same-origin exception is a UX and security nightmare.  Looking at you, IE ...)  There's not really a clean way to have users implicitly grant this privilege in context, in general: the news aggregator wants to show a feed when it first loads, built by script, not in response to a user action.

At the same time, "do you want to relax same-origin?" is a very difficult question to ask users.  I can't think of a simple way to phrase it, off the top of my head.

This /could/ be a purely UA-side feature, but since we want sites to explicitly request this privilege I think we need API for it.  I think we can focus on XHR for the time being.  There are use cases for relaxed same-origin with canvas.getImageData(), e.g., but I can't think of one that couldn't be worked around easily enough with a cross-origin binary XHR.

Anyone have clever ideas for API?  Here's a strawman proposal: add an attribute "crossOrigin" to XMLHttpRequest.  If |req.send()| is called on an XMLHttpRequest |req| with |req.crossOrigin = true|, then it's up to UAs to allow or deny the request.  If the request is denied, the UA can pretend like the request timed out.  (This obviously doesn't address the UI problem.)
Comment 1 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-06 18:03:26 PDT
(In reply to Chris Jones [:cjones] [:warhammer] from comment #0)
> But seriously, we need a way to enable this.  There are many use cases. 
> Probably the most compelling case for the purposes of this bug is a
> client-side-only, Google Reader-style news aggregator.

How about allowing apps to register interest in RSS feeds (using Web Intents or registerProtocolHandler or something new) and then have browser UI for RSS feeds that lets the user choose to view a feed in a registered Web app?

Question: what access to cross-origin content is actually needed here? An app can load a cross-origin page into an IFRAME. What else is needed?

And what's wrong with Google Reader that makes you want to do it "client-side-only"?
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-06 18:19:06 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #1)
> (In reply to Chris Jones [:cjones] [:warhammer] from comment #0)
> > But seriously, we need a way to enable this.  There are many use cases. 
> > Probably the most compelling case for the purposes of this bug is a
> > client-side-only, Google Reader-style news aggregator.
> 
> How about allowing apps to register interest in RSS feeds (using Web Intents
> or registerProtocolHandler or something new) and then have browser UI for
> RSS feeds that lets the user choose to view a feed in a registered Web app?
> 

What manages the RSS feed subscriptions in your example?  

> Question: what access to cross-origin content is actually needed here? An
> app can load a cross-origin page into an IFRAME. What else is needed?
> 

Here are some more use cases that would need cross-origin permissions
 - RSS feed
 - screen-scraping in order to automatically summarize articles
 - fetch images from a flickr feed, say, allow editing / mashing up

> And what's wrong with Google Reader that makes you want to do it
> "client-side-only"?

Privacy, scalability, ...?  Doesn't really matter though.
Comment 3 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-10-06 18:22:27 PDT
I do think that we'll want to allow privileged applications raw TCP socket access. There are lots of use cases out there for communicating with network connected hardware out there.

Raw TCP socket access is certainly more scary than XHR access in many ways (can reach hardware behind firewalls, etc), but it's actually safer than cross-origin XHR in *one* way. With raw TCP socket access the browser won't inject the users credentials (cookis, auth:-headers) into the request. So you can't really do anything that you couldn't do directly from the server. Except accessing things behind firewalls of course :-)

Another data point is that one of the most requested privileges for android apps is internet access. Which I *think* is equivalent to raw TCP socket access. Apparently this is something that almost every android app asks for. Though I do think we can get that down significantly by offering the normal same-origin-plus-CORS-plus-websocket security model of the web.

And in the cases where we grant raw TCP socket access, we can also grant "anonymous" XHR access. I.e. the ability to make XHR requests which doesn't include the users credentials.

In either case, I definitely think that the app should need to ask for elevated privileges to get this access though.
Comment 4 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-06 18:49:32 PDT
(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #1)
> > (In reply to Chris Jones [:cjones] [:warhammer] from comment #0)
> > > But seriously, we need a way to enable this.  There are many use cases. 
> > > Probably the most compelling case for the purposes of this bug is a
> > > client-side-only, Google Reader-style news aggregator.
> > 
> > How about allowing apps to register interest in RSS feeds (using Web Intents
> > or registerProtocolHandler or something new) and then have browser UI for
> > RSS feeds that lets the user choose to view a feed in a registered Web app?
> 
> What manages the RSS feed subscriptions in your example?

The Web application. Browser UI would only be required for the initial access grant.

>  - fetch images from a flickr feed, say, allow editing / mashing up

Flickr already allows this via JSON-P.

> > And what's wrong with Google Reader that makes you want to do it
> > "client-side-only"?
> 
> Privacy, scalability, ...?  Doesn't really matter though.

Actually it does matter, because it affects what you'd be willing to do on a server.
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-06 18:49:55 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #4)
> >  - fetch images from a flickr feed, say, allow editing / mashing up
> 
> Flickr already allows this via JSON-P.

Hmm, maybe not the actual image data.
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-06 18:58:39 PDT
Flickr does support feeds though, so if you use browser UI to establish a connection between a Flickr feed and a Web app, then we could grant the app read access to the contents of <link rel="enclosure"> resources.

Another idea for the API could be <input type="feed">, where the browser UI pops up a list of the feeds exposed by your open tabs or frequently visited sites or something.
Comment 7 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-10-06 19:03:00 PDT
Flickr could easily make image data available now by using CORS. This would work great with the new crossorigin attribute added to <img> in combination with <canvas>. (though I think we should add <img>.getImageData() and <video>.getImageData() as well)
Comment 8 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-06 19:28:22 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #4)
> (In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #1)
> > > (In reply to Chris Jones [:cjones] [:warhammer] from comment #0)
> > > > But seriously, we need a way to enable this.  There are many use cases. 
> > > > Probably the most compelling case for the purposes of this bug is a
> > > > client-side-only, Google Reader-style news aggregator.
> > > 
> > > How about allowing apps to register interest in RSS feeds (using Web Intents
> > > or registerProtocolHandler or something new) and then have browser UI for
> > > RSS feeds that lets the user choose to view a feed in a registered Web app?
> > 
> > What manages the RSS feed subscriptions in your example?
> 
> The Web application. Browser UI would only be required for the initial
> access grant.
> 

OK, I understand now.  Some issues
 - what happens if two installed apps want to read a different set of RSS feeds for different reasons?
 - what happens if one application wants to read an RSS feed from its own domain, for its own use?
 - privacy problem when switching from default RSS reader X to reader Y: if X still maintains a set of subscriptions and tries to access them, Y will be used as the handler, which leaks info from X to Y without the user necessarily knowing

I think this approach would reduce to granting particular applications "RSS privileges" for their own use, which seems overly specific.

> >  - fetch images from a flickr feed, say, allow editing / mashing up
> 
> Flickr already allows this via JSON-P.
> 

JSON-P is horrible :P.  But more importantly, it has the same economic problem as CORS: JSON-P requires server-side changes for web apps that aren't needed for native apps, which leaves web apps at an inherent disadvantage with no leverage to change the situation.

> > > And what's wrong with Google Reader that makes you want to do it
> > > "client-side-only"?
> > 
> > Privacy, scalability, ...?  Doesn't really matter though.
> 
> Actually it does matter, because it affects what you'd be willing to do on a
> server.

Requiring a Google-sized server infrastructure is tremendously unfair economically to Joe Webauthor, and in the grand scheme of things, it's worse for users because maintaining a secure and privacy-respecting server infrastructure is tremendously hard.  I don't think this example matters because it doesn't really bear on whether cross-origin privileges are fundamentally too insecure or difficult.  For example, we don't let web apps run machine code (well, unless they call themselves "extensions") not because computationally-intensive work can be offloaded to servers, but because JS is fast enough to outweigh the fundamental problems of machine code.
Comment 9 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-06 19:33:52 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> Another idea for the API could be <input type="feed">, where the browser UI
> pops up a list of the feeds exposed by your open tabs or frequently visited
> sites or something.

But then you have to specify not only RSS but the format of the feeds, so you know what data can be loaded by the app.  Right?  Do we really want to do that?
Comment 10 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-06 19:55:43 PDT
(In reply to Chris Jones [:cjones] [:warhammer] from comment #8)
> OK, I understand now.  Some issues
>  - what happens if two installed apps want to read a different set of RSS
> feeds for different reasons?

The user could direct different feeds to different apps. I'm not sure what the problem is.

>  - what happens if one application wants to read an RSS feed from its own
> domain, for its own use?

They can just do that using XHR I guess. What's the problem?

>  - privacy problem when switching from default RSS reader X to reader Y: if
> X still maintains a set of subscriptions and tries to access them, Y will be
> used as the handler, which leaks info from X to Y without the user
> necessarily knowing

I imagine we'd simply give apps permission to read particular feeds. Switching default readers would have to be some kind of mass permission copy. If that's a one-time thing, no additional information would be leaked.

> JSON-P is horrible :P.  But more importantly, it has the same economic
> problem as CORS: JSON-P requires server-side changes for web apps that
> aren't needed for native apps, which leaves web apps at an inherent
> disadvantage with no leverage to change the situation.

"No leverage" overstates the case. There are plenty of popular Web apps out there that would benefit from CORS access. There are existing incentives to provide it.

(In reply to Chris Jones [:cjones] [:warhammer] from comment #9)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> > Another idea for the API could be <input type="feed">, where the browser UI
> > pops up a list of the feeds exposed by your open tabs or frequently visited
> > sites or something.
> 
> But then you have to specify not only RSS but the format of the feeds, so
> you know what data can be loaded by the app.  Right?  Do we really want to
> do that?

You could give the app XHR access to the feed URL and, possibly, to any <link rel="enclosure"> URIs found as the feed URL is parsed. The constraints on the feed format would be light. We'd have to require the feed be parsed via HTML or XML parsing, that's about it.

(I'm not really sure about <link rel="enclosure"> ... probably the editing use-case can be addressed by just having the user drag-and-drop or copy/paste the image they want to edit into the editor.)
Comment 11 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-06 22:22:27 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10)
> (In reply to Chris Jones [:cjones] [:warhammer] from comment #8)
> > OK, I understand now.  Some issues
> >  - what happens if two installed apps want to read a different set of RSS
> > feeds for different reasons?
> 
> The user could direct different feeds to different apps. I'm not sure what
> the problem is.
> 

Then registerProtocolHandler() doesn't work, and using an intent-like system would be weird because the browser would have to handle the intent and remember each individual feed permission granted.  It promotes the feed to a first-class web citizen.

> >  - what happens if one application wants to read an RSS feed from its own
> > domain, for its own use?
> 
> They can just do that using XHR I guess. What's the problem?
> 

I was thinking this would interact badly with registerProtocolHandler(), but it wouldn't.  Never mind.

> > JSON-P is horrible :P.  But more importantly, it has the same economic
> > problem as CORS: JSON-P requires server-side changes for web apps that
> > aren't needed for native apps, which leaves web apps at an inherent
> > disadvantage with no leverage to change the situation.
> 
> "No leverage" overstates the case. There are plenty of popular Web apps out
> there that would benefit from CORS access. There are existing incentives to
> provide it.
> 

OK, desktop web apps can apply leverage, which indirectly helps web apps on phones better compete with native apps.

> (In reply to Chris Jones [:cjones] [:warhammer] from comment #9)
> > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> > > Another idea for the API could be <input type="feed">, where the browser UI
> > > pops up a list of the feeds exposed by your open tabs or frequently visited
> > > sites or something.
> > 
> > But then you have to specify not only RSS but the format of the feeds, so
> > you know what data can be loaded by the app.  Right?  Do we really want to
> > do that?
> 
> You could give the app XHR access to the feed URL and, possibly, to any
> <link rel="enclosure"> URIs found as the feed URL is parsed. The constraints
> on the feed format would be light. We'd have to require the feed be parsed
> via HTML or XML parsing, that's about it.
> 

That doesn't scale to choosing an image from a flickr feed, e.g., but the drag-n-drop idea sounds plausible (if we had a way to do that on touch screens ...).  How would this work for choosing an audio clip to edit, say?  How would a user choose an article for a reader to screen-scrape and reformat?

What it sounds like all this boils down to is granting generic cross-origin request privileges to particular apps for particular URLs.  Then if the fetched data is HTML/XML-y, users can grant the requesting app access to embedded resources without a second explicit request from the app.  Those sound like orthogonal things to me.  I'm not so keen on the second part because (i) problem above of presenting UI for non-visual or really large items; (ii) practical issue of standardizing around HTML/XML-ish format for items ("Why not JSON?" is the first thing they'll say).  I could see something interesting being made of this though; I wonder if this could be implemented for simple cases as a "src" for <input>, say something like <input type="file" accept="image/*" src="[URI]">.  Apps could set the src to a data: URI for the flickr editor case, after they've parsed the stream.

What I *am* keen on is allowing the grant of particular cross-origin request privileges.  If this all makes sense, how does the proposal in comment 0 suit?

Hm, I just realized that there's a security issue with granting persistent cross-origin privileges to a particular URL: if say the owner of say ocallahan.org loses his domain registration, and another site takes it, then the app will still keep permissions to content that the user didn't explicitly grant.  https is a solution, but yuck, that would severely limit the usefulness of this.  Hmm...  Maybe not worth losing sleep over.
Comment 12 Lari Temmes 2011-10-24 07:30:25 PDT
I really hope we could have way to grant cross-domain priviliges for any domain. In my case, it's needed because i'm streaming live video from server A (localhost, using VLC), but loading HTML from server B (internet). I'm trying to do some realtime pixel manipulation for video canvas and getImageData() fails (Security error). My application is kiosk-style environment, so disabling security checks is not a issue and there should be some way to do this. It could be against spec, i don't know about that, but it should be noted that browsers are used more and more for specific applications that have only single purpose and normal security methods are not always needed. 

AFAIK, there is already solution for this, it's 'Access-Control-Allow-Origin' HTTP header that server sends when requested content is allowed to be used in cross-domain environment. However, i'm streaming video content from VLC, and it's not possible to modify HTTP-headers it generates, it's not possible to request cross-domain priviliges this way.

For me, it would be enough to have user setting that would toggle cross-domain checks on/off completely, but more elegant solution would be list of domains somewhere in settings UI.
Comment 13 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-24 13:28:11 PDT
The right thing to do there is to modify your video server to send Access-Control-Allow-Origin. VLC is open-source even.
Comment 14 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-24 17:32:41 PDT
Yes, this bug covers cases like client-side news aggregators in which it's not practical to guarantee Access-Control-Allow-Origin for all source servers.  When you control the server, that's not an issue.
Comment 15 Lari Temmes 2011-10-24 23:40:06 PDT
My needs were just an example, proposed solution would fix this issue for all cases. And since this functionality is already done (Access-Control-Allow-Origin) in Firefox, it only needs to be exposed to config file/settings UI. 

I will, however, see if patching VLC would be solution for my problem.
Comment 16 Lari Temmes 2011-10-26 01:37:38 PDT
I compiled latest version of VLC with modifications to send mentioned header to browser. My modifications seem to work as they should, since Firebug shows correct headers (meaning: Access-Control-Allow-Origin: *). However, i still get Security Error. Look's like it completely ignores this header.

Javascript code looks like this (just the frame processing loop, without any code that modified image data):

	copy.drawImage(video, 0, 0);
	output.clearRect(PAINTRECT.x, PAINTRECT.y, WinW, WinH);
	var imageData = copy.getImageData(0,0,VidW,VidH);
	/* do something with data here */
	output.drawImage(copycanvas, 0, 0, WinW, WinH);

Commenting out getImageData result video to be shown, with getImageData, it causes Security Error and script stops there. 

Am i missing something?
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-26 01:47:02 PDT
Yeah, we probably need to implement http://www.whatwg.org/specs/web-apps/current-work/#attr-media-crossorigin and you need to use it. Please file a new bug for this.
Comment 18 Lari Temmes 2011-10-26 04:16:11 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #17)
> Yeah, we probably need to implement
> http://www.whatwg.org/specs/web-apps/current-work/#attr-media-crossorigin
> and you need to use it. Please file a new bug for this.

Hmm, that's intresting, but i don't really see how it has much to do with this problem. This situation and problem is well documented in here, and it's already in spec :

http://www.w3.org/TR/cors/ (search for "Not tainting the canvas element")

Like i mentioned before, Firefox ignores mentioned headers when dealing with media files. I don't know, if those headers are used when using functions like XMLHttpRequest() but for getImageData() they are ignored.
Comment 19 Lari Temmes 2011-10-26 04:23:59 PDT
And i would like to add, spec only mentions toDataURL(), but it should be the same for getImageData()?
Comment 20 Boris Zbarsky [:bz] (still a bit busy) 2011-10-26 04:58:00 PDT
> http://www.w3.org/TR/cors/ (search for "Not tainting the canvas element")

Yes, but the point is that CORS is not applied to media by default because that would break web sites.  It needs to be opt-in on the part of the HTML embedding the media, and we don't implement that opt-in yet.

Now can you please stop adding noise to this bug and do what Robert asked you to do in comment 17, since it looks like what I asked you to do in bug 604496 comment 8 is already handled by the spec?  So all we need now is to add an implementation of that spec.  But that's not _this_ bug; it needs a separate bug on the Video/Audio component to track it.
Comment 21 Justin Lebar (not reading bugmail) 2012-01-31 21:28:28 PST
Why does this block bug 715782, b2g demo phone?  This seems like a somewhat experimental feature to me.
Comment 22 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-01-31 21:39:17 PST
What seems experimental about it?  How would I write a client-side news aggregator without cross-origin XHR?

But, orthogonally, no this isn't hard-blocking anything Mozilla is planning for demo-phone.
Comment 23 Justin Lebar (not reading bugmail) 2012-01-31 22:32:56 PST
> What seems experimental about it?  How would I write a client-side news aggregator without cross-
> origin XHR?

Just the idea that we should design an API for implementing entirely client-side RSS readers, I guess.  Surely this API is necessary if you want an entirely client-side news aggregator.  But I think the question is whether or not we want apps like that, or whether the client-server approach taken by Google Reader is sufficient.
Comment 24 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-01-31 22:39:32 PST
This has been hashed out in excruciating detail above.  If you have new information to add, please do.

If your opinion is that "no, this use case doesn't seem valid", I'll chalk that up if the decision comes down to a vote.
Comment 25 Daniel Veditz [:dveditz] 2012-03-21 16:50:04 PDT
(In reply to Justin Lebar [:jlebar] from comment #23)
> > What seems experimental about it?
> 
> Just the idea that we should design an API for implementing entirely
> client-side RSS readers, I guess.

UserScripts (GreaseMonkey and equivalents) support such loosening for XHR and it's been a popular and well-used feature. Can't think of a single one that's a client-side RSS reader. For "privileged applications" some measured amount of "loosening" could be useful.

The amount of relaxation and the mechanism needs a security review. Such apps could be poking holes in the security of the other-origin sites (maliciously or not) and we need to put the user in control. First step is to make them aware. Blanket cross-site access like Firefox chrome is a terrible idea. Could the app manifest declare a whitelist of 3rd-party sites it needs to access, probably via limited particular APIs like XHR, sockets, WebGL?
Comment 26 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-03-21 17:07:52 PDT
Sorry, the discussion refocused this bug around XHR (IMHO), but that wasn't reflected in the bug title.  Would prefer to keep this feature separate from the raw socket discussion, but the secreviews will be similar I suspect.

Yes, we can have manifests declare a whitelist.  I suspect that's at least an 80% solution to this use case.  (Doesn't work for a full Reader-style aggregator though, obviously.)
Comment 27 Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]] 2012-03-28 14:23:17 PDT
need to sched the sec-review on this one please work with me to do so
Comment 28 James Lal [:lightsofapollo] (inactive) 2012-05-22 09:48:15 PDT
This is a dep of P1 apps calendar, email.
Comment 29 Justin Lebar (not reading bugmail) 2012-05-22 10:43:16 PDT
I thought e-mail was using raw TCP sockets, not XHR.

Why does calendar care about cross-origin XHR?
Comment 30 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-22 10:49:22 PDT
CalDAV.
Comment 31 James Lal [:lightsofapollo] (inactive) 2012-05-22 10:51:09 PDT
ActiveSync (Msft Exchange) uses HTTP.

Calendar, Email and maybe Contacts will sync with it as part of P1 requirements as an additional backend.

Calendar also will use CalDav (WebDav/HTTP based protocol) as a backend (think calendar.[google|yahoo|etc..].com).

This is all in addition to the raw socket access Email needs for imap/smtp (and eventually pop).
Comment 32 Justin Lebar (not reading bugmail) 2012-05-22 11:04:55 PDT
Sync with Exchange is a P1?  That does not make sense to me.  iPhone 1 shipped without Exchange support, didn't it?

But I guess that's besides the point if we need this for CalDAV.  Thanks for clarifying.
Comment 33 Andrew Sutherland [:asuth] 2012-05-22 11:12:05 PDT
(In reply to Justin Lebar [:jlebar] from comment #32)
> Sync with Exchange is a P1?  That does not make sense to me.  iPhone 1
> shipped without Exchange support, didn't it?

Hotmail is #1 in Brazil.  Hotmail's supported sync mechanisms are POP (sucky experience), or ActiveSync.  Exchange is also very important, too.  But of course, any discussions of e-mail prioritization should be discussed elsewhere.
Comment 34 Lucas Adamski [:ladamski] 2012-05-24 06:09:55 PDT
Assuming that each app still gets its own cookie and password store, I don't see any risk for relaxing XHR same-origin restrictions for trusted and certified apps.  That's the current spec'ed behavior for the apps security model.
Comment 35 Justin Lebar (not reading bugmail) 2012-05-24 08:01:14 PDT
> I don't see any risk for relaxing XHR same-origin restrictions for trusted and certified 
> apps.

You mean, all trusted apps automagically get this privilege by virtue of being trusted, or any trusted app may apply for this privilege?

In any case, it would be a shame if my RSS reader had to be trusted; we should think about whether we can do this for untrusted apps too.
Comment 36 Lucas Adamski [:ladamski] 2012-05-24 08:12:04 PDT
Yes, but to be clear, only privileged assets in a trusted app can bypass same-origin.  Non-app content still has to obey it as its bound by the browser security model.  

I don't think you can relax this for any untrusted content; you'd punch a big hole in CSRF defenses (i.e. an ad or other 3rd-party content in an untrusted Facebook app could CSRF Facebook).
Comment 37 Justin Lebar (not reading bugmail) 2012-05-24 08:16:49 PDT
> Yes, but to be clear, only privileged assets in a trusted app can bypass same-origin.

Yes to what?

> I don't think you can relax this for any untrusted content; you'd punch a big hole in 
> CSRF defenses

I see.  That's a lot more concerning than the fact that you could find your way into intranets.
Comment 38 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-05-24 15:01:10 PDT
I think we should do this by always making requests with the "anonymous" flag set. That means that we'll never set any cookies automatically (The site can still do that by using setRequestHeader, but it have to provide the actual cookie values).

That should make it no more possible to CSRF a site than you can from the server.

However that still means that it's possible to read data from behind firewalls. Until we figure out how to handle that I think that means that we can only expose this to trusted apps.
Comment 39 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-05-30 00:15:12 PDT
Here's what I think we should do in this bug:

Add a argument to the XMLHttpRequest constructor, so something like:

new XMLHttpRequest({ anon: true, system: true });

When the 'system' is set to true the XHR object do not do any cross-origin security checks. Including for things like reading arbitrary response headers, and sending arbitrary request headers.

When the 'anon' is set to true the XHR object makes the request with the LOAD_ANONYMOUS flag set to true which causes no cookie headers or other authentication data to be added to the request by gecko.


For now we would only let trusted apps make requests with the 'system' flag set to true, and only when the 'anon' flag is also set to true.
Comment 40 Ben Francis [:benfrancis] 2012-06-01 09:44:53 PDT
I think I came across another use case for this today in the v1 B2G browser. I need to fetch favicons from different domains to store in global history in IndexedDB. The browser API tells the browser about favicon URLs, but doesn't return the actual favicon file. We could potentially change the API to return the file, but that's not ideal and may not work in all cases. Requiring all web sites to set Access-Control-Allow-Origin HTTP headers for favicons isn't really feasible!

BTW. on the subject of the calendar use case, CalDAV is a superset of WebDAV which is a superset of HTTP so cross-origin HTTP alone may not give you all the verbs etc. you need, you might need TCP socket access for that? It would enable you to subscribe to read-only iCalendar feeds over HTTP though, which is a start.
Comment 41 Philipp von Weitershausen [:philikon] 2012-06-01 10:16:47 PDT
Gonna take a stab at this.
Comment 42 Philipp von Weitershausen [:philikon] 2012-06-01 17:18:23 PDT
Created attachment 629403 [details] [diff] [review]
wip 1

I think this is how Jonas wants this to work. It's missing any kind of permission checking, but that's not the biggest problem. nsXMLHttpRequest::Initialize is no longer used from content, since XMLHttpRequest has the new Paris bindings now (see dom/bindings).

Need to find out whether there's a way to

(a) do dictionaries in WebIDL (might be easy to implement or work around if missing)
(b) define a custom constructor/constructor args with Paris bindings

bz, any guidance would be appreciated!
Comment 43 Boris Zbarsky [:bz] (still a bit busy) 2012-06-01 17:26:26 PDT
Huh, I had no idea we had dictionaries in XPIDL now!

For WebIDL, we currently don't have dictionary support at all.  As in, it needs parser support and then codegen.  I _think_ it shouldn't be too bad in the codegen; I'm not sure about how much work the parser end would be.

You could do a poor-man's dictionary for now with an optional "object" argument and explicit JSAPI property gets in the callee....  But if you have the time to do dictionaries, even just the parser part, that would rock.  I can definitely help with the codegen end, but I don't know the parser that well yet.
Comment 44 Johnny Stenback (:jst, jst@mozilla.com) 2012-06-01 17:40:22 PDT
What we've done (or at least attempted to do) for dictionaries in at least one other case is to define the dictionary in XPIDL, and make the type of the dictionary argument in the WebIDL be "any" (with implicitJSContext set in Bindings.conf). That translates into a jsval and JSContext* argument in the called C++ code, and then the called C++ code can use the dictionary helper stuff that smaug wrote.

See http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/dictionary_helper_gen.conf and http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsDOMEvent.cpp#388 for ideas of how the dictionary stuff works in XPIDL.

That's probably the path of least resistance until we have proper dictionary support in WebIDL.
Comment 45 Philipp von Weitershausen [:philikon] 2012-06-01 17:48:44 PDT
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #44)
> What we've done (or at least attempted to do) for dictionaries in at least
> one other case is to define the dictionary in XPIDL, and make the type of
> the dictionary argument in the WebIDL be "any" (with implicitJSContext set
> in Bindings.conf). That translates into a jsval and JSContext* argument in
> the called C++ code, and then the called C++ code can use the dictionary
> helper stuff that smaug wrote.

Sounds great! Do you know how I can get the Paris bindings to to do that on my constructor/initializer? I guess I'm not seeing how nsDOMEvent::InitFromCtor/Initialize are called. nsXMLHttpRequest::Initialize definitely isn't called anymore.
Comment 46 Johnny Stenback (:jst, jst@mozilla.com) 2012-06-01 17:57:42 PDT
nsXMLHttpRequest::Constructor() (inline in nsXMLHttpRequest.h) is called from the new bindings. The current constructor doesn't take any arguments, but see http://www.w3.org/TR/WebIDL/#Constructor for more. Also, I don't know if we have codegen that deals with constructors with arguments yet, or that we've ever tested that code if we have it.
Comment 47 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-06-01 18:18:18 PDT
Comment on attachment 629403 [details] [diff] [review]
wip 1

Review of attachment 629403 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good other than that it needs tests and security bits.

::: content/base/src/nsXMLHttpRequest.cpp
@@ +1719,5 @@
>  
>  bool
>  nsXMLHttpRequest::IsSystemXHR()
>  {
> +  return mIsSystem || !!nsContentUtils::IsSystemPrincipal(mPrincipal);

You can remove the !!. Not sure why those were there in the first place.
Comment 48 Boris Zbarsky [:bz] (still a bit busy) 2012-06-01 18:36:18 PDT
> Also, I don't know if we have codegen that deals with constructors with arguments yet

We do.

> or that we've ever tested that code if we have it.

We haven't.  Though I just added a basic test (that the right types of arguments at least are passed); see http://hg.mozilla.org/integration/mozilla-inbound/rev/fdb686511fc4
Comment 49 Philipp von Weitershausen [:philikon] 2012-06-01 18:55:06 PDT
Yeah I just looked through the tests [1] and traced the way that the constructor hook (which ends up calling MyClass::Constructor) is created in Codegen.py [2]. I might I have everything to make this work the way Johnny suggested.

[1] https://mxr.mozilla.org/mozilla-central/source/dom/bindings/parser/tests/test_constructor.py#44
[2] https://mxr.mozilla.org/mozilla-central/source/dom/bindings/Codegen.py#560
Comment 50 Boris Zbarsky [:bz] (still a bit busy) 2012-06-01 19:16:09 PDT
Note that you'll probably need bug 760749 fixed first, but I'm hoping that will happen mid-next-week (Kyle gets back from vacation Monday, give him two days to catch up on e-mail, then hopefully he'll review).
Comment 51 Philipp von Weitershausen [:philikon] 2012-06-01 19:28:25 PDT
(In reply to Boris Zbarsky (:bz) from comment #50)
> Note that you'll probably need bug 760749 fixed first

Even for "any" constructor argument, as Johnny suggested? But in any case, I'm happy to work on top of your patch.
Comment 52 Boris Zbarsky [:bz] (still a bit busy) 2012-06-01 19:31:06 PDT
> Even for "any" constructor argument, as Johnny suggested?

Oh, no.  "any" should just work.
Comment 53 Philipp von Weitershausen [:philikon] 2012-06-03 13:57:05 PDT
Created attachment 629656 [details] [diff] [review]
wip 2

Gallia est pacata.

Still lacking permission checks and tests, but this now properly integrates into the new bindings. Tested manually and it works. I should point out, however, that I haven't done anything about workers yet apart from making them compile.
Comment 54 Justin Lebar (not reading bugmail) 2012-06-03 14:43:08 PDT
> I think I came across another use case for this today in the v1 B2G browser. I need to fetch 
> favicons from different domains to store in global history in IndexedDB.

We can do <img src="the-favicon-url">, of course.

That's not ideal for a variety of reasons (principally, we don't control caching of the favicons and have no way to guarantee that they'll be there), so I agree that using this API would be an improvement.
Comment 55 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-06-03 17:35:03 PDT
Comment on attachment 629656 [details] [diff] [review]
wip 2

Review of attachment 629656 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsXMLHttpRequest.h
@@ +210,5 @@
>      mPrincipal = aPrincipal;
>      BindToOwner(aOwnerWindow);
>      mBaseURI = aBaseURI;
> +    mIsAnon = false;
> +    mIsSystem = false;

Hmm.. I think setting these in the ctor should be enough. I'd rather not rely on remembering to set these if we add more Construct functions.

::: dom/workers/XMLHttpRequest.cpp
@@ +1465,5 @@
>  XMLHttpRequest*
> +XMLHttpRequest::Constructor(JSContext* aCx,
> +                            JSObject* aGlobal,
> +                            const Optional<jsval>& aParams,
> +                            ErrorResult& aRv)

File a followup to implement in workers.
Comment 56 Philipp von Weitershausen [:philikon] 2012-06-04 10:23:26 PDT
(In reply to Jonas Sicking (:sicking) from comment #55)
> ::: dom/workers/XMLHttpRequest.cpp
> @@ +1465,5 @@
> >  XMLHttpRequest*
> > +XMLHttpRequest::Constructor(JSContext* aCx,
> > +                            JSObject* aGlobal,
> > +                            const Optional<jsval>& aParams,
> > +                            ErrorResult& aRv)
> 
> File a followup to implement in workers.

Filed bug 761227.
Comment 57 Philipp von Weitershausen [:philikon] 2012-06-04 19:20:33 PDT
Created attachment 630039 [details] [diff] [review]
v1

It's ready. Well, apart from the two follow-ups (bug 761227, bug 761479).
Comment 58 Philipp von Weitershausen [:philikon] 2012-06-04 19:22:27 PDT
Watch the try fireworks here: https://tbpl.mozilla.org/?tree=Try&rev=9c9b598b6a1d
Comment 59 Philipp von Weitershausen [:philikon] 2012-06-06 18:42:54 PDT
Created attachment 630801 [details] [diff] [review]
v1.1

rebased, ready to land
Comment 60 Philipp von Weitershausen [:philikon] 2012-06-07 11:33:15 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/57c5763ac3ac
Comment 61 Paul Theriault [:pauljt] 2012-06-08 01:03:08 PDT
Should there be another follow-up bug raised to implement the control that only trusted apps should be able to obtain this permission (instead of the whitelist in preferences)?
Comment 62 Graeme McCutcheon [:graememcc] 2012-06-08 04:20:06 PDT
https://hg.mozilla.org/mozilla-central/rev/57c5763ac3ac

(Merged by Ed Morley)
Comment 63 Daniel Veditz [:dveditz] 2012-06-11 19:46:53 PDT
Comment on attachment 630801 [details] [diff] [review]
v1.1

Review of attachment 630801 [details] [diff] [review]:
-----------------------------------------------------------------

drive-by review comments...

::: content/base/src/nsXMLHttpRequest.cpp
@@ +575,5 @@
> +nsXMLHttpRequest::InitParameters(JSContext* aCx, const jsval* aParams)
> +{
> +  XMLHttpRequestParameters* params = new XMLHttpRequestParameters();
> +  nsresult rv = params->Init(aCx, aParams);
> +  NS_ENSURE_SUCCESS(rv, rv);

If params->Init() fails we leak params

@@ +579,5 @@
> +  NS_ENSURE_SUCCESS(rv, rv);
> +
> +  // Check for permissions.
> +  nsCOMPtr<nsPIDOMWindow> window = do_QueryInterface(GetOwner());
> +  NS_ENSURE_TRUE(window && window->GetDocShell(), NS_OK);

If the window or the docshell don't exist we leak params

@@ +588,5 @@
> +    nsCOMPtr<nsIDocument> doc = do_QueryInterface(window->GetExtantDocument());
> +    NS_ENSURE_TRUE(doc, NS_OK);
> +
> +    nsCOMPtr<nsIURI> uri;
> +    doc->NodePrincipal()->GetURI(getter_AddRefs(uri));

You should use GetOrigin() instead which handles digging into the innermost URI and punycoding and cases when the URI doesn't match the origin. Or URIIsChromeOrInPref() should take a principal and have that done in there, or add another helper to nsContentUtils (like ::GetAsciiOrigin) that does the right thing for you.

@@ +596,5 @@
> +    }
> +  }
> +
> +  mIsAnon = params->mozAnon;
> +  mIsSystem = params->mozSystem;

looks params is leaked no matter what
Comment 64 Philipp von Weitershausen [:philikon] 2012-06-11 20:09:33 PDT
(In reply to Paul Theriault [:pauljt] from comment #61)
> Should there be another follow-up bug raised to implement the control that
> only trusted apps should be able to obtain this permission (instead of the
> whitelist in preferences)?

Yes. We'll have to do this for pretty much every API.

(In reply to Daniel Veditz [:dveditz] from comment #63)
> drive-by review comments...

Thanks! Ouch, leaks. Filed bug 763814.

> > +    nsCOMPtr<nsIDocument> doc = do_QueryInterface(window->GetExtantDocument());
> > +    NS_ENSURE_TRUE(doc, NS_OK);
> > +
> > +    nsCOMPtr<nsIURI> uri;
> > +    doc->NodePrincipal()->GetURI(getter_AddRefs(uri));
> 
> You should use GetOrigin() instead which handles digging into the innermost
> URI and punycoding and cases when the URI doesn't match the origin. Or
> URIIsChromeOrInPref() should take a principal and have that done in there,
> or add another helper to nsContentUtils (like ::GetAsciiOrigin) that does
> the right thing for you.

Thanks for pointer! Will be sure to take that into account when we rewrite this to use the permissions manager.
Comment 65 Stephen Donner [:stephend] 2012-06-25 16:10:06 PDT
This sounds important to verify; is there a clear QA-reproducible testcase that
we can run through, and if so, can you please change [qa?] to [qa+] in the
whiteboard status, with steps?  Thanks!
Comment 66 Philipp von Weitershausen [:philikon] 2012-06-25 16:13:38 PDT
(In reply to Stephen Donner [:stephend] from comment #65)
> This sounds important to verify; is there a clear QA-reproducible testcase

User-facing? No. You could, of course, write some HTML test cases and verify them manually. The mochitests that were added as part of this patch do this in an automated fashion.
Comment 67 Florian Bender 2012-09-17 13:01:17 PDT
Can somebody please summarize what has changed on the JS/web side with this patch? E. g. a description of the new parameters that can be passed to XHR? Docs are still needed …
Comment 68 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-09-17 22:31:11 PDT
This adds the ability to specify two parameters when constructing an XHR object:

new XMLHttpRequest({ anon: true, system: true });

Both are optional and default to false.


new XMLHttpRequest({ anon: true });

is equivalent to the AnonXMLHttpRequest from the XHR spec [1]. So far noone has implemented the AnonXMLHttpRequest constructor, and the anon flag received positive feedback when I suggested it on the list.


new XMLHttpRequest({ anon: true, system: true });

is *only* available in reviewed (privileged) apps. The 'system' flag may only be set to true if the 'anon' flag is also set to true. When 'system' is set to true, it allows the XHR object to make cross-site requests without using CORS.

Again, this is *only* available to reviewed apps, and can't be used in web pages opened in the Firefox browser since it wouldn't be safe.


[1] http://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html
Comment 69 Florian Bender 2012-09-29 09:07:17 PDT
Thank you. I just amended the MDN article: https://developer.mozilla.org/en-US/docs/DOM/XMLHttpRequest#XMLHttpRequest%28%29

Is this enough as documentation?
Comment 70 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-09-30 19:05:56 PDT
I clarified the docs a little bit. Please note that CORS didn't remove any functionality, it just added functionality. This adds further functionality.
Comment 71 Florian Bender 2012-10-01 05:36:25 PDT
Thank you. 

One question is left: In which Firefox version will this arrive? I used Firefox 18 although this bug targets Milestone 16. 

I guess Firefox 18 (??) for developers [1] should also be amended?

[1] https://developer.mozilla.org/en-US/docs/Firefox_18_for_developers
Comment 72 Matěj Cepl 2012-11-04 15:02:19 PST
So, just to clarify ... there will be no third-party podcatcher for Firefox OS (will be there a default one at least)? Negative reply would sound pretty bad for me, as listening to podcasts is for me (and I guess I am not the only one) the most often activity on my phone. Transfer of multimegabyte media files through a server seems like a good way how to eliminate all independent developers. Is there any better bug for this?
Comment 73 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-11-04 15:48:58 PST
writing a podcast app is completely unrelated to this bug. Please file a separate bug on that. This bug is fixed.
Comment 74 Matěj Cepl 2012-11-06 16:48:06 PST
(In reply to Jonas Sicking (:sicking) from comment #73)
> writing a podcast app is completely unrelated to this bug. Please file a
> separate bug on that. This bug is fixed.

Filed as bug 809280
Comment 75 super420840 2015-10-18 11:58:11 PDT
Created attachment 8675438 [details] [diff] [review]
Clicker

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Which older supported branches are affected by this flaw?

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

How likely is this patch to cause regressions; how much testing does it need?
Comment 76 super420840 2015-10-18 12:00:13 PDT
[Tracking Requested - why for this release]:

[Blocking Requested - why for this release]:

Note You need to log in before you can comment on or make changes to this bug.