Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: CSP object-src and unsafe-inline #28

Merged

Conversation

dinosk
Copy link
Member

@dinosk dinosk commented Aug 16, 2018

  • ADD object-src: 'none' as suggested by the CSP evaluator tool,
    and default-src: 'unsafe-inline' to allow the use of
    Flask-DebugToolbar, the latter should be removed in production
    deployments.

Addresses inveniosoftware/cookiecutter-invenio-instance#60

Signed-off-by: Dinos Kousidis [email protected]

@lnielsen
Copy link
Member

I think by default we should have a secure configuration, and only if you enable debug should we add the unsecure configuration.

@dinosk dinosk self-assigned this Aug 16, 2018
@dinosk dinosk force-pushed the 60-enable-flask-debugtoolbar branch from f058026 to ac7395a Compare August 16, 2018 11:55
@dinosk
Copy link
Member Author

dinosk commented Aug 16, 2018

I was thinking the same thing, I tried adding the variable name to explain the reason for appending the config var but maybe it could be done in a more elegant way, which doesn't come to me right now.

* ADD object-src: 'none' as suggested by the CSP evaluator tool,
  and default-src: 'unsafe-inline' to allow the use of
  Flask-DebugToolbar when running in DEBUG mode.

Signed-off-by: Dinos Kousidis <[email protected]>
@dinosk dinosk force-pushed the 60-enable-flask-debugtoolbar branch from ac7395a to 1aa6bb9 Compare August 16, 2018 11:59
@slint
Copy link
Member

slint commented Aug 16, 2018

The only issue I see is that we'll get a lot of "works on my machine, but not in production" for other things as well... Adding inline <script> tags is pretty common in invenio-search-ui, etc to initialize angular apps. Something like adding a CSP nonce in a script tag we inject ourselves might be worth looking into:

{% if app.debug %}
<script nonce="{{ csp_nonce() }}">
  var DEBUG_TOOLBAR_STATIC_PATH = "{{ url_for('_debug_toolbar.static', filename='') }}";
</script>
{% endif %}

(this comes from https://github.com/mgood/flask-debugtoolbar/blob/24362399645f66c82914da8b4da57f8dfcf450eb/flask_debugtoolbar/templates/base.html#L2)

@dinosk
Copy link
Member Author

dinosk commented Aug 16, 2018

This is a good point, but I don't see an easy way to modify the script tag that Flask-Debugtoolbar appends to the html page. One way would be to run some code that removes it every time and adds the script tag with the nonce, but it might be getting too complex.

@slint
Copy link
Member

slint commented Aug 16, 2018

Adding the tag only would be enough (and just let the one provided by Flask-DebugToolbar to fail because of CSP). Though I think you're right it will get complex... Let's maybe extract to another issue and postpone for another (possibly UI-related) sprint.

@tiborsimko tiborsimko merged commit 1aa6bb9 into inveniosoftware:master Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants