From ec1fd113c1360d0487115d4fc94cfb07954b459c Mon Sep 17 00:00:00 2001 From: Tomas Dvorak Date: Fri, 15 Nov 2024 17:02:02 +0100 Subject: [PATCH] opensearch keystore used for key/truststore passwords (#20923) * opensearch keystore used for key/truststore passwords * code cleanup * Add changelog * Added integration test for opensearch keystore cli * add `-x` to add OS keystore secrets, bypassing the prompt --------- Co-authored-by: Matthias Oesterheld --- changelog/unreleased/issue-20926.toml | 5 ++ .../OpensearchDistributionProvider.java | 2 +- .../S3RepositoryConfiguration.java | 16 ++++- .../variants/KeystoreContributor.java | 26 ++++++++ .../OpensearchSecurityConfiguration.java | 18 ++++-- .../opensearch/cli/AbstractOpensearchCli.java | 11 ++-- .../opensearch/cli/OpensearchCli.java | 11 +++- .../cli/OpensearchCommandLineProcess.java | 21 +++---- .../opensearch/cli/OpensearchKeystoreCli.java | 20 ++++-- .../OpensearchConfiguration.java | 13 +++- .../cli/OpensearchKeystoreCommandLineIT.java | 62 +++++++++++++++++++ 11 files changed, 168 insertions(+), 37 deletions(-) create mode 100644 changelog/unreleased/issue-20926.toml create mode 100644 data-node/src/main/java/org/graylog/datanode/configuration/variants/KeystoreContributor.java create mode 100644 data-node/src/test/java/org/graylog/datanode/opensearch/cli/OpensearchKeystoreCommandLineIT.java diff --git a/changelog/unreleased/issue-20926.toml b/changelog/unreleased/issue-20926.toml new file mode 100644 index 000000000000..444b9f3202b5 --- /dev/null +++ b/changelog/unreleased/issue-20926.toml @@ -0,0 +1,5 @@ +type = "c" +message = "Data node: store keystore and truststore passwords in Opensearch keystore" + +issues = ["20926"] +pulls = ["20923"] diff --git a/data-node/src/main/java/org/graylog/datanode/configuration/OpensearchDistributionProvider.java b/data-node/src/main/java/org/graylog/datanode/configuration/OpensearchDistributionProvider.java index 457b8acff116..4926db7e92e0 100644 --- a/data-node/src/main/java/org/graylog/datanode/configuration/OpensearchDistributionProvider.java +++ b/data-node/src/main/java/org/graylog/datanode/configuration/OpensearchDistributionProvider.java @@ -51,7 +51,7 @@ public OpensearchDistributionProvider(final Configuration localConfiguration) { this(Path.of(localConfiguration.getOpensearchDistributionRoot()), OpensearchArchitecture.fromOperatingSystem()); } - OpensearchDistributionProvider(final Path opensearchDistributionRoot, OpensearchArchitecture architecture) { + public OpensearchDistributionProvider(final Path opensearchDistributionRoot, OpensearchArchitecture architecture) { this.distribution = Suppliers.memoize(() -> detectInDirectory(opensearchDistributionRoot, architecture)); } diff --git a/data-node/src/main/java/org/graylog/datanode/configuration/S3RepositoryConfiguration.java b/data-node/src/main/java/org/graylog/datanode/configuration/S3RepositoryConfiguration.java index de4e8404b8da..71ceef3a48d0 100644 --- a/data-node/src/main/java/org/graylog/datanode/configuration/S3RepositoryConfiguration.java +++ b/data-node/src/main/java/org/graylog/datanode/configuration/S3RepositoryConfiguration.java @@ -18,14 +18,15 @@ import com.github.joschi.jadconfig.Parameter; import com.github.joschi.jadconfig.converters.BooleanConverter; +import com.google.common.collect.ImmutableMap; import org.apache.commons.lang3.StringUtils; +import org.graylog.datanode.configuration.variants.KeystoreContributor; import org.graylog2.configuration.Documentation; import java.util.Arrays; import java.util.Map; -import java.util.Set; -public class S3RepositoryConfiguration { +public class S3RepositoryConfiguration implements KeystoreContributor { @Documentation("S3 repository access key for searchable snapshots") @Parameter(value = "s3_client_default_access_key") @@ -100,4 +101,15 @@ private boolean noneBlank(String... properties) { private boolean allBlank(String... properties) { return Arrays.stream(properties).allMatch(StringUtils::isBlank); } + + @Override + public Map getKeystoreItems() { + final ImmutableMap.Builder config = ImmutableMap.builder(); + if (isRepositoryEnabled()) { + config.put("s3.client.default.access_key", getS3ClientDefaultAccessKey()); + config.put("s3.client.default.secret_key", getS3ClientDefaultSecretKey()); + + } + return config.build(); + } } diff --git a/data-node/src/main/java/org/graylog/datanode/configuration/variants/KeystoreContributor.java b/data-node/src/main/java/org/graylog/datanode/configuration/variants/KeystoreContributor.java new file mode 100644 index 000000000000..004dbfc3ddb6 --- /dev/null +++ b/data-node/src/main/java/org/graylog/datanode/configuration/variants/KeystoreContributor.java @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +package org.graylog.datanode.configuration.variants; + +import java.util.Map; + +public interface KeystoreContributor { + /** + * @return collection of key-value pairs that should be added to the opensearch keystore (holding secrets) + */ + Map getKeystoreItems(); +} diff --git a/data-node/src/main/java/org/graylog/datanode/configuration/variants/OpensearchSecurityConfiguration.java b/data-node/src/main/java/org/graylog/datanode/configuration/variants/OpensearchSecurityConfiguration.java index e38395cfdc55..67220ab537db 100644 --- a/data-node/src/main/java/org/graylog/datanode/configuration/variants/OpensearchSecurityConfiguration.java +++ b/data-node/src/main/java/org/graylog/datanode/configuration/variants/OpensearchSecurityConfiguration.java @@ -48,7 +48,7 @@ import java.util.Optional; import java.util.stream.Collectors; -public class OpensearchSecurityConfiguration { +public class OpensearchSecurityConfiguration implements KeystoreContributor { private static final Logger LOG = LoggerFactory.getLogger(OpensearchSecurityConfiguration.class); @@ -106,23 +106,19 @@ public Map getProperties() throws GeneralSecurityException, IOEx config.put("plugins.security.ssl.transport.keystore_type", KEYSTORE_FORMAT); config.put("plugins.security.ssl.transport.keystore_filepath", transportCertificate.location().getFileName().toString()); // todo: this should be computed as a relative path - config.put("plugins.security.ssl.transport.keystore_password", new String(transportCertificate.password())); config.put("plugins.security.ssl.transport.keystore_alias", CertConstants.DATANODE_KEY_ALIAS); config.put("plugins.security.ssl.transport.truststore_type", TRUSTSTORE_FORMAT); config.put("plugins.security.ssl.transport.truststore_filepath", TRUSTSTORE_FILE.toString()); - config.put("plugins.security.ssl.transport.truststore_password", new String(truststore.password())); config.put("plugins.security.ssl.http.enabled", "true"); config.put("plugins.security.ssl.http.keystore_type", KEYSTORE_FORMAT); config.put("plugins.security.ssl.http.keystore_filepath", httpCertificate.location().getFileName().toString()); // todo: this should be computed as a relative path - config.put("plugins.security.ssl.http.keystore_password", new String(httpCertificate.password())); config.put("plugins.security.ssl.http.keystore_alias", CertConstants.DATANODE_KEY_ALIAS); config.put("plugins.security.ssl.http.truststore_type", TRUSTSTORE_FORMAT); config.put("plugins.security.ssl.http.truststore_filepath", TRUSTSTORE_FILE.toString()); - config.put("plugins.security.ssl.http.truststore_password", new String(truststore.password())); // enable client cert auth config.put("plugins.security.ssl.http.clientauth_mode", "OPTIONAL"); @@ -133,6 +129,18 @@ public Map getProperties() throws GeneralSecurityException, IOEx return config.build(); } + @Override + public Map getKeystoreItems() { + final ImmutableMap.Builder config = ImmutableMap.builder(); + if (securityEnabled()) { + config.put("plugins.security.ssl.transport.keystore_password_secure", new String(transportCertificate.password())); + config.put("plugins.security.ssl.transport.truststore_password_secure", new String(truststore.password())); + config.put("plugins.security.ssl.http.keystore_password_secure", new String(httpCertificate.password())); + config.put("plugins.security.ssl.http.truststore_password_secure", new String(truststore.password())); + } + return config.build(); + } + private Map filterConfigurationMap(final Map map, final String... keys) { Map result = map; for (final String key : List.of(keys)) { diff --git a/data-node/src/main/java/org/graylog/datanode/opensearch/cli/AbstractOpensearchCli.java b/data-node/src/main/java/org/graylog/datanode/opensearch/cli/AbstractOpensearchCli.java index 6541c95bca55..4f2b87195446 100644 --- a/data-node/src/main/java/org/graylog/datanode/opensearch/cli/AbstractOpensearchCli.java +++ b/data-node/src/main/java/org/graylog/datanode/opensearch/cli/AbstractOpensearchCli.java @@ -20,7 +20,6 @@ import org.apache.commons.exec.DefaultExecuteResultHandler; import org.apache.commons.exec.DefaultExecutor; import org.apache.commons.exec.PumpStreamHandler; -import org.graylog.datanode.opensearch.configuration.OpensearchConfiguration; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -41,19 +40,17 @@ public abstract class AbstractOpensearchCli { private final Path binPath; /** - * * @param configPath Opensearch CLI tools adapt configuration stored under OPENSEARCH_PATH_CONF env property. * This is why wealways want to set this configPath for each CLI tool. - * @param bin location of the actual executable binary that this wrapper handles + * @param bin location of the actual executable binary that this wrapper handles */ private AbstractOpensearchCli(Path configPath, Path bin) { this.configPath = configPath; this.binPath = bin; } - public AbstractOpensearchCli(OpensearchConfiguration config, String binName) { - this(config.datanodeDirectories().getOpensearchProcessConfigurationDir(), - checkExecutable(config.opensearchDistribution().getOpensearchBinDirPath().resolve(binName))); + protected AbstractOpensearchCli(Path configDir, Path binDir, String binName) { + this(configDir, checkExecutable(binDir.resolve(binName))); } private static Path checkExecutable(Path path) { @@ -72,7 +69,7 @@ protected String runBatch(String... args) { * to questions the tool is asking. It's scripting unfriendly. Our way around this is to * provide an input stream of expected responses, each delimited by \n. There is no validation * and no logic, just the expected order of responses. - * @param args arguments of the command, in opensearch-keystore create, the create is the first argument + * @param args arguments of the command, in opensearch-keystore create, the create is the first argument * @return All the STDOUT and STDERR of the process merged into one String. */ protected String runWithStdin(List answersToPrompts, String... args) { diff --git a/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchCli.java b/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchCli.java index cd00589999f4..c26bfdeea120 100644 --- a/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchCli.java +++ b/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchCli.java @@ -18,6 +18,8 @@ import org.graylog.datanode.opensearch.configuration.OpensearchConfiguration; +import java.nio.file.Path; + /** * Collection of opensearch CLI tools. All of them need to have OPENSEARCH_PATH_CONF preconfigured, so they operate * on the correct version of configuration. @@ -27,7 +29,14 @@ public class OpensearchCli { private final OpensearchKeystoreCli keystore; public OpensearchCli(OpensearchConfiguration config) { - this.keystore = new OpensearchKeystoreCli(config); + this( + config.datanodeDirectories().getOpensearchProcessConfigurationDir(), + config.opensearchDistribution().getOpensearchBinDirPath() + ); + } + + public OpensearchCli(Path configDir, Path binDir) { + this.keystore = new OpensearchKeystoreCli(configDir, binDir); } public OpensearchKeystoreCli keystore() { diff --git a/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchCommandLineProcess.java b/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchCommandLineProcess.java index 86eb2b60b995..eae2377ac07c 100644 --- a/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchCommandLineProcess.java +++ b/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchCommandLineProcess.java @@ -100,31 +100,24 @@ private void writeOpenSearchConfig(final OpensearchConfiguration config) { public OpensearchCommandLineProcess(OpensearchConfiguration config, ProcessListener listener) { fixJdkOnMac(config); - configureS3RepositoryPlugin(config); + configureOpensearchKeystoreSecrets(config); final Path executable = config.opensearchDistribution().getOpensearchExecutable(); writeOpenSearchConfig(config); resultHandler = new CommandLineProcessListener(listener); commandLineProcess = new CommandLineProcess(executable, List.of(), resultHandler, config.getEnv()); } - private void configureS3RepositoryPlugin(OpensearchConfiguration config) { - if (config.s3RepositoryConfiguration().isRepositoryEnabled()) { - final OpensearchCli opensearchCli = new OpensearchCli(config); - configureS3Credentials(opensearchCli, config); - } else { - LOG.info("No S3 repository configuration provided, skipping plugin initialization"); - } - } - - private void configureS3Credentials(OpensearchCli opensearchCli, OpensearchConfiguration config) { + private void configureOpensearchKeystoreSecrets(OpensearchConfiguration config) { + final OpensearchCli opensearchCli = new OpensearchCli(config); LOG.info("Creating opensearch keystore"); final String createdMessage = opensearchCli.keystore().create(); LOG.info(createdMessage); - LOG.info("Setting opensearch s3 repository keystore secrets"); - opensearchCli.keystore().add("s3.client.default.access_key", config.s3RepositoryConfiguration().getS3ClientDefaultAccessKey()); - opensearchCli.keystore().add("s3.client.default.secret_key", config.s3RepositoryConfiguration().getS3ClientDefaultSecretKey()); + final Map keystoreItems = config.getKeystoreItems(); + keystoreItems.forEach((key, value) -> opensearchCli.keystore().add(key, value)); + LOG.info("Added {} keystore items", keystoreItems.size()); } + private static Map getOpensearchConfigurationArguments(OpensearchConfiguration config) { Map allArguments = new LinkedHashMap<>(config.asMap()); diff --git a/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchKeystoreCli.java b/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchKeystoreCli.java index bf3fb6cad6e0..9e5b25972b1d 100644 --- a/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchKeystoreCli.java +++ b/data-node/src/main/java/org/graylog/datanode/opensearch/cli/OpensearchKeystoreCli.java @@ -16,23 +16,25 @@ */ package org.graylog.datanode.opensearch.cli; -import org.graylog.datanode.opensearch.configuration.OpensearchConfiguration; - +import java.nio.file.Path; +import java.util.Arrays; import java.util.Collections; +import java.util.List; public class OpensearchKeystoreCli extends AbstractOpensearchCli { - OpensearchKeystoreCli(OpensearchConfiguration config) { - super(config, "opensearch-keystore"); + public OpensearchKeystoreCli(Path configDir, Path binDir) { + super(configDir, binDir, "opensearch-keystore"); } /** * Create a new opensearch keystore. This command expects that there is no keystore. If there is a keystore, * it will respond YES to override existing. + * * @return STDOUT/STDERR of the execution as one String */ public String create() { - return runWithStdin(Collections.singletonList("Y"),"create"); + return runWithStdin(Collections.singletonList("Y"), "create"); } /** @@ -40,6 +42,12 @@ public String create() { * in the command line history). So we have to work around that and provide the value in STDIN. */ public void add(String key, String secretValue) { - runWithStdin(Collections.singletonList(secretValue), "add", key); + runWithStdin(List.of(secretValue), "add", "-x", key); // -x allows input from stdin, bypassing the prompt + } + + public List list() { + final String rawResponse = runWithStdin(Collections.emptyList(), "list"); + final String[] items = rawResponse.split("\n"); + return Arrays.asList(items); } } diff --git a/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/OpensearchConfiguration.java b/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/OpensearchConfiguration.java index 30181168ef7c..180f04cc7bb8 100644 --- a/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/OpensearchConfiguration.java +++ b/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/OpensearchConfiguration.java @@ -20,6 +20,7 @@ import org.graylog.datanode.OpensearchDistribution; import org.graylog.datanode.configuration.DatanodeDirectories; import org.graylog.datanode.configuration.S3RepositoryConfiguration; +import org.graylog.datanode.configuration.variants.KeystoreContributor; import org.graylog.datanode.configuration.variants.OpensearchSecurityConfiguration; import org.graylog.datanode.process.Environment; import org.graylog.shaded.opensearch2.org.apache.http.HttpHost; @@ -28,6 +29,8 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; public record OpensearchConfiguration( OpensearchDistribution opensearchDistribution, @@ -45,7 +48,7 @@ public record OpensearchConfiguration( String nodeSearchCacheSize, Map additionalConfiguration -) { +) implements KeystoreContributor { public Map asMap() { Map config = new LinkedHashMap<>(); @@ -123,4 +126,12 @@ public HttpHost getClusterBaseUrl() { public boolean securityConfigured() { return opensearchSecurityConfiguration() != null; } + + + @Override + public Map getKeystoreItems() { + Stream keystoreContributorStream = Stream.of(opensearchSecurityConfiguration, s3RepositoryConfiguration); + return keystoreContributorStream.flatMap(config -> config.getKeystoreItems().entrySet().stream()) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + } } diff --git a/data-node/src/test/java/org/graylog/datanode/opensearch/cli/OpensearchKeystoreCommandLineIT.java b/data-node/src/test/java/org/graylog/datanode/opensearch/cli/OpensearchKeystoreCommandLineIT.java new file mode 100644 index 000000000000..86bd4a36cdd6 --- /dev/null +++ b/data-node/src/test/java/org/graylog/datanode/opensearch/cli/OpensearchKeystoreCommandLineIT.java @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +package org.graylog.datanode.opensearch.cli; + +import jakarta.annotation.Nonnull; +import org.assertj.core.api.Assertions; +import org.graylog.datanode.OpensearchDistribution; +import org.graylog.datanode.configuration.OpensearchArchitecture; +import org.graylog.datanode.configuration.OpensearchDistributionProvider; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.net.URISyntaxException; +import java.nio.file.Path; +import java.util.List; + +class OpensearchKeystoreCommandLineIT { + + @Test + void testKeystoreLifecycle(@TempDir Path tempDir) throws URISyntaxException { + final OpensearchCli cli = createCli(tempDir); + final String createdResponse = cli.keystore().create(); + + Assertions.assertThat(createdResponse).contains("Created opensearch keystore"); + + cli.keystore().add("s3.client.default.access_key", "foo"); + cli.keystore().add("s3.client.default.secret_key", "bar"); + + final List response = cli.keystore().list(); + Assertions.assertThat(response) + .hasSize(3) // two keys and one internal seed + .contains("s3.client.default.access_key") + .contains("s3.client.default.secret_key"); + } + + private OpensearchCli createCli(Path tempDir) throws URISyntaxException { + final Path binDirPath = detectOpensearchBinDir(); + return new OpensearchCli(tempDir, binDirPath); + } + + @Nonnull + private Path detectOpensearchBinDir() throws URISyntaxException { + final Path opensearchDistRoot = Path.of(getClass().getResource("/").toURI()).getParent().resolve("opensearch"); + final OpensearchDistributionProvider distributionProvider = new OpensearchDistributionProvider(opensearchDistRoot, OpensearchArchitecture.fromOperatingSystem()); + final OpensearchDistribution opensearchDistribution = distributionProvider.get(); + return opensearchDistribution.getOpensearchBinDirPath(); + } +}