TLS: only support TLS 1.2 / 1.3

[roidelapluie]

TLS 1.0 and 1.1 are considered unsecure, while TLS 1.2 would imply that
we add more configuration variables, e.g. to pick cipher suites. TLS1.3
means that cipher suites are not configurable anyway.

Last releases of Prometheus support TLS 1.3 (since Prometheus 2.13).

Signed-off-by: Julien Pivotto roidelapluie@inuits.eu

[brian-brazil]

I believe we said that we'd stick purely with Go defaults.

[roidelapluie]

I think that only tls 1.3 might be too far but we should at least disable 1.0 and 1.1 as it won't achieve a good score at ssllabs and isn't supported by browsers anymore (or won't be soon).

[roidelapluie]

We can also argue that config.PreferServerCipherSuites = true should be set (but that is less critical)

[roidelapluie]

I believe we said that we'd stick purely with Go defaults.

Well, we can leave this as it but I would not be surprise that it does not pass security audit.

[brian-brazil]

I would not be surprise that it does not pass security audit.

That's a matter for Go then, fine tuning TLS settings over time is not something you want to get into.

[roidelapluie]

Well I don't think so. If you look at other software that do support HTTPS they all support fine tuning ciphers and TLS version. Why would we be the only ones not to?

[roidelapluie]

One point about at least tls 1.2 is that probably lots of OS-provided clients (like curl in red hat) does not support tls 1.3 widely

[brian-brazil]

Why would we be the only ones not to?

Because that way lies madness, there's already over 10 different configurables and that'll only continue to grow over time and in ways that you can't forsee config wise. This is what was discussed back when we first agreed to do tls, that we'll stick with Go defaults with no configurability. If you have a setup that requires a more niche setup, you can use a reverse proxy.

[discordianfish]

I'd say we support 1.2 and 1.3.

Re/ options for 1.2: Not a strong opinion. But I think it would be fine to just hardcode whatever is currently considered best practice.

I'm not really concerned about the maintenance overhead. Yes, this stuff changes but so do kernel metric, procfs format and others. So I'll merge this if @SuperQ agrees.

[roidelapluie]

Adapted, with commit text: TLS 1.0 and 1.1 are deprecated by major vendors (e.g. browsers).

[SuperQ]

I agree with Brian, we don't want to go deep into the madness of all TLS options. We do want to go with Golang defaults to avoid too many flags and the support issues with users flipping all the flags because they can.

But setting a couple of sane defaults is probably worthwhile. And I agree with @discordianfish, hardcoding these is fine, especially as a first iteration.

[discordianfish]

LGTM

[SuperQ]

Can you add a note in the README about the minimum TLS version?

[brian-brazil]

My question is how do we decide what "sane defaults" are? The plan up to this point was to delegate that decision entirely to Go, figuring out what those settings should be on an ongoing basis (and when going via a library which users then need to pull in) is likely to end up being non-trivial.

[roidelapluie]

My question is how do we decide what "sane defaults" are? The plan up to this point was to delegate that decision entirely to Go, figuring out what those settings should be on an ongoing basis (and when going via a library which users then need to pull in) is likely to end up being non-trivial.

Don't you agree that tls 1.0 should be removed and that it is not go responsibility?

[brian-brazil]

Don't you agree that tls 1.0 should be removed and that it is not go responsibility?

That's not really the point. The point is that if we put ourselves in a position where we make security decisions like this, then we need to keep on making them on a regular and consistent basis, ensure that such decisions get promptly released into ~16 binaries implemented in 2 languages plus deal with requests to change them from users - whether due to general concern by users or the particulars of their setup. Delegating to a 3rd party avoids this.

That is something that we could do, but it is not what was agreed and discussed. Thus we'd need sort out the details of all that before going down this path as it's not merely a matter of one-off hardcoding some settings.

[roidelapluie]

@SuperQ updated readme

[SuperQ]

LGTM

[brian-brazil]

We still need to resolve the question of how us using non-default settings would be managed, as this was not the path you proposed and thus that was discussed back at the dev summit. For example if a PR comes in to me in common changing the hardcoding, what should I do?

[roidelapluie]

We are only hardcoding TLS version here ; not ciphers etc.

[brian-brazil]

I don't really see that making a difference, as this is a question of an overall policy framework. E.g. why not disable 1.2? When would 1.2 be disabled? For what reasons would it be acceptable to enable 1.0/1.1?

[roidelapluie]

Qualys SSL Labs is recognized in the industry.

Here is our result with go defaults:

And our result with go defaults + tls 1.0 and 1.1 disabled:

[roidelapluie]

Qualys refers to this: https://blog.qualys.com/ssllabs/2018/11/19/grade-change-for-tls-1-0-and-tls-1-1-protocols which refers to this: http://www.rfc-editor.org/info/rfc7525

[brian-brazil]

To be clear I'm looking for a policy, not the reasoning behind the changes in this particular PR.

I'm aware of sslabs, that's not what I'm asking about. I worried more about why should we aim for this particular A rating, versus any other rating? That RFC is from 5 years ago, how do we determine what RFCs to follow and on what timeline? What browsers are we okay with breaking? Which aren't we okay with breaking? What about past releases of Prometheus?

Why should we care only about browsers, and not programmatic use? For example this PR would break pushgateway support in our client libraries, as we officially support some older versions of Python and Java that only do up to TLS 1.0. There'll also be user code talking to the Prometheus and Alertmanager APIs.

As you can see this is a quite nuanced issue, and I've yet to have answers to my above questions that'd help me as future maintainer of this code once it moves to common evaluate inbound PRs in the years to come. I'm not willing to address this on a PR-to-PR basis as users look to change the various hardcoded 10+ TLS server configurables (nor do I think it's sane to have an ever-shifting TLS configuration from a general project security stance standpoint), there needs to be a plausible coherent policy established in advance. I'd suggest starting a doc if you want to go down this path.

 

