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

CSP compliance #182

Closed
martin-lueders opened this issue Mar 15, 2022 · 4 comments
Closed

CSP compliance #182

martin-lueders opened this issue Mar 15, 2022 · 4 comments

Comments

@martin-lueders
Copy link

While trying to tighten the security measures of a web page, I am working on, using the django CSP middleware, I noticed issues with django_comments:

In order to 'hide' the honeypot fields, the templates use inline styles, e.g. form.html:

{% load comments i18n %}
<form action="{% comment_form_target %}" method="post">{% csrf_token %}
  {% if next %}
    <div><input type="hidden" name="next" value="{{ next }}"/></div>{% endif %}
  {% for field in form %}
    {% if field.is_hidden %}
      <div>{{ field }}</div>
    {% else %}
      {% if field.errors %}{{ field.errors }}{% endif %}
      <p
              {% if field.errors %} class="error"{% endif %}
              {% if field.name == "honeypot" %} style="display:none;"{% endif %}>
        {{ field.label_tag }} {{ field }}
      </p>
    {% endif %}
  {% endfor %}
  <p class="submit">
    <input type="submit" name="post" class="submit-post" value="{% trans "Post" %}"/>
    <input type="submit" name="preview" class="submit-preview" value="{% trans "Preview" %}"/>
  </p>
</form>

These inline styles cause the messages:

Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-0EZqoz+oBhx7gF4nvY2bSqoGyy4zLjNF+SDQXGp/ZrY='), or a nonce ('nonce-...') is required to enable inline execution.

As the suggested measures (unsafe-inline, hash or nonce) are discouraged, the best solution would be to use a css style-sheet, which defines a class (e.g. comments-no-display), which then can be set in the template. It only would require to load that css sheet in the base template, but would allow for tighter CSP settings.

Would it be possible to include this in the next release?

@claudep
Copy link
Member

claudep commented Mar 15, 2022

What about replacing the style="display:none;" by the hidden attribute? It seems to be widely implemented now (https://caniuse.com/?search=hidden).

@martin-lueders
Copy link
Author

@claudep Yes, I guess that also would possible and does not required the extra style-sheet. Indeed it seems to be widely implemented. I just tried this in my project, and it works for me, but I have not tested on a wide variety of browsers.

@claudep
Copy link
Member

claudep commented Mar 15, 2022

Would you like to suggest a patch with this change?

@martin-lueders
Copy link
Author

Good idea. Attached are the two patch files
patches.tar.gz
.

claudep added a commit that referenced this issue Mar 22, 2022
This change improves CSP compliancy.
Thanks Martin Lueders for the report and patch.
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

No branches or pull requests

2 participants