Skip to content
This repository has been archived by the owner on Sep 20, 2022. It is now read-only.

Add COEP and COEP header middleware decorator and wildcard route #4

Merged
merged 1 commit into from
Aug 23, 2021

Conversation

gzuidhof
Copy link
Contributor

@gzuidhof gzuidhof commented Aug 23, 2021

To be able to use Starboard notebook to its full potential we need to set the COOP and COEP header. This is required for Atomics and SharedArrayBuffer to be used since the Spectre vulnerabilities (you can check in the browser console by typing crossOriginIsolated which should be true if everything is good).

This PR introduces a middleware (not sure what they are called in Tornado, but I suppose its implemented as a decorator) that sets these headers to the correct value.


With these headers we basically opt in to a stricter set of security rules (which is not a bad idea anyway!), but there's a bit of work required before everything will work again:

  1. All assets loaded from a different origin must have the crossorigin attribute set. So it would be <script src="https://content.illumidesk.com/my-javascript-file.js" crossorigin></script>. This likely means that we have to change some setting in our lms build pipeline that adds these tags. In webpack it's output.crossOriginLoading.
  2. Alternatively we can proxy all requests from the CDN through the formgradernext server (it's not that crazy, we already do it for the index.html).
  3. For option 1 to work, we need to set the correct CORS headers (ACAO, aka Access-Control-Allow-Origin) for content.illumidesk.com. We can set that to a wildcard * (most CDNs will do this, alternatively it could check the origin and see if it's in a whitelist - but that requires server logic in front of it). Screenshot of issue:
  4. All paths of content.illumidesk.com should have this header:
{
          "key": "Cross-Origin-Resource-Policy",
          "value": "cross-origin"
}
  1. The paths under https://content.illumidesk.com/npm/starboard-notebook (e.g. npm/starboard-notebook@0.13.2/dist/index.html) should additionally have the following headers set:
      {
        "key": "Cross-Origin-Embedder-Policy",
        "value": "require-corp"
      },
      {
        "key": "Cross-Origin-Opener-Policy",
        "value": "same-origin"
      }

(We have a crazy hack from Stefan to not actually require step 5, but we should not rely on that).

@gzuidhof
Copy link
Contributor Author

As for ownership, I was hoping @rupeshparab you could help with step 1 and 2, whereas 3 through 5 are probably best picked up by @jgwerner.

Of course I'm around to help (but a bit infrequently: going on holiday tomorrow - but I'm bringing my laptop and will be working a couple of afternoons).

@@ -46,6 +54,7 @@ def get(self):

handlers = [
(r"/formgradernext/?", LMSHandler),
(r"/formgradernext/.*", LMSHandler),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative to how it is done in #2.

Copy link
Contributor

@rupeshparab rupeshparab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rupeshparab rupeshparab merged commit 8b99ffe into main Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants