Enforce Transport TLS check on all licenses. #79602
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Historically, we haven't enabled the transport TLS bootstrap check for trial licenses because: - We wanted to make the experience of trial license users as easy as possible and configuring transport TLS was considered cumbersome. - Trial licenses have a limited lifetime so that minimizes the impact of this potentially insecure configuration. With security on by default project we are: - Enabling security by default for basic and trial licenses - We offer an easy, automated way for users to configure transport TLS - Enabling by default this bootstrap check for basic licenses. It doesn't make much sense for us to enforce the bootstrap check on basic licenses but not on trial and given that the concerns that were driving the original decision are not there or have been partly alleviated, this commit changes our behavior so that we enable the TLS bootstrap check regardless of the license level.
jkakavas
added
>enhancement
:Security/License
|
Pinging @elastic/es-security (Team:Security) |
jkakavas
commented
Oct 21, 2021
| @@ -243,15 +243,7 @@ public void registerLicense(final PutLicenseRequest request, final ActionListene | |||
| // because the defaults there mean that security can be "off", even if the setting is "on" | |||
| // BUT basic licenses are explicitly excluded earlier in this method, so we don't need to worry | |||
| if (XPackSettings.SECURITY_ENABLED.get(settings)) { | |||
| // TODO we should really validate that all nodes have xpack installed and are consistently configured but this | |||
| // should happen on a different level and not in this code | |||
| if (XPackLicenseState.isTransportTlsRequired(newLicense, settings) | |||
There was a problem hiding this comment.
We don't need to check TLS transport and security when installing a new license. If transport TLS is not enabled when security is enabled, then we'd have failed to start the node in the first place ( or we will fail when this moves to production mode - but it is not an effect of the license, as the check applies to all licenses now ) .
tvernum
approved these changes
Oct 25, 2021
...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/TransportTLSBootstrapCheck.java
Show resolved
Hide resolved
Co-authored-by: Tim Vernum <[email protected]>
ywangd
approved these changes
Oct 25, 2021
| // should happen on a different level and not in this code | ||
| if (XPackLicenseState.isTransportTlsRequired(newLicense, settings) | ||
| && XPackSettings.TRANSPORT_SSL_ENABLED.get(settings) == false | ||
| && isProductionMode(settings, clusterService.localNode())) { |
There was a problem hiding this comment.
Should we also remove isProductionMode and isBoundToLoopback from this class as well?
lockewritesdocs
pushed a commit
to lockewritesdocs/elasticsearch
that referenced
this pull request
Oct 28, 2021
Historically, we haven't enabled the transport TLS bootstrap check for trial licenses because: - We wanted to make the experience of trial license users as easy as possible and configuring transport TLS was considered cumbersome. - Trial licenses have a limited lifetime so that minimizes the impact of this potentially insecure configuration. With security on by default project we are: - Enabling security by default for basic and trial licenses - We offer an easy, automated way for users to configure transport TLS - Enabling by default this bootstrap check for basic licenses. It doesn't make much sense for us to enforce the bootstrap check on basic licenses but not on trial and given that the concerns that were driving the original decision are not there or have been partly alleviated, this commit changes our behavior so that we enable the TLS bootstrap check regardless of the license level.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Historically, we haven't enabled the transport TLS bootstrap
check for trial licenses because:
easy as possible and configuring transport TLS was considered
cumbersome.
impact of this potentially insecure configuration.
With security on by default project we are:
transport TLS
It doesn't make much sense for us to enforce the bootstrap check
on basic licenses but not on trial and given that the concerns
that were driving the original decision are not there or have been
partly alleviated, this commit changes our behavior so that we
enable the TLS bootstrap check regardless of the license level.
resolves: #75292