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

Support sandboxed iframe #8921

Open
DeepDiver1975 opened this issue Apr 29, 2024 · 13 comments
Open

Support sandboxed iframe #8921

DeepDiver1975 opened this issue Apr 29, 2024 · 13 comments
Assignees
Labels
bug Something isn't working unconfirmed

Comments

@DeepDiver1975
Copy link

Describe the Bug

Collabora Online cannot be used inside a sandboxed iframe because collabora tries to access the parent document.
See screenshots below for further details

Steps to Reproduce

  1. load Collabora in <iframe sandbox="allow-scripts">
  2. see it failing

Expected Behavior

Shall load

Actual Behavior

Does not load

Screenshots

image
image

Desktop

  • Collabora version: image: collabora/code:23.05.5.2.1
  • OS and version: Debian 12
  • Browser and version: Chromium 124.0.6367.60

Additional Context

ocis docker setup as per https://github.com/owncloud/ocis/tree/master/deployments/examples/ocis_wopi

@meven
Copy link
Contributor

meven commented May 2, 2024

Regarding allow-same-origin, it is required currently for:

  • localstorage accessing and reading, user's settings
  • localization is loaded using XHtmlRequests

For localization this could be changed.
For localstorage, this would require to read and pass to the host window those settings.

Using sandbox="allow-same-origin allow-scripts" is already an isolation improvement, but COOL does not work properly in those conditions.
I am looking into it.

@meven
Copy link
Contributor

meven commented May 2, 2024

Regarding allow-same-origin, it is required currently for:

* localstorage accessing and reading, user's settings

* localization is loaded using XHtmlRequests

For localization this could be changed. For localstorage, this would require to read and pass to the host window those settings.

Using sandbox="allow-same-origin allow-scripts" is already an isolation improvement, but COOL does not work properly in those conditions. I am looking into it.

Using allow-scripts allow-same-origin allow-downloads allow-popups for sandbox allows CODE to work properly.

@DeepDiver1975
Copy link
Author

Yes - but from a security perspective I don't want to grant an application allow-same-origin.

Under the assumption that the ifamed application is compromised by a vulnerability if can leave the iframe and also attack the parent application.
Any interaction between these two applications should happen using Window.postMessage() - see https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage

@meven
Copy link
Contributor

meven commented May 2, 2024

Yes - but from a security perspective I don't want to grant an application allow-same-origin.

I understand, and Collabora Online could support it with some efforts.
allow-same-origin is a lot more restrictive that just communicating with the parent frame.

Collabora Online already uses only postMessage to communicate with the parent frame.
But without allow-same-origin, indeed Collabora Online is a potential attack vector.

@meven
Copy link
Contributor

meven commented May 3, 2024

An option on your end, is to host Collabora-online on a host with a different domain (or just port difference).
That would restrict access from the iframe to the parent application, making it a cross-domain iframe.
That might not be easy to accommodate on your end though.

@mmeeks
Copy link
Contributor

mmeeks commented May 3, 2024

Question is - where are we not using post-message? there is that frame resizing thing; @meven do we have a short list of things to fix that get this wrong ? =)

@meven
Copy link
Contributor

meven commented May 3, 2024

Question is - where are we not using post-message? there is that frame resizing thing; @meven do we have a short list of things to fix that get this wrong ? =)

  • localstorage, and how to persist user settings.
  • localization data, we use a XHR request to fetch them, that can't work
  • the nested iframe to detect resizing, could be replaced with a ResizeObserver

I have a POC branch of Collabora Online with localstorage turned off, localization off and without the resize iframe that runs in Firefox (but not chrome) and firefox has presentation restriction (even with a allow-presentation).

@meven
Copy link
Contributor

meven commented May 3, 2024

The _fileDownloader iframe could also be replaced either with a "<a download" or window.downloads.

@meven
Copy link
Contributor

meven commented May 14, 2024

I have tested the clipboard and it seems affected for complex pasting (but that might another issue).

I have made a branch that has a few features turned off as a baseline to test COOL with sandbox="allow-scripts allow-presentation allow-downloads allow-popups" :

https://github.com/meven/cool-online/tree/meven/iframe-sandboxed

The missing features are the welcome window, l18n, presenting in window is broken and full-screen not-working.

Apart from that, I haven't noticed any problems.

Some compatibility changes are missing (replacing im Map.js the _fileDownloader and _resizeDetector by other means (a download, and ResizeObserver ) that can already be made in master.

@mmeeks
Copy link
Contributor

mmeeks commented May 21, 2024

Getting a new server-side setting storage API implemented is really a larger task if we can't use the browser setting storage; this is a longer term task I think.

@mmeeks
Copy link
Contributor

mmeeks commented Jul 10, 2024

  1. The presentation window is being re-worked to not run JS in a downloaded SVG - which should help there.

  2. I believe @vmiklos just re-wrote the clipboard code to do its downloading where necessary server-to-server which should help make those large-inter-document pastes work even in this case.

The welcome window - I believe we can proxy that via online reasonably easily as we currently do for the LOK image; there should be no need for the client to go to a different server for that; so we're making progress :-)

@DeepDiver1975
Copy link
Author

Hey - any news on this topic? Anything I can help out? ;-) THX a lot

@mmeeks
Copy link
Contributor

mmeeks commented Jul 29, 2024

Sure - good to check - a quick update:

  • we just merged: Private/gokay/separate js #9442 which has a much stricter set of CSP rules
  • that also includes the switch to a more standard 'resize' event instead of this resize-detector iframe.
  • Miklos' copy/paste work is merged & shipping, and being bug-fixed.

So; TODO:

  • I expect we still need a localization solution that doesn't do this XHR request; not sure we've thought enough about that - probably we could post the local required in the initial post and hard-code the right localization files to load in the served cool.html easily enough - for this specific case if that would help

  • configuration: here is where you can help Thomas; we are going to need to store and efficiently serve user configuration data - we have an easy to save registryconfiguration.xcu overlay XML file that we can send you when we shut-down; you can see teh content there in eg. ~/.config/libreoffice/4/user/registrymodifications.xcu on Linux; but we'll want to keep load-time latency down - we could fetch that asynchronously in parallel with document load I guess; from some well-known WOPI URL (per-user, and perhaps per-system - we could stack them) - but that would need some standardization / implementation thought on how that should look. We need 2x well-known-path XML files we can fetch, and one that we can serialize back; that are specific to that user - and (most ideally) we can generate the URLs from trivially and reliably from any WOPI URL - thoughts on that much appreciated ! =) With that in place we can avoid storing any data in the browser.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working unconfirmed
Projects
Status: No status
Development

No branches or pull requests

3 participants