Display security auto-configuration with fancy unicode #82740

albertzaharovits · 2022-01-18T15:46:00Z

~~TODO:~~
* detect if the banner PrintStream encoding supports the fancy unicode codepoints (otherwise, fallback to simpler variants)
* detect (expose) the terminal width, in order to construct the border to fill the width of the terminal
* make sure information doesn't use jansi when it is disabled (there are multiple ways to disable jansi)

Edited 24.01.2022

This PR slightly improves the format of the security auto-configuration information, which is printed on the terminal when
the initial cluster node first starts up.

It uses eye-catching Unicode characters for bullet points.
It also uses Unicode to display a continuous border, for the whole width of the terminal, before and after the information.
In addition, it uses ANSI escape sequences to render some of the information in bold fonts.

It will fallback to using regular characters if the JVM is set-up with a non-UTF encoding for the standard out.

Related: #82364

albertzaharovits · 2022-01-18T16:21:16Z

FWIW here are some sample outputs, for the time being, with a large terminal width:

Vanilla configuration (when no node enrollment token is generated):

When the enrollment token is generated (because network.host: "0.0.0.0"):

cc @jkakavas @bytebilly @lockewritesdocs

...ity/src/main/java/org/elasticsearch/xpack/security/InitialNodeSecurityAutoConfiguration.java

jkakavas · 2022-01-19T10:13:07Z

Looks good to me @albertzaharovits thanks! I think this moves the needle quite a bit, helping the output to stand out. I'd only suggest a few more (5?10?) empty lines before and after to make it more pronounced that this is a standalone section and not logs

jkakavas · 2022-01-19T10:18:32Z

This is how it looks on my machine and with a darker background

bytebilly · 2022-01-19T10:25:40Z

Thanks Albert, looks good!
I'm not sure what happens on "old" terminals or similar ones that don't support fancy output, do you have an idea?

albertzaharovits · 2022-01-19T15:41:03Z

@bytebilly

I'm not sure what happens on "old" terminals or similar ones that don't support fancy output, do you have an idea?

I still need to implement a fallback for that, as per:

detect if the banner PrintStream encoding supports the fancy unicode codepoints (otherwise, fallback to simpler variants)

More exactly the issue is whether the JVM's character encoding that's used to print the information to the terminal "can encode" the special unicode code points (this is what I need to code still), eg UTF-8 can, Latin-1 can not.

But even in the case that the JVM is favorably configured, there is still some potential for funny characters (ie boxes and question marks):

if the terminal is configured with a different "locale" from the ES JVM
if the installed font doesn't accurately represent the glyphs (eg the "info" box in Ioannis' linux print screen, is very different than what I get on my mac).

The above two we cannot detect.

elasticmachine · 2022-01-24T11:59:10Z

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

albertzaharovits · 2022-01-24T12:36:43Z

Things I've noticed in the message, that maybe we could improve:

In the instruction on running docker ES nodes: docker run --name <node-name> -p <host-http-port>:9200 --net elastic -e "ENROLLMENT_TOKEN=<token>" docker.elastic.co/elasticsearch/elasticsearch:8.1.0 , the --name <node-name> -p <host-http-port>:9200 --net elastic part is not strictly necessary, the options are maybe relevant in the scenarios in the docs, but in my context they look like an unnecessary complication.
No equivalent docker command is displayed for running Kibana in docker.
Adding other ES nodes requires adjusting the network.host, or transport.host setting before starting the node with the enrollment token, but this is omitted (prob intentionally) from the instructions.

jkakavas · 2022-01-24T13:00:14Z

In the instruction on running docker ES nodes: docker run --name -p :9200 --net elastic -e "ENROLLMENT_TOKEN=" docker.elastic.co/elasticsearch/elasticsearch:8.1.0 , the --name -p :9200 --net elastic part is not strictly necessary, the options are maybe relevant in the scenarios in the docs, but in my context they look like an unnecessary complication.

