Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove TLS 1.0 as a default SSL protocol #37512

Merged
merged 8 commits into from
Jan 25, 2019
Merged

Remove TLS 1.0 as a default SSL protocol #37512

merged 8 commits into from
Jan 25, 2019

Conversation

tvernum
Copy link
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
Collaborator

Pinging @elastic/es-security

@tvernum
Copy link
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
Member

@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
@@ -154,7 +154,7 @@ private XPackSettings() {
}
}, 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
Member

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
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
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

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

gwbrown 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
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
Contributor Author

tvernum commented Jan 24, 2019

@elasticmachine run elasticsearch-ci/2

@tvernum
Copy link
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
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
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
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
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkakavas jkakavas jkakavas approved these changes

@jaymode jaymode jaymode 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 @gwbrown @jaymode @jkakavas @colings86