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

Disallow new onion names with subdomains; remove tv2.dk and iqss.harvard #221

Merged
merged 3 commits into from
Jan 23, 2025

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Jan 23, 2025

Status

Ready for review

Description

Review Checklist

  • Changes to onboarded.txt are accurate
  • The file default.rulesets.TIMESTAMP.gz has been updated, extracting that file and inspecting the contents of the JSON file produces the expected rules
  • The ruleset has been verified by modifying the HTTPS Everywhere configuration in a Tor Browser instance pointing to Path Prefix: https://raw.githubusercontent.com/freedomofpress/securedrop-https-everywhere-ruleset/$BRANCH_NAME
  • index.html has been updated using ./update_index.sh

Post-Deployment Checklist

  • Added/modified onion names have been updated in the SecureDrop Directory

Using an extra subdomain causes issues in Tor Browser, because the
first-party domain is incorrectly determined.

Tor has asked us (in #219) to stop adding new domains, which is simple
enough. A new check verifies there's no period in the part before
".securedrop.tor.onion". If there is and it's not in the exemption list,
error out.

Refs #219.
Not currently listed in our directory.

Fixes #220.
Copy link
Collaborator

@nathandyer nathandyer left a comment

Choose a reason for hiding this comment

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

LGTM; left one comment about a nit, which may be intentional.

Approving but waiting to merge until I confirm with @legoktm

"webapps.aljazeera.net",
"www.apache.be",
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two new exceptions are the only ones prefixed with www - is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just a coincidence, these entries have to match how they're listed in onboarded.txt: https://github.com/freedomofpress/securedrop-https-everywhere-ruleset/blob/470ac857ae1c320794f46e3be0bede206e92e10e/onboarded.txt

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, makes sense - thanks for confirming!

@nathandyer nathandyer added this pull request to the merge queue Jan 23, 2025
@nathandyer
Copy link
Collaborator

No changes necessary in SD directory

Merged via the queue into main with commit 83a8831 Jan 23, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Remove TV2 and IQSS from the ruleset
2 participants