Add the Enroll Kibana API #72207

jkakavas · 2021-04-26T08:54:01Z

This change adds the Enroll Kibana API that allows a kibana instance to
configure itself to communicate with a secured elasticsearch cluster

This change adds the Enroll Client API that allows a client to configure itself to communicate with a secured elasticsearch cluster

elasticmachine · 2021-04-26T08:54:05Z

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

jkakavas · 2021-04-26T08:54:42Z

CC @azasypkin

.../core/src/main/java/org/elasticsearch/xpack/core/action/TransportClientEnrollmentAction.java

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

rest-api-spec/src/main/resources/rest-api-spec/api/security.enroll_client.json

BigPandaToo

Added few comments, mostly nit. Feel free to merge after you addressing them

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

client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java

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

...ain/java/org/elasticsearch/xpack/core/security/action/enrollment/ClientEnrollmentAction.java

...java/org/elasticsearch/xpack/security/action/enrollment/TransportClientEnrollmentAction.java

BigPandaToo

Left few comments and suggestions

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

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

...t/rest-high-level/src/main/java/org/elasticsearch/client/security/NodeEnrollmentRequest.java

client/rest-high-level/src/test/java/org/elasticsearch/client/SecurityIT.java

...java/org/elasticsearch/xpack/security/action/enrollment/TransportKibanaEnrollmentAction.java

...org/elasticsearch/xpack/security/action/enrollment/TransportKibanaEnrollmentActionTests.java

...java/org/elasticsearch/xpack/security/action/enrollment/TransportKibanaEnrollmentAction.java

BigPandaToo

LGTM

albertzaharovits

I've left a couple of points. They need to be addressed before merging.
Nonetheless I have approved. Do ping me if there's anything I should clarify.

...igh-level/src/test/java/org/elasticsearch/client/security/ClientErnollmentResponseTests.java

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

albertzaharovits · 2021-06-18T13:43:26Z

...java/org/elasticsearch/xpack/security/rest/action/enrollment/RestKibanaEnrollmentAction.java

+     * @param settings the node's settings
+     * @param licenseState the license state that will be used to determine if security is licensed
+     */
+    public RestKibanaEnrollmentAction(Settings settings, XPackLicenseState licenseState) {


Only if it is irking you as well, I would prefer using Enroll rather than Enrollment, when possible.

will do, no worries

...java/org/elasticsearch/xpack/security/action/enrollment/TransportKibanaEnrollmentAction.java

albertzaharovits · 2021-06-18T14:32:41Z

...java/org/elasticsearch/xpack/security/action/enrollment/TransportKibanaEnrollmentAction.java

+        ActionFilters actionFilters) {
+        super(KibanaEnrollmentAction.NAME, transportService, actionFilters, KibanaEnrollmentRequest::new);
+        this.environment = environment;
+        // Should we use a specific origin for this ? Are we satisfied with the auditability of the change password request as-is ?


I personally prefer a different origin, but this is by no means something I would hold this PR for. We've been very lax about origins.

...java/org/elasticsearch/xpack/security/action/enrollment/TransportKibanaEnrollmentAction.java

albertzaharovits · 2021-06-18T15:36:40Z

...java/org/elasticsearch/xpack/security/action/enrollment/TransportKibanaEnrollmentAction.java

+            client.execute(NodesInfoAction.INSTANCE, nodesInfoRequest, ActionListener.wrap(nodesInfoResponse -> {
+                final List<String> nodeList = new ArrayList<>();
+                for (NodeInfo nodeInfo : nodesInfoResponse.getNodes()) {
+                    nodeList.add(nodeInfo.getInfo(HttpInfo.class).getAddress().publishAddress().toString());


Publish address only exposes a single address (per node). Hopefully the one reachable from Kibana (but no way to tell for sure).

It is safer to show all the addresses, but that is not possible if we must also support binding to 0.0.0.0 (ie instead of showing publish addresses we can show bind addresses, but bind can be 0.0.0.0 which is not routable).

So there is no perfect solution. I would choose to show bind addresses because the setup script won't use 0.0.0.0. Your call.

Good catch. I didn't think this through as much as I should have

I would choose to show bind addresses because the setup script won't use 0.0.0.0

The only problem with this is docker where we bind on 0.0.0.0 actually. I 'll think this through on Monday and get back with a suggestion

Thought this through a little more:

Kibana already can communicate with an ES node ( it just sent us a request ). The idea here is that we return the http addresses for other nodes so that it might connect to other nodes if it wants/needs.

bind_address is always 0.0.0.0 in docker

I think publish_address is an appropriate compromise for this. We can, however, get bind_address and return those unless it is 0.0.0.0, when we would fallback to publish_address.

@azasypkin , @legrego ping for visibility and feedback.

It sounds like publish address is what we would want here. While there are no guarantees that Kibana will be able to reach ES at that address, the docs claim that the publish address must be reachable by all clients (if I understand correctly).

I'm not opposed to also including the bind address in the response if it's not 0.0.0.0, however.

Same here, returning of publish_address or publish_address + bind_address[] (excluding 0.0.0.0) sounds like a reasonable approach to me.

lockewritesdocs

Added some suggestions for updates to the Enroll Kibana API doc page.

lockewritesdocs · 2021-06-21T19:46:08Z

x-pack/docs/en/rest-api/security/enroll-kibana.asciidoc

+
+[[security-api-kibana-enrollment-prereqs]]
+==== {api-prereq-title}
+


Suggested change

* You must have the `enroll` <<privileges-list-cluster,cluster privilege>> to use this API.

I think that this privilege is the same as the enroll node API.

We are not adding the named privilege after all, but thanks this reminds me to remove the reference from the enroll node API !

x-pack/docs/en/rest-api/security/enroll-kibana.asciidoc

lockewritesdocs · 2021-06-21T19:56:56Z

x-pack/docs/en/rest-api/security/enroll-kibana.asciidoc

+  ]
+}
+--------------------------------------------------
+<1> The password for the `kibana_system` user


Suggested change

<1> The password for the `kibana_system` user

<1> The password for the `kibana_system` user.

x-pack/docs/en/rest-api/security/enroll-kibana.asciidoc

...curity/src/test/resources/org/elasticsearch/xpack/security/action/enrollment/README.asciidoc

jkakavas · 2021-06-22T06:40:04Z

@albertzaharovits I've addressed your feedback but 2 points ( getCertificate vs getCertificateChain and bind vs publish address ) which need a little more discussion

jkakavas · 2021-06-23T07:33:06Z

@albertzaharovits do you want to take another look please and see if something needs to be resolved/discussed more or we should move forward with merging this? 🙏

jkakavas · 2021-06-23T15:49:36Z

@albertzaharovits do you want to take another look please and see if something needs to be resolved/discussed more or we should move forward with merging this?

We went through remaining points in a zoom.

…d Kibana doesn't need those

StoreKeyConfig#x509Certificates was introduced in elastic#72207 in order to retrieve all certificates from the keystore used for HTTP TLS and then filter out to get the CA certificate. Since we explicitly add the CA key/cert as a PrivateKeyEntry on startup, we can iterate on the PrivateKeyEntry objects instead.

StoreKeyConfig#x509Certificates was introduced in #72207 in order to retrieve all certificates from the keystore used for HTTP TLS and then filter out to get the CA certificate. Since we explicitly add the CA key/cert as a PrivateKeyEntry on startup, we can iterate on the PrivateKeyEntry objects instead.

Add the Enroll Client API

85fcede

This change adds the Enroll Client API that allows a client to configure itself to communicate with a secured elasticsearch cluster

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

jkakavas requested review from BigPandaToo and albertzaharovits April 26, 2021 08:54

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

jkakavas commented Apr 26, 2021

View reviewed changes

.../core/src/main/java/org/elasticsearch/xpack/core/action/TransportClientEnrollmentAction.java Outdated Show resolved Hide resolved

jkakavas added 15 commits April 26, 2021 12:05

fix doc test skip

6e1333c

fix typos

48a5bde

remove unused import

429186b

fix logger usage

1eeafe4

fix validation and tests

9b3ca8a

fix test and add api as operator only

db3714c

Merge remote-tracking branch 'origin/master' into enroll-client-api

99e79db

Move API to the security plugin

516f7a3

Read HTTP CA and key from they keystore used for the HTTP layer

bc7e265

Change API registered endpoint

cf8e26d

fixes

5ab6222

Merge remote-tracking branch 'origin/master' into enroll-client-api

c8c1c63

Disable tests that fail

5b3a62e

revert unwanted change

b3c1f46

Merge remote-tracking branch 'origin/master' into enroll-client-api

1aea706

jkakavas commented Apr 29, 2021

View reviewed changes

client/rest-high-level/src/main/java/org/elasticsearch/client/ClusterClient.java Outdated Show resolved Hide resolved

sethmlarson reviewed Apr 29, 2021

View reviewed changes

rest-api-spec/src/main/resources/rest-api-spec/api/security.enroll_client.json Outdated Show resolved Hide resolved

sethmlarson reviewed Apr 29, 2021

View reviewed changes

rest-api-spec/src/main/resources/rest-api-spec/api/security.enroll_client.json Show resolved Hide resolved

address feedback

e3a5e02

BigPandaToo reviewed May 6, 2021

View reviewed changes

jkakavas requested a review from lockewritesdocs June 17, 2021 12:14

BigPandaToo suggested changes Jun 17, 2021

View reviewed changes

jkakavas added 2 commits June 18, 2021 09:51

address feedback

041a9d3

Merge remote-tracking branch 'origin/master' into enroll-client-api

2c6d13a

jkakavas commented Jun 18, 2021

View reviewed changes

...java/org/elasticsearch/xpack/security/action/enrollment/TransportKibanaEnrollmentAction.java Outdated Show resolved Hide resolved

jkakavas requested a review from BigPandaToo June 18, 2021 12:36

BigPandaToo approved these changes Jun 18, 2021

View reviewed changes

albertzaharovits approved these changes Jun 18, 2021

View reviewed changes

lockewritesdocs reviewed Jun 21, 2021

View reviewed changes

jkakavas added 3 commits June 22, 2021 09:14

partially address feedback

0e3c305

address docs feedback

69324d8

don't zerofill array

4957eb0

jkakavas requested a review from albertzaharovits June 22, 2021 06:40

unused import

c90ab22

Merge remote-tracking branch 'origin/master' into enroll-client-api

30bc6af

jkakavas added 3 commits June 23, 2021 19:32

Removed addresses from the Enroll Kibana API response as we determine…

06f0ff7

…d Kibana doesn't need those

update fingerprint as we're updating the CA certificate in httpCa.o12

09f8256

Merge remote-tracking branch 'origin/master' into enroll-client-api

3b27062

jkakavas merged commit 82e7fbd into elastic:master Jun 23, 2021

jkakavas deleted the enroll-client-api branch June 23, 2021 19:58

jkakavas mentioned this pull request Jun 24, 2021

Calculate SHA256 fingerprint for enrollment token #74511

Merged

jkakavas mentioned this pull request Jul 1, 2021

Remove redundant method from StoreKeyConfig #74836

Merged

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


		[[security-api-kibana-enrollment-prereqs]]
		==== {api-prereq-title}


	* You must have the `enroll` <<privileges-list-cluster,cluster privilege>> to use this API.

	<1> The password for the `kibana_system` user
	<1> The password for the `kibana_system` user.

Add the Enroll Kibana API #72207

Add the Enroll Kibana API #72207

Uh oh!

Conversation

jkakavas commented Apr 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Apr 26, 2021

Uh oh!

jkakavas commented Apr 26, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BigPandaToo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BigPandaToo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BigPandaToo left a comment

Choose a reason for hiding this comment

Uh oh!

albertzaharovits left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lockewritesdocs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

jkakavas commented Apr 26, 2021 •

edited

Loading