From 0a4431ddc55480d58581bb299bc1c7d9bca342fd Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 20 Apr 2020 15:37:22 +1000 Subject: [PATCH 1/2] Support empty keystore password for JDK 11 and lower --- .../security/cli/HttpCertificateCommand.java | 2 +- .../security/cli/CertificateToolTests.java | 8 ++-- .../cli/HttpCertificateCommandTests.java | 46 +++++++++++-------- 3 files changed, 33 insertions(+), 23 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 604482ffb6f77..61f64912f5c2f 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 @@ -864,7 +864,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 d26e4100bbf9e..510b60e91020f 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 1c8a954e610a5..3665ab2e644a1 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 @@ -142,7 +142,7 @@ public void testGenerateSingleCertificateSigningRequest() throws Exception { final String password = randomPassword(); terminal.addSecretInput(password); - terminal.addSecretInput(password); // confirm + runForNonEmptyPattern(password, () -> terminal.addSecretInput(password)); // confirm terminal.addTextInput(outFile.toString()); @@ -163,7 +163,8 @@ public void testGenerateSingleCertificateSigningRequest() throws Exception { wasEncrypted.set(true); return password.toCharArray(); }); - assertTrue("Password should have been required to decrypted key", wasEncrypted.get()); + runForNonEmptyPattern(password, + () -> assertTrue("Password should have been required to decrypted key", wasEncrypted.get())); final Path esReadmePath = zipRoot.resolve("elasticsearch/README.txt"); assertThat(esReadmePath, isRegularFile()); @@ -186,13 +187,13 @@ 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))); + runForNonEmptyPattern(password, () -> 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))); + runForNonEmptyPattern(password, () -> assertThat(yml, not(containsString(password)))); // Should not be a CA directory in CSR mode assertThat(zipRoot.resolve("ca"), not(pathExists())); @@ -257,7 +258,7 @@ public void testGenerateSingleCertificateWithExistingCA() throws Exception { final String password = randomPassword(); terminal.addSecretInput(password); - terminal.addSecretInput(password); // confirm + runForNonEmptyPattern(password, () -> terminal.addSecretInput(password)); // confirm terminal.addTextInput(outFile.toString()); @@ -292,13 +293,13 @@ 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))); - assertThat(readme, not(containsString(caPassword))); + runForNonEmptyPattern(password, () -> assertThat(readme, not(containsString(password)))); + runForNonEmptyPattern(caPassword, () -> assertThat(readme, not(containsString(caPassword)))); // Verify the yml assertThat(yml, containsString(p12Path.getFileName().toString())); - assertThat(yml, not(containsString(password))); - assertThat(yml, not(containsString(caPassword))); + runForNonEmptyPattern(password, () -> assertThat(yml, not(containsString(password)))); + runForNonEmptyPattern(caPassword, () -> assertThat(yml, not(containsString(caPassword)))); // Should not be a CA directory when using an existing CA. assertThat(zipRoot.resolve("ca"), not(pathExists())); @@ -347,7 +348,7 @@ public void testGenerateMultipleCertificateWithNewCA() throws Exception { final String caPassword = randomPassword(); terminal.addSecretInput(caPassword); - terminal.addSecretInput(caPassword); // confirm + runForNonEmptyPattern(caPassword, () -> terminal.addSecretInput(caPassword)); // confirm final int certYears = randomIntBetween(1, 8); terminal.addTextInput(certYears + "y"); // node cert validity period @@ -378,7 +379,7 @@ public void testGenerateMultipleCertificateWithNewCA() throws Exception { final String password = randomPassword(); terminal.addSecretInput(password); - terminal.addSecretInput(password); // confirm + runForNonEmptyPattern(password, () -> terminal.addSecretInput(password)); // confirm terminal.addTextInput(outFile.toString()); @@ -422,13 +423,13 @@ 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))); + runForNonEmptyPattern(password, () -> assertThat(readme, not(containsString(password)))); + runForNonEmptyPattern(caPassword, () -> assertThat(readme, not(containsString(caPassword)))); // Verify the yml assertThat(yml, containsString(p12Path.getFileName().toString())); - assertThat(yml, not(containsString(password))); - assertThat(yml, not(containsString(caPassword))); + runForNonEmptyPattern(password, () -> assertThat(yml, not(containsString(password)))); + runForNonEmptyPattern(caPassword, () -> assertThat(yml, not(containsString(caPassword)))); } verifyKibanaDirectory(zipRoot, true, List.of("2. elasticsearch-ca.pem"), @@ -589,7 +590,16 @@ 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 runForNonEmptyPattern(String pattern, Runnable runnable) { + if ("".equals(pattern) == false) { + runnable.run(); + } } private void verifyCertificationRequest(PKCS10CertificationRequest csr, String certificateName, List hostNames, @@ -699,11 +709,11 @@ private void verifyKibanaDirectory(Path zipRoot, boolean expectCAFile, Iterable< assertThat(kibanaReadme, containsString("https://")); assertThat(kibanaReadme, containsString("elasticsearch-ca.pem")); readmeShouldContain.forEach(s -> assertThat(kibanaReadme, containsString(s))); - shouldNotContain.forEach(s -> assertThat(kibanaReadme, not(containsString(s)))); + shouldNotContain.forEach(s -> runForNonEmptyPattern(s, () -> assertThat(kibanaReadme, not(containsString(s))))); assertThat(kibanaYml, containsString("elasticsearch.ssl.certificateAuthorities: [ \"config/elasticsearch-ca.pem\" ]")); assertThat(kibanaYml, containsString("https://")); - shouldNotContain.forEach(s -> assertThat(kibanaYml, not(containsString(s)))); + shouldNotContain.forEach(s -> runForNonEmptyPattern(s, () -> assertThat(kibanaYml, not(containsString(s))))); } private PublicKey getPublicKey(PKCS10CertificationRequest pkcs) throws GeneralSecurityException { From ad06cd29ac38f46a9c0cfd469349b85b60cfaa75 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 22 Apr 2020 10:41:01 +1000 Subject: [PATCH 2/2] Address feedback. --- .../cli/HttpCertificateCommandTests.java | 78 ++++++++++++------- 1 file changed, 51 insertions(+), 27 deletions(-) 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 3665ab2e644a1..88c83c81797eb 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 @@ -142,7 +142,9 @@ public void testGenerateSingleCertificateSigningRequest() throws Exception { final String password = randomPassword(); terminal.addSecretInput(password); - runForNonEmptyPattern(password, () -> terminal.addSecretInput(password)); // confirm + if ("".equals(password) == false) { + terminal.addSecretInput(password); + } // confirm terminal.addTextInput(outFile.toString()); @@ -163,8 +165,9 @@ public void testGenerateSingleCertificateSigningRequest() throws Exception { wasEncrypted.set(true); return password.toCharArray(); }); - runForNonEmptyPattern(password, - () -> 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()); @@ -187,19 +190,26 @@ public void testGenerateSingleCertificateSigningRequest() throws Exception { assertThat(esReadme, containsString(crtName)); assertThat(esReadme, containsString(keyPath.getFileName().toString())); assertThat(esReadme, containsString(ymlPath.getFileName().toString())); - runForNonEmptyPattern(password, () -> 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())); - runForNonEmptyPattern(password, () -> 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, List.of("Certificate Signing Request"), List.of(password, csrPath.getFileName().toString())); + + verifyKibanaDirectory(zipRoot, false, List.of("Certificate Signing Request"), + Stream.of(password, csrPath.getFileName().toString()) + .filter(s -> "".equals(s) == false).collect(Collectors.toList())); } public void testGenerateSingleCertificateWithExistingCA() throws Exception { @@ -258,7 +268,9 @@ public void testGenerateSingleCertificateWithExistingCA() throws Exception { final String password = randomPassword(); terminal.addSecretInput(password); - runForNonEmptyPattern(password, () -> terminal.addSecretInput(password)); // confirm + if ("".equals(password) == false) { + terminal.addSecretInput(password); + } // confirm terminal.addTextInput(outFile.toString()); @@ -293,19 +305,24 @@ public void testGenerateSingleCertificateWithExistingCA() throws Exception { // Verify the README assertThat(readme, containsString(p12Path.getFileName().toString())); assertThat(readme, containsString(ymlPath.getFileName().toString())); - runForNonEmptyPattern(password, () -> assertThat(readme, not(containsString(password)))); - runForNonEmptyPattern(caPassword, () -> assertThat(readme, not(containsString(caPassword)))); + if ("".equals(password) == false) { + assertThat(readme, not(containsString(password))); + } + assertThat(readme, not(containsString(caPassword))); // Verify the yml assertThat(yml, containsString(p12Path.getFileName().toString())); - runForNonEmptyPattern(password, () -> assertThat(yml, not(containsString(password)))); - runForNonEmptyPattern(caPassword, () -> assertThat(yml, not(containsString(caPassword)))); + 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, List.of("2. elasticsearch-ca.pem"), - List.of(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 { @@ -348,7 +365,9 @@ public void testGenerateMultipleCertificateWithNewCA() throws Exception { final String caPassword = randomPassword(); terminal.addSecretInput(caPassword); - runForNonEmptyPattern(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 @@ -379,7 +398,9 @@ public void testGenerateMultipleCertificateWithNewCA() throws Exception { final String password = randomPassword(); terminal.addSecretInput(password); - runForNonEmptyPattern(password, () -> terminal.addSecretInput(password)); // confirm + if ("".equals(password) == false) { + terminal.addSecretInput(password); + } // confirm terminal.addTextInput(outFile.toString()); @@ -423,17 +444,26 @@ public void testGenerateMultipleCertificateWithNewCA() throws Exception { // Verify the README assertThat(readme, containsString(p12Path.getFileName().toString())); assertThat(readme, containsString(ymlPath.getFileName().toString())); - runForNonEmptyPattern(password, () -> assertThat(readme, not(containsString(password)))); - runForNonEmptyPattern(caPassword, () -> 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())); - runForNonEmptyPattern(password, () -> assertThat(yml, not(containsString(password)))); - runForNonEmptyPattern(caPassword, () -> 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, List.of("2. elasticsearch-ca.pem"), - List.of(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,12 +626,6 @@ private String randomPassword() { ); } - private void runForNonEmptyPattern(String pattern, Runnable runnable) { - if ("".equals(pattern) == false) { - runnable.run(); - } - } - private void verifyCertificationRequest(PKCS10CertificationRequest csr, String certificateName, List hostNames, List ipAddresses) throws IOException { // We rebuild the DN from the encoding because BC uses openSSL style toString, but we use LDAP style. @@ -709,11 +733,11 @@ private void verifyKibanaDirectory(Path zipRoot, boolean expectCAFile, Iterable< assertThat(kibanaReadme, containsString("https://")); assertThat(kibanaReadme, containsString("elasticsearch-ca.pem")); readmeShouldContain.forEach(s -> assertThat(kibanaReadme, containsString(s))); - shouldNotContain.forEach(s -> runForNonEmptyPattern(s, () -> assertThat(kibanaReadme, not(containsString(s))))); + shouldNotContain.forEach(s -> assertThat(kibanaReadme, not(containsString(s)))); assertThat(kibanaYml, containsString("elasticsearch.ssl.certificateAuthorities: [ \"config/elasticsearch-ca.pem\" ]")); assertThat(kibanaYml, containsString("https://")); - shouldNotContain.forEach(s -> runForNonEmptyPattern(s, () -> assertThat(kibanaYml, not(containsString(s))))); + shouldNotContain.forEach(s -> assertThat(kibanaYml, not(containsString(s)))); } private PublicKey getPublicKey(PKCS10CertificationRequest pkcs) throws GeneralSecurityException {