Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"no-cors" CSS SOP violation #719

Open
annevk opened this issue Jul 3, 2015 · 69 comments
Open

"no-cors" CSS SOP violation #719

annevk opened this issue Jul 3, 2015 · 69 comments

Comments

@annevk
Copy link
Member

annevk commented Jul 3, 2015

Per our current set of definitions a service worker reveals what resources a "no-cors" CSS stylesheet attached to a document loads. In particular this can leak confidential tokens in the URLs.

Entered the public record here: http://krijnhoetmer.nl/irc-logs/whatwg/20150703#l-286

According to @jakearchibald resource timing (paging @igrigorik) did this first, in both Chrome and Firefox.

I think we should revert both, seems like bad precedent to cut more holes in SOP.

@jakearchibald
Copy link
Contributor

Most requests by CSS are exposed through computed styles, and it's pretty trivial to iterate over all elements to find those. Things like font urls & @import cannot be detected through computed styles, but are exposed by SW & resource timing.

I see CSS more like script, although it's "opaque", it gives up some visibility when it makes requests within the context of the page. Although, as @annevk points out, script gives up visibility by using globals that can be modified, whereas CSS doesn't.

@annevk
Copy link
Member Author

annevk commented Jul 3, 2015

https://bugzilla.mozilla.org/show_bug.cgi?id=1180145 tracks this for Gecko.

@jakearchibald
Copy link
Contributor

@davidben @mikewest what's your take here, are we (and resource priorities) breaking the web by exposing CSS-initiated fetches?

@igrigorik
Copy link
Member

Opened a bug on Resource Timing to track this (w3c/resource-timing#27).

@jakearchibald
Copy link
Contributor

Ping @davidben @mikewest

We have two options here:

  1. Declare CSS-triggered fetches as "page visible" as they are with script-triggered fetches. However, this is leaking new data when it comes to URLs that don't appear in computed styles. Eg font sources, @import urls.
  2. as Anne suggests, remove these from the resource timing API & bypass serviceworker for these requests.

This will trip developers up who host their CSS on a CDN, as it's less likely to have access-control-allow-origin, but it's not worth breaking the web over.

@igrigorik
Copy link
Member

@jakearchibald silly question: if you hide CSS fetches from SW, how am I supposed to offline my app with background images and fonts? Wouldn't that effectively mean that "if you want to be SW-compatible, you should avoid CSS fetches"... because that would break the web. I don't see why we're making this distinction between JS and CSS (JS fetches are "computed fetches" also).

@annevk
Copy link
Member Author

annevk commented Jul 13, 2015

No-cors cross-origin CSS subresources would be hidden. Any other kind of CSS subresources would not be hidden. So there's still plenty of ways to do business. (JavaScript fetches are already observable, as explained upthread.)

@jakearchibald
Copy link
Contributor

@igrigorik

if you hide CSS fetches from SW, how am I supposed to offline my app with background images and fonts?

CSS fetches would only be hidden from SW if the CSS is no-cors cross-origin. You'd be able to get them back with <link rel="stylesheet" href="…" crossorigin>, provided CORS headers.

@igrigorik
Copy link
Member

CSS has no author-level equivalent for "crossorigin" for fetches declared via url() - i.e. I can't specify a CORS policy on resources specified within the CSS file. As such, all url() initiated fetches (read, almost all CSS fetches modulo some odd exceptions like web fonts), would become invisible? Unless I'm missing something here, that would break the web.

Why can SW see and intercept no-cors crossorigin <img> initiated fetches, but same use case then fails under CSS? This seems inconsistent and broken.

p.s. the whole crossorigin business is a total mess - see w3c/resource-hints#32.

@annevk
Copy link
Member Author

annevk commented Jul 13, 2015

No, it depends on how the CSS resource was fetched. Not on how the CSS resource's subresources are fetched.

@igrigorik
Copy link
Member

@annevk ah, ok.. So, coming back to @jakearchibald earlier point, what exactly would be "hidden" here if most URLs are already observable through computed styles? Jake called out fonts earlier, but afaik, those are visible via http://dev.w3.org/csswg/css-font-loading/.

@annevk
Copy link
Member Author

annevk commented Jul 13, 2015

Those observable through computed styles are only exposed if you know where to look, which can be expensive. Very different characteristics from getting a list. And e.g. @import is not affected by that. And it might very well be that CSS font loading has a bug too if it's claiming to be available for no-cors CSS.

@annevk
Copy link
Member Author

annevk commented Jul 13, 2015

@tabatkins ^^

@tabatkins
Copy link
Member

FontFace objects don't expose the url (or ArrayBuffer) they were loaded from. The FontFace itself is there as long as CSS has rights to it (font loading is CORS-restricted per spec), but you can't read anything from it.

@annevk
Copy link
Member Author

annevk commented Jul 13, 2015

@tabatkins no-cors CSS is applied to a page, but you can't access it through CSSOM (you can access computed styles). So the question is what the font API does for such a CSS resource (I suspect it shouldn't work, but that may not be defined).

@davidben
Copy link

Oh yuck. Yeah, I think I agree with Anne that we should remove these requests from SW and Resource Timing unless you add the crossorigin attribute. These kinds of "the contents are secret, but if they happen to parse as foo, you can execute it" security policies are super-hairy. We shouldn't add new ones.

In fact, cross-origin CSS has already bitten us in the past because the CSS parser is extremely error-tolerant. See https://www.linshunghuang.com/papers/css.pdf

@davidben
Copy link

Oh, and I believe this is the corresponding WebKit bug, since I don't see it linked in the paper:
https://bugs.webkit.org/show_bug.cgi?id=29820

ETA: To clarify, this is the WebKit bug for the paper in the above comment that describes a similar issue, not the WebKit bug for the issue being discussed here.

@tabatkins
Copy link
Member

Yeah, makes sense. I'll send a notification to www-style and make the change in Font Loading.

@tabatkins
Copy link
Member

Actually, I ran into some questions about how to implement. I posted https://lists.w3.org/Archives/Public/www-style/2015Jul/0150.html with two possibilities for handling this.

The proposal I prefer still exposes the existence of FontFace objects from the tainted stylesheet, and their loading status and promise. Everything else is hidden, tho.

(If you know the name of the font, its loading status is already exposed to the page via layout/timing channels. This approach would further expose the number of fonts present in tainted sheets and the loading status/promises of the mystery fonts, which wasn't previously available, but any further restrictions seem kinda insane to spec/implement.)

ETA: I could maybe further hide the load status/promise and only expose it when you retrieve the FontFace via one of the "by name" methods, but that adds a lot more complexity.) Or maybe just expose one opaque slab per stylesheet? Then the query methods can return a slab representing just the fonts actually used, so you can load them and view their load status in synchrony. This is kinda similar to how I'd like to expose local fonts at some point.

@nattokirai
Copy link

Creating new flavors of FontFace seems like overdesign to me. If style rules can't be exposed in the OM, I don't think FontFace objects should be exposed in the FontFaceSet. The ready() promise should include fonts from the inaccessible stylesheet.

This is kinda similar to how I'd like to expose local fonts at some point.

Allowing local fonts to be enumerated exposes users to fingerprinting attacks.

@tabatkins
Copy link
Member

Creating new flavors of FontFace seems like overdesign to me. If style rules can't be exposed in the OM, I don't think FontFace objects should be exposed in the FontFaceSet. The ready() promise should include fonts from the inaccessible stylesheet.

I explained in the email why that's complicated and probably unworkable. Please comment on www-style for further discussing of FontFace/etc.

Allowing local fonts to be enumerated exposes users to fingerprinting attacks.

I didn't mention enumerating local fonts; my suggestion was for the exact opposite, actually.

And you can already enumerate local fonts fairly trivially through layout channels.

@horo-t
Copy link
Member

horo-t commented Sep 16, 2015

https://crbug.com/532374 tracks this for Chromium.

@wanderview
Copy link
Member

Some corner cases:

Consider:

  1. a.com/index.html loads stylesheet at b.com/foo.css as no-cors
  2. b.com/foo.css @imports stylesheet at a.com/bar.css
  3. a.com/bar.css loads background-image a.com/snafu.jpg

Should SW and performance see data for snafu.jpg? I think everything @imported under a tainted sheet should be hidden.

@igrigorik, note this is not really covered by the current language in the performance spec:

