Closed rwjblue closed 1 year ago
FWIW, I created a similar issue over in https://github.com/lifeart/ember-class-modifier/issues/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 style=
and generate the correct nonce / hash.
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?
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. 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! ๐
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 ember-cli-content-security-policy
is to add support for hashes.
More specifically, regarding this addon, I guess we have some options:
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?
@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 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.
@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 <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.
@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
@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:
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.
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.
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...
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.
@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.
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.
Do we want to call the signing method with an element, or a string?
How about signSrc
and signScript
? (could also infer from element, if we pass one in)
I agree, I think hashes is a lot better than nonces, because they works with static websites (common for SPAs).
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.
@sandstorm Thanks for your feedback.
- 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.
- 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.
- How about
signSrc
andsignScript
? (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
?
- I think that as soon as a hash is added to
script-src
orstyle-src
(ordefault-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'
.
@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!
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 https://github.com/jelhan/ember-style-modifier/issues/11#issuecomment-496712001
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...