From 666e87c29ba68ebbd8c45b86e36d0daa626e8247 Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Wed, 12 Apr 2017 10:18:56 -0400 Subject: [PATCH] Resolve paths from the current working directory instead of the config directory (elastic/x-pack-elasticsearch#637) This commit changes the resolution of the output and input files so that relative paths will be resolved from the current working directory instead of the x-pack config directory. relates elastic/x-pack-elasticsearch#621 Original commit: elastic/x-pack-elasticsearch@bbfd83c2d578ed5d4350fedd1df810b38c6c5ff0 --- .../xpack/ssl/CertificateTool.java | 38 ++++++++++--------- .../xpack/ssl/CertificateToolTests.java | 21 +++++----- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ssl/CertificateTool.java b/plugin/src/main/java/org/elasticsearch/xpack/ssl/CertificateTool.java index d6aef1cfe40..27de75857e2 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ssl/CertificateTool.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ssl/CertificateTool.java @@ -19,6 +19,8 @@ import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.Terminal; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.network.InetAddresses; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.common.xcontent.ConstructingObjectParser; @@ -27,7 +29,6 @@ import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.env.Environment; -import org.elasticsearch.xpack.XPackPlugin; import javax.security.auth.x500.X500Principal; import java.io.IOException; @@ -138,11 +139,11 @@ public class CertificateTool extends EnvironmentAwareCommand { protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { final boolean csrOnly = options.has(csrSpec); printIntro(terminal, csrOnly); - final Path outputFile = getOutputFile(terminal, outputPathSpec.value(options), env, csrOnly ? DEFAULT_CSR_FILE : DEFAULT_CERT_FILE); + final Path outputFile = getOutputFile(terminal, outputPathSpec.value(options), csrOnly ? DEFAULT_CSR_FILE : DEFAULT_CERT_FILE); final String inputFile = inputFileSpec.value(options); final int keysize = options.has(keysizeSpec) ? keysizeSpec.value(options) : DEFAULT_KEY_SIZE; if (csrOnly) { - Collection certificateInformations = getCertificateInformationList(terminal, inputFile, env); + Collection certificateInformations = getCertificateInformationList(terminal, inputFile); generateAndWriteCsrs(outputFile, certificateInformations, keysize); } else { final String dn = options.has(caDnSpec) ? caDnSpec.value(options) : AUTO_GEN_CA_DN; @@ -151,7 +152,7 @@ public class CertificateTool extends EnvironmentAwareCommand { final int days = options.hasArgument(daysSpec) ? daysSpec.value(options) : DEFAULT_DAYS; CAInfo caInfo = getCAInfo(terminal, dn, caCertPathSpec.value(options), caKeyPathSpec.value(options), keyPass, prompt, env, keysize, days); - Collection certificateInformations = getCertificateInformationList(terminal, inputFile, env); + Collection certificateInformations = getCertificateInformationList(terminal, inputFile); generateAndWriteSignedCertificates(outputFile, certificateInformations, caInfo, keysize, days); } printConclusion(terminal, csrOnly, outputFile); @@ -171,35 +172,38 @@ public class CertificateTool extends EnvironmentAwareCommand { * * @param terminal terminal to communicate with a user * @param outputPath user specified output file, may be {@code null} - * @param env the environment for this tool to resolve files with * @return a {@link Path} to the output file */ - static Path getOutputFile(Terminal terminal, String outputPath, Environment env, String defaultFilename) throws IOException { + static Path getOutputFile(Terminal terminal, String outputPath, String defaultFilename) throws IOException { Path file; if (outputPath != null) { - file = XPackPlugin.resolveConfigFile(env, Strings.cleanPath(outputPath)); + file = resolvePath(outputPath); } else { - file = XPackPlugin.resolveConfigFile(env, defaultFilename); + file = resolvePath(defaultFilename); String input = terminal.readText("Please enter the desired output file [" + file + "]: "); if (input.isEmpty() == false) { - file = XPackPlugin.resolveConfigFile(env, Strings.cleanPath(input)); + file = resolvePath(input); } } return file; } + @SuppressForbidden(reason = "resolve paths against CWD for a CLI tool") + private static Path resolvePath(String pathStr) { + return PathUtils.get(Strings.cleanPath(pathStr)).toAbsolutePath(); + } + /** * This method handles the collection of information about each instance that is necessary to generate a certificate. The user may * be prompted or the information can be gathered from a file * @param terminal the terminal to use for user interaction * @param inputFile an optional file that will be used to load the instance information - * @param env the environment for this tool to resolve files with * @return a {@link Collection} of {@link CertificateInformation} that represents each instance */ - static Collection getCertificateInformationList(Terminal terminal, String inputFile, Environment env) + static Collection getCertificateInformationList(Terminal terminal, String inputFile) throws Exception { if (inputFile != null) { - return parseFile(XPackPlugin.resolveConfigFile(env, inputFile)); + return parseFile(resolvePath(inputFile)); } Map map = new HashMap<>(); boolean done = false; @@ -307,13 +311,14 @@ public class CertificateTool extends EnvironmentAwareCommand { Environment env, int keysize, int days) throws Exception { if (caCertPath != null) { assert caKeyPath != null; - Certificate[] certificates = CertUtils.readCertificates(Collections.singletonList(caCertPath), env); + final String resolvedCaCertPath = resolvePath(caCertPath).toString(); + Certificate[] certificates = CertUtils.readCertificates(Collections.singletonList(resolvedCaCertPath), env); if (certificates.length != 1) { throw new IllegalArgumentException("expected a single certificate in file [" + caCertPath + "] but found [" + certificates.length + "]"); } Certificate caCert = certificates[0]; - PrivateKey privateKey = readPrivateKey(caKeyPath, keyPass, terminal, env, prompt); + PrivateKey privateKey = readPrivateKey(caKeyPath, keyPass, terminal, prompt); return new CAInfo((X509Certificate) caCert, privateKey); } @@ -504,14 +509,13 @@ public class CertificateTool extends EnvironmentAwareCommand { * @param path the path to the private key * @param password the password provided by the user or {@code null} * @param terminal the terminal to use for user interaction - * @param env the environment to resolve files from * @param prompt whether to prompt the user or not * @return the {@link PrivateKey} that was read from the file */ - private static PrivateKey readPrivateKey(String path, char[] password, Terminal terminal, Environment env, boolean prompt) + private static PrivateKey readPrivateKey(String path, char[] password, Terminal terminal, boolean prompt) throws Exception { AtomicReference passwordReference = new AtomicReference<>(password); - try (Reader reader = Files.newBufferedReader(XPackPlugin.resolveConfigFile(env, path), StandardCharsets.UTF_8)) { + try (Reader reader = Files.newBufferedReader(resolvePath(path), StandardCharsets.UTF_8)) { return CertUtils.readPrivateKey(reader, () -> { if (password != null || prompt == false) { return password; diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ssl/CertificateToolTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ssl/CertificateToolTests.java index a8dd357fee1..db466c3ed6b 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ssl/CertificateToolTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ssl/CertificateToolTests.java @@ -22,11 +22,12 @@ import org.bouncycastle.openssl.PEMParser; import org.bouncycastle.pkcs.PKCS10CertificationRequest; import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.network.NetworkAddress; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.XPackPlugin; import org.elasticsearch.xpack.ssl.CertificateTool.CAInfo; import org.elasticsearch.xpack.ssl.CertificateTool.CertificateInformation; import org.elasticsearch.xpack.ssl.CertificateTool.Name; @@ -63,7 +64,6 @@ import java.util.function.Function; import java.util.stream.Collectors; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.instanceOf; /** @@ -89,13 +89,12 @@ public class CertificateToolTests extends ESTestCase { } public void testOutputDirectory() throws Exception { - Environment env = new Environment(Settings.builder().put("path.home", createTempDir()).build()); Path outputDir = createTempDir(); Path outputFile = outputDir.resolve("certs.zip"); MockTerminal terminal = new MockTerminal(); // test with a user provided dir - Path resolvedOutputFile = CertificateTool.getOutputFile(terminal, outputFile.toString(), env, null); + Path resolvedOutputFile = CertificateTool.getOutputFile(terminal, outputFile.toString(), null); assertEquals(outputFile, resolvedOutputFile); assertTrue(terminal.getOutput().isEmpty()); @@ -103,15 +102,15 @@ public class CertificateToolTests extends ESTestCase { Path userPromptedOutputFile = outputDir.resolve("csr"); assertFalse(Files.exists(userPromptedOutputFile)); terminal.addTextInput(userPromptedOutputFile.toString()); - resolvedOutputFile = CertificateTool.getOutputFile(terminal, null, env, "out.zip"); + resolvedOutputFile = CertificateTool.getOutputFile(terminal, null, "out.zip"); assertEquals(userPromptedOutputFile, resolvedOutputFile); assertTrue(terminal.getOutput().isEmpty()); // test with empty user input String defaultFilename = randomAlphaOfLengthBetween(1, 10); - Path expectedDefaultPath = XPackPlugin.resolveConfigFile(env, defaultFilename); + Path expectedDefaultPath = resolvePath(defaultFilename); terminal.addTextInput(""); - resolvedOutputFile = CertificateTool.getOutputFile(terminal, null, env, defaultFilename); + resolvedOutputFile = CertificateTool.getOutputFile(terminal, null, defaultFilename); assertEquals(expectedDefaultPath, resolvedOutputFile); assertTrue(terminal.getOutput().isEmpty()); } @@ -150,8 +149,7 @@ public class CertificateToolTests extends ESTestCase { } } - Collection certInfos = CertificateTool.getCertificateInformationList(terminal, null, - new Environment(Settings.builder().put("path.home", createTempDir()).build())); + Collection certInfos = CertificateTool.getCertificateInformationList(terminal, null); logger.info("certificate tool output:\n{}", terminal.getOutput()); assertEquals(numberOfInstances, certInfos.size()); for (CertificateInformation certInfo : certInfos) { @@ -487,4 +485,9 @@ public class CertificateToolTests extends ESTestCase { return Files.write(path, instances, StandardCharsets.UTF_8); } + + @SuppressForbidden(reason = "resolve paths against CWD for a CLI tool") + private static Path resolvePath(String path) { + return PathUtils.get(path).toAbsolutePath(); + } }