Skip to content

Remove TLS 1.0 as a default SSL protocol#37512

Merged
tvernum merged 8 commits intoelastic:masterfrom
tvernum:default-no-tls10
Jan 25, 2019
Merged

Remove TLS 1.0 as a default SSL protocol#37512
tvernum merged 8 commits intoelastic:masterfrom
tvernum:default-no-tls10

Conversation

@tvernum
Copy link
Copy Markdown
Contributor

@tvernum tvernum commented Jan 16, 2019

The default value for ssl.supported_protocols no longer includes TLSv1
as this is an old protocol with known security issues.
Administrators can enable TLSv1.0 support by configuring the
appropriate ssl.supported_protocols setting, for example:

xpack.security.http.ssl.supported_protocols: ["TLSv1.2","TLSv1.1","TLSv1"]

Relates: #36021

@tvernum
Remove TLS 1.0 as a default SSL protocol
335b4c0 
The default value for ssl.supported_protocols no longer includes TLSv1
as this is an old protocol with known security issues.
Administrators can enable TLSv1.0 support by configuring the
appropriate `ssl.supported_protocols` setting, for example:

xpack.security.http.ssl.supported_protocols: ["TLSv1.2","TLSv1.1","TLSv1"]

Relates: elastic#36021
@tvernum tvernum requested review from jaymode and jkakavas January 16, 2019 04:54
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Jan 16, 2019

Pinging @elastic/es-security

@tvernum
Copy link
Copy Markdown
Contributor Author

tvernum commented Jan 16, 2019

This is currently WIP because I need to make the same change in #37287 when it is merged.

I will raise a companion PR to deprecate the use of TLS1.0 in 6.x

jkakavas
jkakavas approved these changes Jan 16, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Maybe not in the scope of this PR, but do we want to start throwing [WARN] messages when TLS1 or older is used?

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java Outdated
}, Setting.Property.NodeScope);

public static final List<String> DEFAULT_SUPPORTED_PROTOCOLS = Arrays.asList("TLSv1.2", "TLSv1.1", "TLSv1");
public static final List<String> DEFAULT_SUPPORTED_PROTOCOLS = Arrays.asList("TLSv1.2", "TLSv1.1" );
Copy link
Copy Markdown
Contributor

@jkakavas jkakavas Jan 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove extra space

@tvernum
Copy link
Copy Markdown
Contributor Author

tvernum commented Jan 16, 2019

do we want to start throwing [WARN] messages when TLS1 or older is used?

For 6.7 we'll issue a deprecation warning if TLS1.0 is used without being explicitly enabled.

I'm not sure we want to issue warnings for things that users intentionally configure. We could but then we probably should warn on verification_mode: none and a bunch of other things.

jaymode
jaymode approved these changes Jan 16, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AthenaEryma AthenaEryma mentioned this pull request Jan 16, 2019
Closed
26 tasks
@AthenaEryma
Copy link
Copy Markdown
Contributor

AthenaEryma commented Jan 16, 2019

We should also add a WARN level issue to the Deprecations API (#36024) if the default protocols list is in use (i.e. if it is not set explicitly).

This would probably be best to add to 6.x at the same time as the deprecation warnings, but you don't have bandwidth @tvernum I'd be happy to do it.

@tvernum tvernum mentioned this pull request Jan 23, 2019
Merged
@tvernum tvernum removed the WIP label Jan 24, 2019
@tvernum
Copy link
Copy Markdown
Contributor Author

tvernum commented Jan 24, 2019 •
edited
Loading

@jkakavas @jaymode
I've added (a4cfb24) this to the 7.0 breaking changes doc if you'd like to cast your eye over that.

@tvernum tvernum mentioned this pull request Jan 24, 2019
Merged
@tvernum
Copy link
Copy Markdown
Contributor Author

tvernum commented Jan 24, 2019

@elasticmachine run elasticsearch-ci/2

@tvernum
Copy link
Copy Markdown
Contributor Author

tvernum commented Jan 24, 2019

@elasticmachine run elasticsearch-ci/2

04:23:41 
04:23:41 > Task :x-pack:plugin:ccr:internalClusterTest FAILED
04:23:41 Tests with failures:
04:23:41   - org.elasticsearch.xpack.ccr.FollowerFailOverIT.testReadRequestsReturnsLatestMappingVersion
04:23:41   - org.elasticsearch.xpack.ccr.FollowerFailOverIT.testAddNewReplicasOnFollower
04:23:41

I can't reproduce this, and I can't see how it's related (but it has failed twice)

@tvernum
Copy link
Copy Markdown
Contributor Author

tvernum commented Jan 24, 2019

@elasticmachine run elasticsearch-ci/2

jaymode
jaymode approved these changes Jan 24, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still LGTM

@tvernum
Copy link
Copy Markdown
Contributor Author

tvernum commented Jan 25, 2019 

11:49:37 FAILURE 11.8s J2 | ReservedRealmIntegTests.testDisablingUser <<< FAILURES!
11:49:37    > Throwable #1: java.lang.AssertionError: Shard [.security-6][0] is still locked after 5 sec waiting
11:49:37    > 	at __randomizedtesting.SeedInfo.seed([D784343A8527B3AC:87758EACCFD15F3A]:0)
11:49:37    > 	at org.elasticsearch.test.InternalTestCluster.assertAfterTest(InternalTestCluster.java:2386)
11:49:37    > 	at org.elasticsearch.test.ESIntegTestCase.afterInternal(ESIntegTestCase.java:594)
11:49:37    > 	at org.elasticsearch.test.ESIntegTestCase.cleanUpCluster(ESIntegTestCase.java:2231)
11:49:37    > 	at java.lang.Thread.run(Thread.java:748)

@elasticmachine run elasticsearch-ci/2

@tvernum tvernum merged commit 03690d1 into elastic:master Jan 25, 2019
@jaymode jaymode mentioned this pull request Jan 29, 2019
Closed
3 tasks
@EvanXiaa EvanXiaa mentioned this pull request Apr 30, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaymode jaymode jaymode approved these changes

@jkakavas jkakavas jkakavas approved these changes

Assignees

No one assigned

Labels

>breaking :Security/TLS SSL/TLS, Certificates v7.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@tvernum @elasticmachine @AthenaEryma @jaymode @jkakavas @colings86