Retain reference to stdout for exceptional cases #77460
Conversation
In exceptional cases, there is a need for the ES process to print to the user's "console" without the output appearing in log files. An example is sensitive information such as the initial password for an administrative user. In these cases we would like to print to System.out instead of using log4j. However, we intentionally redirect stdout to go a log4j logger, because that is the preferred place to capture the sorts of messages that are typically printed to System.out This change introduces a stashed reference to the original stdout PrintWriter before we redirect to log4g in BootstrapInfo, that can be used in said cases. It also updates the relevant code that was printing the sensitive information to the log, to use this newly introduced reference.
jkakavas
added
>enhancement
:Core/Infra/Core
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
| @Override | ||
| public void accept(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) { | ||
| final PrintStream out = BootstrapInfo.getOriginalStandardOut(); | ||
| if (null == out) { |
There was a problem hiding this comment.
How is this case possible? We could add a not null assertion when setting in BootstrapInfo?
There was a problem hiding this comment.
Yes, you are right. There should be nothing else calling BootstrapInfo#init and there is no setter for originalStandardOut so I think we are ok ?
| LOGGER.info("-----------------------------------------------------------------"); | ||
| LOGGER.info(""); | ||
| private void outputOnSuccess(SecureString elasticPassword, SecureString kibanaSystemPassword, PrintStream out) { | ||
| out.println(); |
There was a problem hiding this comment.
The stream could still have been closed, in which case we are not attached to a terminal. So we need to catch that case, and probably log a warning?
There was a problem hiding this comment.
Yes, I'll check this first before even generating the passwords. If we can't show them, there is no need to even generate them
| @Override | ||
| public void accept(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) { | ||
| final PrintStream out = BootstrapInfo.getOriginalStandardOut(); | ||
| // Check if it has been closed, try to write something so that we trigger PrintStream#ensureOpen |
There was a problem hiding this comment.
Not sure if there is any other way to check if stdout is closed
|
@elasticmachine update branch |
|
If both of you are happy with this, @tvernum you can merge this at will |
|
|
||
| /** | ||
| * This method is invoked by {@link Elasticsearch#main(String[])} to startup elasticsearch. | ||
| */ |
There was a problem hiding this comment.
We should isolate this, otherwise other forbidden uses can creep into bootstrap init.
| // Check if it has been closed, try to write something so that we trigger PrintStream#ensureOpen | ||
| out.println(); | ||
| if (out.checkError()) { | ||
| outputOnError(new IllegalStateException("Stashed standard output stream is closed.")); |
There was a problem hiding this comment.
Is this really an illegal state? It just means that we’ve closed the streams, which is normal when not attached to a console.
|
@elasticmachine update branch |
|
Turned out to be a Github UI hiccup, so I just ended up re-running a full CI round for no reason 🎉 |
4 participants
In exceptional cases, there is a need for the ES process to print to
the user's "console" without the output appearing in log files.
An example is sensitive information such as the initial password for
an administrative user.
In these cases we would like to print to System.out instead of using
log4j.
However, we intentionally redirect stdout to go a log4j logger,
because that is the preferred place to capture the sorts of messages
that are typically printed to System.out
This change introduces a stashed reference to the original stdout
PrintWriter before we redirect to log4g in BootstrapInfo, that
can be used in said cases.
It also updates the relevant code that was printing the sensitive
information to the log, to use this newly introduced reference.
Supersedes: #77299
Co-authored-by: Tim Vernum [email protected]