nextcloud / richdocuments

📑 Collabora Online for Nextcloud
https://nextcloud.com/collaboraonline
349 stars 115 forks source link

WASM headers for online #3258

Closed juliushaertl closed 1 week ago

juliushaertl commented 11 months ago

As asked for by @mmeeks we would need to have wasm-unsafe-eval CSP headers set for WASM experiments currently taking place.

I checked back and we have a partly mechanism available to set those by https://github.com/nextcloud/server/pull/38082 this is however not used yet by Nextcloud Talk, as there the wasm is running in a service worker which doesn't require this.

We should be able to add those headers of course, just some additional questions:

@mmeeks To you have a need to run the online wasm in the main thread? As far as I understood this could have a significant impact on browser responsiveness.

mmeeks commented 11 months ago

Nope - we should run our WASM off the main thread, and do load, rendering etc. in the background; while continuing to render the UI in the main-thread with the same front-end code as now.

juliushaertl commented 11 months ago

In that case the header should not be needed as far as @danxuliu told me

mmeeks commented 11 months ago

Hmm - ok ? it seems that we need the parent frame of our iframe to have this @Ashod can you provide more details; quite possibly I've mistaken what's up here I think. Quite possibly our startup WASM runs in the main thread initially (not sure).

Ashod commented 11 months ago

I believe we need these two headers when serving the parent:

Cross-Origin-Opener-Policy: same-origin
Cross-Origin-Embedder-Policy: require-corp

Also had to add script-src unsafe-eval to the CSP header of our frame/js/wasm serving, but we will find out if that is required when we load with the above headers as the missing unsafe-eval was mentioned in the error message.

juliushaertl commented 11 months ago

unsafe-eval would be quite an impactful change security wise.

For the others I pushed a quick PR to https://github.com/nextcloud/richdocuments/pull/3260 which should achieve setting those headers for Nextcloud. However we should of course get more clarify about which headers to add in the end, why and its implications.

mmeeks commented 11 months ago

unsafe-eval sounds bad; but I think we can do that with wasm-eval in future :-) thanks Julius.

Ashod commented 10 months ago

For the others I pushed a quick PR to #3260 which should achieve setting those headers for Nextcloud. However we should of course get more clarify about which headers to add in the end, why and its implications.

@juliushaertl, I'd like to run some tests locally. Any chance to get the draft PR polished so I can build locally and test? Thank in advance!

juliushaertl commented 10 months ago

@Ashod Pushed and should be testable with that now.

Reference for what it currently set currently set:

Screenshot 2023-11-16 at 09 15 20
Ashod commented 10 months ago

Thanks @juliushaertl. The PR branch being out-of-date shouldn't be an issue?

@caolanm, does #3260 work for you? Are you able to apply it and build?

caolanm commented 10 months ago

I have this patched richdocuments and a local nextcloud install. I can see it has an effect, but right now if I open a document, without wasm, then I just get an error of:

[getWopiUrl] http://localhost/nextcloud/index.php/apps/richdocuments/wopi/files/7_oca1b28l9m0b url.js:42:9 The resource at “http://localhost:9980/browser/70f697e3da/cool.html?WOPISrc=http%3A%2F%2Flocalhost%2Fnextcloud%2Findex.php%2Fapps%2Frichdocuments%2Fwopi%2Ffiles%2F7_oca1b28l9m0b&title=%2FDocuments%2FWelcome%20to%20Nextcloud%20Hub.docx&lang=en&closebutton=1&revisionhistory=1” was blocked due to its Cross-Origin-Resource-Policy header (or lack thereof). See https://developer.mozilla.org/docs/Web/HTTP/Cross-Origin_Resource_Policy_(CORP)# files

juliushaertl commented 10 months ago

Seems this was caused by those two headers: https://github.com/nextcloud/richdocuments/issues/3258#issuecomment-1781169694

I pushed a fix up to the PR.