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

Cross-origin content #63

Closed
domenic opened this issue Aug 30, 2021 · 5 comments
Closed

Cross-origin content #63

domenic opened this issue Aug 30, 2021 · 5 comments
Assignees
Labels

Comments

@domenic
Copy link

domenic commented Aug 30, 2021

Hi, great to see the explainer for the model element! It seems pretty cool and most of the design decisions make sense to me.

One thing I noticed was recurring text such as

However, there are cases where these current options cannot render content. This might be due to security restrictions

or

Rendered <model> data is not exposed to / extractable by the page in this proposal, so no tainting is required

which seem to imply that <model> might be aiming for the legacy security model used by things like <img>, where you can display cross-origin data (even without CORS enabled for the resource).

Given Spectre, and even before then the move towards ensuring the same-origin policy is respected, this seems like a bad idea for new resource-inclusion technologies. All recent resource-inclusion technologies such as CSS fonts or JS modules, have instead relied on CORS.

So, I'm hoping that <model> can do the same, and require CORS for cross-origin data.

(Another related modern security practice is to require correct Content-Type headers and not use MIME sniffing; that might be too much detail for the explainer though.)

@hober hober added the model label Aug 31, 2021
@hober hober changed the title [model] Cross-origin content Cross-origin content Aug 31, 2021
@othermaciej
Copy link
Contributor

These comments are not referring to loading of cross-origin content via the model element. Rather, this is considering implementations of model where the actual rendering is affected by information from the environment or content outside the page (or even other content from the page) for realistic shading/lighting. If it were possible to paint a model element to a canvas, then tainting would be required in such cases. However, that is not proposed here.

That said, it's probably good to be explicit in the explainer about whether loading of cross-origin content is allowed by default or requires CORS.

@grorg
Copy link
Contributor

grorg commented Sep 7, 2021

Agreed. Would using the crossorigin attribute be enough to cover the canvas tainting case? I will add that.

This document should probably also address what happens if the model data itself references cross-origin content. e.g. a texture used for a material is loaded from somewhere other than the source of the model file. Maybe the easiest thing to do for now is disallow such references?

@domenic
Copy link
Author

domenic commented Sep 7, 2021

Would using the crossorigin attribute be enough to cover the canvas tainting case? I will add that.

I think the simpler and more modern approach would be to just require the content be exposed with CORS. (I.e., use mode "cors" instead of mode "no-cors".)

Then you can never taint a canvas with these things:

  • Either the resource was exposed with CORS, so no tainting is needed since the site consented to you reading the contents; or
  • The resource was not exposed with CORS, and so totally fails to load.

Maybe the easiest thing to do for now is disallow such references?

This would be equivalent to fetching such items with mode of "same-origin". I think it would also be relatively easy to just load with mode "cors", which would let them reference public textures and such.

@grorg
Copy link
Contributor

grorg commented Sep 7, 2021

Thanks for the explanation.

grorg added a commit that referenced this issue Sep 7, 2021
This might be temporary while investigating #63, where there
is an alternate suggestion.
@marcoscaceres
Copy link
Contributor

Closing as this is immersive-web/model-element#56

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

No branches or pull requests

5 participants