Skip to content

Commit

Permalink
fix: Revert enabling CSP (apache#24476)
Browse files Browse the repository at this point in the history
  • Loading branch information
kgabryje authored Jun 21, 2023
1 parent d60040d commit fb08e0e
Show file tree
Hide file tree
Showing 14 changed files with 42 additions and 84 deletions.
1 change: 0 additions & 1 deletion UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/...`,
Expand Down
42 changes: 21 additions & 21 deletions docs/docs/security.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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()`.

```
<script nonce="{{ csp_nonce() }}">
/* my script */
</script>
default-src 'self' 'unsafe-eval' 'unsafe-inline'
```

* Some dashboards load images using data URIs and require `data:` in their `img-src`
Expand All @@ -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

Expand All @@ -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": { ...
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
38 changes: 4 additions & 34 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1363,42 +1363,12 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
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,
}

#
Expand Down
6 changes: 1 addition & 5 deletions superset/initialization/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -613,11 +613,7 @@ def __call__(

# 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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,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" nonce="{{ csp_nonce() }}">
<script language="javascript">
var modelActions = new AdminActions();
</script>

Expand Down
2 changes: 1 addition & 1 deletion superset/templates/appbuilder/general/widgets/search.html
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
</form>
</div>

<script nonce="{{ csp_nonce() }}">
<script>
(function($) {
function checkSearchButton() {
var hasFilter = $('.filters tr').length;
Expand Down
2 changes: 1 addition & 1 deletion superset/templates/superset/export_dashboards.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
specific language governing permissions and limitations
under the License.
#}
<script nonce="{{ csp_nonce() }}">
<script>
window.onload = function() {

// See issue #7353, window.open fails
Expand Down
2 changes: 1 addition & 1 deletion superset/templates/superset/form_view/csv_scripts.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
specific language governing permissions and limitations
under the License.
#}
<script nonce="{{ csp_nonce() }}">
<script>
$('#delimiter').on('change', function () {
var delimiterOptions = $('#delimiter').val();
if (delimiterOptions?.includes("other")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@
</div>
{% endblock %}
{% block add_tail_js %}
<script src="{{url_for('appbuilder.static',filename='js/ab_keep_tab.js')}}" nonce="{{ csp_nonce() }}"></script>
<script src="{{url_for('appbuilder.static',filename='js/ab_keep_tab.js')}}"></script>
{% endblock %}
{% block tail_js %}
{{ super() }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
specific language governing permissions and limitations
under the License.
#}
<script nonce="{{ csp_nonce() }}">
<script>
var db = $("#database");
var schema = $("#schema");

Expand Down
8 changes: 4 additions & 4 deletions superset/templates/superset/models/database/macros.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
under the License.
#}
{% macro testconn() %}
<script nonce="{{ csp_nonce() }}">
<script>
$("#sqlalchemy_uri").parent()
.append('<button id="testconn" class="btn btn-sm btn-primary">{{ _("Test Connection") }}</button>');
$("#testconn").click(function(e) {
Expand Down Expand Up @@ -72,19 +72,19 @@
{% endmacro %}

{% macro expand_extra_textarea() %}
<script nonce="{{ csp_nonce() }}">
<script>
$('#extra').attr('rows', '5');
</script>
{% endmacro %}

{% macro expand_encrypted_extra_textarea() %}
<script nonce="{{ csp_nonce() }}">
<script>
$('#encrypted_extra').attr('rows', '5');
</script>
{% endmacro %}

{% macro expand_server_cert_textarea() %}
<script nonce="{{ csp_nonce() }}">
<script>
$('#server_cert').attr('rows', '5');
</script>
{% endmacro %}
2 changes: 1 addition & 1 deletion superset/templates/superset/partials/asset_bundle.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
with development version #}
<!-- Bundle js {{ filename }} START -->
{% for entry in js_manifest(filename) %}
<script src="{{ entry }}" async nonce="{{ csp_nonce() }}"></script>
<script src="{{ entry }}" async></script>
{% endfor %}
<!-- Bundle js {{ filename }} END -->
{% endmacro %}
Expand Down
14 changes: 3 additions & 11 deletions superset/templates/superset/theme.html
Original file line number Diff line number Diff line change
Expand Up @@ -1340,15 +1340,7 @@ <h4 class="modal-title">Source Code</h4>
{% endblock %}
{% block tail_js %}
{{ super() }}
<script
src="https://code.jquery.com/jquery-1.10.2.min.js"
integrity="sha256-C6CB9UYIS9UJeqinPHWTHVqh/E1uhG5Twh+Y5qFQmYg="
crossorigin="anonymous"
nonce="{{ csp_nonce() }}"></script>
<script
src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js"
integrity="sha384-Tc5IQib027qvyjSMfHjOMaLkfuWVxZxUPnCJA7l2mCWNIpG9mGCD8wGNIcPD7Txa"
crossorigin="anonymous"
nonce="{{ csp_nonce() }}"></script>
<script src="{{ assets_prefix }}/static/assets/stylesheets/less/cosmo/cosmoTheme.js" nonce="{{ csp_nonce() }}"></script>
<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>
{% endblock %}

0 comments on commit fb08e0e

Please sign in to comment.