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

Replace Cross-Origin-Resource-Policy: 'same-site' with 'same-origin' #6768

Merged

Conversation

evilaliv3
Copy link
Contributor

@evilaliv3 evilaliv3 commented Mar 13, 2023

The configuration is recommended by the official Mozilla documentation that has a warning that inform that same-site is considered less secure than an origin. https://developer.mozilla.org/en-US/docs/Web/HTTP/Cross-Origin_Resource_Policy

Screenshot 2023-03-14 at 00-03-15 Cross-Origin Resource Policy (CORP) - HTTP MDN


Test plan:

  • CI is happy, and:

Clean install

  • Check out this branch, make build-debs, make staging.
  • Installation completes successfully, and /etc/apache2/sites-available/{source,journalist}.conf are provisioned as in this PR

Upgrade testing

  • Provision staging servers (VMs OK) against tip of develop
  • Inspect /etc/apache2/sites-available/{source,journalist}.conf and notice a Cross-Origin Resource Policy of "same-site"
  • Check out this branch, and run make build-debs. Copy the securedrop-app-code package you built to the app server (scp $PATH_TO_YOUR_SECUREDROP_ROOT_DIR/build/focal/securedrop-app-code_2.6.0~rc1+focal_amd64.deb sdadmin@<app.server.ip.address>:).
  • Install the .deb using dpkg, and inspect the apache2 config files again. Observe a Cross-Origin Resoure Policy of "same-origin"

@evilaliv3 evilaliv3 requested a review from a team as a code owner March 13, 2023 23:04
@rocodes
Copy link
Contributor

rocodes commented Mar 16, 2023

Hi @evilaliv3, thank you for this PR and for all of your contributions to SecureDrop <3 Changes look good (CI failure known/unrelated), we just want to make sure this doesn't affect any of the Onion Name aliasing before we proceed.

The configuration is recommended by the official Mozilla documentation that has a warning that
inform that same-site is considered less secure than an origin.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Cross-Origin_Resource_Policy
@zenmonkeykstop
Copy link
Contributor

Thanks for this @evilaliv3, rebasing to pull in some CI tests. It might need a postint change to update servers which already have the same-site config but we can sort that out.

@rocodes
Copy link
Contributor

rocodes commented May 29, 2023

Hey @evilaliv3, I just added a line in the securedrop-app-code postinst to handle upgrade cases, and I'm going to update the test plan in the issue description. Hope that's alright :)

@zenmonkeykstop zenmonkeykstop requested a review from cfm May 31, 2023 16:08
Copy link
Member

@cfm cfm left a comment

Choose a reason for hiding this comment

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

Upgrade scenario looks good! Your merge, @rocodes.

  • Provision staging servers (VMs OK) against tip of develop

  • Inspect /etc/apache2/sites-available/{source,journalist}.conf and notice a Cross-Origin-Resource-Policy of "same-site"

    vagrant@app-staging:~$ grep "Cross-Origin-Resource-Policy" /etc/apache2/sites-available/*.conf 
    /etc/apache2/sites-available/journalist.conf:Header onsuccess unset Cross-Origin-Resource-Policy
    /etc/apache2/sites-available/journalist.conf:Header always set Cross-Origin-Resource-Policy "same-site"
    /etc/apache2/sites-available/source.conf:Header onsuccess unset Cross-Origin-Resource-Policy
    /etc/apache2/sites-available/source.conf:Header always set Cross-Origin-Resource-Policy "same-site"
  • Check out this branch, and run make build-debs. Copy the securedrop-app-code package you built to the app server (scp $PATH_TO_YOUR_SECUREDROP_ROOT_DIR/build/focal/securedrop-app-code_2.6.0~rc1+focal_amd64.deb sdadmin@<app.server.ip.address>:).

  • Install the .deb using dpkg, and inspect the apache2 config files again. Observe a Cross-Origin-Resource-Policy of "same-origin"

    vagrant@app-staging:~$ grep "Cross-Origin-Resource-Policy" /etc/apache2/sites-available/*.conf 
    /etc/apache2/sites-available/journalist.conf:Header onsuccess unset Cross-Origin-Resource-Policy
    /etc/apache2/sites-available/journalist.conf:Header always set Cross-Origin-Resource-Policy "same-origin"
    /etc/apache2/sites-available/source.conf:Header onsuccess unset Cross-Origin-Resource-Policy
    /etc/apache2/sites-available/source.conf:Header always set Cross-Origin-Resource-Policy "same-origin"

@rocodes
Copy link
Contributor

rocodes commented Jun 1, 2023

Thanks @cfm. Before merge, can you confirm your apache2 service on the app server comes back up without error? Having a problems on my app server staging VM today but may be unrelated.

@cfm
Copy link
Member

cfm commented Jun 5, 2023

I've retested the upgrade scenario in a fresh staging environment and see no problems:

vagrant@app-staging:~$ sudo cat /var/lib/tor/services/sourcev3/hostname
4uni4nncsyiebfgib3lvjjutxsr4ylis42ba7hss6mtdpfhxpispcvyd.onion
vagrant@app-staging:~$ torify curl -Is 4uni4nncsyiebfgib3lvjjutxsr4ylis42ba7hss6mtdpfhxpispcvyd.onion | grep "Cross-Origin-Resource-Policy"
Cross-Origin-Resource-Policy: same-site
vagrant@app-staging:~$ sudo apt install ./securedrop-app-code_2.6.0~rc1+focal_amd64.deb
vagrant@app-staging:~$ torify curl -Is 4uni4nncsyiebfgib3lvjjutxsr4ylis42ba7hss6mtdpfhxpispcvyd.onion | grep "Cross-Origin-Resource-Policy"
Cross-Origin-Resource-Policy: same-origin

@rocodes rocodes merged commit 52212b0 into freedomofpress:develop Jun 5, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants