Enroll node API #72129

jkakavas · 2021-04-22T19:08:30Z

Enroll node API can be used by new nodes in order to join an
existing cluster that has security features enabled. The response
of a call to this API contains all the necessary information that
the new node requires in order to configure itself and bootstrap
trust with the existing cluster.

Enroll node API can be used by new nodes in order to join an existing cluster that has security features enabled. The response of a call to this API contains all the necessary information that the new node requires in order to configure itself and bootstrap trust with the existing cluster.

elasticmachine · 2021-04-22T19:08:34Z

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

docs/reference/cluster/enroll-node.asciidoc

docs/reference/cluster.asciidoc

BigPandaToo · 2021-04-23T19:55:28Z

Just added couple comments, but will take a closer look on Monday

API can be called by the startup process or a user with appropriate privileges while elasticsearch is in the enrollment mode to obtain an enrollment token used to enroll a new node to the cluster or configure a new client to communicate with the cluster. Resolve: elastic#71438 Related: elastic#72129

jkakavas · 2021-04-26T06:30:12Z

~~HLRC changes incoming~~ added in a444410

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java

… as ASN.1 DER encoded, Base64 encoded strings instead of PKCS12. PKCS12 makes sense for storage on disk, but not that much as an intermediate format for transmission

jkakavas · 2021-05-05T16:34:40Z

@elasticmachine update branch

...n/java/org/elasticsearch/xpack/security/action/enrollment/TransportNodeEnrollmentAction.java

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/StoreKeyConfig.java

client/rest-high-level/src/main/java/org/elasticsearch/client/SecurityClient.java

albertzaharovits · 2021-05-06T05:54:03Z

...n/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java

        "cluster:admin/repository/analyze/blob",
-        "cluster:admin/repository/analyze/blob/read"
+        "cluster:admin/repository/analyze/blob/read",
+        NodeEnrollmentAction.NAME


Does this mean that invoking client-soon-to-be-node has to be an operator?
If it's using an API key it is not currently possible. Even if it where, it doesn't sound to useful. When we release the enroll credential we explicitly want any client with that credential to be able to use it to join the cluster.

The idea is that this should not be used in ESS, even if someone is superuser or grants themselves the enroll privilege. We don't "support" operator privileges in any other circumstance, so I don't see this as a problem.

If it's using an API key it is not currently possible.

I've missed that part, will take a look

Chatted a little with Yang. Wasn't aware of the API key limitation for operator only. In retrospect, we don't actually need the API keys to be operator only and since the enrollment mode settings is not going to be a dynamic one, then it can't also be operator only. I think we'd need to depend on setting the enrollment mode to false on ESS instances and not whitelist the setting so that administrators cannot overwrite it in ESS deployment settings

To check that I'm following, you're proposing introducing a setting that will "enable" the enroll node API.
The setting is static and it must be false (API not available) for ESS. Am I right? Can it be false by default, so that the new startup scripts (the ones generating or consuming the enrollment tokens) have to set it to true?

I agree with the setting proposal, and we can discuss if it is static, or dynamic for operators only, as well as its default value.

It doesn't have to be part of this PR, but raise an issue with your thoughts, please.

We synced:

The enrollment mode enabled setting is static ( introduced in Create enrollment token API #72186)

The default value is false and we will be setting it to true while/when auto-generating configuration

We're introducing a base rest handler for all enrollment APIs that check that security is enabled and that enrollment mode is enabled, and return an error if not.

Setting wlll not be whitelisted in ESS so users won't be able to enable it.

.../main/java/org/elasticsearch/xpack/core/security/action/enrollment/NodeEnrollmentAction.java

albertzaharovits · 2021-05-06T06:07:32Z

client/rest-high-level/src/main/java/org/elasticsearch/client/SecurityClient.java

+
+
+    /**
+     * Allows a node to join to a secured cluster using the Enroll Node API.


I would highlight that this API allows the caller to stand-up a node that is able to join the secure cluster, which also means that the caller has the capacity to "impersonate" the cluster to any other clients.

I'll add something to that effect. I think the most important part is to clearly call this out in the privilege description but no objection to adding information in the APIs description

@albertzaharovits can you elaborate on "which also means that the caller has the capacity to "impersonate" the cluster to any other clients. " ? What specific threat do you have in mind ?

An astute user might infer that this endpoint is very sensitive, given that a rogue node can siphon all the data from the cluster (even this, I think, is worth mentioning somewhere).
But it's more than this, because the caller doesn't have to join the cluster (which might be otherwise monitored, and the attacker be detected), it can use the API response to forge himself certificates that can impersonate any cluster node to a client.

@albertzaharovits worth noting that an attacker needs also to be in a position to MITM legitimate clients in order to impersonate the server ( assuming that by "impersonate any node" you mean that the caller of the API can get a valid signed certificate signed by the same CA that would have signed the legitimate nodes certificates )

@jkakavas One must pass an enroll token to clients to have them configured; the same enroll token that's used to register nodes. There can be an attacker that intercepts client-cluster communication.

Yes that is my argument too, one needs to be MITM. I'm not trying to downplay the attack, just noting the constraints

albertzaharovits · 2021-05-06T06:34:31Z

...n/java/org/elasticsearch/xpack/security/action/enrollment/TransportNodeEnrollmentAction.java

+            final List<String> nodeList = new ArrayList<>();
+            for (DiscoveryNode node : clusterService.state().getNodes()) {
+                nodeList.add(node.getAddress().toString());
+            }


What do you think if we use the output of NodeEnrollmentAction.NAME with the transport metric parameter?
It gives me more confidence that we're returning the right addresses, and the client might already be used to parsing that.

I'm not sure I understood the suggestion @albertzaharovits .You mean that we should return enroll:enroll_node:192.168.1.2:9300 instead of 192.168.1.2:9300 ? What does this buy us ?

Bleah... sorry, I messed up the clipboard buffers.
I meant NodeEnrollmentAction.NAME -> NodesInfoAction.NAME .

Specifically, I mean that the nodes info API internally uses

elasticsearch/server/src/main/java/org/elasticsearch/transport/TransportInfo.java

Line 88 in f379216

private String formatPublishAddressString(String propertyName, TransportAddress publishAddress) {

.

makes sense, I'm calling this for the enroll client API transport action, might as well do this here too, no objections

albertzaharovits

I had a couple of comments, but overall it does what it's supposed to do.
We can discuss it a bit, but we might need to polish it when all the pieces are in-place, so lets not overdo it now.

BigPandaToo

Approoving considering you address Albert's comments

jkakavas · 2021-05-06T18:41:53Z

Thanks for the round @albertzaharovits . I addressed your feedback or commented accordingly, if we could do another round on Monday before I go, that'd be awesome <3

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/StoreKeyConfig.java

albertzaharovits · 2021-05-10T18:53:27Z

Provided you find resolve the remaining comments.

jkakavas · 2021-05-10T21:52:53Z

Thanks for the round @albertzaharovits , I'll merge this if CI is green before I go, otherwise please do merge at will

Method to be called by the startup process while elasticsearch is in the enrollment mode to obtain an enrollment token used to enroll a new node to the cluster. Resolve: elastic#71438 Related: elastic#72129

enrollment mode to obtain an enrollment token used to enroll a new node to the cluster. Resolve: elastic#71438 Related: elastic#72129

* Create enrollment token Method to be called by the startup process while elasticsearch is in the enrollment mode to obtain an enrollment token used to enroll a new node to the cluster. Resolve: #71438 Related: #72129

jkakavas added 2 commits April 22, 2021 21:49

fix doc reference

dad583d

jkakavas added >enhancement :Security/Security Security issues without another label v8.0.0 labels Apr 22, 2021

jkakavas requested review from BigPandaToo and albertzaharovits April 22, 2021 19:08

elasticmachine added the Team:Security Meta label for security team label Apr 22, 2021

jkakavas commented Apr 22, 2021

View reviewed changes

docs/reference/cluster/enroll-node.asciidoc Outdated Show resolved Hide resolved

Use the correct API method

5f0a02b

BigPandaToo reviewed Apr 23, 2021

View reviewed changes

docs/reference/cluster/enroll-node.asciidoc Outdated Show resolved Hide resolved

docs/reference/cluster.asciidoc Outdated Show resolved Hide resolved

BigPandaToo mentioned this pull request Apr 23, 2021

Create enrollment token API #72186

Closed

jkakavas added 4 commits April 26, 2021 13:18

fix typo

7c52561

add HLRC and fix docs test

a444410

add action as operator only

9ae6ce7

Merge remote-tracking branch 'origin/master' into node-enrollement-api

d7e21bf

ywangd reviewed Apr 27, 2021

View reviewed changes

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java Outdated Show resolved Hide resolved

jkakavas added 5 commits April 29, 2021 11:00

Move API to the security plugin

fe7e92a

Merge remote-tracking branch 'origin/master' into node-enrollement-api

61bcc42

move API to security plugin. Change API specification to return files…

7db5a3b

… as ASN.1 DER encoded, Base64 encoded strings instead of PKCS12. PKCS12 makes sense for storage on disk, but not that much as an intermediate format for transmission

fix

4b9bbcc

Mute test temporarily

990517a

Merge branch 'master' into node-enrollement-api

0b249cd