Discuss issues in SSR in README (investigate mitigation)

[rwjblue]

Since element modifiers do not run at all in SSR / FastBoot mode, we should discuss in the README the caveats an app may have when using this addon. Specifically, if you use {{style}} those styles will not be present when the app is ran via SSR/FastBoot.

There are a few plausible ways to mitigate this, I'd love to have a discussion to help make a path forward...

[rwjblue]

FWIW, I created a similar issue over in lifeart/ember-class-modifier#2 to kick off a similar discussion over there.

[jelhan]

Providing a CSP safe way to set styles on an element is one of the main motivation for this addon. If I didn't miss something there isn't any way to render these styles in SSR / Fastboot without violating a strict Content-Security-Policy, isn't it? Nonce and hashes could only be used for elements but not for attributes if I recall correctly. Therefor this feels like a conflict of objectives.

But the limitation for SSR / FastBoot should be mentioned in README of course.

[rwjblue]

Sorry to pester @sandstrom, maybe you know the answer to the above question?

[rwjblue]

I was thinking that since the whole HTML content is coming from the server in the SSR case, we could scan for inline style= and generate the correct nonce / hash.

[rwjblue]

I couldn't get a hash of the inline style attribute to work in my local testing (and various StackOverflow posts seem to indicate that its not supposed to?), but I can see us working around the issue by generating hashes/nonces and using inline <style> elements + an AST transform.

Specifically, we could transform at build time from:

<p
  {{style
    border="1px"
    padding="1em !important"
  }}
>
</p>
<p {{style color="red"}}>Red!</p>

Into:

{{#let 
  (-gather-styles
    border="1px"
    padding="1em !important")
  (-gather-styles
    color="red")

as |styles1 styles2|}}

<p class="{{styles1.class}}" {{style styles1.directives}}></p>
<p class="{{styles2.class}}" {{style styles2.directives}}>Red!</p>

{{/let}}

Where the -gather-styles helper does something like:

export default helper(function(params, hash) {
  let result = {
    directives: hash
  };

  if (typeof FastBoot !== 'undefined') {
    // add a randomly generated class
    result.class = generateRandomClass();

    let styleHelpers = FastBoot.require('ember-style-modifier/node-file-for-gathering-stuff');
    styleHelpers.registerStyle(result.class, hash);
  }
});

Then we'd need to hook into the result object in FastBoot land to add a single <style></style> that we would hash and include in our headers.

---

Sorry, that was a ton of content, is this making any sense?

[rwjblue]

FWIW, I was testing around with various approaches in this codesandbox if you want an easy way to play with CSP...

[sandstrom]

@rwjblue No worries. But it seems like you beat me to it 😄

I agree, I don't see a way to make nonces/hashes work with style attributes (and using e.g. data-style="color: red" plus some JS to write that onto the element upon page load won't make sense for SSR I assume).

Your idea with unique class names is great, I think that should work!

---

Another solution (not sure we should do this though) is:

<!-- before -->
<p {{style color="red"}}>Red!</p>

<!-- after -->
<style type="text/css" nonce="abc" id="def">
  style#def + p {
    color: red;   
   }
</style>
<p>Red!</p>
<!-- must position style element immediately before the relevant element -->

Would need one inline style element per element with style attribute though, I personally would prefer keeping everything in a single style element (like you suggested).

The only benefit with this method is that we don't need to touch the element (add a class attribute to it), but not sure if that's worth much (or anything?).

So I would still vote for your class idea! 😄

[jelhan]

These approaches sounds great. But I'm not quite sure yet how this would integrate with ember-cli-content-security-policy. The nonce must be a unqiue value for each request served by FastBoot. It must be unguessable and should be generated via a cryptographically secure random number generator. The same nonce must be used for CSP meta tag and CSP header. ember-cli-content-security-policy does not provide any support for nonces yet (besides for testing support). So work seems to be needed at multiple places or do I miss something?

[sandstrom]

@jelhan Yeah, nonces doesn't work well with statically generated sites (not the case with Fastboot, but it is the case with most other Ember deployments), so while there aren't any support for it yet, what would make sense for ember-cli-content-security-policy is to add support for hashes.

More specifically, regarding this addon, I guess we have some options:

1. This addon will generate hashes for the inline style tag that it generates
2. Some hook on the aforementioned CSP-addon will parse all html, look for style tags and generate hashes for them. If that would operate on HTML that the Fastboot process outputs, we don't want to blindly add hashes for every style tag, since such a tag could be malicious (I think, please correct me if any of you disagree).
3. If addons can 'call' other addons (I don't know if that's a thing), this addon could call the CSP-addon with the tag/html and ask it to sign/hash it, that could also be a venue.

[jelhan]

Yeah, nonces doesn't work well with statically generated sites

