Configure security for the initial node cli #74868

albertzaharovits · 2021-07-01T21:20:04Z

When a new cluster node is started up for the first time, in v8, the startup script auto configures the node (mostly Security), by appending to its default yml file.

@jkakavas @BigPandaToo This PR does everything that we discussed.
We need to sync up with the broader team, then dovetail the other pieces (eg displaying the enrollment token).

mark-vieira · 2021-07-01T21:31:35Z

I was under the impression as-per our dicussion that we didn't want to go down the path of modifying user provided config files. I'm still of the opinion this is not something we want to do.

mark-vieira · 2021-07-01T21:50:47Z

For posterity here are my main concerns regarding having Elasticsearch potentially modify configuration files that are intended to be provided by users:

Up until now it was perfectly reasonable for elasticsearch.yml to be read-only. This isn't a "guarantee" per-se, but this is definitely a breaking change as no doubt many deployments make this file read-only for security concerns, and rightfully so. Nothing should be modifying that file. It's pretty reasonable assumption for any administrator to make that configuration files are only read by the consuming process.
Building on (1), it's very feasible that modifying that file may trigger security monitoring. Again, if something touches configuration on a system unexpectedly, that's going to be seen as potentially malicious.
Anyone deploying Elasticsearch via a configuration management system like Ansible, Puppet, Salt, etc is likely going to manage the elasticsearch.yml file via that system. Us modifying that file is then going to throw the system into a cycle where Elasticsearch modifies the file, updates to the deployment blow away those changes and round and round we go.
In deployment environments like Kubernetes the common pattern here is to provide elasticsearch.yml via a ConfigMap, mounted into the pod as a configMap volume. These are not writable, so in effect, this is an extension of item (1).

IMO these are not esoteric uses cases. Folks will encounter this scenario. What is our fallback if the config file isn't writable? How do we handle cases where the changes are overwritten?

jkakavas · 2021-07-02T07:58:44Z

Thanks for your comments Mark!

I was under the impression as-per our dicussion that we didn't want to go down the path of modifying user provided config files. I'm still of the opinion this is not something we want to do.

If you are referring to a discussion in slack a couple of weeks back, there was no decision made during that brief discussion. The concerns that were raised were further discussed within the project and while we acknowledge them, we decided to move forward with the original approach as we think that we can mitigate the concerns and there was no alternative raised that was considered equivalent or better in order to satisfy the project goals and requirements. There was an action item in this discussion to go back to the folks that raised concerns, explain our reasoning and decision, along with the mitigations we think of. We failed to assign an owner for this and it fell through the cracks, apologies for this. This would probably be me, so allow me to address the ones you bring up here, happy to continue the discussion in another medium/forum if this is more helpful.

Up until now it was perfectly reasonable for elasticsearch.yml to be read-only. This isn't a "guarantee" per-se, but this is definitely a breaking change as no doubt many deployments make this file read-only for security concerns, and rightfully so. Nothing should be modifying that file. It's pretty reasonable assumption for any administrator to make that configuration files are only read by the consuming process.

We would write to the elasticsearch.yml only the first time a new node starts, and iff the node is not configured already. If elasticsearch.yml is not writeable, then we will not attempt to write to it and bail the startup script that auto-configures security. The node will start as it would today. In the case this is a new node and the administrator made the file read only before starting elasticsearch without performing any additional configuration then the node will start with security enabled, no TLS configuration. We will be offering tooling ( as we do today ) for the administrator to get access to credentials and interact with the node/cluster as they would do today. I think it is more likely for an administrator that makes the configuration read-only, to bring along their configuration and not depend on us auto-configuring the node security. In this case we wouldn't even attempt to write to the config file in the first place.

Building on (1), it's very feasible that modifying that file may trigger security monitoring. Again, if something touches configuration on a system unexpectedly, that's going to be seen as potentially malicious.

see above.

