-
Notifications
You must be signed in to change notification settings - Fork 687
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
Add support for support TLSv1.3 ciphersuites as for #4769 #5988
Add support for support TLSv1.3 ciphersuites as for #4769 #5988
Conversation
Thank you for this PR. I think we should also work/discuss to figure out how to enable this on the systems already provisioned on Focal. |
While looking at the options to update the existing instances via Maybe someone running a very old version of Tails will not be able to access it. But, rest should be. Again to access the v3 addresses, people must be running a newer version of Tails. So, that takes care of old version of Tails/Tor problem. |
Here is a table showing browser support https://docs.w3cub.com/browser_support_tables/tls1-3, we should be okay with all the working Tor Browsers. |
Agreed, the time for 1.3 has come. The change would land here: securedrop/install_files/ansible-base/roles/app/templates/sites-available/focal/source.conf Lines 19 to 21 in 1a2494d
|
May be we can just do this:
|
Thank you @conorsch @kushaldas : You can find already a commit that applies your full change proposal. |
(quick rebase to include some CI fixes for tests to pass) |
Thank you @evilaliv3 . This means we should also do:
|
Thank you @kushaldas, i agree on this. feel free to proceed to integrate the pull request with those aspect as you probably know already how to do it. thank you. |
While working on the patchset:
|
This works:
|
Thank you @kushaldas. This is actually pretty strange, let me see if apache uses alternatives names for the same ciphers... |
@kushaldas: i think the proper apache2 naming could actually be the following:
|
Yes, typo in my |
I'll take this for a spin in VMs today and report back with a review! |
We now only provide TLSv1.3 on the source interface if there is TLS certificate is enabled.
coverage, installed 5.3, affected <6.0b1, id 41002 It is not released yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested in prod VMs with self-signed certs. testssl.sh
output is good, and functionality in Tor Browser is A-OK. Migration via postinst modified the apache configs as expected.
I rebased this as part of review, so CI will run one more time. Fine to merge after! |
See reference ticket: #4769 where this patch was previously discussed
Changes made in this PR
TLSv1.2
and onlyTLSv1.3
is allowedSSLSessionTickets
is nowoff
( Read this post for details)TLSv1.3
we don't have to mention cipher orderHow to test?
self signed certificate
make build-debs
in this branchsecuredrop-app-code
debian package and install on the app server.TLSv1.3
is allowed.Output from testssl script