-
Notifications
You must be signed in to change notification settings - Fork 12
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
Discuss issues in SSR in README (investigate mitigation) #11
Comments
FWIW, I created a similar issue over in lifeart/ember-class-modifier#2 to kick off a similar discussion over there. |
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. |
Sorry to pester @sandstrom, maybe you know the answer to the above question? |
I was thinking that since the whole HTML content is coming from the server in the SSR case, we could scan for inline |
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 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
Then we'd need to hook into the result object in FastBoot land to add a single Sorry, that was a ton of content, is this making any sense? |
FWIW, I was testing around with various approaches in this codesandbox if you want an easy way to play with CSP... |
@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. 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! 😄 |
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? |
@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 More specifically, regarding this addon, I guess we have some options:
|
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? |
@jelhan But when we have generated a Or am I misunderstanding something? |
Yeah, that's right, but we can't generate that 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 |
@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>` |
Ya, exactly. |
@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 |
@jelhan I think it can be used for inline style elements. Further down on that page it says:
|
@sandstrom This is something different. CSP provides three ways to allow inline JavaScript and CSS styles:
Content-Security-Policy: style-src 'nonce-abcdefghi' <style nonce="abcdefghi">p { color: "red" }</style>
Content-Security-Policy: style-src 'sha256-aeaf0c14ece3e9b2caec8f491230995ce790d86107839ea908e95a0f079282d9' <style>p { color: "red" }</style>
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:
|
Let me draw out how a possible integration with
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. |
That seems pretty good to me.
The idea was to only actually emit one |
I'm willing to do the work on
That's what I meant. "Register styles" should be read as "insert the CSS rules into the |
@jelhan I've tried to get
|
@sandstorm Thanks for your feedback.
Generating the
To support hashes we need to know it's content to generate the hash and if it's a 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.
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
Good point. We must not add a hash if CSP directive includes |
@jelhan Good point, if we are fine with |
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? |
@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. |
Expample of transformation, described in #11 (comment) updated one: |
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. |
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...
The text was updated successfully, but these errors were encountered: