-
-
Notifications
You must be signed in to change notification settings - Fork 881
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
Good SSL configuration by default #1556
Comments
While I agree the default configuration is not great, defaulting to the upstream default configuration seems a common and surprise-less practice (what this module currently does not seems to do according to your message 😒). When using a control-repo and following the role and profile pattern, you have a AFAIAC, I would prefer to see PR to revert settings so that they match the defaults rather than PR that change random bits to provide a better default config. Puppet modules really are building blocks and a commodity for you to use and tune to fit your needs, and I can imagine Enterprise™ Productsⓡ that only support the most broken ciphers with SSLv3 (because you know, 3 is better that 1.3 🤡), for which the default config is working, and that start to fail in all possible ways if the module ship a secure config by default. |
This is not something the average organization would want to tweak. Speaking of experience (we do handle TLS configuration for different organizations), the majority of organizations don't have any opinions or special needs wrg of TLS tweaking. For the big majority of organizations out there the "Mozilla intermediate"-recommendations would be better than the default delivered by nginx. Then there are a small minority of organizations that have special needs on the client side and therefore may need support for a specific list of ciphers, support for ciphers that are considered to be deprecated or even insecure, or perhaps some setup where "ultimate security" is more important than "compatibility with yesterdays browser versions". I absolutely think it makes sense to offer "best recommended practice" as part of the module. Of course, arguably this should be fixed in nginx - but at the other hand, a simple "apt-get upgrade" should not break backward compatibility. I think the puppet module has a bit better flexibility. Maybe we could have the cake and eat it too? Put a simple boolean (or enum) flag there to adjust SSL configuration, if turned off the nginx package defaults will be used, and if turned on, the puppet module will do an attempt to set things up according to the best current practice. I will not argue weather the default ought to be off or on. (It will add a maintenance burden on the module, though). |
Given how the module is organized, a boolean would be tricky to implement, and given the complexity of some templates I would really avoid this. Also, these settings should be kept up-to-date to add / remove ciphers when needed which would require major version bumps. Not ideal. Maybe a lower hanging fruit would be providing examples for the mozilla-modern, mozilla-intermediate and mozilla-old configuration that user can use as starters for building their own Or maybe contributing a generator for the Puppet fragment to configure puppet-nginx through https://github.com/mozilla/ssl-config-generator would make sense? Maybe they would be interested in this (I have no idea)? |
This is a continuation of #859, but since the issue is closed I raise a new one. I think it would be nice if the TLS setup came as secure according to the current best practice by default when using the module. Looking over the default values and updating them is something that has to be done regularly. Currently a site set up with voxpupuli-nginx without any tweaking will get the grade capped to B in the Qualys SSL Labs online analyzer, that's too bad in my opinion.
Personally I'm not much of an expert on those things, but I do like to have a secure setup, and mozilla has a configuration generator that can be used as a point of reference.
Here is from the config generator:
And those are the defaults from the module:
It's quite obvious that the ssl_protocols ought to be adjusted.
As for the ciphers, if I get it right, a lot of those ciphers are considered "obscure". Even if there are no known weaknesses, they may be less reviewed than the more common ciphers. I don't have any strong opinions on this, I just ran a script that reported that some of those ciphers ought to be removed ... but I suppose it's safest to default on the mozilla recommendations.
Other differences observed ... recommended from mozilla:
Defaults set to
5m
andshared:SSL:10m
by the module. I have no idea about those things to be honest.This is also defined in the recommendation, but not defined by the module:
The latter requires a module and if added it may possibly break backward compatibility for some few users.
OCSP stapling is recommended, but set to off as the default in the module configuration. Are there any drawbacks in OCSP stapling?
Path for the trusted cert store and IP-address for resolver is also hardcoded in the recommendation from Mozilla. I suppose trusting the OS defaults is good enough.
Should I look more into this and raise a pull request? (I do have experts both on nginx and on TLS available on my team, so I can get a proper review of the changes from them - but I do not like putting efforts into a pull request without first getting a hint that it probably will be accepted)
The text was updated successfully, but these errors were encountered: