Skip to content

/ trellis

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

Use modern SSL config for Nginx #1127

Merged
merged 4 commits into from Jan 19, 2020
Merged

Use modern SSL config for Nginx #1127

merged 4 commits into from Jan 19, 2020
+34 −13

Conversation

@swalkinshaw
Copy link
Member

swalkinshaw commented Dec 12, 2019

Fixes #1126

Based off of h5bp's config: https://github.com/h5bp/server-configs-nginx/blob/611ed7507bc200b81867423f6061fe79b2f606e8/h5bp/ssl/policy_modern.conf

Drops TLSv1 and TLSv2.1 and updates the cipher suite.

Cipher suite compatibility details: https://cryptcheck.fr/suite/ECDHE+AES:!SHA
TLS 1.2 compatibility: https://caniuse.com/#feat=tls1-2
TLS 1.3 compatibility: https://caniuse.com/#feat=tls1-3

@austinpray
swalkinshaw added 3 commits Dec 12, 2019
@swalkinshaw
Use modern SSL config for Nginx
2801a8b 
Based off of h5bp's config: https://github.com/h5bp/server-configs-nginx/blob/611ed7507bc200b81867423f6061fe79b2f606e8/h5bp/ssl/policy_modern.conf

Drops TLSv1 and TLSv2.1 and updates the cipher suite.

Cipher suite compatibility details: https://cryptcheck.fr/suite/ECDHE+AES:!SHA
TLS 1.2 compatibility: https://caniuse.com/#feat=tls1-2
TLS 1.3 compatibility: https://caniuse.com/#feat=tls1-3
@swalkinshaw
Disable ssl_session_tickets
bd8912e
@swalkinshaw
Update stapling config
Loading status checks…
b6cae78
@swalkinshaw swalkinshaw force-pushed the update-ssl-config branch from 411f0ae to b6cae78 Dec 24, 2019
@swalkinshaw swalkinshaw mentioned this pull request Dec 24, 2019
Closed
@austinpray
austinpray requested changes Dec 24, 2019
View changes
roles/nginx/templates/h5bp/directive-only/ssl.conf Outdated Show resolved Hide resolved
roles/nginx/templates/h5bp/directive-only/ssl.conf
# Ciphers set to best allow protection from Beast, while providing forwarding secrecy, as defined by Mozilla (Intermediate Set) - https://wiki.mozilla.org/Security/Server_Side_TLS#Nginx
ssl_ciphers ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS;
ssl_prefer_server_ciphers on;
ssl_protocols TLSv1.2 TLSv1.3;

This comment has been minimized.

Sign in to view
Copy link
@austinpray

austinpray Dec 24, 2019

Member

👍
roles/nginx/templates/h5bp/directive-only/ssl.conf
ssl_ciphers ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS;
ssl_prefer_server_ciphers on;
ssl_protocols TLSv1.2 TLSv1.3;
ssl_ciphers EECDH+CHACHA20:EECDH+AES;

This comment has been minimized.

Sign in to view
Copy link
@austinpray

austinpray Dec 24, 2019

Member

I need slightly more time to research this.

I usually defer to https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29

which is

ssl_ciphers ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384;

But comparing https://cryptcheck.fr/suite/ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384

vs

https://cryptcheck.fr/suite/EECDH+CHACHA20:EECDH+AES

I don't really fully understand what I'm looking at.

https://github.com/ssllabs/research/wiki/SSL-Server-Rating-Guide#cipher-strength

This comment has been minimized.

Sign in to view
Copy link
@swalkinshaw

swalkinshaw Dec 24, 2019

Author Member

Ditto, it's confusing. Here's more detail on it h5bp/server-configs-nginx#190 (comment)

This comment has been minimized.

Sign in to view
Copy link
@austinpray

austinpray Jan 13, 2020

Member

Well it's been 20 days and I haven't made time to research this. I am using mozilla's recommended ciphers in production:

ssl_ciphers ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384;

Don't let me hold this PR up then.
roles/nginx/templates/h5bp/directive-only/ssl-stapling.conf
@@ -1,9 +1,34 @@
# OCSP stapling...
# ----------------------------------------------------------------------
# | Online Certificate Status Protocol stapling |

