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

Consider to expose decoding time for FCP and LCP? #104

Closed
sefeng211 opened this issue Apr 10, 2024 · 3 comments · Fixed by #105
Closed

Consider to expose decoding time for FCP and LCP? #104

sefeng211 opened this issue Apr 10, 2024 · 3 comments · Fixed by #105
Assignees

Comments

@sefeng211
Copy link

I haven't get a chance to do a proper testing for this, so I'll just refer to Michal's slide for TPAC 2022. Based on the slides, Firefox and Safari doesn't wait for the image to be decoded for FCP, but Chrome does.

Should we update the spec for FCP to wait for the image to be decoded?

I think the spec for LCP only waits for the size of the image to be known, it also doesn't wait for the image to be decoded. Should it also be updated as well?

There's a concern about leaking image decoding time. On one hand, image.decode() can leak the same information, however on the other hand, image.decode() uses a promise, so engines can add arbitrary delays to avoid leaking the same information.

We have some related discussions w3c/largest-contentful-paint#111, WICG/element-timing#65

@noamr
Copy link
Contributor

noamr commented Apr 10, 2024

This is also connected to #62.

@noamr
Copy link
Contributor

noamr commented Oct 14, 2024

We've discussed this with the security folks.
As for as LCP/element-timing goes, the current restriction with Timing-Allow-Origin doesn't make sense because that's a resource-based protection and render time is for the whole document.

So the current proposal is to remove the TAO restrictions, and instead coarsen the render time by 4ms (and clamp to the load time) when the document is not cross-origin-isolated. This would make trying to use timing attacks to figure out information about cross-origin images based on their decoding time not useful.

@noamr
Copy link
Contributor

noamr commented Oct 17, 2024

A more detailed explanation about this for the WG call:

  • The renderTime value is really the paint time of the frame after two conditions are fulfilled: (1) the image is loaded & decoded, (2) the image is added to the DOM.
  • This is potentially a cross-origin leak, as it can expose decoding time information about cross-origin no-cors images.
  • So element timing / LCP for images was guarded by having Timing-Allow-Origin on the image if it's cross-origin. Images failing the TAO test would receive a renderTime of zero.
  • This is an ineffective measure, because resources are rendered together in the same frame, thus receiving the same timing. It's easy enough to add a cross-origin image to the DOM together with a small same-origin image, and receive the render time for both.
  • The alternative proposed is that we use cross-origin isolation for this rather than Timing-Allow-Origin. Cross-origin isolation protects any cross-origin resource on the page from indirect leaks, rather than a particular resource.
  • When cross-origin isolation is not present, we'll still expose renderTime - but over-coarsened (to 4ms resolution). This would allow enough information for performance analysis, while being coarse enough so that it does not expose decoding information in a meaningful way.
  • This has been generally discussed with our internal security team, and will pass through an additional review after it's blessed by the WebPerfWG and we're ready to ship it.

noamr added a commit that referenced this issue Nov 4, 2024
The TAO check does not add any security measure, given rendering same-origin and
cross-origin images in the same frame would result in the same renderTime.

Closes #104
noamr added a commit to noamr/element-timing that referenced this issue Nov 4, 2024
@noamr noamr closed this as completed in #105 Nov 4, 2024
@noamr noamr closed this as completed in 486220f Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants