trieloff
commented
1 week ago @chicharr I think this would play nicely with https://github.com/adobe/helix-rum-js/pull/207
Open cziegeler opened 3 weeks ago
trieloff
commented
1 week ago @chicharr I think this would play nicely with https://github.com/adobe/helix-rum-js/pull/207
cziegeler
commented
5 days ago This could be implemented by allowing two optional data attributes, data-enhancer-version and data-enhancer-hash. The first one determines the version of the rum enhancer script if set. In addition, data-enhancer-hash can be set which then results in adding subresource integrity to the included script tag for the rum enhancer, like:
script.integrity = value of data-enhancer-hash attribute script.setAttribute('crossorigin', 'anonymous');
kptdobe
commented
5 days ago It would be good to be able to get a specific version of the enhancer script and not just the latest.
What are the real life use cases ? If there are, there are other way to temporarily serve a specific version.
This adds an overhead in maintenance, especially that our customers will probably never upgrade. We also want them to use the latest and greatest and make the data collection evolve. We'll be stuck here.
Also, we discussed to use the Helix delivery instead of unpkg and jsdeliver. This will mean will have to add a lot more magic to serve the correct version.
I am definitively against this feature.
cziegeler
commented
5 days ago The real life use case is Subresource integrity and CSP - you can only specify a hash for the javascript if you know what file is served. Customers who do not care about that extra security do not need to use this feature, it is optional. But customers who care have a mechanism to do so.
kptdobe
commented
5 days ago ok but we are introducing a super generic solution that will be overused for a specific problem.
My understanding is: the only problem is for the CS script to be aligned with the hash of the rum-standalone.js and their respective release cycles (i.e. if we update rum-standalone.js, we need to update the CS script - and temporarily serve the old version and the new one, the time for everyone and all caches to be up-to-date).
Customers who do not care about that extra security do not need to use this feature, it is optional. But customers who care have a mechanism to do so.
It should be the other way around: we have 5 customers for which we have implemented a temporary hash solution, we need to productise it. So far, I though we've managed to fix their CSP none-sense... ?
cziegeler
commented
5 days ago I don't think that SRI is none-sense; and we are not talking about 5 customers here. Its at least 50, most likely increasing. The way SRI works is that you add the hash of the script to the script tag, something like (simplified):
The browser refuses to load the script if the hash does not match. Obviously if the script changes, the hash needs to change - therefore, you usually reference a fixed version of a file. If a new version is released, you update the script tag with both, the new reference and the new hash. If you always refer the latest script, then out of the sudden the script changes, the hash in the html does not match, and the site is broken - something you want to avoid - especially as this needs updating the html. SRI is needed for both, the standalone script as well as the rum enhancer script.
kptdobe
commented
5 days ago I am not saying SRI is non-sense.
The current integrity problem we are having is because we serve from NPM via unpkg and jsdeliver.
If we change the delivery to come from our infrastructure (and get rid of unpkg and jsdeliver), we do not have the problem. Or at least, we never considered it so far.
cziegeler
commented
5 days ago The current delivery has a larger integrity problem, agreed. but by using a different mechanism it is not fully going away. I seriously think we need to support SRI - I'm open to any other suggestion on how to solve that.
cziegeler
commented
4 days ago The main question is whether we want to support SRI or not - my take on this is a clear yes as there is a large enough user base for it already. If we answer that with a yes, I think there is no way around being able to fetch specific versions of the scripts (the rum js and the enhancer js). This works with todays delivery mechanism. A new delivery mechanism needs to support that as well. The hashes of each version need to be known, but that's trivial. And then finally, as the rum js is creating a script tag for the enhancer js, the version and the hash for the enhancer script needs to be known by rum js. There are two ways to deal with that: a) pass it in from the outside - as the current PR does b) the rum js knows it - I dont think we want to go there, but I describe it regardless. This would mean that a released version of the rum js has the latest released version of the enhancer js baked in (together with the hash). And this means, whenever a new enhancer js is released, a new rum js needs to be released. The current PR allows this while it does not break any other use case. /cc @trieloff @kptdobe
kptdobe
commented
4 days ago I think we are making a problem we do not have which leads to add more complexity we do not need (and all of this relies on a feature which should not be there).
No customer asked for anything: they get a rum.js loaded on their pages, most of the time, they are even not aware of it. When there is an issue with that script, it is because of CSPs (not allowed to load the script) or WAF (too many requests). SRI does not solve any of those. Either they can add the host to the CSP or we simply disable the script.
The current implementation has the great advantage we can very quickly react and adjust the data collection without depending on the AEM release cycles.
Thus yes, we have a large customer base to test this but no, no one needs it, we decide to add the feature for content integrity reason. While I agree it could be a food practice, this does not exist in our infrastructure today and has never been challenged. It could be, I fully agree, but we need to take that discussion more globally.
The PR technically works for only one reason: we use jsdeliver and unpkg to deliver the files via NPM which is a completely exotic pattern we should get rid of asap. Today, we do not support "versions" other than code branches in our overall infrastructure.
If only the latest version would be accessible (like it should for any code managed via AEM Live), implementing SRI would still be possible but the implementation would have to be different.
cziegeler
commented
4 days ago Lets not intermix different topics, the AEM release cycle has nothing to do with this. We can update the script version in minutes across all customers (and also the hash). If I understand it right, in the future there will be no way to load a specific version of the script, you always get the lastest - which means SRI is out of question. And that prevents any integrity check.
chicharr
commented
3 days ago Hi @cziegeler
We can update the script version in minutes across all customers (and also the hash).
this is interesting. How would that work? would the enhancer version + hash be config vars in the CS environments?
cziegeler
commented
3 days ago @chicharr Kind of, those (version + hash) are configurations that we can change at runtime. Think of it as toggles which have a value and are not just boolean,
While it is possible to request a specific version of the rum script, e.g.
this script then always fetches the latest rum enhancer script.
It would be good to be able to get a specific version of the enhancer script and not just the latest.
One solution would be to pin point the rum enhancer script version when releasing a new rum js version. The downside is that this ties those versions together and when a new rum enhancer version is released, it requires a new rum js release.