-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Send a configurable CSP in every HTML response #9665
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
base: master
Are you sure you want to change the base?
Conversation
…dcube#9422) * \n\s+'file' => __FILE__, * \n\s+'line' => __LINE__, * 'line' => __LINE__, 'file' => __FILE__, * 'file' => __FILE__, 'line' => __LINE__, * rest * more * improve cs * more cs * revert rcube_utils::preg_error changes * impl file/line from backtrace * Revert "revert rcube_utils::preg_error changes"
As stated in roundcube#5574 (comment) redis is supported, adding it to supported backed [skip ci]
The eslint package "eslint-plugin-unicorn" introduced a new rule in version 53.0.0 that suggests to use `String.raw` in order to avoid the need for escaping backslashes. While this certainly is a good idea we cannot follow that rule as long as we support Internet Explorer 11+, which doesn't support template literals (and thus `String.raw`).
Fix Code Spell check by disabling `String.raw` rule
…ith no personal namespace prefix (roundcube#9452)
[skip ci]
[skip ci]
… Windows Reported by Huy Nguyễn Phạm Nhật.
… from user preferences Reported by Huy Nguyễn Phạm Nhật.
…attributes Reported by Valentin T. and Lutz Wolf of CrowdStrike.
[skip ci]
[skip ci]
Added JetBrains PhpStorm project folder (.idea)
Update .gitignore
[skip ci]
The CSP gets adapted to remote objects being allowed or not. It can be configured or disabled via the config option `content_security_policy` (and `content_security_policy_add_allow_remote`).
If people write a lax default CSP they might set the additional config option to the blank string, or false. Then the CSP header should not contain that value.
This way the code always has values it can work with, no matter how good or broken the configuration is.
Previously the code also treated `'false'` (the string) as invalid, but that's a very specific check against a specific edge case, which wouldn't even break the code (but will only make the browser complain), so I'm dropping that check.
The config value might be something else than a string.
In the previous way useless semicolons could have happened.
We still support PHP v7, which doesn't support union types.
phpstan complains that `assertIsArray($config)` will always fail because it doesn't know about the side effects of `require`ing the default config.
Fixes a typo
A "lax" CSP (aka using the config option `content_security_policy_add_allow_remote`) is required to allow using remote ressources like image URLs in the HTML editor. We can't depend this on the intial content being HTML or not because the user might want to change the editor after loading the page, and then add remote ressources.
fda480b to
e326546
Compare
|
I removed the wrong colons and I think I fixed the usage of the cases you mentioned, thank you for those hints! I found another problem: If a mailvelope user wants to automatically lookup missing openpgp keys (to encrypt a message to), the HTTP request is sent from JavaScript directly to the configured keyserver. This would be blocked by a strict CSP. I see two options:
The latter would keep the security advantage of a stricter CSP, and also improve the user's privacy slightly because it won't allow for profiling based on the client IP address. @alecpl Do you have an opinion on this? |
|
That reminds me of a key search in the enigma plugin, which may have the same issue, I suppose. I'm not sure we can fix the HTML composer this way, though. When you compose a reply/forward you deal with the original mail content that should get secured, don't you think? Marking the content safe does not look right. As I said in the beginning this becomes complicated and problematic. |
|
Enigma calls the same JavaScript-function as the Mailvelope-integration. Good point about the replies! I've changed the code to not use the (I'll happily clean up the code before it would be merged, just don't want to stir around in the commits as long as they're continuously reviewed.) |
6128f33 to
112bc97
Compare
The CSP gets adapted to remote objects being allowed or not. It can be configured or disabled via the config option
content_security_policy(andcontent_security_policy_add_allow_remote).The default is pretty weak in order to not introduce a breaking change, but still not useless (e.g. it adds another layer of defence against loading remote objects unless allowed.)
Closes #9638