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

Resolve HTTPS CSRF validation failure due to Referrer-Policy on source interface #3351

Merged
merged 2 commits into from
May 3, 2018

Conversation

redshiftzero
Copy link
Contributor

Status

Ready for review

Description of Changes

Fixes #3333.

Changes proposed in this pull request:

The addition of Referrer-Policy "no-referrer" meant that the CSRF tokens on the source interface did not validate due to the use of WTF_CSRF_SSL_STRICT=True. Setting Referrer-Policy to "same-origin" resolves. Note that Tor Browser does not send Referers from onion services, so there are no negative privacy implications to this change.

Testing

  1. Generate and apply self-signed certs to the source interface running in a staging or prod application server VM.
  2. Ensure that you can successfully submit a document to the source interface.

Deployment

Some sites have applied this change already in production. Their Apache configs will not automatically update to the latest, but the next time they re-run securedrop-admin install this change will mean that they do not regress to the faulty behavior produced by the use of "no-referrer" .

Checklist

If you made changes to the system configuration:

@redshiftzero redshiftzero requested a review from emkll May 2, 2018 22:30
emkll
emkll previously approved these changes May 3, 2018
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @redshiftzero for this change.
I've tested this in a prod vm with a self-signed certificate.
With this patch, I can confirm the submission now properly goes through and I can fully interact with the source interface over https.
I also agree that this change should not affect the privacy properties of the application.
👍 once rebased and passing CI

The addition of Referrer-Policy "no-referrer" meant that the CSRF
tokens on the source interface did not validate due to the use of
WTF_CSRF_SSL_STRICT=True [0]. Setting Referrer-Policy to "same-origin"
resolves [1]. Note that Tor Browser does not send Referers from onion
services [2], so there are no negative privacy implications to this
change.

[0] https://flask-wtf.readthedocs.io/en/stable/config.html
[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy
[2] https://trac.torproject.org/projects/tor/ticket/9623
@redshiftzero redshiftzero force-pushed the resolve-https-referer branch from 851b5d4 to 4fc204a Compare May 3, 2018 17:29
@emkll emkll self-requested a review May 3, 2018 18:15
@codecov-io
Copy link

Codecov Report

Merging #3351 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3351   +/-   ##
========================================
  Coverage    85.81%   85.81%           
========================================
  Files           34       34           
  Lines         2157     2157           
  Branches       238      238           
========================================
  Hits          1851     1851           
  Misses         250      250           
  Partials        56       56

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42c3154...4fc204a. Read the comment docs.

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes in the diff, approving based on my previous comment.

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.

CSRFError on source interface with HTTPS
3 participants