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

Good SSL configuration by default #1556

Open
tobixen opened this issue Jun 15, 2023 · 3 comments
Open

Good SSL configuration by default #1556

tobixen opened this issue Jun 15, 2023 · 3 comments

Comments

@tobixen
Copy link
Contributor

tobixen commented Jun 15, 2023

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:

    # intermediate configuration
    ssl_protocols TLSv1.2 TLSv1.3;
    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:DHE-RSA-CHACHA20-POLY1305;
    ssl_prefer_server_ciphers off;

And those are the defaults from the module:

  ssl_protocols             TLSv1 TLSv1.1 TLSv1.2;
  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;

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:

ssl_session_timeout 1d;
ssl_session_cache shared:MozSSL:10m;  # about 40000 sessions

Defaults set to 5m and shared: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:

ssl_session_tickets off;

# curl https://ssl-config.mozilla.org/ffdhe2048.txt > /path/to/dhparam
ssl_dhparam /path/to/dhparam;

# HSTS (ngx_http_headers_module is required) (63072000 seconds)
add_header Strict-Transport-Security "max-age=63072000" always;

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)

@smortex
Copy link
Member

smortex commented Jun 17, 2023

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 profile::nginx class that wraps nginx with the setting suitable for your organization. I would rather recommend this if it is not what you are currently doing.

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.

@tobixen
Copy link
Contributor Author

tobixen commented Jul 20, 2023

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).

@smortex
Copy link
Member

smortex commented Jul 20, 2023

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 profile::nginx? We already have a few examples in the examples directory but none of them concern the base configuration explicitly, only trivial examples which include nginx and declare a few simple vhosts.

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)?

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

No branches or pull requests

2 participants