Agreed. I think that part of the struggle here is to add just enough information. We don't want to replicate the entirety of a relevant docs page but we do want to add some information that we believe is helpful. I was wary of adding anything related to docker but if we're not able to determine whether we're running in Docker and given the % of folks that will run this in Docker , I'm ++ to having something there. I agree with your suggestion to simplify the command though, maybe simply docker run -e "ENROLLMENT_TOKEN=<token>" docker.elastic.co/elasticsearch/elasticsearch:8.1.0 is enough ?

No equivalent docker command is displayed for running Kibana in docker.

Personally this falls into the too much information category for the startup message but I'm happy to change my mind.

Adding other ES nodes requires adjusting the network.host, or transport.host setting before starting the node with the enrollment token, but this is omitted (prob intentionally) from the instructions.

Yes this is a tricky one. We could assume that if the initial node is bound to not localhost, then the other nodes will be on other hosts for sure and so they'd need to change network.host or transport.host and print a relevant message, or prefix this with an if.. but then again this sounds like a documentation page to me. Interested to here other thoughts on the matter though

jkakavas · 2022-01-24T13:04:04Z

Yes this is a tricky one. We could assume that if the initial node is bound to not localhost, then the other nodes will be on other hosts for sure and so they'd need to change network.host or transport.host and print a relevant message, or prefix this with an if.. but then again this sounds like a documentation page to me. Interested to here other thoughts on the matter though

@bytebilly had caught this in the feature assurance doc but we failed to address this. :doh: I think we should treat it as a bug and fix it. We get the transport addresses from the node we call out to during enrollment and we figure out all IP Addresses we can know of ( for the SANs ) so we could possibly set transport.host to site IF any of the IP addresses that we get as existing cluster members is not in the ones we figured out for the host. I'll open a small PR

jkakavas

Output looks much nicer, added some suggestions and comments but LGTM in general. Happy to discuss some more about text etc but we can always iterate, this moves the needle significantly already!

...ity/src/main/java/org/elasticsearch/xpack/security/InitialNodeSecurityAutoConfiguration.java

distribution/tools/ansi-console/src/main/java/org/elasticsearch/io/ansi/AnsiConsoleLoader.java

jkakavas · 2022-01-24T13:43:48Z

...ution/tools/ansi-console/src/test/java/org/elasticsearch/io/ansi/AnsiConsoleLoaderTests.java

+            assertThat(console.width().get(), is(width));
+        }
+    }
+


Is there a test we can write that would fail if tryExtractPrintCharset fails with a reflection related error ? Could be useful for upgrades

Hmm, I don't have good ideas. The method would fail if the classes in the JANSI lib change structure, following an upgrade, but we can't simulate that in a test.

We do not have tests asserting the auto-configuration message to the terminal.
If we would, we could assert the unicode formatting on at least some platforms.

We assert the message to the terminal in ArchiveGenerateInitialCredentialsTests for some simple things but we need to adjust and fix these tests ( currently muted ) and we can add additional cases for sure

Good, I missed those!

TBC, when those tests are unmuted, I believe we can assert there that the displayed information contains unicode chars (on some platforms, eg not Windows), which also implies that reflection worked.

albertzaharovits · 2022-01-24T15:25:24Z

Okay, this is what it looks like after the above suggestions:

Unless @lockewritesdocs has other suggestions, I'm going to merge this as is.

FWIW we also add color to the text.

albertzaharovits · 2022-01-24T17:32:05Z

@elasticmachine run elasticsearch-ci/part-2-fips

lockewritesdocs · 2022-01-24T17:55:08Z

Unless @lockewritesdocs has other suggestions, I'm going to merge this as is.

I agree with @jkakavas in his previous comment. I do think that we need a separate line for enrolling a new node in Docker, but just enough to get the user up and running. This is only a startup message, and we'll rely on the documentation for more instructive, how-to information.

No equivalent docker command is displayed for running Kibana in docker.

I'm torn on this one. For many users, the current startup message will meet their needs -- they're able to start Elasticsearch with security enabled and can enroll additional nodes. I agree with @jkakavas that we don't want to clutter the startup message unnecessarily, but would we consider adding a link to the docs for Installing Kibana in Docker, which includes the necessary steps? For example:

If you're running in Docker, copy the enrollment token and run:
docker run -e \"ENROLLMENT_TOKEN=<token>\" docker.elastic.co/elasticsearch/elasticsearch:<version>

To enroll Kibana, refer to https://www.elastic.co/guide/en/kibana/8.0/docker.html.

jkakavas · 2022-01-24T17:59:33Z

but would we consider adding a link to the docs for Installing Kibana in Docker, which includes the necessary steps? For example:

My personal preference would be to not add this.
If we're adding any links here, I'd much rather simplify the existing text even more and then add a link to a ( still simple but longer than can fit in a startup message single ) doc page, but we can iterate on this in a follow up

lockewritesdocs · 2022-01-24T18:05:02Z

My personal preference would be to not add this.

I'm 👍 with not adding the link. Just wanted to have the discussion and see where we landed.

lockewritesdocs

👍 @albertzaharovits! Much easier to scan and read.

albertzaharovits · 2022-01-24T18:51:08Z

For test failure https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+pull-request+part-2-fips/361/ I've raised #82977 . Will be merging this after that.

albertzaharovits · 2022-01-24T20:28:33Z

@elasticmachine update branch

This PR slightly improves the format of the security auto-configuration information that is printed on the terminal when the initial cluster node first starts up. It uses eye-catching Unicode characters for bullet points. It also uses Unicode to display a continuous border, for the whole width of the terminal, before and after the information. In addition, it uses ANSI escape sequences to render some of the information in bold fonts. It will fallback to using regular characters if the JVM is set-up with a non-UTF encoding for the standard out.

jkakavas · 2022-01-25T09:12:27Z

I realized ( late enough :) ) that we might want to add some more whitespace after the unicode characters so that it looks better in many terminals. Easy to do in a short followup though :

This PR slightly improves the format of the security auto-configuration information that is printed on the terminal when the initial cluster node first starts up. It uses eye-catching Unicode characters for bullet points. It also uses Unicode to display a continuous border, for the whole width of the terminal, before and after the information. In addition, it uses ANSI escape sequences to render some of the information in bold fonts. It will fallback to using regular characters if the JVM is set-up with a non-UTF encoding for the standard out.

albertzaharovits added 5 commits January 18, 2022 14:05

WIP

ff73d15

Merge branch 'master' into unicode-banner-display

ace158f

terminal length still not detected

8d56819

Spotless

1a674d4

Nit

f796b57

albertzaharovits requested a review from jkakavas January 18, 2022 15:46

albertzaharovits self-assigned this Jan 18, 2022

elasticsearchmachine added the v8.1.0 label Jan 18, 2022

albertzaharovits added :Security/Security Security issues without another label v8.0.0 >bug labels Jan 18, 2022

albertzaharovits changed the title ~~Unicode auto-configuration display~~ Display security auto-configuration with fancy unicode Jan 18, 2022

jkakavas reviewed Jan 19, 2022

View reviewed changes

...ity/src/main/java/org/elasticsearch/xpack/security/InitialNodeSecurityAutoConfiguration.java Outdated Show resolved Hide resolved

albertzaharovits added 9 commits January 20, 2022 06:54

Merge branch 'master' into unicode-banner-display

b0ab407

Border length the width of the terminal

4951602

Merge branch 'master' into unicode-banner-display

089e552

Merge branch 'master' into unicode-banner-display

29b9f79

Detect console charset and fallback

d98599c

suppress forbidden

5ab6a29

optionally disable ansi escape codes

7148258

Nit

04853a3

Some tests

e61226a

albertzaharovits marked this pull request as ready for review January 24, 2022 11:59

elasticmachine added the Team:Security Meta label for security team label Jan 24, 2022

jkakavas approved these changes Jan 24, 2022

View reviewed changes

albertzaharovits mentioned this pull request Jan 24, 2022

Investigate using UTF-8 console output in docker #82939

Open

albertzaharovits added 3 commits January 24, 2022 16:16

Nits

4686054

Merge branch 'master' into unicode-banner-display

7e2085f

Nits

aa73106

Spotless

d0aeb74

lockewritesdocs approved these changes Jan 24, 2022

View reviewed changes

Merge branch 'master' into unicode-banner-display

3d74a79

albertzaharovits merged commit 6687a28 into elastic:master Jan 25, 2022

albertzaharovits deleted the unicode-banner-display branch January 25, 2022 09:00

albertzaharovits mentioned this pull request Jan 25, 2022

Display security auto-configuration with fancy unicode (#82740) #83015

Merged

albertzaharovits mentioned this pull request Jan 25, 2022

Bind to non-localhost for transport in some cases #82973

Merged

jkakavas mentioned this pull request Jan 28, 2022

Better visibility for security auto-configuration information on startup #82364

Closed

pugnascotia added v8.0.0-rc2 and removed v8.0.0 labels Feb 1, 2022

ChrisHegarty mentioned this pull request Feb 15, 2022

Examine usage of setAccessible in AnsiConsoleLoader #83928

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display security auto-configuration with fancy unicode #82740

Display security auto-configuration with fancy unicode #82740

albertzaharovits commented Jan 18, 2022 •

edited

Loading

albertzaharovits commented Jan 18, 2022

jkakavas commented Jan 19, 2022

jkakavas commented Jan 19, 2022

bytebilly commented Jan 19, 2022

albertzaharovits commented Jan 19, 2022

elasticmachine commented Jan 24, 2022

albertzaharovits commented Jan 24, 2022

jkakavas commented Jan 24, 2022

jkakavas commented Jan 24, 2022 •

edited

Loading

jkakavas left a comment

jkakavas Jan 24, 2022

albertzaharovits Jan 24, 2022

jkakavas Jan 24, 2022

albertzaharovits Jan 24, 2022

albertzaharovits Jan 24, 2022 •

edited

Loading

albertzaharovits commented Jan 24, 2022

albertzaharovits commented Jan 24, 2022

lockewritesdocs commented Jan 24, 2022

jkakavas commented Jan 24, 2022

lockewritesdocs commented Jan 24, 2022

lockewritesdocs left a comment

albertzaharovits commented Jan 24, 2022

albertzaharovits commented Jan 24, 2022

jkakavas commented Jan 25, 2022

Display security auto-configuration with fancy unicode #82740

Display security auto-configuration with fancy unicode #82740

Conversation

albertzaharovits commented Jan 18, 2022 • edited Loading

albertzaharovits commented Jan 18, 2022

jkakavas commented Jan 19, 2022

jkakavas commented Jan 19, 2022

bytebilly commented Jan 19, 2022

albertzaharovits commented Jan 19, 2022

elasticmachine commented Jan 24, 2022

albertzaharovits commented Jan 24, 2022

jkakavas commented Jan 24, 2022

jkakavas commented Jan 24, 2022 • edited Loading

jkakavas left a comment

Choose a reason for hiding this comment

jkakavas Jan 24, 2022

Choose a reason for hiding this comment

albertzaharovits Jan 24, 2022

Choose a reason for hiding this comment

jkakavas Jan 24, 2022

Choose a reason for hiding this comment

albertzaharovits Jan 24, 2022

Choose a reason for hiding this comment

albertzaharovits Jan 24, 2022 • edited Loading

Choose a reason for hiding this comment

albertzaharovits commented Jan 24, 2022

albertzaharovits commented Jan 24, 2022

lockewritesdocs commented Jan 24, 2022

jkakavas commented Jan 24, 2022

lockewritesdocs commented Jan 24, 2022

lockewritesdocs left a comment

Choose a reason for hiding this comment

albertzaharovits commented Jan 24, 2022

albertzaharovits commented Jan 24, 2022

jkakavas commented Jan 25, 2022

albertzaharovits commented Jan 18, 2022 •

edited

Loading

jkakavas commented Jan 24, 2022 •

edited

Loading

albertzaharovits Jan 24, 2022 •

edited

Loading