Cockroach: security: Option to configure TLS version and ciphers

[bdarnell]

CockroachDB 20.1 is the first version to use TLS 1.3 by default. This turns out to be a compatibility problem with some versions of Java, including the current LTS release (https://github.com/cockroachdb/docs/issues/7258), and our compatibility testing of TLS 1.3 isn't great (https://github.com/cockroachdb/cockroach/issues/48293) so there may be other headaches lurking.

The java incompatibility is a bug on the java side; patch releases are available but weren't released until April 2020 so they're not ubiquitous yet. There is also a workaround by setting the environment variable GODEBUG=tls13=0; support for this env var has been removed in Go 1.14 (and therefore it will not be supported in CRDB 20.2).

Given the compatibility impact with a major platform, should we consider giving users the option to disable TLS 1.3 (in a form other than an env var, which might be easier to manage in some environments}? Should we consider disabling it by default in 20.1 since it is evidently still going through interoperability headaches in April 2020? Note that if we make any changes to CRDB here, they would presumably appear in the patch release 20.1.1.

[blathers-crl[bot]]

Hi @bdarnell, I've guessed the C-ategory of your issue and suitably labeled it. Please re-label if inaccurate.

While you're here, please consider adding an A- label to help keep our repository tidy.

_{:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[aaron-crl]

Allowing an operator to specify the TLS level that they want would be my preference with a default that balances security and usability. Postgres defaults to using the system openssl.cnf but also allows this to be overridden with a config file and ssl_ciphers in the postgresql.conf file. We might consider exposing similar options?

For the default level: I think that TLS 1.2 adequately meets the balance for security and ubiquity.

As for config options:

• We should enable users that desire perfect forward secrecy and the speed improvements of TLS 1.3 to have access to them, especially since we can already support that.
• We might want to allow election of cipher suites for the particularly security conscious operators.

[rafiss]

• As long as it is not too confusing for the default level to go down after it has already gone up in 20.1.0, I think it would be preferable to make 20.1.1 default to TLS 1.2. It sounds worth it to prevent the headaches for most Java users.
• I agree with adding a config option that can be used in all releases going forward. It looks like the TLS level is set in code with Config.MaxVersion.

[bdarnell]

If we're OK with disabling 1.3 by default, do we even need an option to turn it back on? It would simplify things if we just leave it off for this release and unconditionally turn it on in 20.2 (relying on the patched JDK versions to propagate in the meantime). Options tend to invite more complexity (if there's a max version setting, is there a min version? We don't want people to leave TLS 1.3 disabled for longer than needed for interoperability. At some point in the (hopefully distant) future TLS 1.2 will be deprecated and we'll have to start overriding this setting to force-enable TLS 1.3, etc)

We should enable users that desire perfect forward secrecy and the speed improvements of TLS 1.3 to have access to them, especially since we can already support that.

PFS is available in 1.2 as long as you use an (EC)DHE cipher suite (which is the default). As I understand it, enabling 1.3 is only a real advantage for PFS if you also disable 1.2. And the speed gains are mainly in reduced round trips for the handshake, which matters a lot less for us than in the web context.

We might want to allow election of cipher suites for the particularly security conscious operators.

I'm not a fan of that because cipher specifications tend to be copied and pasted from outdated Apache HOWTOs and often move things backwards instead of forwards. Case in point: we disabled 3DES cipher suites in #17237, which required us to copy and modify the then-current default list of ciphers. We didn't update this list when TLS 1.3 was introduced, so it only contains TLS 1.2 ciphers. However, the Go TLS implementation does not support configuring cipher suites in TLS 1.3, so our now-outdated configuration was being ignored anyway.

[aaron-crl]

If we're OK with disabling 1.3 by default, do we even need an option to turn it back on? It would simplify things if we just leave it off for this release and unconditionally turn it on in 20.2 (relying on the patched JDK versions to propagate in the meantime). Options tend to invite more complexity (if there's a max version setting, is there a min version? We don't want people to leave TLS 1.3 disabled for longer than needed for interoperability. At some point in the (hopefully distant) future TLS 1.2 will be deprecated and we'll have to start overriding this setting to force-enable TLS 1.3, etc)

Defaulting to 1.2 seems reasonable here.

We should enable users that desire perfect forward secrecy and the speed improvements of TLS 1.3 to have access to them, especially since we can already support that.

