Skip to content

Commit

Permalink
Fix certutil http for empty password with JDK 11 and lower (elastic#5…
Browse files Browse the repository at this point in the history 
…5437) (elastic#55565)

 Fix elasticseaerch-certutil http command so that it correctly accepts empty keystore password with JDK version 11 and lower.
  • Loading branch information
@ywangd
ywangd authored Apr 22, 2020
1 parent 777ffd5 commit 32e46bf
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 25 deletions.
2 changes: 1 addition & 1 deletion ...curity/cli/src/main/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ private CertificateTool.CAInfo createNewCA(Terminal terminal) {
terminal.println("IT IS IMPORTANT THAT YOU REMEMBER THIS PASSWORD AND KEEP IT SECURE");
terminal.println("");
final char[] password = readPassword(terminal, "CA password: ", true);
return new CertificateTool.CAInfo(caCert, keyPair.getPrivate(), true, password.length == 0 ? null : password);
return new CertificateTool.CAInfo(caCert, keyPair.getPrivate(), true, password);
} catch (GeneralSecurityException | CertIOException | OperatorCreationException e) {
throw new IllegalArgumentException("Cannot generate CA key pair", e);
}
Expand Down
8 changes: 4 additions & 4 deletions ...security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -563,10 +563,10 @@ public void testCreateCaAndMultipleInstances() throws Exception {

final int days = randomIntBetween(7, 1500);

final String caPassword = randomAlphaOfLengthBetween(4, 16);
final String node1Password = randomAlphaOfLengthBetween(4, 16);
final String node2Password = randomAlphaOfLengthBetween(4, 16);
final String node3Password = randomAlphaOfLengthBetween(4, 16);
final String caPassword = randomFrom("", randomAlphaOfLengthBetween(4, 16));
final String node1Password = randomFrom("", randomAlphaOfLengthBetween(4, 16));
final String node2Password = randomFrom("", randomAlphaOfLengthBetween(4, 16));
final String node3Password = randomFrom("", randomAlphaOfLengthBetween(4, 16));

final String node1Ip = "200.181." + randomIntBetween(1, 250) + "." + randomIntBetween(1, 250);
final String node2Ip = "200.182." + randomIntBetween(1, 250) + "." + randomIntBetween(1, 250);
Expand Down
70 changes: 50 additions & 20 deletions ...y/cli/src/test/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommandTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -146,7 +145,9 @@ public void testGenerateSingleCertificateSigningRequest() throws Exception {

final String password = randomPassword();
terminal.addSecretInput(password);
terminal.addSecretInput(password); // confirm
if ("".equals(password) == false) {
terminal.addSecretInput(password);
} // confirm

terminal.addTextInput(outFile.toString());

Expand All @@ -167,7 +168,9 @@ public void testGenerateSingleCertificateSigningRequest() throws Exception {
wasEncrypted.set(true);
return password.toCharArray();
});
assertTrue("Password should have been required to decrypted key", wasEncrypted.get());
if ("".equals(password) == false) {
assertTrue("Password should have been required to decrypted key", wasEncrypted.get());
}

final Path esReadmePath = zipRoot.resolve("elasticsearch/README.txt");
assertThat(esReadmePath, isRegularFile());
Expand All @@ -190,22 +193,26 @@ public void testGenerateSingleCertificateSigningRequest() throws Exception {
assertThat(esReadme, containsString(crtName));
assertThat(esReadme, containsString(keyPath.getFileName().toString()));
assertThat(esReadme, containsString(ymlPath.getFileName().toString()));
assertThat(esReadme, not(containsString(password)));
if ("".equals(password) == false) {
assertThat(esReadme, not(containsString(password)));
}

// Verify the yml
assertThat(yml, not(containsString(csrPath.getFileName().toString())));
assertThat(yml, containsString(crtName));
assertThat(yml, containsString(keyPath.getFileName().toString()));
assertThat(yml, not(containsString(password)));
if ("".equals(password) == false) {
assertThat(yml, not(containsString(password)));
}

// Should not be a CA directory in CSR mode
assertThat(zipRoot.resolve("ca"), not(pathExists()));

// No CA in CSR mode
verifyKibanaDirectory(zipRoot,
false,
verifyKibanaDirectory(zipRoot, false,
Collections.singletonList("Certificate Signing Request"),
Arrays.asList(password, csrPath.getFileName().toString()));
Stream.of(password, csrPath.getFileName().toString())
.filter(s -> "".equals(s) == false).collect(Collectors.toList()));
}

public void testGenerateSingleCertificateWithExistingCA() throws Exception {
Expand Down Expand Up @@ -264,7 +271,9 @@ public void testGenerateSingleCertificateWithExistingCA() throws Exception {

final String password = randomPassword();
terminal.addSecretInput(password);
terminal.addSecretInput(password); // confirm
if ("".equals(password) == false) {
terminal.addSecretInput(password);
} // confirm

terminal.addTextInput(outFile.toString());

Expand Down Expand Up @@ -299,19 +308,24 @@ public void testGenerateSingleCertificateWithExistingCA() throws Exception {
// Verify the README
assertThat(readme, containsString(p12Path.getFileName().toString()));
assertThat(readme, containsString(ymlPath.getFileName().toString()));
assertThat(readme, not(containsString(password)));
if ("".equals(password) == false) {
assertThat(readme, not(containsString(password)));
}
assertThat(readme, not(containsString(caPassword)));

// Verify the yml
assertThat(yml, containsString(p12Path.getFileName().toString()));
assertThat(yml, not(containsString(password)));
if ("".equals(password) == false) {
assertThat(yml, not(containsString(password)));
}
assertThat(yml, not(containsString(caPassword)));

// Should not be a CA directory when using an existing CA.
assertThat(zipRoot.resolve("ca"), not(pathExists()));

verifyKibanaDirectory(zipRoot, true, Collections.singletonList("2. elasticsearch-ca.pem"),
Arrays.asList(password, caPassword, caKeyPath.getFileName().toString()));
Stream.of(password, caPassword, caKeyPath.getFileName().toString())
.filter(s -> "".equals(s) == false).collect(Collectors.toList()));
}

public void testGenerateMultipleCertificateWithNewCA() throws Exception {
Expand Down Expand Up @@ -354,7 +368,9 @@ public void testGenerateMultipleCertificateWithNewCA() throws Exception {

final String caPassword = randomPassword();
terminal.addSecretInput(caPassword);
terminal.addSecretInput(caPassword); // confirm
if ("".equals(caPassword) == false) {
terminal.addSecretInput(caPassword);
} // confirm

final int certYears = randomIntBetween(1, 8);
terminal.addTextInput(certYears + "y"); // node cert validity period
Expand Down Expand Up @@ -385,7 +401,9 @@ public void testGenerateMultipleCertificateWithNewCA() throws Exception {

final String password = randomPassword();
terminal.addSecretInput(password);
terminal.addSecretInput(password); // confirm
if ("".equals(password) == false) {
terminal.addSecretInput(password);
} // confirm

terminal.addTextInput(outFile.toString());

Expand Down Expand Up @@ -429,17 +447,26 @@ public void testGenerateMultipleCertificateWithNewCA() throws Exception {
// Verify the README
assertThat(readme, containsString(p12Path.getFileName().toString()));
assertThat(readme, containsString(ymlPath.getFileName().toString()));
assertThat(readme, not(containsString(password)));
assertThat(readme, not(containsString(caPassword)));
if ("".equals(password) == false) {
assertThat(readme, not(containsString(password)));
}
if ("".equals(caPassword) == false) {
assertThat(readme, not(containsString(caPassword)));
}

// Verify the yml
assertThat(yml, containsString(p12Path.getFileName().toString()));
assertThat(yml, not(containsString(password)));
assertThat(yml, not(containsString(caPassword)));
if ("".equals(password) == false) {
assertThat(yml, not(containsString(password)));
}
if ("".equals(caPassword) == false) {
assertThat(yml, not(containsString(caPassword)));
}
}

verifyKibanaDirectory(zipRoot, true, Collections.singletonList("2. elasticsearch-ca.pem"),
Arrays.asList(password, caPassword, caPath.getFileName().toString()));
Stream.of(password, caPassword, caPath.getFileName().toString())
.filter(s -> "".equals(s) == false).collect(Collectors.toList()));
}

public void testParsingValidityPeriod() throws Exception {
Expand Down Expand Up @@ -596,7 +623,10 @@ private List<String> randomHostNames() {
private String randomPassword() {
// We want to assert that this password doesn't end up in any output files, so we need to make sure we
// don't randomly generate a real word.
return randomAlphaOfLength(4) + randomFrom('~', '*', '%', '$', '|') + randomAlphaOfLength(4);
return randomFrom(
"",
randomAlphaOfLength(4) + randomFrom('~', '*', '%', '$', '|') + randomAlphaOfLength(4)
);
}

private void verifyCertificationRequest(PKCS10CertificationRequest csr, String certificateName, List<String> hostNames,
Expand Down

0 comments on commit 32e46bf

Please sign in to comment.