Fix Unsafe inline scripts by adding nonce as CSP #3832
Conversation
| response = self.get_response(request) | ||
| csp_policy = "script-src 'nonce-{}' 'strict-dynamic' 'unsafe-eval';".format(nonce) | ||
|
|
||
| response['Content-Security-Policy'] = csp_policy |
There was a problem hiding this comment.
This one is configurable in the .ini, secure_content_security_policy, I believe you need to merge it somehow here instead. You could potentially just string replace 'nonce' with 'nonce-{}'...
There was a problem hiding this comment.
And add 'strict-dynamic' to the default of the config value.
There was a problem hiding this comment.
Note that there's already a def ContentSecurityPolicyMiddleware in the same file..
| @@ -267,7 +267,7 @@ else: | |||
|
|
|||
| <div class="clipboard-content"></div> | |||
|
|
|||
| <script type="text/javascript"> | |||
| <script nonce="${request.csp_nonce if request else csp_nonce}" type="text/javascript"> | |||
There was a problem hiding this comment.
why the 2 different options?
desktop/core/src/desktop/conf.py
Outdated
|
|
||
| CSP_NONCE = Config( | ||
| key="csp_nonce", | ||
| help=_('CSP NONCE can be true or false'), |
There was a problem hiding this comment.
Need to improve help message
| @@ -69,6 +70,83 @@ | |||
| from libsaml.conf import CDP_LOGOUT_URL | |||
| from urllib.parse import urlparse | |||
|
|
|||
| import base64 | |||
There was a problem hiding this comment.
Nit: Importing twice? Check line 21
| import os | ||
|
|
||
| # Number of random bytes in the nonce: | ||
| NONCE_BYTES = 24 |
There was a problem hiding this comment.
Any specific reason for setting this as 24?
| request.csp_nonce = nonce | ||
|
|
||
| def process_response(self, request, response): | ||
| # Add the secure CSP if it doesn't exist |
There was a problem hiding this comment.
Is the comment correct here?
| if not CSP_NONCE.get(): | ||
| return response |
There was a problem hiding this comment.
If let's say this condition is True, then it will return but isn't this regressing from the old change where it was setting Content-Security-Policy header in response and then returning?
| nonce_found, has_nonce = nonce_exists(response) | ||
| if has_nonce: |
There was a problem hiding this comment.
Do we really need 2 return values here?
| @@ -502,3 +502,13 @@ def __init__(self, data, encoder=DjangoJSONEncoder, safe=True, | |||
| kwargs.setdefault('content_type', 'application/json') | |||
| data = json.dumps(data, cls=encoder, **json_dumps_params) | |||
| super(JsonResponse, self).__init__(content=data, **kwargs) | |||
|
|
|||
|
|
|||
| def nonce_attribute(context): | |||
desktop/core/src/desktop/views.py
Outdated
There was a problem hiding this comment.
Why are these changes present in this file?
|
This PR is stale because it has been open 45 days with no activity and is not labeled "Prevent stale". Remove "stale" label or comment or this will be closed in 10 days. |
What changes were proposed in this pull request?
How was this patch tested?
Please review Hue Contributing Guide before opening a pull request.