This comment has been minimized.

Sign in to view
Copy link
@austinpray

austinpray Dec 24, 2019

Member

LGTM
roles/nginx/templates/h5bp/directive-only/ssl.conf
@@ -25,7 +21,7 @@ ssl_session_timeout 24h;
#
# Note that you'll have to define and rotate the keys securely by yourself. In absence
# of such infrastructure, consider turning off session tickets:
#ssl_session_tickets off;
ssl_session_tickets off;

This comment has been minimized.

Sign in to view
Copy link
@austinpray

austinpray Dec 24, 2019

Member

👍 this is a good default.

mozilla/server-side-tls#135
@swalkinshaw @austinpray
Update roles/nginx/templates/h5bp/directive-only/ssl.conf
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
258c884 
Co-Authored-By: Austin Pray <austin@austinpray.com>
@swalkinshaw

This comment has been minimized.

Sign in to view
Copy link
Member Author

swalkinshaw commented Jan 19, 2020

Doing some testing with these new settings:

image
@swalkinshaw

This comment has been minimized.

Sign in to view
Copy link
Member Author

swalkinshaw commented Jan 19, 2020

image

Not sure those WEAK warnings are necessarily bad? (or avoidable)
@austinpray

This comment has been minimized.

Sign in to view
Copy link
Member

austinpray commented Jan 19, 2020

@swalkinshaw afaik the Mozilla ciphers have the same warnings. If you excluded those the browser support would be unacceptably modern.
@swalkinshaw

This comment has been minimized.

Sign in to view
Copy link
Member Author

swalkinshaw commented Jan 19, 2020

@TangRufus are you able to give this a try?
@TangRufus

This comment has been minimized.

Sign in to view
Copy link
Member

TangRufus commented Jan 19, 2020 •
edited

Tested on a remote server. Looks good to me.
However, I must admit I don't fully understand all the changes, especially ssl_ciphers and #ssl_early_data on;.

Question: Are we going to bundle the whole https://github.com/h5bp/server-configs-nginx/tree/28cb47df4227f8b8ae562e3a35e0d663a6593279/h5bp in Trellis?
@swalkinshaw

This comment has been minimized.

Sign in to view
Copy link
Member Author

swalkinshaw commented Jan 19, 2020

It would be nice to update them all but that's a much bigger task so I'd rather get the important SSL updates in first.
@swalkinshaw swalkinshaw merged commit 0e0a6a1 into master Jan 19, 2020
6 checks passed
6 checks passed
ci/circleci: syntax-check-python-2-ansible-2.7 Your tests passed on CircleCI!
Details
ci/circleci: syntax-check-python-2-ansible-2.8 Your tests passed on CircleCI!
Details
ci/circleci: syntax-check-python-2-ansible-latest Your tests passed on CircleCI!
Details
ci/circleci: syntax-check-python-3-ansible-2.7 Your tests passed on CircleCI!
Details
ci/circleci: syntax-check-python-3-ansible-2.8 Your tests passed on CircleCI!
Details
ci/circleci: syntax-check-python-3-ansible-latest Your tests passed on CircleCI!
Details
@swalkinshaw swalkinshaw deleted the update-ssl-config branch Jan 19, 2020
@austinpray

This comment has been minimized.

Sign in to view
Copy link
Member

austinpray commented Jan 20, 2020

Should we tag a minor (maybe even major) release and put out a little FYI? This should impact browser support a fair bit. Don't wanna blindside people updating.
@swalkinshaw

This comment has been minimized.

Sign in to view
Copy link
Member Author

swalkinshaw commented Jan 20, 2020

Yes I planned to. Unless you mean like asap? I'll do one tonight/tomorrow
@austinpray

This comment has been minimized.

Sign in to view
Copy link
Member

austinpray commented Jan 20, 2020

Oh no ASAP implied: just meant at some point 👍
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@austinpray austinpray

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@swalkinshaw @austinpray @TangRufus
You can’t perform that action at this time.