I'm not sure if that's a big difference for hashes if talking about FastBoot. The styles might not be predictable at build time. E.g. they might depend on some property of the model. If the styles are predictable at build time we might just recommend to use a CSS class instead?

[sandstrom]

@jelhan But when we have generated a <style> element (as rwjblue described above), we know what content it contains, which is all we need to generate the hash.

Or am I misunderstanding something?

[jelhan]

@jelhan But when we have generated a <style> element (as rwjblue described above), we know what content it contains, which is all we need to generate the hash.

Or am I misunderstanding something?

Yeah, that's right, but we can't generate that <style> element (or to be more precise: it's content) at compile time cause the styles might not be known at these time. It has to be done at render time (SSR / FastBoot) for each request to support all use cases. E.g. the styles might depend on some data fetched from backend or on the current time. In that cases the styles aren't known at compile time but must be calculated at build time.

I'm not 100%ly sure if I fully understood rwjblue's approach but if I got it right, it calculates the styles at render time. At compile time it adds a template helper additionally to the element modifier cause template helpers are executed in SSR/FastBoot. These template helper calculates the styles, adds them to the <style> element with a unique class name and adds that unique class to the element.

[sandstrom]

@jelhan Yeah, that's also how I interpreted his approach, and at render time we can calculate the hash. Something like this:

// inside the styleHelper (pseudo-code)
let cssString = "…";
let hash = computeSha512(cssString);
return `<style type="text/css" integrity="${hash}">${cssString}</style>`

[rwjblue]

Ya, exactly.

[jelhan]

@sandstrom I wasn't aware that CSP Level 3 integrates with Subresource Integrity in some cases. Haven't read the latest draft of CSP Level 3 in detail yet but as far as I got it yet I'm not sure if our use case is supported. It's only mentioned in 6.6.1. Script directive checks and 8.4. Allowing external JavaScript via hashes but not in 6.6.3. Element Matching Algorithms. Therefor I guess it's only supported for <script> elements that references external JavaScript but not for <style> elements having inline styles. If this is true and Subresource Integrity is not supported for our use case the hash must be included in CSP, which is either served as HTTP header or <meta> element.

[sandstrom]

@jelhan I think it can be used for inline style elements. Further down on that page it says:

Note: Hashes apply to inline script and inline style. If the "'unsafe-hashes'" source expression is present, they will also apply to event handlers, style attributes and javascript: navigations.
– https://w3c.github.io/webappsec-csp/#matching-elements

[jelhan]

@sandstrom This is something different. CSP provides three ways to allow inline JavaScript and CSS styles:

1. Nonces

Content-Security-Policy: style-src 'nonce-abcdefghi'

<style nonce="abcdefghi">p { color: "red" }</style>

2. Hashes

Content-Security-Policy: style-src 'sha256-aeaf0c14ece3e9b2caec8f491230995ce790d86107839ea908e95a0f079282d9'

<style>p { color: "red" }</style>

3. unsafe-inline

Content-Security-Policy: style-src 'unsafe-inline'

<style>p { color: "red" }</style>

The last one disables the security feature at all and therefor isn't recommended.

Allowing external JavaScript via hashes is another story. As far as I got it CSP Level 3 integrates with Subresource Integrity for this use case. But that's limited for external JavaScript. At least that's what I got from § 8.4. Allowing external JavaScript via hashes. Please note that it's also using a different algorithm to determine the hash:

Note: The CSP spec specifies that the contents of an inline script element or event handler needs to be encoded using UTF-8 encode before computing its hash. [SRI] computes the hash on the raw resource that is being fetched instead. This means that it is possible for the hash needed to whitelist an inline script block to be different that the hash needed to whitelist an external script even if they have identical contents.

[jelhan]

Let me draw out how a possible integration with ember-cli-content-security-policy might look like:

ember-cli-content-security-policy exposes a contentSecurityPolicy service. This service provides a sign method that expects an element as first and only argument. The sign method of contentSecurityPolicy services calculates the hash of the elements content accordingly to CSP spec. It registers the element / hash combination in a local key-value-store (e.g. Map). It generates the CSP including all registers hashes and sets the CSP header using response.headers.set() method provided by fastboot service. It updates the CSP <header> tag if used. ember-style-modifier calls the sign method whenever it registers styles as described in rwjblue's approach.

We might later support using nonces but should start with hashes cause hashes also supports use cases like Prember. Hashes have the drawback of adding more to response size compared to nonces especially if used for multiple elements.

[rwjblue]

That seems pretty good to me.

ember-style-modifier calls the sign method whenever it registers styles as described in rwjblue's approach.

The idea was to only actually emit one <style> tag (not one for each usage of {{style}} modifier). Of course, this doesn't really change anything in your suggested path forward, I just wanted to point it out...

[jelhan]

That seems pretty good to me.

I'm willing to do the work on ember-cli-content-security-policy side. We don't need a RFC for this one, do we?

ember-style-modifier calls the sign method whenever it registers styles as described in rwjblue's approach.

The idea was to only actually emit one <style> tag (not one for each usage of {{style}} modifier). Of course, this doesn't really change anything in your suggested path forward, I just wanted to point it out...

That's what I meant. "Register styles" should be read as "insert the CSS rules into the style element created". Actually this is a very important point as using hashes doesn't scale well for multiple elements. You need one hash per element and even a sha256 hash adds 64 chars to header size. This would blow up quickly.

[sandstrom]

@jelhan I've tried to get integrity="…" to work for <style> but I cannot get it to work. So what I wrote earlier is either wrong or browsers don't support it yet. Fortunately what you outline above (2, hashes) works fine, so we're still good on that front.

1. For the API of ember-content-security-policy, should we leave it up to the calling addon to render the element? Or should that be the responsibility of ember-content-security-policy?

   I assume we'd want that responsibility to fall on the calling addon? In that case, and if we'd like to support the integrity attribute in the future, such a future method would need to return a promise that will resolve with the hash. We don't have to build all of that now, just something to keep in mind.

2. Do we want to call the signing method with an element, or a string?

3. How about signSrc and signScript? (could also infer from element, if we pass one in)

4. I agree, I think hashes is a lot better than nonces, because they works with static websites (common for SPAs).

5. I think that as soon as a hash is added to script-src or style-src (or default-src), the browsers CSP engine will ignore 'unsafe-inline' directives. Just something to keep in mind + probably point out in the readme.

[jelhan]

@sandstorm Thanks for your feedback.

1. For the API of ember-content-security-policy, should we leave it up to the calling addon to render the element? Or should that be the responsibility of ember-content-security-policy?
   I assume we'd want that responsibility to fall on the calling addon? In that case, and if we'd like to support the integrity attribute in the future, such a future method would need to return a promise that will resolve with the hash. We don't have to build all of that now, just something to keep in mind.

Generating the <style> (or <script>) element should be responsibility of consumer/calling addon. ember-cli-content-security-policy should only be responsible for ensuring that the element won't violate CSP. For now that's only adding hash to CSP, which whitelists that element. If we add an option to use nonces later, that would also mean adding a nonce to the element passed in.

2. Do we want to call the signing method with an element, or a string?

To support hashes we need to know it's content to generate the hash and if it's a <style> or <script> element to know which directive the hash should be added to. Both is available if passing the element (tagName, textContent). If we decide to support nonces in future, we would need to alter the element (add a nonce attribute). So passing the element should give us a simple API and create flexibility at the same time.

Also I'm not sure how we could ensure that only one hash per element is added if method is called multiple times, if we pass a string and not the reference to that element.

3. How about signSrc and signScript? (could also infer from element, if we pass one in)

I'm not quite sure why this would be needed. Do you have a use case in mind? Wouldn't this be just an alias of sign with an additional assertion that element has expected tagName?

5. I think that as soon as a hash is added to script-src or style-src (or default-src), the browsers CSP engine will ignore 'unsafe-inline' directives. Just something to keep in mind + probably point out in the readme.

Good point. We must not add a hash if CSP directive includes 'unsafe-inline'.

[sandstrom]

@jelhan Good point, if we are fine with .sign() mutating the passed in element we don't need to return a promise. Regarding the method name, let's go with .sign() as you suggested!

[jelhan]

I could need some guidance on getting startet with Glimmer AST Plugins. I've read the registering a Plugin section of ember-cli-htmlbars docs. I also had a look at the ASTPlugins being part of ember-test-selectors and @css-blocks/glimmer. They are looking quite different. Which one is the correct / modern approach? Is there any documentation / talk / article about Glimmer ASTPlugins?

I also had a look at ember-template-recast and ember-angle-brackets-codemod. I got the feeling that codemods are a different topic and the strategies used there aren't fitting our use case here, isn't it?

[sandstrom]

@jelhan I think that codemods is something different (you run them once on your code base and commit the changes (after review) to your repo, e.g. when upgrading from Ember 2.x to Ember 3.x.

Unfortunately I don't know anything about AST plugins, so I cannot be of any help there.

[lifeart]

Expample of transformation, described in #11 (comment)

https://astexplorer.net/#/gist/8f6e4d346714c8d3365ab4b0e6d806e6/1cd4357717991559f6988f70b621fb5c21bfc9da

updated one:

https://astexplorer.net/#/gist/5b54910c780b597882abe306d3497905/b8bc4f0fefd66d4b0496d285c28d408996e2a228

[jelhan]

This known limitation is documented in #117. Extending modifier capabilities to resolve it, is discussed in this RFC issue. Currently a preinsert hook, which supports rehydration capability as well, is discussed as preferred solution. Please find details in this comment.

I'm going to close this issue as the targeted way forward is addressing the issue at framework level.