search

quattor / configuration-modules-core

Node Configuration Manager Components for Everyone
www.quattor.org
Other
7 stars 56 forks source link

ncm-metaconfig: nginx default ssl settings outdated #1717

Open wdpypere opened 2 months ago

wdpypere commented 2 months ago

Looking at the nginx schema I see following defaults:

    "ciphersuite" : cipherstring[] = list("TLSv1")
    "protocol" : sslprotocol[] = list("TLSv1", "TLSv1.1", "TLSv1.2", "TLSv1.3")

This is quite weak, and outdated. Do we regard it as our responsibility to provide secure defaults? Otherwise I would change these to:

    "ciphersuite" : cipherstring[] = list("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")
    "protocol" : sslprotocol[] = list("TLSv1.2", "TLSv1.3")

which would work on el8 (nginx 1.17) and beyond. Maybe lower as well but I don't have a el7 machine to test.

ned21 commented 2 months ago

+1 to not shipping weak defaults. I think there's a couple of options:

  1. Remove the default value, let users experience a compile error and choose their own value.
  2. Adjust the default value as you suggest, at the risk of breaking EL7 and storing up problems for future maintainers.

Both options are backwards incompatible, the first leads to a template compile error with trivial fix, the second will likely work seamlessly for people but carries the risk of breaking a deployed app somewhere.

jrha commented 2 months ago

My vote would be for :two: with an appropriate call-out in the release notes.