Add a tool for creating enrollment tokens #74890

jkakavas · 2021-07-05T06:30:00Z

This change introduces a CLI tool that can be used to create
enrollment tokens. It doesn't require credentials, but simply
write access to the local filesystem of a node. It uses an
auto-generated user in the file-realm with superuser role.

For this purpose, this change also introduces a base class for a
CLI tool that can be used by any CLI tool needs to perform actions
against an ES node as a superuser without requiring credentials
from the user. It is worth noting that this doesn't change our
existing thread model, because already an actor with write access
to the filesystem of an ES node, can become superuser (again, by
adding a superuser to the file realm, albeit manually).

This change introduces a CLI tool that can be used to create enrollment tokens. It doesn't require credentials, but simply write access to the local filesystem of a node. It uses an auto-generated user in the file-realm with superuser role. For this purpose, this change also introduces a base class for a CLI tool that can be used by any CLI tool needs to perform actions against an ES node as a superuser without requiring credentials from the user. It is worth noting that this doesn't change our existing thread model, because already an actor with write access to the fs of an ES node, can become superuser (again, by adding a superuser to the file realm, albeit manually).

elasticmachine · 2021-07-05T06:30:03Z

Pinging @elastic/es-security (Team:Security)

tvernum

Drive by ...

docs/reference/commands/create-entrollment-token.asciidoc

.../security/src/main/java/org/elasticsearch/xpack/security/tool/BaseRunAsSuperuserCommand.java

docs/reference/commands/create-entrollment-token.asciidoc

…l execute the command if they so wish to do

.../security/src/main/java/org/elasticsearch/xpack/security/tool/BaseRunAsSuperuserCommand.java

tvernum · 2021-07-09T08:52:02Z

Thanks for requesting a review, but my original comments were just a drive-by, I don't think I need to do a full review unless you specifically want it.

Feel free to merge when you have the OK from the project team.

jkakavas · 2021-07-09T08:57:59Z

Thanks for requesting a review, but my original comments were just a drive-by, I don't think I need to do a full review unless you specifically want it.

I just triggered the button on the top right mostly so that you get the PR in the GH list of PRs for convenience. Thanks for the drive-by comments !

...rc/main/java/org/elasticsearch/xpack/security/enrollment/tool/CreateEnrollmentTokenTool.java

.../security/src/main/java/org/elasticsearch/xpack/security/tool/BaseRunAsSuperuserCommand.java

...st/java/org/elasticsearch/xpack/security/enrollment/tool/BaseRunAsSuperuserCommandTests.java

...st/java/org/elasticsearch/xpack/security/enrollment/tool/CreateEnrollmentTokenToolTests.java

jkakavas · 2021-07-12T20:56:27Z

part-1-fips failed because of #75221, unrelated flaky test

jkakavas · 2021-07-12T20:56:43Z

@elasticsearchmachine run elasticsearch-ci/part-1-fips

BigPandaToo

LGTM

lockewritesdocs · 2021-07-13T15:40:40Z

@elasticmachine run elasticsearch-ci/docs

lockewritesdocs

Added a few non-blocking suggested changes. LGTM otherwise 🥳

docs/reference/commands/create-enrollment-token.asciidoc

Co-authored-by: Adam Locke <[email protected]>

albertzaharovits · 2021-07-15T06:00:05Z

.../security/src/main/java/org/elasticsearch/xpack/security/tool/BaseRunAsSuperuserCommand.java

+            settings = env.settings();
+        }
+
+        ensureFileRealmEnabled(settings);


Because file realms don't have Secure Settings, I would move this before the password prompt, and maybe make it the base implementation of validate.

We chatted and we're moving ensureFileRealmEnabled(settings); higher up but not adding it to a default implementation of validate()

albertzaharovits · 2021-07-15T06:02:20Z

.../security/src/main/java/org/elasticsearch/xpack/security/tool/BaseRunAsSuperuserCommand.java

+            attributesChecker.check(terminal);
+            final boolean forceExecution = options.has(force);
+            checkClusterHealthWithRetries(newEnv, terminal, 5, forceExecution);
+            executeCommand(terminal, options, newEnv);


nit: I would pass the username and password to this method instead of making them class members. It's more clear the lifecycle like this.

we'd need to pass them as parameters to all checkClusterHealthWithRetries, executeCommand, cleanup. I like the fact that we keep ownership of the password here and close it in all cases etc. Happy to change, but I miss the point of "It's more clear the lifecycle like this." , I'll ping you to chat

we chatted and I made the change as suggested

albertzaharovits

LGTM
I only had two very minor comments.

This change introduces a CLI tool that can be used to create enrollment tokens. It doesn't require credentials, but simply write access to the local filesystem of a node. It uses an auto-generated user in the file-realm with superuser role. For this purpose, this change also introduces a base class for a CLI tool that can be used by any CLI tool needs to perform actions against an ES node as a superuser without requiring credentials from the user. It is worth noting that this doesn't change our existing thread model, because already an actor with write access to the fs of an ES node, can become superuser (again, by adding a superuser to the file realm, albeit manually). Co-authored-by: Adam Locke <[email protected]>

jkakavas added 2 commits July 5, 2021 09:26

Merge remote-tracking branch 'origin/master' into create-tokens-cli

7c2d68a

jkakavas added :Security/Security Security issues without another label v8.0.0 labels Jul 5, 2021

jkakavas requested review from albertzaharovits and BigPandaToo July 5, 2021 06:30

elasticmachine added the Team:Security Meta label for security team label Jul 5, 2021

jkakavas mentioned this pull request Jul 5, 2021

Reset elastic password cli tool #74892

Merged

formatting

81cfa9e

tvernum reviewed Jul 5, 2021

View reviewed changes

bytebilly reviewed Jul 5, 2021

View reviewed changes

docs/reference/commands/create-entrollment-token.asciidoc Outdated Show resolved Hide resolved

docs/reference/commands/create-entrollment-token.asciidoc Outdated Show resolved Hide resolved

jkakavas added 4 commits July 5, 2021 15:15

address feedback

e64daab

reduce visibility

24228ba

Allow users to override the health check against the cluster and stil…

57c6a07

…l execute the command if they so wish to do

Merge remote-tracking branch 'origin/master' into create-tokens-cli

ec4dd43

jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Jul 8, 2021

Bring along updates from elastic#74890

c5c1759

Merge remote-tracking branch 'origin/master' into create-tokens-cli

51e8e8c

jkakavas requested review from tvernum and bytebilly July 8, 2021 21:05

tvernum reviewed Jul 9, 2021

View reviewed changes

.../security/src/main/java/org/elasticsearch/xpack/security/tool/BaseRunAsSuperuserCommand.java Outdated Show resolved Hide resolved

BigPandaToo suggested changes Jul 10, 2021

View reviewed changes

jkakavas added 3 commits July 12, 2021 11:07

address feedback

7b41749

Merge remote-tracking branch 'origin/master' into create-tokens-cli

a8be2c4

don't explicitly check for multiple file realms, node does that already

f501712

jkakavas requested review from BigPandaToo and removed request for bytebilly July 12, 2021 09:26

jkakavas requested a review from lockewritesdocs July 12, 2021 14:51

BigPandaToo approved these changes Jul 13, 2021

View reviewed changes

Rename file and add link to the top-level commands page

44ccacb

lockewritesdocs approved these changes Jul 13, 2021

View reviewed changes

jkakavas and others added 2 commits July 13, 2021 21:34

Apply doc suggestions from code review

b5e7dfe

Co-authored-by: Adam Locke <[email protected]>

Merge remote-tracking branch 'origin/master' into create-tokens-cli

b373f85

jkakavas force-pushed the create-tokens-cli branch from 5d96a9b to b373f85 Compare July 14, 2021 12:05

lockewritesdocs mentioned this pull request Jul 14, 2021

[DOCS] Stack updates for security ON by default #75153

Closed

12 tasks

albertzaharovits reviewed Jul 15, 2021

View reviewed changes

albertzaharovits approved these changes Jul 15, 2021

View reviewed changes

address feedback

6e7dd05

jkakavas merged commit cb37989 into elastic:master Jul 15, 2021

jakelandis added v8.0.0-alpha1 >enhancement and removed v8.0.0 labels Jul 26, 2021

thomheymann mentioned this pull request Jul 27, 2021

Interactive setup mode elastic/kibana#106881

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a tool for creating enrollment tokens #74890

Add a tool for creating enrollment tokens #74890

jkakavas commented Jul 5, 2021

elasticmachine commented Jul 5, 2021

tvernum left a comment

tvernum commented Jul 9, 2021

jkakavas commented Jul 9, 2021

jkakavas commented Jul 12, 2021

jkakavas commented Jul 12, 2021

BigPandaToo left a comment

lockewritesdocs commented Jul 13, 2021

lockewritesdocs left a comment

albertzaharovits Jul 15, 2021

jkakavas Jul 15, 2021

albertzaharovits Jul 15, 2021

jkakavas Jul 15, 2021

jkakavas Jul 15, 2021

albertzaharovits left a comment

Add a tool for creating enrollment tokens #74890

Add a tool for creating enrollment tokens #74890

Conversation

jkakavas commented Jul 5, 2021

elasticmachine commented Jul 5, 2021

tvernum left a comment

Choose a reason for hiding this comment

tvernum commented Jul 9, 2021

jkakavas commented Jul 9, 2021

jkakavas commented Jul 12, 2021

jkakavas commented Jul 12, 2021

BigPandaToo left a comment

Choose a reason for hiding this comment

lockewritesdocs commented Jul 13, 2021

lockewritesdocs left a comment

Choose a reason for hiding this comment

albertzaharovits Jul 15, 2021

Choose a reason for hiding this comment

jkakavas Jul 15, 2021

Choose a reason for hiding this comment

albertzaharovits Jul 15, 2021

Choose a reason for hiding this comment

jkakavas Jul 15, 2021

Choose a reason for hiding this comment

jkakavas Jul 15, 2021

Choose a reason for hiding this comment

albertzaharovits left a comment

Choose a reason for hiding this comment