@discordianfish There is active disagreement on this issue, merging this is not kosher. I ask that you promptly correct this oversight.

[discordianfish]

My position is that this still is part of the node-exporter. If we extract this package and you take over ownership, you can establish whatever process you want and change the TLS settings as you see fit.

[roidelapluie]

To be clear I'm looking for a policy, not the reasoning behind the changes in this particular PR.

I'm aware of sslabs, that's not what I'm asking about. I worried more about why should we aim for this particular A rating, versus any other rating? That RFC is from 5 years ago, how do we determine what RFCs to follow and on what timeline? What browsers are we okay with breaking? Which aren't we okay with breaking? What about past releases of Prometheus?

As far as I am concerned, disabling TLS1.0 and 1.1 is a no brainer. I have seen different customers just asking for everything tls1.1 and 1.0 go away; and I assume that those very same customers are the same who would like https in the prometheus server too, instead of only at the LB level.

While that does not help with a general policy, I would agree to let that setting defaulting to TLS1.2 but enable more flexibility with the TLS configuration, enabling setting the min version, the ciphers, etc. So maybe we need to take that discussion on the developers mailing list to reach a new concensus about this? I am also glad to help you with maintaining the https package when we take it to common, and becoming one of the maintainers.

Why should we care only about browsers, and not programmatic use? For example this PR would break pushgateway support in our client libraries, as we officially support some older versions of Python and Java that only do up to TLS 1.0. There'll also be user code talking to the Prometheus and Alertmanager APIs.

As this is disabled by default, people with old software can simply not enable TLS and use weaker reverse proxies as they do now. We do not "break" anyone with this.

As you can see this is a quite nuanced issue, and I've yet to have answers to my above questions that'd help me as future maintainer of this code once it moves to common evaluate inbound PRs in the years to come. I'm not willing to address this on a PR-to-PR basis as users look to change the various hardcoded 10+ TLS server configurables (nor do I think it's sane to have an ever-shifting TLS configuration from a general project security stance standpoint), there needs to be a plausible coherent policy established in advance. I'd suggest starting a doc if you want to go down this path.

I guess, yes, I can work on a document. At the same time, I do not want to hold the 1.0.0 release of the node_exporter for this.