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

Add warning on unsupported settings to elasticsearch keystore command #31489

Closed
ppf2 opened this issue Jun 21, 2018 · 7 comments
Closed

Add warning on unsupported settings to elasticsearch keystore command #31489

ppf2 opened this issue Jun 21, 2018 · 7 comments
Labels
:Core/Infra/Settings Settings infrastructure and APIs >enhancement :Security/Security Security issues without another label

Comments

@ppf2
Copy link
Member

ppf2 commented Jun 21, 2018

Elasticsearch version : 6.2.1

In the documentation of elasticsearch-keystore, we noted the following:

Only some settings are designed to be read from the keystore. See documentation for each setting to see if it is supported as part of the keystore.

There is an existing ticket on documenting these settings.

This one covers what happens if the end user ends up adding a setting Elasticsearch does not know of via elasticsearch-keystore. The following is accepted by the command:

./elasticsearch-keystore add mysetting

will result in Elasticsearch not starting up upon a restart, failing a bootstrap check.

[2018-06-20T17:40:32,952][WARN ][o.e.b.ElasticsearchUncaughtExceptionHandler] [node-1] uncaught exception in thread [main]
org.elasticsearch.bootstrap.StartupException: java.lang.IllegalArgumentException: unknown secure setting [mysetting] please check that any required plugins are installed, or check the breaking changes documentation for removed settings
	at org.elasticsearch.bootstrap.Elasticsearch.init(Elasticsearch.java:125) ~[elasticsearch-6.2.1.jar:6.2.1]
	at org.elasticsearch.bootstrap.Elasticsearch.execute(Elasticsearch.java:112) ~[elasticsearch-6.2.1.jar:6.2.1]
	at org.elasticsearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:86) ~[elasticsearch-6.2.1.jar:6.2.1]
	at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:124) ~[elasticsearch-cli-6.2.1.jar:6.2.1]

Have we considered adding validation at the time of adding a string via elasticsearch-keystore? Or simply throw a warning back on every invocation of ./elasticsearch-keystore add <setting_name> to warn users that if they have added an unknown setting, Elasticsearch will fail bootstrap check and not start up?

Also, it will be nice to add this check to the list of bootstrap checks in the documentation. Thx!

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

@colings86 colings86 added the :Security/Security Security issues without another label label Jun 21, 2018
@tvernum tvernum added the :Core/Infra/Settings Settings infrastructure and APIs label Jun 21, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@bleskes
Copy link
Contributor

bleskes commented Jun 21, 2018

@jkakavas @rjernst any opinions here?

@rjernst
Copy link
Member

rjernst commented Jun 21, 2018

I would love to have this, but it is technically difficult given the current settings code. When launching the keystore cli, we do not load plugins, yet plugins can have secure settings. In order to validate which settings are allowed (and their type, eg string vs file), we would need to load this information from all plugins. But we do not want to actually load plugins. A long time ago @s1monw was working on a prototype for separating constructing the plugin class (where we could get the setting objects from), and and initialization of the plugin, but I don't know where that was left off.

@s1monw
Copy link
Contributor

s1monw commented Jun 25, 2018

@rjernst the reason way this is not separated yet is that REALM settings are not real settings and requires SPI to be loaded in order to get them fully constructed. Once this is resolved we can load settings separately and potentially move the plugin parts out of server to get this stuff on our tools end. (not even sure that is needed).

@jaymode
Copy link
Member

jaymode commented Jun 25, 2018

The work to make the realm settings real settings is being worked on by @tvernum in #30241

@rjernst
Copy link
Member

rjernst commented Aug 29, 2019

Closing this in favor of #46148 which outlines a specific task of needing keystore validation.

@rjernst rjernst closed this as completed Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Settings Settings infrastructure and APIs >enhancement :Security/Security Security issues without another label
Projects
None yet
Development

No branches or pull requests

8 participants