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.

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

Enable TLSv1.3 by default for JDKs with support #38103

Merged
merged 1 commit into from
Feb 1, 2019

Conversation

jaymode
Copy link
Member

@jaymode jaymode commented Jan 31, 2019

This commit enables the use of TLSv1.3 with security by enabling us to
properly map TLSv1.3 in the supported protocols setting to the
algorithm for a SSLContext. Additionally, we also enable TLSv1.3 by
default on JDKs that support it.

An issue was uncovered with the MockWebServer when TLSv1.3 is used that
ultimately winds up in an endless loop when the client does not trust
the server's certificate. Due to this, SSLConfigurationReloaderTests
has been pinned to TLSv1.2.

Closes #32276

This commit enables the use of TLSv1.3 with security by enabling us to
properly map `TLSv1.3` in the supported protocols setting to the
algorithm for a SSLContext. Additionally, we also enable TLSv1.3 by
default on JDKs that support it.

An issue was uncovered with the MockWebServer when TLSv1.3 is used that
ultimately winds up in an endless loop when the client does not trust
the server's certificate. Due to this, SSLConfigurationReloaderTests
has been pinned to TLSv1.2.

Closes elastic#32276
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

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

return "TLSv1.2";
}

String algorithm = "SSL";
Copy link
Member

Choose a reason for hiding this comment

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

I really like we got rid of this switch :)

@jkakavas
Copy link
Member

jkakavas commented Feb 1, 2019

An issue was uncovered with the MockWebServer when TLSv1.3 is used that
ultimately winds up in an endless loop when the client does not trust
the server's certificate.

Do we have an upstream issue ? I'm curious to see the details

@jaymode
Copy link
Member Author

jaymode commented Feb 1, 2019

Do we have an upstream issue ? I'm curious to see the details

I need to open one. I discussed with @tbrooks8 elsewhere. Essentially what happens is the server gets into a loop where it thinks it needs to wrap data but never produces any data each time it wraps. I tested against our NIO and Netty transports which do not exhibit this behavior.

@jaymode jaymode merged commit 2ca2220 into elastic:master Feb 1, 2019
@jaymode jaymode deleted the tlsv13 branch February 1, 2019 15:34
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 1, 2019
* master:
  Replace awaitBusy with assertBusy in atLeastDocsIndexed (elastic#38190)
  Adjust SearchRequest version checks (elastic#38181)
  AwaitsFix testClientSucceedsWithVerificationDisabled (elastic#38213)
  Zen2ify RareClusterStateIT (elastic#38184)
  ML: Fix error race condition on stop _all datafeeds and close _all jobs (elastic#38113)
  AwaitsFix PUT mapping with _doc on an index that has types (elastic#38204)
  Allow built-in monitoring_user role to call GET _xpack API (elastic#38060)
  Update geo_shape docs to include unsupported features (elastic#38138)
  [ML] Remove "8" prefixes from file structure finder timestamp formats (elastic#38016)
  Disable bwc tests while backporting elastic#38104 (elastic#38182)
  Enable TLSv1.3 by default for JDKs with support (elastic#38103)
  Fix _host based require filters (elastic#38173)
  RestoreService should update primary terms when restoring shards of existing indices (elastic#38177)
  Throw if two inner_hits have the same name (elastic#37645)
jasontedor added a commit to gwbrown/elasticsearch that referenced this pull request Feb 1, 2019
* elastic/master: (54 commits)
  Introduce retention leases versioning (elastic#37951)
  Correctly disable tests for FIPS JVMs (elastic#38214)
  AwaitsFix testAbortedSnapshotDuringInitDoesNotStart (elastic#38227)
  Preserve ILM operation mode when creating new lifecycles (elastic#38134)
  Enable trace log in FollowerFailOverIT (elastic#38148)
  SnapshotShardsService Simplifications (elastic#38025)
  Default include_type_name to false in the yml test harness. (elastic#38058)
  Disable bwc preparing to backport of#37977, elastic#37857 and elastic#37872 (elastic#38126)
  Adding ml_settings entry to HLRC and Docs for deprecation_info (elastic#38118)
  Replace awaitBusy with assertBusy in atLeastDocsIndexed (elastic#38190)
  Adjust SearchRequest version checks (elastic#38181)
  AwaitsFix testClientSucceedsWithVerificationDisabled (elastic#38213)
  Zen2ify RareClusterStateIT (elastic#38184)
  ML: Fix error race condition on stop _all datafeeds and close _all jobs (elastic#38113)
  AwaitsFix PUT mapping with _doc on an index that has types (elastic#38204)
  Allow built-in monitoring_user role to call GET _xpack API (elastic#38060)
  Update geo_shape docs to include unsupported features (elastic#38138)
  [ML] Remove "8" prefixes from file structure finder timestamp formats (elastic#38016)
  Disable bwc tests while backporting elastic#38104 (elastic#38182)
  Enable TLSv1.3 by default for JDKs with support (elastic#38103)
  ...
jaymode added a commit to jaymode/elasticsearch that referenced this pull request Feb 4, 2019
This commit fixes the pinning of SSLContexts to TLSv1.2 in the
SSLConfigurationReloaderTests. The pinning was added for the initial
creation of clients and webservers but the updated contexts would
default to TLSv1.3, which is known to cause hangs with the
MockWebServer that we use.

Relates elastic#38103
Closes elastic#38247
jaymode added a commit that referenced this pull request Feb 4, 2019
This commit fixes the pinning of SSLContexts to TLSv1.2 in the
SSLConfigurationReloaderTests. The pinning was added for the initial
creation of clients and webservers but the updated contexts would
default to TLSv1.3, which is known to cause hangs with the
MockWebServer that we use.

Relates #38103
Closes #38247
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants