From 32e46bf5526991a2adb23c69e937124c49584e1a Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 22 Apr 2020 15:03:10 +1000 Subject: [PATCH] Fix certutil http for empty password with JDK 11 and lower (#55437) (#55565) Fix elasticseaerch-certutil http command so that it correctly accepts empty keystore password with JDK version 11 and lower. --- .../security/cli/HttpCertificateCommand.java | 2 +- .../security/cli/CertificateToolTests.java | 8 +-- .../cli/HttpCertificateCommandTests.java | 70 +++++++++++++------ 3 files changed, 55 insertions(+), 25 deletions(-) diff --git a/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommand.java b/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommand.java index 10de695be651a..76f36c1f06d3f 100644 --- a/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommand.java +++ b/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommand.java @@ -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); } diff --git a/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java b/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java index 02a254fc5120a..f9d78f758c78c 100644 --- a/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java +++ b/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java @@ -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); diff --git a/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommandTests.java b/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommandTests.java index 818570ace03fe..7c46d45c09cc9 100644 --- a/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommandTests.java +++ b/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommandTests.java @@ -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; @@ -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()); @@ -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()); @@ -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 { @@ -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()); @@ -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 { @@ -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 @@ -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()); @@ -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 { @@ -596,7 +623,10 @@ private List 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 hostNames,