PFS is available in 1.2 as long as you use an (EC)DHE cipher suite (which is the default). As I understand it, enabling 1.3 is only a real advantage for PFS if you also disable 1.2. And the speed gains are mainly in reduced round trips for the handshake, which matters a lot less for us than in the web context.

Would we provide an option to allow only (EC)DHE for users that want PFS now or punt this until 20.2?

We might want to allow election of cipher suites for the particularly security conscious operators.

I'm not a fan of that because cipher specifications tend to be copied and pasted from outdated Apache HOWTOs and often move things backwards instead of forwards. Case in point: we disabled 3DES cipher suites in #17237, which required us to copy and modify the then-current default list of ciphers. We didn't update this list when TLS 1.3 was introduced, so it only contains TLS 1.2 ciphers. However, the Go TLS implementation does not support configuring cipher suites in TLS 1.3, so our now-outdated configuration was being ignored anyway.

Thanks for this, I hadn't realized Go made that edict. I strongly dislike that Go decided what ciphers they consider secure and hope they will reverse the decision (and I'm apparently not alone: https://github.com/golang/go/issues/29349). In all fairness this is _generally_ only an issue when dealing with companies with regulatory or policy constraints around cryptologic implementations but I think we might care about those.

However, from a pure security perspective, I don't have 100% faith that $ciphers in ANY framework will stand the test of time. As TLS 1.3 is implemented for Golang, if a cipher suite is later determined to be vulnerable (either cryptographically or by implementation). Companies like us would have to wait until an upstream framework provider (Golang) issues a patch that we can use to recompile our binaries and then publish them to CRDB operators who then have to deploy them. During the period of time between discovery and patched binaries the only mitigation I can think of offering operators would be to create a local TLS proxy using non-CRDB technology that IS configurable to drop support for the offending implementation.

TL;DR: I think we should revert to TLS 1.2 for now and have a discussion about cipher configurability in its own right.

[bdarnell]

Would we provide an option to allow only (EC)DHE for users that want PFS now or punt this until 20.2?

That would be 20.2, if at all (since we haven't had anyone actually ask for this, and they can also guarantee that they only get PFS ciphers in their client configurations to accomplish the same thing).

However, from a pure security perspective, I don't have 100% faith that $ciphers in ANY framework will stand the test of time.

Agreed. There should at least be a blacklist option so you can knock out problematic ones quickly should the need arise. I'd much prefer a blacklist model than a "copy the list and edit" model, even though it's a little less expressive and doesn't let you tweak priorities.

wait until an upstream framework provider (Golang) issues a patch

At least for that part we could easily make our own patch to remove the offending cipher suite (and a little less easily integrate that patch into our build process) if we had to. But yes, it would still require a new cockroachdb binary release to be built and deployed.

To be fair, the TLS 1.3 cipher suite list is much smaller than that used by older versions of TLS (5 suites, really just 3 when you remove ones that differ only in key sizes. And Go doesn't implement the two AES-CCM variants, only AES-GCM and CHACHA20-POLY1305). So it's already nearly a cipher monoculture (oligoculture?) and a flaw in one cipher that is fixed by moving to one of the other ciphers on the list seems remote enough that I'm not sure it's worth building in configurability for that specific event in advance. It seems much more likely that the next dozen security flaws in TLS will be found in the handshake protocol or something like that rather than something that could be fixed by swapping cipher suites.

[aaron-crl]

So it's already nearly a cipher monoculture (oligoculture?) and a flaw in one cipher that is fixed by moving to one of the other ciphers on the list seems remote enough that I'm not sure it's worth building in configurability for that specific event in advance.

Agreed.

It seems much more likely that the next dozen security flaws in TLS will be found in the handshake protocol or something like that rather than something that could be fixed by swapping cipher suites.

Definitely fair given the recent history of TLS security issues.

Doesn't sound like configuration is worth the engineering cycles at this time, so unless we have a specific customer ask for something that is not TLS 1.2 I'm happy to simply support that until 20.2 (at the earliest).

[bdarnell]

OK, we're going to disable TLS 1.3 in CRDB 20.1.0. I'm going to leave it enabled on master so that we can continue interoperability testing, with the intention of having it enabled in 20.2.

[knz]

@bdarnell @aaron-crl do you think we should have the configuration option in 20.2 after all? If so we'll need to start planning the work.

[bdarnell]

I don't think so. I've opened https://github.com/cockroachdb/docs/issues/7820 to document the potential need to upgrade java versions.

[knz]

Closing since we are not making this configurable after all.