Anyone deploying Elasticsearch via a configuration management system like Ansible, Puppet, Salt, etc is likely going to manage the elasticsearch.yml file via that system. Us modifying that file is then going to throw the system into a cycle where Elasticsearch modifies the file, updates to the deployment blow away those changes and round and round we go.

We will not modify elasticsearch.yml in this case. We expect that anyone deploying elasticsearch via CM, will be responsible to configure security themselves and not depend on us auto-configuring security for them. Even if that would be the case, our auto-configuration would only kick in the first time a node is brought up, so subsequent edits to the yml file by CM would persist and there would be no "round and round"

In deployment environments like Kubernetes the common pattern here is to provide elasticsearch.yml via a ConfigMap, mounted into the pod as a configMap volume. These are not writable, so in effect, this is an extension of item (1).

ECK, ECE, ESS, and anything that has orchestration is out of scope for the security on by default project. We will not be attempting to auto-confiure security in these cases, there are better ways ( said orchestration ) to achieve that.

mark-vieira · 2021-07-02T14:20:51Z

We failed to assign an owner for this and it fell through the cracks, apologies for this.

No worries, that's what pull request reviews are for 😉

We will not modify elasticsearch.yml in this case. We expect that anyone deploying elasticsearch via CM, will be responsible to configure security themselves and not depend on us auto-configuring security for them.

How will we determine this? We can't really reliable know how ES is being deployed other than the packaging type. In the case where folks don't configure security for whatever reason I suppose it'll just be as you say, where we update the file on the first go, and then it'll get subsequently overwritten. I still contend that's a bit of an awkward behavior, as if I'm creating a new ES deployment and naively don't setup security, it'll come up iniitally with security enabled and then later be disbled again.

ECK, ECE, ESS, and anything that has orchestration is out of scope for the security on by default project. We will not be attempting to auto-confiure security in these cases, there are better ways ( said orchestration ) to achieve that.

I wasn't referring to our own orchestration solutions, just more generally, anyone deploying ES on k8s themselves. I suppose this is just another extension of the read-only case where we simply press on w/o enabling security?

mark-vieira · 2021-07-02T14:26:22Z

FYI, I've added @pugnascotia as a reviewer for this since it touches startup scripts we'll want packaging tests for the scenarios described above (config is read-only, config is subsequently overwritten, etc).

jkakavas · 2021-07-02T15:24:35Z

How will we determine this? We can't really reliable know how ES is being deployed other than the packaging type. In the case where folks don't configure security for whatever reason I suppose it'll just be as you say, where we update the file on the first go, and then it'll get subsequently overwritten. I still contend that's a bit of an awkward behavior, as if I'm creating a new ES deployment and naively don't setup security, it'll come up iniitally with security enabled and then later be disbled again.

We can work on our heuristics on what doesn't trigger the auto-configuration but I assume we can never be 100% sure there is no (ab)use case we did not cover. There is xpack.security.enrollment.enabled which when set to false causes the auto-configuration to bail, the easiest solution will be to set this to the yaml file that is being provisioned by your CM.

I wasn't referring to our own orchestration solutions, just more generally, anyone deploying ES on k8s themselves. I suppose this is just another extension of the read-only case where we simply press on w/o enabling security?

My answer still applies, for other orchestration solutions. We press on without auto-configuring TLS and creating an elastic user password. Security is enabled by default.

jkakavas

LGTM. I added some comments for consideration but I'm happy for this to be merged as a CLI tool to get folks try it out in alpha and do subsequent review rounds when we introduce the changes to make this part of the startup of the node

jkakavas · 2021-07-20T08:19:32Z

...lugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/ConfigInitialNode.java

+    private static final int HTTP_CERTIFICATE_DAYS = 2 * 365;
+    private static final int HTTP_KEY_SIZE = 4096;
+
+    private final OptionSpec<Void> strictOption = parser.accepts("strict", "Error if auto config cannot be performed for any reason");


I'm torn on whether we need this. I think the user base we're targeting with this change will not know/want to set it, but the change is minimal and with no added complexity so +1 to keep it. Did you think of other cases where this would be helpful too?

...lugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/ConfigInitialNode.java

jkakavas · 2021-07-20T08:21:00Z

...lugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/ConfigInitialNode.java

+        return "Enter password for the elasticsearch keystore : ";
+    }
+
+    Terminal.Verbosity expectedNoopVerbosityLevel() {


why do we need this two, especially when the both return NORMAL ?

jkakavas · 2021-07-20T09:42:16Z

...lugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/ConfigInitialNode.java

+                throw new UserException(ExitCodes.CONFIG, null);
+            }
+            // If the node's yml configuration is not readable, most probably auto-configuration isn't run under the suitable user
+            if (false == Files.isReadable(ymlPath)) {


Why do we do this as an extra check and not as and additional or clause above ?

Different exit codes, but maybe the distinction is not important here.

jkakavas · 2021-07-20T09:49:02Z

...lugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/ConfigInitialNode.java

+        // but clients (configured manually, outside of the enrollment process) might indeed need it and
+        // it is impossible to use the keystore because it is password protected because it contains the key
+        try {
+            fullyWriteFile(instantAutoConfigDir, HTTP_AUTOGENERATED_CA_NAME + ".crt", false, stream -> {


It probably makes sense to make this world readable as it might be used by any process (that isn't necessarily run by the same user that runs elatsticsearch )

jkakavas · 2021-07-20T09:52:23Z

...lugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/ConfigInitialNode.java

+        // the HTTP CA PEM file is provided "just in case", the node configuration doesn't use it
+        // but clients (configured manually, outside of the enrollment process) might indeed need it and
+        // it is impossible to use the keystore because it is password protected because it contains the key
+        try {


nit: Can we move writing all files under the same block ?

...lugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/ConfigInitialNode.java

distribution/src/bin/elasticsearch-env

jkakavas · 2021-07-20T18:33:47Z

@elasticmachine update branch

elasticmachine · 2021-07-20T20:44:44Z

Pinging @elastic/es-delivery (Team:Delivery)

elasticmachine · 2021-07-20T20:44:45Z

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

…ck/security/cli/ConfigInitialNode.java Co-authored-by: Ioannis Kakavas <[email protected]>

BigPandaToo · 2021-07-21T01:43:29Z

...lugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/ConfigInitialNode.java

+                    bw.write(String.format(Locale.ROOT, "# %-79s #", autoConfigDate));
+                    // TODO add link to docs
+                    bw.newLine();
+                    bw.write("###################################################################################");


Don't we want to put an instruction here on how to recover (remove this section and enroll the node) if a new cluster was mistakenly configured instead of joining an existing one?

BigPandaToo

I added a comment about adding more information/recovery documentation into the yml, but it can be added later I think

albertzaharovits · 2021-07-21T05:01:18Z

@pugnascotia @mark-vieira we decided to repurposed this PR so instead of hooking security auto configuration into the node startup it is now a separate cmd line tool. We still want to have it hooked at node startup (and package installation), but I was unable to test it properly (packaging tests are no joke) in time for the 8 alpha release.
But we need to provide a way for the other stack components to have an easy way to start a node with Security enabled in a similar fashion to what they'll automatically get in 8.
This is a compromise PR.

mark-vieira · 2021-07-22T20:00:51Z

Do we not plan to add any unit tests for ConfigInitialNode? Is the intention to implement all coverage here in the form of packaging tests?

This introduces a new cmd line tool that generates the security configuration for a new node in a new cluster (as opposed to joining an existing cluster). The security configuration consists of TLS key and certificates, which are stored in a directory inside the config path, as well as settings appended to the elasticsearch.yml referencing the aforementioned certs.

albertzaharovits added 16 commits June 7, 2021 13:03

Auto config

85d2bd2

No security settings validation in the auto-config

d9c67d4

Transport certificates without password

db94ef8

Mess

9a67ac5

Merge branch 'master' into auto_conf_security

574bb3b

Merge branch 'master' into auto_conf_security

43c046d

Merge branch 'master' into auto_conf_security

972d478

Reasonable

30e8909

Without HTTPS

f7577c2

Almost, but without HTTPS, and without logging

0fa3654

WIP almost

30f6acf

Done draft

b49052d

precommit

489f91e

Merge branch 'master' into auto_conf_security

5509f0d

Revert access modifier

5b82c15

Check if data dir is empty

0f38a6b

albertzaharovits added >enhancement :Security/Security Security issues without another label v8.0.0 labels Jul 1, 2021

albertzaharovits requested review from BigPandaToo and jkakavas July 1, 2021 21:20

albertzaharovits self-assigned this Jul 1, 2021

mark-vieira added the :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts label Jul 1, 2021

mark-vieira requested a review from pugnascotia July 2, 2021 14:21

albertzaharovits added 8 commits July 20, 2021 07:46

Fix password reuse

353eb3e

Separate cmd line

f17a921

track new binaries in tests

d36d80c

Merge branch 'master' into auto_conf_security

f67354b

More revert of run as bin/elasticsearch

a9b4989

Try

3e42bff

Try 2

a25f022

Meh

00557ac

jkakavas approved these changes Jul 20, 2021

View reviewed changes

God bless

440aeab

Merge branch 'master' into auto_conf_security

ef69a79

albertzaharovits marked this pull request as ready for review July 20, 2021 20:44

elasticmachine added Team:Delivery Meta label for Delivery team Team:Security Meta label for security team labels Jul 20, 2021

Revert elasticsearch-env

c3be9a7

albertzaharovits changed the title ~~Auto configure security for the initial node~~ Configure ecurity for the initial node cli Jul 20, 2021

albertzaharovits changed the title ~~Configure ecurity for the initial node cli~~ Configure security for the initial node cli Jul 20, 2021

Update x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpa…

9abd975

…ck/security/cli/ConfigInitialNode.java Co-authored-by: Ioannis Kakavas <[email protected]>

BigPandaToo reviewed Jul 21, 2021

View reviewed changes

BigPandaToo approved these changes Jul 21, 2021

View reviewed changes

albertzaharovits merged commit b9bc7a6 into elastic:master Jul 21, 2021

elastic deleted a comment from jkakavas Jul 21, 2021

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

Configure security for the initial node cli #74868

Configure security for the initial node cli #74868

Uh oh!

Conversation

albertzaharovits commented Jul 1, 2021

Uh oh!

mark-vieira commented Jul 1, 2021

Uh oh!

mark-vieira commented Jul 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkakavas commented Jul 2, 2021

Uh oh!

mark-vieira commented Jul 2, 2021

Uh oh!

mark-vieira commented Jul 2, 2021

Uh oh!

jkakavas commented Jul 2, 2021

Uh oh!

jkakavas left a comment

Choose a reason for hiding this comment

Uh oh!

jkakavas Jul 20, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jkakavas Jul 20, 2021

Choose a reason for hiding this comment

Uh oh!

jkakavas Jul 20, 2021

Choose a reason for hiding this comment

Uh oh!

albertzaharovits Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

jkakavas Jul 20, 2021

Choose a reason for hiding this comment

Uh oh!

jkakavas Jul 20, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jkakavas commented Jul 20, 2021

Uh oh!

elasticmachine commented Jul 20, 2021

Uh oh!

elasticmachine commented Jul 20, 2021

Uh oh!

BigPandaToo Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

BigPandaToo left a comment

Choose a reason for hiding this comment

Uh oh!

albertzaharovits commented Jul 21, 2021

Uh oh!

mark-vieira commented Jul 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mark-vieira commented Jul 1, 2021 •

edited

Loading

mark-vieira commented Jul 22, 2021 •

edited

Loading