-
Notifications
You must be signed in to change notification settings - Fork 8
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
Set Onion-Location header #693
Comments
Do we have the address of the onion service? Is that something that's okay to post in this issue? |
Currently it's Assuming you wire up a settings-based config where |
Thanks for that information, Conor. I can definitely wire up the settings as you describe. Though, I wonder, now, if the best way to take an env var and serve it out as a header is at the nginx level, rather than at the Django level? Though there might be other issues involved in that I'm not aware of. |
I admit my kneejerk reaction is that HTTP header manipulation is best handled via nginx, rather than at the app-code layer. For the CSP, though, we're injecting headers via app code settings, and that makes lots of sense. One nice feature of preferring app code for managing the logic is that via containerization, no infra involvement would be required to update the options, web team could own it fully. Perhaps that's a bit of a moot point, given that the underlying keypair for the Onion service is still managed in infra secrets somewhere... @harrislapiroff What do you think? My preference is always to enable web team to manage as much as possible directly, but if for pragmatic reasons y'all would prefer that infra manage the Onion logic, happy to accommodate. |
One thing not mentioned yet: Is there any reason not to make it something that can be customized from the Wagtail admin? Are we concerned about bad actors getting db or admin access more easily than env access? (Though we do also host GPG public keys and our crypto wallet addresses in the database—and I believe infra has integrity checks for the latter—which seems reasonably analagous.) I do think I'd prefer to see this as django middleware rather than nginx logic for the portability reasons @conorsch mentioned. |
Nice, I really like the suggestion of supporting interactions via Wagtail admin, so that even more folks than infra & web can manage the header. No concerns there security wise, if folks have access to the admin interface on e.g. SDO, they already have the ability to do a lot of damage by updating directory entries etc. |
Not seeing the "onion available" notification in Tor Browser 9.5, even after this change was deployed: The header is being served:
but the value doesn't quite match the spec (emphasis added):
So what we want to see is something like:
Given that there are at least two places where the Onion value is used, i.e. Onion-Location header & in the footer, maybe some additional formatting is warranted. But as currently implemented, we don't have the functionality we want to consider the issue resolved. |
Sorry, should have mentioned in chat that we can add the protocol in the admin under footer settings! Any reason we wouldn't want the protocol in both places? |
Well I'll be: that was simple enough. After making the manual edit in the admin panel, both the footer shows the https prefix (not strictly required, but not a problem), and more importantly the header is indeed serving well. Thank you for clarifying, @harrislapiroff! |
securedrop.org has a corresponding onion service (v2 for now). We should advertise it via
Onion-Location
, which Tor Browser 9.5 will interpret to show an ".onion available" button.https://community.torproject.org/onion-services/advanced/onion-location/
Example screenshot:
The text was updated successfully, but these errors were encountered: