You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2023/06/29 15:38:25 UTC
[superset] branch master updated: chore: Un-revert enabling CSP by default (#24543)
This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 38df1a873f chore: Un-revert enabling CSP by default (#24543)
38df1a873f is described below
commit 38df1a873f3bbfcdc5a7af6aaf7b17f8c7a9e08c
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Thu Jun 29 17:38:17 2023 +0200
chore: Un-revert enabling CSP by default (#24543)
---
UPDATING.md | 1 +
docs/docs/security.mdx | 48 +++++++++++-----------
.../src/models/ExtensibleFunction.ts | 3 +-
superset/config.py | 38 +++++++++++++++--
superset/initialization/__init__.py | 6 ++-
.../appbuilder/general/widgets/base_list.html | 3 +-
.../appbuilder/general/widgets/search.html | 3 +-
superset/templates/superset/export_dashboards.html | 3 +-
.../templates/superset/form_view/csv_scripts.html | 4 +-
.../form_view/csv_to_database_view/edit.html | 3 +-
.../form_view/database_schemas_selector.html | 3 +-
.../{export_dashboards.html => macros.html} | 20 +++------
.../templates/superset/models/database/macros.html | 9 ++--
.../templates/superset/partials/asset_bundle.html | 3 +-
superset/templates/superset/theme.html | 15 +++++--
15 files changed, 102 insertions(+), 60 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index b95cedc1e9..f55102d15e 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -34,6 +34,7 @@ assists people when migrating to a new version.
### Breaking Changes
+- [24262](https://github.com/apache/superset/pull/24262): Enabled `TALISMAN_ENABLED` flag by default and provided stricter default Content Security Policy
- [24415](https://github.com/apache/superset/pull/24415): Removed the obsolete Druid NoSQL REGEX operator.
- [24423](https://github.com/apache/superset/pull/24423): Removed deprecated APIs `/superset/slice_json/...`, `/superset/annotation_json/...`
- [24400](https://github.com/apache/superset/pull/24400): Removed deprecated APIs `/superset/recent_activity/...`, `/superset/fave_dashboards_by_username/...`, `/superset/fave_dashboards/...`, `/superset/created_dashboards/...`, `/superset/user_slices/`, `/superset/created_slices/...`, `/superset/fave_slices/...`, `/superset/favstar/...`,
diff --git a/docs/docs/security.mdx b/docs/docs/security.mdx
index a81149afb5..ab6d41e895 100644
--- a/docs/docs/security.mdx
+++ b/docs/docs/security.mdx
@@ -176,9 +176,9 @@ a certain resource type or policy area. You can check possible directives
It's extremely important to correctly configure a Content Security Policy when deploying Superset to
prevent many types of attacks. Superset provides two variables in `config.py` for deploying a CSP:
-- `TALISMAN_ENABLED` defaults to `False`; set this to `True` in order to implement a CSP
-- `TALISMAN_CONFIG` holds the actual the policy definition (_see example below_) as well as any
- other arguments to be passed to Talisman.
+- `TALISMAN_ENABLED` defaults to `True`; set this to `False` in order to disable CSP
+- `TALISMAN_CONFIG` holds the actual the policy definition (*see example below*) as well as any
+other arguments to be passed to Talisman.
When running in production mode, Superset will check at startup for the presence
of a CSP. If one is not found, it will issue a warning with the security risks. For environments
@@ -187,10 +187,20 @@ this warning using the `CONTENT_SECURITY_POLICY_WARNING` key in `config.py`.
#### CSP Requirements
-- Superset needs both the `'unsafe-eval'` and `'unsafe-inline'` CSP keywords in order to operate.
+* Superset needs the `style-src unsafe-inline` CSP directive in order to operate.
```
- default-src 'self' 'unsafe-eval' 'unsafe-inline'
+ style-src 'self' 'unsafe-inline'
+ ```
+
+* Only scripts marked with a [nonce](https://content-security-policy.com/nonce/) can be loaded and executed.
+Nonce is a random string automatically generated by Talisman on each page load.
+You can get current nonce value by calling jinja macro `csp_nonce()`.
+
+ ```
+ <script nonce="{{ csp_nonce() }}">
+ /* my script */
+ </script>
```
- Some dashboards load images using data URIs and require `data:` in their `img-src`
@@ -206,21 +216,11 @@ this warning using the `CONTENT_SECURITY_POLICY_WARNING` key in `config.py`.
connect-src 'self' https://api.mapbox.com https://events.mapbox.com
```
-This is a basic example `TALISMAN_CONFIG` that implements the above requirements, uses `'self'` to
-limit content to the same origin as the Superset server, and disallows outdated HTML elements by
-setting `object-src` to `'none'`.
+* Other CSP directives default to `'self'` to limit content to the same origin as the Superset server.
+
+In order to adjust provided CSP configuration to your needs, follow the instructions and examples provided in
+[Content Security Policy Reference](https://content-security-policy.com/)
-```python
-TALISMAN_CONFIG = {
- "content_security_policy": {
- "default-src": ["'self'", "'unsafe-inline'", "'unsafe-eval'"],
- "img-src": ["'self'", "data:"],
- "worker-src": ["'self'", "blob:"],
- "connect-src": ["'self'", "https://api.mapbox.com", "https://events.mapbox.com"],
- "object-src": "'none'",
- }
-}
-```
#### Other Talisman security considerations
@@ -229,15 +229,15 @@ of which `content_security_policy` is only one. Those can be found in the
[Talisman documentation](https://pypi.org/project/flask-talisman/) under _Options_.
These generally improve security, but administrators should be aware of their existence.
-In particular, the default option of `force_https = True` may break Superset's Alerts & Reports
+In particular, the option of `force_https = True` (`False` by default) may break Superset's Alerts & Reports
if workers are configured to access charts via a `WEBDRIVER_BASEURL` beginning
-with `http://`. As long as a Superset deployment enforces https upstream, e.g.,
-through a loader balancer or application gateway, it should be acceptable to set this
-option to `False`, like this:
+with `http://`. As long as a Superset deployment enforces https upstream, e.g.,
+through a loader balancer or application gateway, it should be acceptable to keep this
+option disabled. Otherwise, you may want to enable `force_https` like this:
```python
TALISMAN_CONFIG = {
- "force_https": False,
+ "force_https": True,
"content_security_policy": { ...
```
diff --git a/superset-frontend/packages/superset-ui-core/src/models/ExtensibleFunction.ts b/superset-frontend/packages/superset-ui-core/src/models/ExtensibleFunction.ts
index 78901dd565..5a247d751e 100644
--- a/superset-frontend/packages/superset-ui-core/src/models/ExtensibleFunction.ts
+++ b/superset-frontend/packages/superset-ui-core/src/models/ExtensibleFunction.ts
@@ -22,9 +22,8 @@
*/
export default class ExtensibleFunction extends Function {
+ // @ts-ignore
constructor(fn: Function) {
- super();
-
// eslint-disable-next-line @typescript-eslint/no-unsafe-return, no-constructor-return
return Object.setPrototypeOf(fn, new.target.prototype);
}
diff --git a/superset/config.py b/superset/config.py
index abb73e9f56..4ac95925ec 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -1365,12 +1365,42 @@ TEST_DATABASE_CONNECTION_TIMEOUT = timedelta(seconds=30)
CONTENT_SECURITY_POLICY_WARNING = True
# Do you want Talisman enabled?
-TALISMAN_ENABLED = False
+TALISMAN_ENABLED = True
# If you want Talisman, how do you want it configured??
TALISMAN_CONFIG = {
- "content_security_policy": None,
- "force_https": True,
- "force_https_permanent": False,
+ "content_security_policy": {
+ "default-src": ["'self'"],
+ "img-src": ["'self'", "data:"],
+ "worker-src": ["'self'", "blob:"],
+ "connect-src": [
+ "'self'",
+ "https://api.mapbox.com",
+ "https://events.mapbox.com",
+ ],
+ "object-src": "'none'",
+ "style-src": ["'self'", "'unsafe-inline'"],
+ "script-src": ["'self'", "'strict-dynamic'"],
+ },
+ "content_security_policy_nonce_in": ["script-src"],
+ "force_https": False,
+}
+# React requires `eval` to work correctly in dev mode
+TALISMAN_DEV_CONFIG = {
+ "content_security_policy": {
+ "default-src": ["'self'"],
+ "img-src": ["'self'", "data:"],
+ "worker-src": ["'self'", "blob:"],
+ "connect-src": [
+ "'self'",
+ "https://api.mapbox.com",
+ "https://events.mapbox.com",
+ ],
+ "object-src": "'none'",
+ "style-src": ["'self'", "'unsafe-inline'"],
+ "script-src": ["'self'", "'unsafe-inline'", "'unsafe-eval'"],
+ },
+ "content_security_policy_nonce_in": ["script-src"],
+ "force_https": False,
}
#
diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py
index 6354606006..94a506b83a 100644
--- a/superset/initialization/__init__.py
+++ b/superset/initialization/__init__.py
@@ -606,7 +606,11 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods
# Talisman
talisman_enabled = self.config["TALISMAN_ENABLED"]
- talisman_config = self.config["TALISMAN_CONFIG"]
+ talisman_config = (
+ self.config["TALISMAN_DEV_CONFIG"]
+ if self.superset_app.debug
+ else self.config["TALISMAN_CONFIG"]
+ )
csp_warning = self.config["CONTENT_SECURITY_POLICY_WARNING"]
if talisman_enabled:
diff --git a/superset/templates/appbuilder/general/widgets/base_list.html b/superset/templates/appbuilder/general/widgets/base_list.html
index 3b376d243b..4842511c5f 100644
--- a/superset/templates/appbuilder/general/widgets/base_list.html
+++ b/superset/templates/appbuilder/general/widgets/base_list.html
@@ -17,6 +17,7 @@
under the License.
#}
{% import 'appbuilder/general/lib.html' as lib %}
+{% import "superset/macros.html" as macros %}
{% set can_add = "can_add" | is_item_visible(modelview_name) %}
{% set can_show = "can_show" | is_item_visible(modelview_name) %}
@@ -56,7 +57,7 @@
{{ lib.render_pagination(page, page_size, count, modelview_name) }}
{{ lib.render_set_page_size(page, page_size, count, modelview_name) }}
</div>
- <script language="javascript">
+ <script language="javascript" nonce="{{ macros.get_nonce() }}">
var modelActions = new AdminActions();
</script>
diff --git a/superset/templates/appbuilder/general/widgets/search.html b/superset/templates/appbuilder/general/widgets/search.html
index 22c6b629c1..e59b96848e 100644
--- a/superset/templates/appbuilder/general/widgets/search.html
+++ b/superset/templates/appbuilder/general/widgets/search.html
@@ -17,6 +17,7 @@
under the License.
#}
{% import 'appbuilder/general/lib.html' as lib %}
+{% import "superset/macros.html" as macros %}
<div class="list-search-container">
<form id="filter_form" class="form-search" method="get">
@@ -44,7 +45,7 @@
</form>
</div>
-<script>
+<script nonce="{{ macros.get_nonce() }}">
(function($) {
function checkSearchButton() {
var hasFilter = $('.filters tr').length;
diff --git a/superset/templates/superset/export_dashboards.html b/superset/templates/superset/export_dashboards.html
index 9744424b67..775256ff26 100644
--- a/superset/templates/superset/export_dashboards.html
+++ b/superset/templates/superset/export_dashboards.html
@@ -16,7 +16,8 @@
specific language governing permissions and limitations
under the License.
#}
-<script>
+{% import "superset/macros.html" as macros %}
+<script nonce="{{ macros.get_nonce() }}">
window.onload = function() {
// See issue #7353, window.open fails
diff --git a/superset/templates/superset/form_view/csv_scripts.html b/superset/templates/superset/form_view/csv_scripts.html
index bb7b94b1a3..b2d12b1aea 100644
--- a/superset/templates/superset/form_view/csv_scripts.html
+++ b/superset/templates/superset/form_view/csv_scripts.html
@@ -16,7 +16,9 @@ KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
#}
-<script>
+{% import "superset/macros.html" as macros %}
+
+<script nonce="{{ macros.get_nonce() }}">
$('#delimiter').on('change', function () {
var delimiterOptions = $('#delimiter').val();
if (delimiterOptions?.includes("other")) {
diff --git a/superset/templates/superset/form_view/csv_to_database_view/edit.html b/superset/templates/superset/form_view/csv_to_database_view/edit.html
index a0ae43792e..9bbda3366f 100644
--- a/superset/templates/superset/form_view/csv_to_database_view/edit.html
+++ b/superset/templates/superset/form_view/csv_to_database_view/edit.html
@@ -18,6 +18,7 @@
#}
{% extends "appbuilder/base.html" %}
{% import 'appbuilder/general/lib.html' as lib %}
+{% import "superset/macros.html" as macros %}
{% set begin_sep_label = '<td class="col-sm-2" style="border-left: 0; border-top: 0;">' %}
{% set end_sep_label = '</td>' %}
{% set begin_sep_field = '<td style="border-right: 0; border-top: 0;">' %}
@@ -132,7 +133,7 @@
</div>
{% endblock %}
{% block add_tail_js %}
-<script src="{{url_for('appbuilder.static',filename='js/ab_keep_tab.js')}}"></script>
+<script src="{{url_for('appbuilder.static',filename='js/ab_keep_tab.js')}}" nonce="{{ macros.get_nonce() }}"></script>
{% endblock %}
{% block tail_js %}
{{ super() }}
diff --git a/superset/templates/superset/form_view/database_schemas_selector.html b/superset/templates/superset/form_view/database_schemas_selector.html
index ac827c1330..a02ea46098 100644
--- a/superset/templates/superset/form_view/database_schemas_selector.html
+++ b/superset/templates/superset/form_view/database_schemas_selector.html
@@ -16,7 +16,8 @@ KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
#}
-<script>
+{% import "superset/macros.html" as macros %}
+<script nonce="{{ macros.get_nonce() }}">
var db = $("#database");
var schema = $("#schema");
diff --git a/superset/templates/superset/export_dashboards.html b/superset/templates/superset/macros.html
similarity index 67%
copy from superset/templates/superset/export_dashboards.html
copy to superset/templates/superset/macros.html
index 9744424b67..943c11d25b 100644
--- a/superset/templates/superset/export_dashboards.html
+++ b/superset/templates/superset/macros.html
@@ -16,18 +16,8 @@
specific language governing permissions and limitations
under the License.
#}
-<script>
- window.onload = function() {
-
- // See issue #7353, window.open fails
- var a = document.createElement('a');
- a.href = window.location + '&action=go';
- a.download = 'dashboards.json';
- document.body.appendChild(a);
- a.click();
- document.body.removeChild(a);
-
- window.location = '{{ dashboards_url }}';
-
- };
-</script>
+{% macro get_nonce() %}
+ {% if csp_nonce is defined %}
+ {{ csp_nonce() }}
+ {% endif %}
+{% endmacro %}
diff --git a/superset/templates/superset/models/database/macros.html b/superset/templates/superset/models/database/macros.html
index f1f7a36cde..5f86e7df3b 100644
--- a/superset/templates/superset/models/database/macros.html
+++ b/superset/templates/superset/models/database/macros.html
@@ -16,8 +16,9 @@
specific language governing permissions and limitations
under the License.
#}
+{% import "superset/macros.html" as macros %}
{% macro testconn() %}
- <script>
+ <script nonce="{{ macros.get_nonce() }}">
$("#sqlalchemy_uri").parent()
.append('<button id="testconn" class="btn btn-sm btn-primary">{{ _("Test Connection") }}</button>');
$("#testconn").click(function(e) {
@@ -72,19 +73,19 @@
{% endmacro %}
{% macro expand_extra_textarea() %}
- <script>
+ <script nonce="{{ macros.get_nonce() }}">
$('#extra').attr('rows', '5');
</script>
{% endmacro %}
{% macro expand_encrypted_extra_textarea() %}
- <script>
+ <script nonce="{{ macros.get_nonce() }}">
$('#encrypted_extra').attr('rows', '5');
</script>
{% endmacro %}
{% macro expand_server_cert_textarea() %}
- <script>
+ <script nonce="{{ macros.get_nonce() }}">
$('#server_cert').attr('rows', '5');
</script>
{% endmacro %}
diff --git a/superset/templates/superset/partials/asset_bundle.html b/superset/templates/superset/partials/asset_bundle.html
index 066c9f64e6..41b6d7cf8a 100644
--- a/superset/templates/superset/partials/asset_bundle.html
+++ b/superset/templates/superset/partials/asset_bundle.html
@@ -16,12 +16,13 @@
specific language governing permissions and limitations
under the License.
#}
+{% import "superset/macros.html" as macros %}
{% macro js_bundle(filename) %}
{# HTML comment is needed for webpack-dev-server to replace assets
with development version #}
<!-- Bundle js {{ filename }} START -->
{% for entry in js_manifest(filename) %}
- <script src="{{ entry }}" async></script>
+ <script src="{{ entry }}" async nonce="{{ macros.get_nonce() }}"></script>
{% endfor %}
<!-- Bundle js {{ filename }} END -->
{% endmacro %}
diff --git a/superset/templates/superset/theme.html b/superset/templates/superset/theme.html
index 856796a4c4..3f6c8fb074 100644
--- a/superset/templates/superset/theme.html
+++ b/superset/templates/superset/theme.html
@@ -17,6 +17,7 @@
under the License.
#}
{% extends "superset/basic.html" %}
+{% import "superset/macros.html" as macros %}
{% block body %}
<body>
@@ -1340,7 +1341,15 @@
{% endblock %}
{% block tail_js %}
{{ super() }}
- <script src="https://code.jquery.com/jquery-1.10.2.min.js"></script>
- <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js"></script>
- <script src="{{ assets_prefix }}/static/assets/stylesheets/less/cosmo/cosmoTheme.js"></script>
+ <script
+ src="https://code.jquery.com/jquery-1.10.2.min.js"
+ integrity="sha256-C6CB9UYIS9UJeqinPHWTHVqh/E1uhG5Twh+Y5qFQmYg="
+ crossorigin="anonymous"
+ nonce="{{ macros.get_nonce() }}"></script>
+ <script
+ src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js"
+ integrity="sha384-Tc5IQib027qvyjSMfHjOMaLkfuWVxZxUPnCJA7l2mCWNIpG9mGCD8wGNIcPD7Txa"
+ crossorigin="anonymous"
+ nonce="{{ macros.get_nonce() }}"></script>
+ <script src="{{ assets_prefix }}/static/assets/stylesheets/less/cosmo/cosmoTheme.js" nonce="{{ macros.get_nonce() }}"></script>
{% endblock %}