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

Ssl optimizations and advanced configuration #69

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

bartoszbetka
Copy link
Contributor

Resolves #56

@bartoszbetka bartoszbetka force-pushed the ssl-optimizations-and-advenced-configuration branch 4 times, most recently from 22fbabe to 305ed52 Compare March 6, 2018 17:04
@cameel cameel changed the title Ssl optimizations and advenced configuration Ssl optimizations and advanced configuration Mar 7, 2018
@bartoszbetka bartoszbetka force-pushed the ssl-optimizations-and-advenced-configuration branch from 305ed52 to 69196a3 Compare March 8, 2018 15:44
@cameel
Copy link
Contributor

cameel commented Mar 8, 2018

The two ssl.conf files are almost identifcal. Extract repeating stuff into a single place.

@bartoszbetka bartoszbetka force-pushed the ssl-optimizations-and-advenced-configuration branch from 69196a3 to e07bff7 Compare March 9, 2018 16:00
@bartoszbetka bartoszbetka force-pushed the ssl-optimizations-and-advenced-configuration branch from a3c7b93 to 126f212 Compare April 26, 2018 15:16
kubernetes/config-maps/nginx-proxy/default.conf.j2 Outdated Show resolved Hide resolved
kubernetes/config-maps/nginx/ssl.conf Outdated Show resolved Hide resolved
kubernetes/config-maps/nginx/ssl.conf Outdated Show resolved Hide resolved
ssl_stapling_verify on;

# Enable HSTS
add_header Strict-Transport-Security "max-age=63072000; includeSubdomains";
Copy link
Contributor

Choose a reason for hiding this comment

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

This max-age has a significant gotcha. Once we switch dev or any other cluster to HTTPS, switching back to HTTP will require clearing client's cache.

Please at least add some explanations:

# HSTS tells the client to only access the site over HTTPS. This is mostly relevant to browsers
# but other types clients may implement it too as long as they have some persistent storage.
# See https://www.owasp.org/index.php/HTTP_Strict_Transport_Security_Cheat_Sheet
# Note that max-age here is 63072000 seconds (2 years) which means that once you start serving
# traffic over HTTPS, you won't be able to switch back to HTTP without clearing client cache.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may cause a problem when traffic with HTTP and HTTPS are allowed.

kubernetes/config-maps/nginx/ssl.conf Outdated Show resolved Hide resolved
kubernetes/config-maps/nginx/ssl.conf Outdated Show resolved Hide resolved
kubernetes/config-maps/nginx/ssl.conf Outdated Show resolved Hide resolved

# Enable server-side protection
ssl_prefer_server_ciphers on;
ssl_ciphers "EECDH+AESGCM:EDH+AESGCM:AES256+EECDH:AES256+EDH";
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add some comments explaining why you chose those specific ciphers. Best if you can add a link to some authoritative recommendation.

Copy link
Contributor

Choose a reason for hiding this comment

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

This generator from Mozilla can give you some good values: https://mozilla.github.io/server-side-tls/ssl-config-generator/

Copy link
Contributor

@cameel cameel Jun 4, 2018

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use value from https://cipherli.st/

ssl_certificate_key /etc/ssl/secrets/nginx-proxy-ssl.key;

# Add Diffie-Helman group
ssl_dhparam /etc/ssl/secrets/nginx-proxy-dhparam.pem;
Copy link
Contributor

Choose a reason for hiding this comment

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

How big is this file and how long does it take to generate it?

It would be best to generate it during container build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a small file which is generate fast. This solution requires additional script and causes generate new .pem file for new deployment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure we want to use it ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why want to it? Ok, so I will do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked, how much time need to generate this file and it's not deterministic and random on each execution. For generate 2048bit key, it's need sometimes 10s and sometimes 40s. The openssl inform that Generating DH parameters, 2048 bit long safe prime, generator 2 This is going to take a long time. We still want this solution ?

kubectl create secret generic nginx-storage-secrets \
--from-file=nginx-storage-ssl.crt=concent-secrets/nginx-storage-ssl.crt \
--from-file=nginx-storage-ssl.key=concent-secrets/nginx-storage-ssl.key \
--from-file=nginx-storage-dhparam.pem=concent-secrets/nginx-storage-dhparam.pem
Copy link
Contributor

Choose a reason for hiding this comment

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

Is nginx-storage-dhparam.pem really a secret?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kubernetes/config-maps/nginx/ssl.conf Outdated Show resolved Hide resolved
kubernetes/config-maps/nginx/ssl.conf Outdated Show resolved Hide resolved
kubernetes/config-maps/nginx/ssl.conf Show resolved Hide resolved
kubernetes/config-maps/nginx/ssl.conf Show resolved Hide resolved
ssl_stapling_verify on;

# Enable HSTS
add_header Strict-Transport-Security "max-age=63072000; includeSubdomains";
Copy link
Contributor

Choose a reason for hiding this comment

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

This form does not follow the RFC. Here's an alternative:
https://gist.github.com/plentz/6737338#gistcomment-1299170

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you read my comment in issue about preload option ?

kubernetes/config-maps/nginx/ssl.conf Outdated Show resolved Hide resolved
@bartoszbetka bartoszbetka force-pushed the ssl-optimizations-and-advenced-configuration branch from 126f212 to f5c0ff6 Compare September 25, 2018 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants