solved issue "certutil: large passwords not set" #30944

x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateTool.java

@@ -108,6 +108,11 @@ public class CertificateTool extends LoggingAwareMultiCommand {
         Pattern.compile("[a-zA-Z0-9!@#$%^&{}\\[\\]()_+\\-=,.~'` ]{1," + MAX_FILENAME_LENGTH + "}");
     private static final int DEFAULT_KEY_SIZE = 2048;
 
+    // Older versions of OpenSSL had a max internal password length.
+    // We issue warnings when writing files with passwords that would not be usable in those versions of OpenSSL.
+    static final String OLD_OPENSSL_VERSION = "1.1.0";
+    static final int MAX_PASSWORD_OLD_OPENSSL = 50;
+
     /**
      * Wraps the certgen object parser.
      */
@@ -349,9 +354,8 @@ CAInfo getCAInfo(Terminal terminal, OptionSet options, Environment env) throws E
         private CAInfo loadPkcs12CA(Terminal terminal, OptionSet options, Environment env) throws Exception {
             Path path = resolvePath(options, caPkcs12PathSpec);
             char[] passwordOption = getChars(caPasswordSpec.value(options));
-
-            Map<Certificate, Key> keys = withPassword("CA (" + path + ")", passwordOption,
-                terminal, password -> CertParsingUtils.readPkcs12KeyPairs(path, password, a -> password));
+            Map<Certificate, Key> keys = withPassword("CA (" + path + ")", passwordOption, terminal, false,
+                password -> CertParsingUtils.readPkcs12KeyPairs(path, password, a -> password));
 
             if (keys.size() != 1) {
                 throw new IllegalArgumentException("expected a single key in file [" + path.toAbsolutePath() + "] but found [" +
@@ -391,10 +395,11 @@ CAInfo generateCA(Terminal terminal, OptionSet options) throws Exception {
 
             if (options.hasArgument(caPasswordSpec)) {
                 char[] password = getChars(caPasswordSpec.value(options));
+                checkAndConfirmPasswordLengthForOpenSSLCompatibility(password, terminal, false);
                 return new CAInfo(caCert, keyPair.getPrivate(), true, password);
             }
             if (options.has(caPasswordSpec)) {
-                return withPassword("CA Private key", null, terminal, p -> new CAInfo(caCert, keyPair.getPrivate(), true, p.clone()));
+                return withPassword("CA Private key", null, terminal, true, p -> new CAInfo(caCert, keyPair.getPrivate(), true, p.clone()));
             }
             return new CAInfo(caCert, keyPair.getPrivate(), true, null);
         }
@@ -541,9 +546,9 @@ static void writePkcs12(String fileName, OutputStream output, String alias, Cert
                                 char[] password, Terminal terminal) throws Exception {
             final KeyStore pkcs12 = KeyStore.getInstance("PKCS12");
             pkcs12.load(null);
-            withPassword(fileName, password, terminal, p12Password -> {
+            withPassword(fileName, password, terminal, true, p12Password -> {
                 if (isAscii(p12Password)) {
-                    pkcs12.setKeyEntry(alias, pair.key, p12Password, new Certificate[]{pair.cert});
+                    pkcs12.setKeyEntry(alias, pair.key, p12Password, new Certificate[] { pair.cert });
                     if (caCert != null) {
                         pkcs12.setCertificateEntry("ca", caCert);
                     }
@@ -784,7 +789,7 @@ void generateAndWriteSignedCertificates(Path output, boolean writeZipFile, Optio
                             final String keyFileName = entryBase + ".key";
                             outputStream.putNextEntry(new ZipEntry(keyFileName));
                             if (usePassword) {
-                                withPassword(keyFileName, outputPassword, terminal, password -> {
+                                withPassword(keyFileName, outputPassword, terminal, true, password -> {
                                     pemWriter.writeObject(pair.key, getEncrypter(password));
                                     return null;
                                 });
@@ -924,16 +929,47 @@ static PEMEncryptor getEncrypter(char[] password) {
         return new JcePEMEncryptorBuilder("AES-128-CBC").setProvider(BC_PROV).build(password);
     }
 
-    private static <T, E extends Exception> T withPassword(String description, char[] password, Terminal terminal,
+    /**
+     * Checks whether the supplied password exceeds the maximum length supported by older OpenSSL versions.
+     * A warning message is printed to the terminal if the password is too long. If {@code confirm} is true, then the user
+     * (via the terminal) is asked to confirm whether to continue with the potentially problematic password.
+     * @return {@code false} if the password is too long <em>and</em> the user elects to reject it, otherwise {@code true}.
+     */
+    static boolean checkAndConfirmPasswordLengthForOpenSSLCompatibility(char[] password, Terminal terminal, boolean confirm) {
+        if (password.length > MAX_PASSWORD_OLD_OPENSSL) {
+            terminal.println(
+                Verbosity.SILENT,
+                "Warning: Your password exceeds "
+                    + MAX_PASSWORD_OLD_OPENSSL
+                    + " characters. Versions of OpenSSL older than "
+                    + OLD_OPENSSL_VERSION
+                    + " may not be able to read this file."
+            );
+            if (confirm) {
+                return terminal.promptYesNo("Do you want to continue?", true);
+            }
+        }
+        return true;
+    }
+
+    private static <T, E extends Exception> T withPassword(String description, char[] password, Terminal terminal, boolean checkLength,
                                                            CheckedFunction<char[], T, E> body) throws E {
         if (password == null) {
-            char[] promptedValue = terminal.readSecret("Enter password for " + description + " : ");
-            try {
-                return body.apply(promptedValue);
-            } finally {
-                Arrays.fill(promptedValue, (char) 0);
+            while (true) {
+                char[] promptedValue = terminal.readSecret("Enter password for " + description + " : ");
+                if (checkLength && checkAndConfirmPasswordLengthForOpenSSLCompatibility(promptedValue, terminal, true) == false) {
+                    continue;
+                }
+                try {
+                    return body.apply(promptedValue);
+                } finally {
+                    Arrays.fill(promptedValue, (char) 0);
+                }
             }
         } else {
+            if (checkLength) {
+                checkAndConfirmPasswordLengthForOpenSSLCompatibility(password, terminal, false);
+            }
             return body.apply(password);
         }
     }

x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommand.java

@@ -938,6 +938,9 @@ private char[] readPassword(Terminal terminal, String prompt, boolean confirm) {
                         continue;
                     }
                 }
+                if (CertificateTool.checkAndConfirmPasswordLengthForOpenSSLCompatibility(password, terminal, confirm) == false) {
+                    continue;
+                }
                 return password;
             } else {
                 terminal.println(Terminal.Verbosity.SILENT, "Passwords must be plain ASCII");

x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java

@@ -39,14 +39,14 @@
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.SecuritySettingsSourceField;
+import org.elasticsearch.xpack.core.ssl.PemUtils;
 import org.elasticsearch.xpack.security.cli.CertificateTool.CAInfo;
 import org.elasticsearch.xpack.security.cli.CertificateTool.CertificateAuthorityCommand;
 import org.elasticsearch.xpack.security.cli.CertificateTool.CertificateCommand;
 import org.elasticsearch.xpack.security.cli.CertificateTool.CertificateInformation;
 import org.elasticsearch.xpack.security.cli.CertificateTool.GenerateCertificateCommand;
 import org.elasticsearch.xpack.security.cli.CertificateTool.Name;
 import org.elasticsearch.xpack.core.ssl.CertParsingUtils;
-import org.hamcrest.Matchers;
 import org.junit.After;
 import org.junit.BeforeClass;
 
@@ -70,6 +70,7 @@
 import java.security.KeyPair;
 import java.security.KeyStore;
 import java.security.Principal;
+import java.security.PrivateKey;
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
 import java.security.interfaces.RSAKey;
@@ -93,6 +94,8 @@
 import static org.hamcrest.Matchers.in;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
@@ -361,7 +364,7 @@ public void testGeneratingSignedPemCertificates() throws Exception {
                             GeneralNames.fromExtensions(x509CertHolder.getExtensions(), Extension.subjectAlternativeName);
                     assertSubjAltNames(subjAltNames, certInfo);
                 }
-                assertThat(p12, Matchers.not(pathExists()));
+                assertThat(p12, not(pathExists()));
             }
         }
     }
@@ -382,6 +385,75 @@ public void testErrorMessageOnInvalidKeepCaOption() {
         assertThat(e.getMessage(), containsString("Generating certificates without providing a CA is no longer supported"));
     }
 
+    public void testHandleLongPasswords() throws Exception {
+        final Path tempDir = initTempDir();
+
+        final MockTerminal terminal = new MockTerminal();
+        Environment env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", tempDir).build());
+
+        final Path caFile = tempDir.resolve("ca.p12");
+        final Path pemZipFile = tempDir.resolve("cert.zip").toAbsolutePath();
+
+        final String longPassword = randomAlphaOfLengthBetween(51, 256);
+
+        boolean expectPrompt = randomBoolean();
+        final CertificateAuthorityCommand caCommand = new PathAwareCertificateAuthorityCommand(caFile);
+        final OptionSet gen1Options = caCommand.getParser()
+            .parse("-ca-dn", "CN=Test-Ca", (expectPrompt ? "-pass" : "-pass=" + longPassword), "-out", caFile.toString());
+
+        if (expectPrompt) {
+            terminal.addSecretInput(longPassword);
+            terminal.addTextInput("y"); // Yes, really use it
+        }
+        caCommand.execute(terminal, gen1Options, env);
+        assertThat(terminal.getOutput(), containsString("50 characters"));
+        assertThat(terminal.getOutput(), containsString("OpenSSL"));
+        assertThat(terminal.getOutput(), containsString("1.1.0"));
+
+        terminal.reset();
+        final GenerateCertificateCommand genCommand = new PathAwareGenerateCertificateCommand(caFile, pemZipFile);
+        final OptionSet gen2Options = genCommand.getParser().parse(
+            "-ca", "<ca>",
+            "-ca-pass", longPassword,
+            (expectPrompt ? "-pass" : "-pass=" + longPassword),
+            "-out", "<node2>",
+            "-name", "cert",
+            "-pem"
+        );
+
+        if (expectPrompt) {
+            terminal.addSecretInput(longPassword);
+            terminal.addTextInput("n"); // No, don't really use it
+            terminal.addSecretInput(longPassword);
+            terminal.addTextInput("y"); // This time, yes we will use it
+        }
+        genCommand.execute(terminal, gen2Options, env);
+        assertThat(terminal.getOutput(), containsString("50 characters"));
+        assertThat(terminal.getOutput(), containsString("OpenSSL"));
+        assertThat(terminal.getOutput(), containsString("1.1.0"));
+
+        assertThat(pemZipFile, pathExists());
+
+        final KeyStore caKeyStore = CertParsingUtils.readKeyStore(caFile, "PKCS12", longPassword.toCharArray());
+        Certificate caCert = caKeyStore.getCertificate("ca");
+        assertThat(caCert, notNullValue());
+
+        FileSystem zip = FileSystems.newFileSystem(new URI("jar:" + pemZipFile.toUri()), Collections.emptyMap());
+        Path zipRoot = zip.getPath("/");
+
+        final Path keyPath = zipRoot.resolve("cert/cert.key");
+        final PrivateKey key = PemUtils.readPrivateKey(keyPath, () -> longPassword.toCharArray());
+        assertThat(key, notNullValue());
+
+        final Path certPath = zipRoot.resolve("cert/cert.crt");
+        final Certificate[] certificates = CertParsingUtils.readCertificates(List.of(certPath));
+        assertThat(certificates, arrayWithSize(1));
+        assertThat(
+            ((X509Certificate) certificates[0]).getIssuerX500Principal(),
+            equalTo(((X509Certificate) caCert).getSubjectX500Principal())
+        );
+    }
+
     public void testGetCAInfo() throws Exception {
         Environment env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", createTempDir()).build());
         Path testNodeCertPath = getDataPath("/org/elasticsearch/xpack/security/cli/testnode.crt");
@@ -520,7 +592,7 @@ public void testNameValues() throws Exception {
     public void testCreateCaAndMultipleInstances() throws Exception {
         final Path tempDir = initTempDir();
 
-        final Terminal terminal = new MockTerminal();
+        final MockTerminal terminal = new MockTerminal();
         Environment env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", tempDir).build());
 
         final Path caFile = tempDir.resolve("ca.p12");
@@ -689,7 +761,7 @@ public void testTrustBetweenPEMandPKCS12() throws Exception {
         Path zip2Root = zip2FS.getPath("/");
 
         final Path ca2 = zip2Root.resolve("ca/ca.p12");
-        assertThat(ca2, Matchers.not(pathExists()));
+        assertThat(ca2, not(pathExists()));
 
         final Path node2Cert = zip2Root.resolve("node02/node02.crt");
         assertThat(node2Cert, pathExists());
@@ -881,6 +953,51 @@ private static Path resolvePath(String path) {
         return PathUtils.get(path).toAbsolutePath();
     }
 
+    private String generateCA(Path caFile, MockTerminal terminal, Environment env) throws Exception {
+        final int caKeySize = randomIntBetween(4, 8) * 512;
+        final int days = randomIntBetween(7, 1500);
+        final String caPassword = randomFrom("", randomAlphaOfLengthBetween(4, 80));
+
+        final CertificateAuthorityCommand caCommand = new PathAwareCertificateAuthorityCommand(caFile);
+        final OptionSet caOptions = caCommand.getParser().parse(
+            "-ca-dn", "CN=My ElasticSearch Cluster",
+            "-pass", caPassword,
+            "-out", caFile.toString(),
+            "-keysize", String.valueOf(caKeySize),
+            "-days", String.valueOf(days)
+        );
+        caCommand.execute(terminal, caOptions, env);
+
+        // Check output for OpenSSL compatibility version
+        if (caPassword.length() > 50) {
+            assertThat(terminal.getOutput(), containsString("OpenSSL"));
+        } else {
+            assertThat(terminal.getOutput(), not(containsString("OpenSSL")));
+        }
+
+        assertThat(caFile, pathExists());
+
+        return caPassword;
+    }
+
+    /**
+     * Converting jimfs Paths into strings and back to paths doesn't work with the security manager.
+     * This class works around that by sticking with the original path objects
+     */
+    private class PathAwareCertificateAuthorityCommand extends CertificateAuthorityCommand {
+        private final Path caFile;
+
+        private PathAwareCertificateAuthorityCommand(Path caFile) {
+            this.caFile = caFile;
+        }
+
+        @Override
+        Path resolveOutputPath(Terminal terminal, OptionSet options, String defaultFilename) {
+            // Needed to work within the security manager
+            return caFile;
+        }
+    }
+
     /**
      * Converting jimfs Paths into strings and back to paths doesn't work with the security manager.
      * This class works around that by sticking with the original path objects
@@ -907,30 +1024,4 @@ Path resolveOutputPath(Terminal terminal, OptionSet options, String defaultFilen
             return outFile;
         }
     }
-
-    private String generateCA(Path caFile, Terminal terminal, Environment env) throws Exception {
-        final int caKeySize = randomIntBetween(4, 8) * 512;
-        final int days = randomIntBetween(7, 1500);
-        final String caPassword = randomFrom("", randomAlphaOfLengthBetween(4, 16));
-
-        final CertificateAuthorityCommand caCommand = new CertificateAuthorityCommand() {
-            @Override
-            Path resolveOutputPath(Terminal terminal, OptionSet options, String defaultFilename) {
-                // Needed to work within the security manager
-                return caFile;
-            }
-        };
-        final OptionSet caOptions = caCommand.getParser().parse(
-            "-ca-dn", "CN=My ElasticSearch Cluster",
-            "-pass", caPassword,
-            "-out", caFile.toString(),
-            "-keysize", String.valueOf(caKeySize),
-            "-days", String.valueOf(days)
-        );
-        caCommand.execute(terminal, caOptions, env);
-
-        assertThat(caFile, pathExists());
-
-        return caPassword;
-    }
 }