Skip to content

Retain reference to stdout for exceptional cases #77460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jkakavas merged 9 commits into from
Sep 9, 2021
Merged

Retain reference to stdout for exceptional cases #77460

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6d5c2fd
Retain reference to stdout for exceptional cases
jkakavas Sep 8, 2021
2f221b4
Add package tests
jkakavas Sep 8, 2021
0152f99
address feedback:
jkakavas Sep 8, 2021
bc256ed
Merge branch 'master' into print-console
elasticmachine Sep 8, 2021
ec3f566
further feedback
jkakavas Sep 9, 2021
330320b
Merge branch 'master' into print-console
elasticmachine Sep 9, 2021
dfe8e88
Merge branch 'print-console' of github.com:jkakavas/elasticsearch int…
jkakavas Sep 9, 2021
d4fa04c
Fix tests
jkakavas Sep 9, 2021
5087a48
Merge remote-tracking branch 'origin/master' into print-console
jkakavas Sep 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions ...s/src/test/java/org/elasticsearch/packaging/test/ArchiveGenerateInitialPasswordTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,35 @@ public void test30NoAutoGenerationWhenAutoConfigurationDisabled() throws Excepti
assertThat(usersAndPasswords.isEmpty(), is(true));
}

public void test40VerifyAutogeneratedCredentials() throws Exception {
public void test40NoAutogenerationWhenDaemonizedWithTty() throws Exception {
/* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */
assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS);
stopElasticsearch();
ServerUtils.enableSecurityAutoConfiguration(installation);
Shell.Result result = runElasticsearchStartCommand(null, true, true);
Map<String, String> usersAndPasswords = parseUsersAndPasswords(result.stdout);
assertThat(usersAndPasswords.isEmpty(), is(true));
}

public void test50NoAutogenerationWhenNotDaemonizedWithoutTty() throws Exception {
/* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */
assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS);
stopElasticsearch();
Shell.Result result = runElasticsearchStartCommand(null, false, false);
Map<String, String> usersAndPasswords = parseUsersAndPasswords(result.stdout);
assertThat(usersAndPasswords.isEmpty(), is(true));
}

public void test60NoAutogenerationWhenDaemonizedWithoutTty() throws Exception {
/* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */
assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS);
stopElasticsearch();
Shell.Result result = runElasticsearchStartCommand(null, true, false);
Map<String, String> usersAndPasswords = parseUsersAndPasswords(result.stdout);
assertThat(usersAndPasswords.isEmpty(), is(true));
}

public void test70VerifyAutogeneratedCredentials() throws Exception {
/* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */
assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS);
stopElasticsearch();
Expand All @@ -81,7 +109,7 @@ public void test40VerifyAutogeneratedCredentials() throws Exception {
}
}

