From fb08e0ecfc81cba37e26620a7b6d88fbd5658cb8 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Wed, 21 Jun 2023 19:19:15 +0200 Subject: [PATCH] fix: Revert enabling CSP (#24476) --- UPDATING.md | 1 - docs/docs/security.mdx | 42 +++++++++---------- .../src/models/ExtensibleFunction.ts | 3 +- superset/config.py | 38 ++--------------- superset/initialization/__init__.py | 6 +-- .../appbuilder/general/widgets/base_list.html | 2 +- .../appbuilder/general/widgets/search.html | 2 +- .../templates/superset/export_dashboards.html | 2 +- .../superset/form_view/csv_scripts.html | 2 +- .../form_view/csv_to_database_view/edit.html | 2 +- .../form_view/database_schemas_selector.html | 2 +- .../superset/models/database/macros.html | 8 ++-- .../superset/partials/asset_bundle.html | 2 +- superset/templates/superset/theme.html | 14 ++----- 14 files changed, 42 insertions(+), 84 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 432af7af4d..41a120f310 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -34,7 +34,6 @@ 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 c2b3c72eae..56e058e581 100644 --- a/docs/docs/security.mdx +++ b/docs/docs/security.mdx @@ -181,7 +181,7 @@ 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 `True`; set this to `False` in order to disable 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. @@ -192,20 +192,10 @@ this warning using the `CONTENT_SECURITY_POLICY_WARNING` key in `config.py`. #### CSP Requirements -* Superset needs the `style-src unsafe-inline` CSP directive in order to operate. +* Superset needs both the `'unsafe-eval'` and `'unsafe-inline'` CSP keywords in order to operate. ``` - 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()`. - - ``` - + default-src 'self' 'unsafe-eval' 'unsafe-inline' ``` * Some dashboards load images using data URIs and require `data:` in their `img-src` @@ -221,11 +211,21 @@ You can get current nonce value by calling jinja macro `csp_nonce()`. connect-src 'self' https://api.mapbox.com https://events.mapbox.com ``` -* 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/) +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'`. +```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 @@ -234,15 +234,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 option of `force_https = True` (`False` by default) may break Superset's Alerts & Reports +In particular, the default option of `force_https = True` 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 keep this -option disabled. Otherwise, you may want to enable `force_https` like this: +through a loader balancer or application gateway, it should be acceptable to set this +option to `False`, like this: ```python TALISMAN_CONFIG = { - "force_https": True, + "force_https": False, "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 5a247d751e..78901dd565 100644 --- a/superset-frontend/packages/superset-ui-core/src/models/ExtensibleFunction.ts +++ b/superset-frontend/packages/superset-ui-core/src/models/ExtensibleFunction.ts @@ -22,8 +22,9 @@ */ 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 3334a55948..d62003991a 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1363,42 +1363,12 @@ TEST_DATABASE_CONNECTION_TIMEOUT = timedelta(seconds=30) CONTENT_SECURITY_POLICY_WARNING = True # Do you want Talisman enabled? -TALISMAN_ENABLED = True +TALISMAN_ENABLED = False # If you want Talisman, how do you want it configured?? TALISMAN_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'", "'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, + "content_security_policy": None, + "force_https": True, + "force_https_permanent": False, } # diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 93890371be..3d7d9817f7 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -613,11 +613,7 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods # Talisman talisman_enabled = self.config["TALISMAN_ENABLED"] - talisman_config = ( - self.config["TALISMAN_DEV_CONFIG"] - if self.superset_app.debug - else self.config["TALISMAN_CONFIG"] - ) + talisman_config = 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 e4ea2ed9ae..3b376d243b 100644 --- a/superset/templates/appbuilder/general/widgets/base_list.html +++ b/superset/templates/appbuilder/general/widgets/base_list.html @@ -56,7 +56,7 @@ {{ lib.render_pagination(page, page_size, count, modelview_name) }} {{ lib.render_set_page_size(page, page_size, count, modelview_name) }} - diff --git a/superset/templates/appbuilder/general/widgets/search.html b/superset/templates/appbuilder/general/widgets/search.html index 04d58d6d98..22c6b629c1 100644 --- a/superset/templates/appbuilder/general/widgets/search.html +++ b/superset/templates/appbuilder/general/widgets/search.html @@ -44,7 +44,7 @@ - + {% 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 46ca3eb874..ac827c1330 100644 --- a/superset/templates/superset/form_view/database_schemas_selector.html +++ b/superset/templates/superset/form_view/database_schemas_selector.html @@ -16,7 +16,7 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. #} - {% endmacro %} {% macro expand_encrypted_extra_textarea() %} - {% endmacro %} {% macro expand_server_cert_textarea() %} - {% endmacro %} diff --git a/superset/templates/superset/partials/asset_bundle.html b/superset/templates/superset/partials/asset_bundle.html index 60bc9aa2f8..066c9f64e6 100644 --- a/superset/templates/superset/partials/asset_bundle.html +++ b/superset/templates/superset/partials/asset_bundle.html @@ -21,7 +21,7 @@ with development version #} {% for entry in js_manifest(filename) %} - + {% endfor %} {% endmacro %} diff --git a/superset/templates/superset/theme.html b/superset/templates/superset/theme.html index 7fb29fc968..856796a4c4 100644 --- a/superset/templates/superset/theme.html +++ b/superset/templates/superset/theme.html @@ -1340,15 +1340,7 @@ {% endblock %} {% block tail_js %} {{ super() }} - - - + + + {% endblock %}