Trusted Types compatibility for Emscripten threads

[aaronshim]

We noticed that worker-loading code in Emscripten will not work on a page with Trusted Types enabled. Given that this incompatibility will prevent Trusted Types enforcement (which has significant security benefits mitigating DOM XSS) for sites that rely on Emscripten, we wish to make the worker creation callsites compatible with browsers that support Trusted Types.

This change will be a no-op for browsers that do not support Trusted Types, but will allow worker creation for browsers that enforce it.

Please do not hesitate to reach out with any questions, and thank you for your consideration!

[welcome]

Thank you for submitting a pull request! Someone will be along to review it shortly.

[sbc100]

Since this add a fair bit of code I wonder if we could make this into an option? Or maybe find a way to do it using --pre-js.. some kind of hook or polyfill and developers who want this feature can opt into?

[aaronshim]

Making it an available option sounds great-- would the code changes in library_pthread.js and worker.js follow the pattern of the #if PTHREADS_DEBUG compilation directive? In which other files should I wire up this new option to link to the compilation directives? Thanks!

[kripken]

Both possibilities @sbc100 mentioned sound reasonable to me too.

For an option, yes, it would look like #if TRUSTED_TYPES ifdefs and so forth. The setting would be defined in settings.js.

A polyfill seems like it could be even simpler, though. That is, it looks like wrapping around Worker and importScripts could achieve this, and that wrapping could be added dynamically in code that runs before emscripten. I don't know much about trusted types, though - is there a reason that can't work?

[aaronshim]

I can take a look at using a Polyfill to wrap the two calls-- which section of code do those live in?

Are there concerns about shipping this extra bit of code to users who aren't on TT-compatible browsers? (The actual logic would be a no-op, but concerns around shipping extra ~bits of code.)

[kripken]

I can take a look at using a Polyfill to wrap the two calls-- which section of code do those live in?

A polyfill could actually be separate from emscripten - basically a JS file that is included right before the emscripten code (that's a potential benefit I think). Or the user could include it with a --pre-js.

But if this is going to be a common thing, then adding a setting probably makes sense. I don't know much about TT myself - is that expected?

Are there concerns about shipping this extra bit of code to users who aren't on TT-compatible browsers? (The actual logic would be a no-op, but concerns around shipping extra ~bits of code.)

Yes, I'm afraid so. It's very easy to add a little bit of code for various web features, and then the default output keeps increasing in size, which is annoying even if each addition is reasonable by itself. So we try hard to avoid that.

(But I guess this goes back to my last question, if eventually TT become very common, then maybe we do just want to pay that cost by default...)

[sbc100]

I wonder if you could develop this as a polyfil outside of emscripten (using --pre-js). If it becomes a much requested feature we could then integrate it as a setting.

[aaronshim]

Maybe I'm not understanding correctly, but do you mean polyfilling the calls to new Worker(...) and importScripts(...) to bless all inputs as Trusted Types? Unfortunately, this would defeat the purpose of Trusted Types, since the goal is to lock down calls to these DOM sinks unless explicitly allowed by these extra API calls.

In this instance, we want to explicitly allow the calls to the new Worker(...) and importScripts(...) at these callsites (because they were judged to be a part of the Emscripten code-loading process), while not allowing other calls in the same context (since they could be user or injected code).

To address the concern of increasing delivered code size to users not interested in Trusted Types compatibility, having an option would be sufficient, right?

We envision Trusted Types enforcement being commonplace in the future, since it's the best mitigation against DOM XSS that we have available as a Web Platform API, but we are stuck in a cyclic dependency: We need to have as many commonly-used libraries support the option of being Trusted Types compatible to drive adoption, but many maintainers want to see more widespread adoption as a justification for accepting PRs to add TT compatibility. For Emscripten, we think it'd be worth it to add the compatibility, since it's such a foundational dependency, having a non-compatible Emscripten would effectively mean that WASM support and TT protections would be mutually exclusive features.

Thanks so much for your help-- I really appreciate the time that you've taken to talk through this! 😄

[sbc100]

It sounds like maybe a setting (in settings.js) is the best idea for now. The setting can default to 0 initially and folks can easily opt in, and if it ever becomes ubiquitous we can consider changing the default. Does that sounds reasonable?

[aaronshim]

That sounds great! I modified the PR code to add the option-- does this look correct to you? Thanks!

…

On Thu, Sep 2, 2021, 7:59 PM Sam Clegg ***@***.***> wrote: It sounds like maybe a setting (in settings.js) is the best idea for now. The setting can default to 0 initially and folks can easily opt in, and if it ever becomes ubiquitous we can consider changing the default. Does that sounds reasonable? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#14962 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJCFUBASQZIYXWRGGJ2HJDUAAFVJANCNFSM5C6LD22Q> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[kripken]

Thanks!

Looks good aside from a final style comment.

[kripken]

Perfect, thanks!