1357966 - Don't report resource timing values for image, -moz-image-rect, font, and cursor from cross-origin stylesheets

Status: RESOLVED WORKSFORME

(Core :: Layout, enhancement, P2)

Description

[Jet Villegas (inactive)]

Bug 1180145 prevents the identification of external resources loaded by cross-origin stylesheets. The mitigation is achieved by not reporting the time spent on loading the external resources, which may be identifiable by the timing differences vs. same-origin resource loads.

This bug will add similar mitigations for image, -moz-image-rect, font, and cursor resource loads from cross-origin stylesheets.

Comment 1

[Frederik Braun [:freddy]]

This is a follow-up of bug 1180145. Adding security flags for tracking.

Comment 2

[Frederik Braun [:freddy]]

Jet, any updates? This is unassigned for 2 months now.

Comment 3

[Jet Villegas (inactive)]

(In reply to Frederik Braun [:freddyb] from comment #2)
> Jet, any updates? This is unassigned for 2 months now.

Please see my comment here about when we expect to start work on this:
https://bugzilla.mozilla.org/show_bug.cgi?id=1180145#c79

Comment 4

[Wennie]

Hi Jet:

I have assigned these security bugs to you to reassign them to appropriate developers in your team to investigate and fix them.

Thanks!

Wennie

Comment 5

[Frederik Braun [:freddy]]

Maire, can you help us find an owner for this?
According to comment 3 this was on hold until Stylo is "done", but given that it's the default style system for a while and we even unshipped the previous one, I'd like us to take a look.

Comment 6

[Maire Reavy [:mreavy]]

Hi Jonathan -- After the work for Variable Fonts is done, can you take a look at this bug and assess the level of effort to fix it?

Comment 7

[Sean Voisen (:svoisen)]

Hi Boris – Per our discussion, thank you for making time to investigate this. I imagine jwatt can serve as a resource for guidance here once he's back after next week.

Comment 8

[Boris Chiou [:boris]]

I'm trying to understand what should I do in this bug.

First, Bug 1180145 added an API in `nsITimedChannel`, so we could just use `nsITimedChannel::SetReportResourceTiming()` to make the flag true, and then `PerformanceTimingData::Create()` wouldn't create the entry. In Bug 1180145 we did this for `HTTPBaseChannel` and `NullHTTPChannel`; However, we still need to do the same thing for `imgRequestProxy` for the image stuff, I guess, because it seems only 3 classes use nsITimedChannel:
(1) HTTPBaseChannel
(2) NullHTTPChannel
(3) imgRequestProxy
, and (1) & (2) had been finished.

Still not sure how to do this for font and cursor. Keep looking at this. Probably we only need do this for the classes which use nsITimedChannel?

Comment 9

[Boris Chiou [:boris]]

OK. I'm a little bit confused by this bug title. It seems we added tests for image, -moz-image-rect, font, and cursor resource loads from cross-origin stylesheets in Bug 1180145 [1], and those are passed. So what should we do in this bug? Or what we want is focus on something like `cursor: url(#aaa)`, and #aaa is defined in a cross-origin stylesheet? Maybe heycam could help me. :)

[1] https://searchfox.org/mozilla-central/rev/dd965445ec47fbf3cee566eff93b301666bda0e1/dom/tests/mochitest/general/file_resource_timing_nocors.html#58-91

Comment 10

[Boris Chiou [:boris]]

(In reply to Boris Chiou [:boris] from comment #9)
> OK. I'm a little bit confused by this bug title. It seems we added tests for
> image, -moz-image-rect, font, and cursor resource loads from cross-origin
> stylesheets in Bug 1180145 [1], and those are passed. So what should we do
> in this bug? Or what we want is focus on something like `cursor: url(#aaa)`,
> and #aaa is defined in a cross-origin stylesheet? Maybe heycam could help
> me. :)
> 
> [1]
> https://searchfox.org/mozilla-central/rev/
> dd965445ec47fbf3cee566eff93b301666bda0e1/dom/tests/mochitest/general/
> file_resource_timing_nocors.html#58-91

Oh, so this is for non-stylesheet subresources, I guess. Probably I need a simple example (so I could write a test case for this).

Comment 11

[Boris Chiou [:boris]]

Hi Cameron,

This is about cross-origin stylesheet, and I'm not familiar with this, so would you mind giving me a simple example code to test this? (e.g. How to test the cursor from a cross-origin stylesheet? [1] has an example, but it seems that's not the case.) Thanks.

[1] https://searchfox.org/mozilla-central/rev/dd965445ec47fbf3cee566eff93b301666bda0e1/dom/tests/mochitest/general/file_resource_timing_nocors.html#58-91

Comment 12

[Cameron McCormack (:heycam)]

I'm also a bit confused about what this bug should be covering.

The test you link to from bug 1180145 covered cross-origin style sheets linking to cross-origin resources.  That test is passing, and my own local testing shows that its resource timing is correctly not being reported (at least for cursor, which is what I tested).

Freddy or Jonathan, can you clarify what we need to be doing here?

Comment 13

[Frederik Braun [:freddy]]

Oh. looking at the cases in generateCss.sjs, I agree. Those seem to be covered. It looks like this bug here was filed based on an assumption that has since then invalidated.

What bug 1180145 is missing though, are tests that show we *do* resource timing for things that come with a CORS header.

Though looking at <https://searchfox.org/mozilla-central/source/dom/tests/mochitest/general/file_resource_timing_nocors.html#152-155>, it seems we still report timing for cross-origin fonts.
But that's another bug: bug 1423602

Comment 14

[Boris Chiou [:boris]]

(In reply to Frederik Braun [:freddyb] from comment #13)
> Oh. looking at the cases in generateCss.sjs, I agree. Those seem to be
> covered. It looks like this bug here was filed based on an assumption that
> has since then invalidated.

Thanks. Looks like image, -moz-image-rect, and cursor are covered by bug 1180145, and we have a separate bug for font. Maybe this bug could be closed. Jonathan, wdyt?

Comment 15

[Cameron McCormack (:heycam)]

Yes, let's close this bug WORKSFORME and open a new bug for tests that resource timing does work in cases where it should:

(In reply to Frederik Braun [:freddyb] from comment #13)
> What bug 1180145 is missing though, are tests that show we *do* resource
> timing for things that come with a CORS header.

Comment 16

[Boris Chiou [:boris]]

(In reply to Cameron McCormack (:heycam) from comment #15)
> Yes, let's close this bug WORKSFORME and open a new bug for tests that
> resource timing does work in cases where it should:
> 
> (In reply to Frederik Braun [:freddyb] from comment #13)
> > What bug 1180145 is missing though, are tests that show we *do* resource
> > timing for things that come with a CORS header.

Thanks, heycam. Let me file a new bug for this.