opencloud-eu
We can't add default CSP rules reliably
Describe the bug
When I want to add a new CSP rule to the set of default CSP rules, I can't be certain that it reaches all OpenCloud installations.
Steps to reproduce
- Release OpenCloud
- Wait until admins have set up their own OpenCloud instances with customized
csp.yamlfiles - Release another version of OpenCloud which needs a new CSP rule (e.g. our upcoming download-server so that we can check in the web ui, whether the installed version is up to date).
Expected behavior
When I add a new CSP rule to the default csp.yaml it gets merged into the custom csp.yaml of all the existing OpenCloud instances which have their own custom csp.yaml.
Actual behavior
If an OpenCloud instance already has a custom csp.yaml it will always overrule the default csp.yaml - as in: fully replace. There is no merging.
Additional context
Kind of relates to this feature request: https://github.com/opencloud-eu/opencloud/issues/1388 where we want to be able to add CSP rules via web ui apps.
Please note that this is P2 because we want to bring an update check to the web ui, which will require an additional CSP rule for contacting the download server from the web ui. We can only do that for the default, not for OpenCloud instances which deviated from the default.
@tbsbdr && @kulmann what csp.yaml should have higher priority? The custom user generated one or the one distributed by us (in case of overlapping configurations)?
@dragonchaser I would always say that user generated config takes precedence over our defaults. But: I think it's not possible to have conflicts, right? if both our defaults and the user generated csp.yaml have an entry that is exactly the same we only need it once (avoid the duplication). But everything else needs to be merged. Do you have examples for "conflicts" in mind where this is not true?
@kulmann that is exactly my point, example:
we have a csp config for service xzy, user has a csp config for service xzy, I'd always pick the users config, assuming the user is the source of truth and throw away our config
I'd always pick the users config [...] and throw away our config
Why would you throw away anything at all?! Did you see https://github.com/opencloud-eu/opencloud/issues/1388 ? IMO we don't have a way around merging the CSP rules into one big resulting CSP ruleset.
After talking to @kulmann we agreed on a complete merger of user-settings and our presets.
After talking to @kulmann we agreed on a complete merger of user-settings and our presets.
that + debug output of the result 😇
This is a behavioral change. Previously, admins could be sure only the CSP rules they configured were used. That we replace the built in default is the expected behavior.
Being able to add CSP rules should be an additional config option or a subsequent step, maybe by reading them from the installed web apps.
But please do not change the current behavior of replacing the built in defaults. Otherwise, admins have no way of removing CSP rules, eg. to prevent the web ui from accessing github after web apps have been installed.
After discussion in refinement this morning, we moved the update server check back to the drawing board, I guess this is on hold for now as well. Maybe we can reuse the merge code in #1617 later for #1388 .
/cc @kulmann @tbsbdr
@dragonchaser could you please write down the reasons why it "went back to the drawing board"?
@kulmann because there was no consensus on how the actual behaviour/implementation should look like
A recently discussed idea to mitigate this issue was:
-
- merge CSP rules from the defaults provided in the binary and the csp.yaml file the admin provided via
PROXY_CSP_CONFIG_FILE_LOCATION
- merge CSP rules from the defaults provided in the binary and the csp.yaml file the admin provided via
-
- add a new env variable (e.g.
PROXY_CSP_OVERRIDE_CONFIG_FILE_LOCATION) which would overrule the outcome of (i)
- add a new env variable (e.g.
This would fix e.g. the update check for already existing deployments while giving admins the tooling to completely overwrite our CSP rules. My experience is that you usually want to provide additional CSP rules, so my assumption is, that (i) would ease the lives of admins, while having (ii) in place would give them all the control they might need.
Is there any reason against this approach?
