Problem/Motivation
My understanding is that the only valid sources for script-src-attr
and style-src-attr
are 'none'
, 'unsafe-inline'
, or 'unsafe-hashes'
(+ corresponding hashes) - any network sources can be removed.
If the *-attr
directive matches a fallback it should continue to be omitted from the policy, but if it contains a unique value it should be shortened.
Proposed resolution
Add another helper method for Csp::getHeaderValue()
to handle *-attr
-specific optimizations that removes any network sources for the policy.
This optimization should only be applied if the sources do not match a fallback directive, which would allow omitting the directive completely.
Input | Output | Notes |
---|---|---|
script-src-attr 'self' 'unsafe-inline' https://example.com |
script-src-attr 'unsafe-inline' |
|
script-src-attr 'self' 'unsafe-hashes' 'sha256-abcde' |
script-src-attr 'unsafe-hashes' 'sha256-abcde' |
|
script-src-attr 'self' 'unsafe-inline' 'unsafe-hashes' 'sha256-abcde' |
script-src-attr 'unsafe-hashes' 'sha256-abcde' |
CSP Level 2 browsers don't support *-attr; hashes should disable 'unsafe-inline' in CSP Level 3 browser |
script-src-attr 'self' https://example.com |
script-src-attr 'none' |
|
script-src-attr * |
script-src-attr 'none' |
|
script-src-attr 'sha256-abcde' |
script-src-attr 'none' |
Omit hashes if 'unsafe-hashes' is not present |
|
script-src 'self' https://example.com |
|
|
|
|
|
|
Comments
Comment #2
gappleI think
script-src-attr 'self'
would also be equivalent toscript-src-attr 'none'
.If the site configuration is set to
'none'
it will override any dynamic changes (which would currently break CKEditor). If it's set ton/a
it may be omitted and fallback, enabling unwanted sources. I think this makes keeping'self'
as an option in the UI necessary, but the resulting policy would not need to include it.Comment #4
gappleI think most cases listed in the description should now be covered, except for
'self'
Comment #5
egranty CreditAttribution: egranty commentedIt case of omitting it's sufficiently reduce the secirity:
does allow only inline event handlers and javascript-navigation in the support browsers, while:
does allow any inline scripts including
<script>...</script>
.Yes, you can remove unsupported sources, but who know full list of these?
As for now the
'report-sample'
is missed, but what happens in future? Who will track CSP spec changes and do support tests across all browsers?I think it's better just to throw diag message on invalid sources. Any enforced modifying userd CSP leads to responcibility if something gies wrong.
Comment #6
gapple@egranty
In your example:
The
script-src-elem
directive is not explicitly defined, so it will fall back to the value of thescript-src
directive. As such the three directives are equivalent to justscript-src 'unsafe-inline'
, and both inline code blocks (<script>inlineFunction()</script>
) and event attributes (<a onclick="inlineFunction()">Click me!</a>
), will be permitted. In order to only allow inline attributes and not inline code blocks,script-src-elem
would need to be defined - in this case with'none'
.A more expected real world example would include a host-source for
script-src-elem
(and inscript-src
for browsers that don't yet support Level 3):The previous example could not be automatically optimized (since
script-src-attr
doesn't include'self'
), but would be effectively equivalent to:Since the not explicitly defined
script-src-attr
would fall back to the value ofscript-src
.----
Your point that spec changes could cause issues is important to consider in what and how optimizations are done. I do watch the CSP spec for changes, and for questions that others have about the spec to ensure I have a solid understanding of it and this module aligns to it as best as possible. Current optimizations are only done where they are reasonably safe, have a clear set of cases to test against, and are actually likely to be encountered and provide a benefit (e.g. I see you've also commented on the issue about optimizing host-source protocols - I don't think that it's something that will typically occur, it's probably something a user should address if it does, and there's not a big benefit compared to the complexity, so it's not likely to be implemented).
The current implementation for optimizing attribute directives only removes values that are known to not have an affect when included in the directive. If new keywords are introduced to the spec they will be allowed by default and it's not possible to introduce a source value to the spec that's not a quoted keyword, so I don't think there's a concern regarding this particular optimization.
https://git.drupalcode.org/project/csp/-/blob/20b79b90120a7e07aed655686d...
The configuration form is a little more restrictive - limiting the options to ones that are know to be valid for a particular directive, and emitting errors where known invalid values are seen. This means that if a new keyword is introduced to the spec the form will need an update - it's still possible to modify the directive directly in configuration (or add it programmatically) and it will be output in the header, but it would be removed from config if the admin form were to be saved while it's not an available option.
Comment #7
gappleI think the current optimizations are good as-is.
I would like to have a specific, practical use case for further optimizations.