public void test50PasswordAutogenerationOnlyOnce() throws Exception {
public void test80PasswordAutogenerationOnlyOnce() throws Exception {
/* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */
assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS);
stopElasticsearch();
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ static void init(
final Environment initialEnv) throws BootstrapException, NodeValidationException, UserException {
// force the class initializer for BootstrapInfo to run before
// the security manager is installed
BootstrapInfo.init();
BootstrapInfo.init(System.out);

INSTANCE = new Bootstrap();

Expand Down
11 changes: 10 additions & 1 deletion server/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.elasticsearch.core.SuppressForbidden;

import java.io.PrintStream;
import java.util.Dictionary;
import java.util.Enumeration;

Expand All @@ -19,6 +20,8 @@
@SuppressForbidden(reason = "exposes read-only view of system properties")
public final class BootstrapInfo {

private static PrintStream originalStandardOut;

/** no instantiation */
private BootstrapInfo() {}

Expand Down Expand Up @@ -46,6 +49,11 @@ public static boolean isSystemCallFilterInstalled() {
return Natives.isSystemCallFilterInstalled();
}

/**
* Returns a reference to the original System.out
*/
public static PrintStream getOriginalStandardOut() { return originalStandardOut; }

/**
* codebase location for untrusted scripts (provide some additional safety)
* <p>
Expand Down Expand Up @@ -110,7 +118,8 @@ public static Dictionary<Object,Object> getSystemProperties() {
return SYSTEM_PROPERTIES;
}

public static void init() {
public static void init(PrintStream originalStandardOut) {
BootstrapInfo.originalStandardOut = originalStandardOut;
}

}
54 changes: 27 additions & 27 deletions ...in/java/org/elasticsearch/xpack/security/GenerateInitialBuiltinUsersPasswordListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.DocWriteRequest;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.bootstrap.BootstrapInfo;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.index.engine.VersionConflictEngineException;
import org.elasticsearch.xpack.core.security.user.ElasticUser;
import org.elasticsearch.xpack.core.security.user.KibanaSystemUser;
import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
import org.elasticsearch.xpack.security.support.SecurityIndexManager;

import java.io.PrintStream;
import java.util.function.BiConsumer;

import static org.elasticsearch.xpack.security.tool.CommandUtils.generatePassword;
Expand All @@ -36,6 +38,11 @@ public GenerateInitialBuiltinUsersPasswordListener(NativeUsersStore nativeUsersS

@Override
public void accept(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) {
final PrintStream out = BootstrapInfo.getOriginalStandardOut();
if (null == out) {
Copy link
Member

@rjernst rjernst Sep 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this case possible? We could add a not null assertion when setting in BootstrapInfo?

Copy link
Contributor Author

@jkakavas jkakavas Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

outputOnError(new IllegalStateException("Stashed standard output stream is not available."));
return;
}
if (previousState.equals(SecurityIndexManager.State.UNRECOVERED_STATE)
&& currentState.equals(SecurityIndexManager.State.UNRECOVERED_STATE) == false
&& securityIndexManager.indexExists() == false) {
Expand All @@ -57,7 +64,7 @@ public void accept(SecurityIndexManager.State previousState, SecurityIndexManage
WriteRequest.RefreshPolicy.IMMEDIATE,
ActionListener.wrap(
r -> {
outputOnSuccess(elasticPassword, kibanaSystemPassword);
outputOnSuccess(elasticPassword, kibanaSystemPassword, out);
}, this::outputOnError
)
);
Expand All @@ -66,32 +73,25 @@ public void accept(SecurityIndexManager.State previousState, SecurityIndexManage
}
}

private void outputOnSuccess(SecureString elasticPassword, SecureString kibanaSystemPassword) {
LOGGER.info("");
LOGGER.info("-----------------------------------------------------------------");
LOGGER.info("");
LOGGER.info("");
LOGGER.info("");
LOGGER.info("Password for the elastic user is: " + elasticPassword);
LOGGER.info("");
LOGGER.info("");
LOGGER.info("");
LOGGER.info("Password for the kibana_system user is: " + kibanaSystemPassword);
LOGGER.info("");
LOGGER.info("");
LOGGER.info("Please note these down as they will not be shown again.");
LOGGER.info("");
LOGGER.info("You can use 'bin/elasticsearch-reset-elastic-password' at any time");
LOGGER.info("in order to reset the password for the elastic user.");
LOGGER.info("");
LOGGER.info("");
LOGGER.info("You can use 'bin/elasticsearch-reset-kibana-system-password' at any time");
LOGGER.info("in order to reset the password for the kibana_system user.");
LOGGER.info("");
LOGGER.info("");
LOGGER.info("");
LOGGER.info("-----------------------------------------------------------------");
LOGGER.info("");
private void outputOnSuccess(SecureString elasticPassword, SecureString kibanaSystemPassword, PrintStream out) {
out.println();
Copy link
Member

@rjernst rjernst Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@jkakavas jkakavas Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

out.println("-----------------------------------------------------------------");
out.println();
out.println("Password for the elastic user is: " + elasticPassword);
out.println();
out.println("Password for the kibana_system user is: " + kibanaSystemPassword);
out.println();
out.println("Please note these down as they will not be shown again.");
out.println();
out.println();
out.println("You can use 'bin/elasticsearch-reset-elastic-password' at any time");
out.println("in order to reset the password for the elastic user.");
out.println();
out.println("You can use 'bin/elasticsearch-reset-kibana-system-password' at any time");
out.println("in order to reset the password for the kibana_system user.");
out.println();
out.println("-----------------------------------------------------------------");
out.println();
}

private void outputOnError(Exception e) {
Expand Down