Feat: add ssl_supported_protocols option #1055
Conversation
kares
changed the title
| @@ -66,6 +66,8 @@ module APIConfigs | |||
| # Set the keystore password | |||
| :keystore_password => { :validate => :password }, | |||
|
|
|||
| :ssl_supported_protocols => { :validate => ['TLSv1.1', 'TLSv1.2', 'TLSv1.3'], :default => [], :list => true }, | |||
There was a problem hiding this comment.
ES also (still) supports: SSLv3, TLSv1 and SSLv2Hello but since these are disabled by default in an up-to-date Java I do not think we need to support them give even an old 6.0 cluster is expected to run on at least Java 8.
There was a problem hiding this comment.
I'm slightly wary of removing something that is a valid option with Elasticsearch, maybe we should keep it but warn?
There was a problem hiding this comment.
we had a discussion with @jsvd on the topic and part of that outcome was that we start with >= TLSv1.1 (for all plugins), in this case despite ES' support for legacy SSLv3 and TLSv1.
existing plugin versions using TLS <= 1.1, on an up-to-date JDK, need tampering with the java.security files so if they're upgrading they either upgrade to secure TLS or we'll hear them complaining on the feature per plugin basis (the default - not setting the option is expected to work for them as before).
There was a problem hiding this comment.
I reran the travis tests and there's still one configuration failing: SECURE_INTEGRATION=true INTEGRATION=true ELASTIC_STACK_VERSION=8.x SNAPSHOT=true LOG_LEVEL=info
The logs don't seem to show anything relevant, may require some investigation to rule out as unrelated.
This reverts commit 0655b45.
good catch, the issue was simply 8.x requiring auth credentials (for _refresh and friends), this slipped under the sleeve with the 🔴 storm on CI. now all 🟢 |
There was a problem hiding this comment.
Code looks good, I'd like to get feedback from @karenzone on the docs changes, and from @jsvd on whether we need to continue supporting TLS < v1.1 as it is a valid configuration for Elasticsearch, or that we can safely drop support
|
|
||
| * Value type is <<string,string>> | ||
| * Allowed values are: `'TLSv1.1'`, `'TLSv1.2'`, `'TLSv1.3'` | ||
| * Default depends on the JDK being used. With up-to-date Logstash, the default is `['TLSv1.2', 'TLSv1.3']`. |
There was a problem hiding this comment.
I wonder if it makes sense to have a Logstash page equivalent to https://www.elastic.co/guide/en/elasticsearch/reference/current/jdk-tls-versions.html for Logstash for a single point of reference, given that this is copied across multiple plugins.
cc @karenzone
There was a problem hiding this comment.
It does @robbavey ! That refactor should happen separately from that PR, but I agree we should centralize this information. Pls open a docs issue so the idea isn't lost.
There was a problem hiding this comment.
Issue created - elastic/logstash#13962
| @@ -66,6 +66,8 @@ module APIConfigs | |||
| # Set the keystore password | |||
| :keystore_password => { :validate => :password }, | |||
|
|
|||
| :ssl_supported_protocols => { :validate => ['TLSv1.1', 'TLSv1.2', 'TLSv1.3'], :default => [], :list => true }, | |||
There was a problem hiding this comment.
I'm slightly wary of removing something that is a valid option with Elasticsearch, maybe we should keep it but warn?
While it makes sense for ES to still allow the possibility of a client using tls1.0 because you many need to have a component sending to ES directly that can only do tls1.0 or lower, on the Logstash ES output we are essentially connecting only to ES, and all ES versions we support also support tls1.1+. We can certainly but a higher bar on the encryption parameters used between Elastic components. |
|
Thanks for the explanations @kares, @jsvd - makes sense. Code LGTM, leaving to @karenzone for final approval on doc |
The new
ssl_supported_protocolsoption default is aligned with what the Manticore/HttpClient defaults to-> whatever the SSL engine defaults to -> on up to date Java (>= 8) it's
TLSv1.3,TLSv1.2Having a configuration option (similar to ES'
xpack.security.http.ssl.supported_protocols: TLSv1.3,TLSv1.2) allows for more flexibility.This PR also establishes confidence that TLSv1.3 is working and supported on recent LS (JVM) versions.