For each resource fetched by the current browsing context, excluding resources fetched by cross-origin stylesheets fetched with no-cors policy, perform the following steps:

This raises further issues like this:

  1. a.com/index.html loads stylesheet at b.com/foo.css as no-cors
  2. b.com/foo.css @imports stylesheet at a.com/bar.css
  3. a.com/bar.css loads background-image a.com/snafu.jpg
  4. a.com/index.html loads stylesheet at a.com/thepain.css
  5. a.com/thepain.css @imports stylesheet at a.com/bar.css
  6. a.com/bar.css loads background-image a.com/snafu.jpg

Here snafu.jpg should be hidden in step 3, but what happens to snafu.jpg in step 6? It seems the stylesheet should not be shared from tainted import with non-tainted import. I'm not sure if the image cache would prevent a network load in step 6, though.

@sicking
Copy link

sicking commented Oct 23, 2015

I don't think we should prevent requests coming from a no-cors stylesheet from going through a SW.

Yes. I understand that this exposes data that isn't currently exposed. And yes, I understand that there is some security risk involved in that. But I don't think that automatically means that we must not do this.

In short, I think the question is more complex and nuanced than that.

Currently, if I host a file on a webserver and serve it with a "text/css" mimetype, a lot of the data in that file can be read by third parties.

A third party can link to that stylesheet, create an element which matches one of the rules in the stylesheet, and then use getComputedStyle to figure out the properties that were set by the rule, and what those properties were set to.

You can relatively easily use a dictionary to scan probable class names to extract all rules that simply match on a class. You can even scan all possible ascii-classnames shorter than N characters.

That means, that as an author, if I put a text/css resource on a webserver, I already need to count on that most of the bytes in that file is readable by third parties.

It's also not as simple as "exposing more data" == "less safe for developers".

Trying to send developers the message that "these here bytes are leaked, but these other bytes, in the @import rule, those are safe, feel free to put sensitive data in those URLs" is a significantly more complex than saying "the contents of the stylesheet can be read by third parties. Don't put sensitive data in stylesheets". Such complexity often leads to security bugs.

For example, the fact that background-image URL isn't exposed to a SW might make developers think that the URL is protected, when in fact .getComputedStyle leaks the very same information. I.e. it is very easy for developers to get a false sense of security. This is also made worse by the fact that developers can legitimately claim that some urls, like @import, are in fact safe.

Then there is of course the problem that the implementation complexity will most likely lead to a few security bugs here and there. We had a recent example of this where a developer had seen that CORS says that only certain values can be sent in the Content-Type header without triggering a preflight. So the developer built CSRF-protection by checking for a particular content type. However it turned out that different browsers apply slightly different rules to Content-Type parsing leading to the ability for an attacker to send a cross-site request in some browsers with a Content-Type that this website didn't expect.

The Content-Type rule in CORS is my fault. I think it was a bad decision and it has lead to security problems.

What we're talking about here is orders of magnitude more complex, and so I feel quite confident that it will lead to orders of magnitude more problems. Both security problems and other problems.

And yes, I am aware that we block access to the CSSOM for cross-origin stylesheets that weren't fetched with CORS. But that was added because at the time we didn't have the restriction that the stylesheet had to be served as "text/css". The attacks that we saw was due to websites pointing at cross-origin HTML files, and then took advantage of the lax CSS parsing rules to extract information from the HTML.

So, all in all, I think the cost here is quite high to browser developers, as can be seen by all the edge cases that are being debated. I agree that there is some security value to developers, but I also think there is a very real security risk to developers (in addition to the cost of not being able to intercept CDN hosted stylesheet resources).

And again, I realize that more information is being exposed here. If your counter argument is simply that that means that we have to block these intercepts, then you might want to reread this comment.

I think we would be better served giving developers a real security tool here. I know that @annevk had a proposal some time back for a header which blocked cross-site reads of resources like images/stylesheets/scripts. Implementing something like that seems like a better way to protect the data debated here.

@jakearchibald
Copy link
Contributor

jakearchibald commented Aug 24, 2018

My concerns about this:

It's difficult for developers to understand which parts of their CSS is protected when loaded cross-origin:

//cross-origin/user.css

@import 'user/styles/david-smith.css';

//cross-origin/user/styles/david-smith.css

/* This is David's favourite font */
@font-face {
  font-family: 'david-loves-consolas';
  src: url('/fonts/consolas-v123.woff2');
}

body {
  font-family: david-loves-consolas;
}

.user-avatar {
  background: url('/images/me-and-samantha.jpg');
}

.davids-mother-is-called-jane {
  background: green;
}

Assuming we make the change proposed in this issue, what's private?

We can look at the computed style for body, and determine that the user is called david, and they love consolas. Again, using computed styles, we can determine they have a friend or relative called samantha, who is displayed in the image. We could use guesswork to figure out David's mother is called Jane.

The bits of information that this PR protects is David's surname, as it's only in the @import which isn't usually exposed. We also protect the version of the font (I think? Or is this exposed elsewhere?).

My advice to developers would be "Don't put private data in CSS". I realise we do our best to hide the source, as we do with JS, but we don't hide fetches from JS.

@annevk
Copy link
Member Author

annevk commented Aug 24, 2018

@jakearchibald one of the reasons JavaScript modules requires CORS is precisely to not have to hide import fetches from service workers. We also do not provide CSSOM access to cross-origin style sheets for this reason. Would you try to argue we should? And yes, you should probably not put private information in CSS, but we should also not regress in what we protect.

@yoavweiss
Copy link

yoavweiss commented Aug 24, 2018

FWIW, https://jsbin.com/pigihubuxa/edit?html,output shows that at least Chrome and Safari do expose CSSOM (and the background image URL info) for default cross-origin CSS fetches. Firefox does not.

@jakearchibald
Copy link
Contributor

@annevk

We also do not provide CSSOM access to cross-origin style sheets for this reason. Would you try to argue we should?

No, but "we expose some URLs from cross-origin CSS" should probably go one way or the other.

@jakearchibald
Copy link
Contributor

As an aside, isn't script and CSS source highly vulnerable to meltdown/spectre?

@davidben
Copy link

Indeed! See also CORB and friends. The legacy thing where the web platform has all these "you can execute this but not read through contents" CORS bypasses are kind of a disaster. I need to swap this particular issue back in, but having to sacrifice ergonomics here to plug a vulnerability would not be at all surprising to me.

@annevk
Copy link
Member Author

annevk commented Aug 25, 2018

SOP bypasses* (this seems an increasingly futile battle, but I'll keep trying)

@mfalken
Copy link
Member

mfalken commented Oct 29, 2018

F2F: This was discussed partly in WebAppSec and in a dedicated breakout session:

If I may attempt a summary.

Arguments in favor of making the requests skip the service worker:

  • It's arguably a SOP bypass.
  • There have been real instances of CSS leaking private data, ca. 2011 (Eric Lawrence to try to provide more details about this).
  • Firefox changed to at least partially block this for Resource Timing: https://bugzilla.mozilla.org/show_bug.cgi?id=1180145
  • Allowing a SOP bypass because it's already exposed via other APIs or side-effects seems unsatisfying; not moving where we want to be.

Arguments in favor of going to the service worker:

  • The information is already exposed via Resource Timing; getComputedStyle(); and side-effects of the import itself on geometry/etc.
  • Changing the behavior now could break sites.
  • It would become difficult to explain to developers when subresource requests go to the service worker and when they don't.
  • There would be some complexity in specing/implementing skipping, due to nested imports and redirects.

Observations:

  • Any new APIs won't make subresource requests from no-cors contexts. If we were to start from scratch, we'd have made CSS cors-only.
  • A core problem is there is no way for developers to make cors import requests. You can only do cors from <link>.

Outcomes:

  • We want to timebox this decision (there were some references to mid-November, or this year).
  • Youenn (WebKit) is to check internally to try to make a decision about what they want to do.
  • @wanderview (Chrome) volunteered to add metrics to estimate how many sites would be affected.

(I don't know how to cc Eric L or Youenn on GitHub.) (edit: @youennf )

@yoavweiss
Copy link

cc @youennf @ericlaw1979

@youennf
Copy link

youennf commented Oct 29, 2018

  • A core problem is there is no way for developers to make cors import requests. You can only do cors from <link>.

Is there a corresponding GitHub issue tracking this?

@aliams
Copy link
Contributor

aliams commented Jan 3, 2019

@wanderview were you able to get metrics for this yet by any chance?

@wanderview
Copy link
Member

I spent a couple days working on this, but it turned out to be more complex than I had time for. The issue I ran into is that chrome does not initiate the loads for things like background images, etc, until they are used by a style flush. This is quite a bit after the sheet was loaded and parsed, so none of the tainting information is available. That information would have to be plumbed though. It was a bit too much for me since I'm not familiar with chrome's style system.

@mfalken
Copy link
Member

mfalken commented Jul 8, 2019

I've just landed some metrics at https://crbug.com/898497. Very early data from the past few Chrome Canaries suggests that 0.87% of page loads involved a service worker intercepting a request from a no-cors stylesheet. 14% of page loads involved a controller service worker at all, and of those page loads 6% had a no-cors stylesheet making a request.

@mfalken
Copy link
Member

mfalken commented Jul 8, 2019

My personal interpretation of this is that it's likely in the range where we can deprecate, and the implementation complexity doesn't seem so bad (Blink already tracked the "origin-clean" bit for nested imports and redirects, AFAICT). Therefore, of the arguments against deprecating at #719 (comment), I think the strongest one is developer confusion. Not sure how much of a blocker that should be.

@youennf
Copy link

youennf commented Jul 23, 2020

FWIW, Safari Tech Preview and Safari latest betas skip service worker for those loads.
So far, we have not heard of breakage.

@annevk
Copy link
Member Author

annevk commented Jul 23, 2020

That is exciting, I've reopened https://bugzilla.mozilla.org/show_bug.cgi?id=1201160 so we can reconsider this for Firefox.

@mfalken
Copy link
Member

mfalken commented Oct 16, 2020

Thanks, at TPAC I wanted to see if @youennf had any updates on how skipping these went.

Chrome data show 1.5% of page loads have a service worker intercepting a request from a no-cors stylesheet, which is quite high. But it's possible in many cases the worker is not doing anything with the request.

@youennf
Copy link

youennf commented Oct 16, 2020

We received no complaint so far so this seems web compatible.
It could be interesting to see how affected the offline experience would be for those sites.

@mfalken
Copy link
Member

mfalken commented Oct 20, 2020

Good idea. The chromestatus page lists some sample URLs that are affected.

Would it be possible to see if any of those have a degraded offline experience on a version of Safari that has the change?

@asutherland
Copy link

From a spec perspective, the changes for this would actually happen elsewhere and result in resources being fetched with a service-workers mode of "none", yes?

@mfalken
Copy link
Member

mfalken commented Nov 20, 2020

(fixing account) Agreed, it would be part of CSS I imagine.

@annevk
Copy link
Member Author

annevk commented Nov 20, 2020

Yeah, though this repository should probably take ownership of getting it tested as CSS hasn't even integrated with Fetch and its integration with HTML is also patchy.

@mfalken
Copy link
Member

mfalken commented Jan 22, 2021

(belated) Notes from Service Worker WG meeting on Nov 20:

  • Safari Tech Preview is skipping service workers for no-cors requests from CSS.
  • We should see whether the offline experience of sites here https://chromestatus.com/metrics/feature/timeline/popularity/2941 are impacted in Safari Tech Preview.
  • Affecting 1.5% of page loads will be considered very high by Blink API Owners.
  • CSS would need to integrate with Fetch to write the spec for this.

@sicking
Copy link

sicking commented Feb 13, 2021

I'm curious how we are thinking about the issue here in relation to the fact that a lot of the same information is exposed through the CSSOM in most major browsers, Safari and Chrome included [1].

What would be the security win if we just fix this issue without also fixing the CSSOM issue?

Are there plans to also fix the CSSOM issue? Has any experiments been done to see what percentage of websites would be impacted? Or any experiments shipped with that support disabled?

I'm also curious if anyone has heard of this issue leading to security problems? This design has been shipping for several years now so we've had opportunity to see if this is a real-world problem or just a theoretical one.

I think it's well established that no-cors cross-site stylesheets has lead to security issues (as referenced in [2]). However the question at hand in this issue is if locking down URL access while still exposing a lot of other properties provides meaningful security benefits.

[1] https://jsbin.com/pigihubuxa/edit?html,output
[2] #719 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests