Skip to content

Commit

Permalink
Merge pull request #967 from gregschohn/FixProxyConfigFileParsingAndW…
Browse files Browse the repository at this point in the history 
…eakTls

Parse config file w/ jackson and set better TLS defaults
  • Loading branch information
@gregschohn
gregschohn committed Sep 19, 2024
2 parents b277fec + a902b14 commit f7d6fb6
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 23 deletions.
3 changes: 2 additions & 1 deletion TrafficCapture/dockerSolution/src/main/docker/elasticsearchWithSearchGuard/Dockerfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ RUN sed 's/searchguard/plugins.security/g' $ELASTIC_SEARCH_CONFIG_FILE | \
# but maybe not for a demo to show individual steps
RUN /root/enableTlsConfig.sh $ELASTIC_SEARCH_CONFIG_FILE
# Alter this config line to either enable(searchguard.disabled: false) or disable(searchguard.disabled: true) HTTP auth
RUN echo "searchguard.disabled: false" >> $ELASTIC_SEARCH_CONFIG_FILE
RUN echo -n "searchguard.disabled: false" >> $ELASTIC_SEARCH_CONFIG_FILE && \
echo -n "plugins.security.ssl.http.enabled_protocols: ['TLSv1.2', 'TLSv1.3']" >> $PROXY_TLS_CONFIG_FILE

RUN sed -i '/^-Xms/i # Increase default heap to 80% RAM, Requires JDK >= 10' $ELASTIC_SEARCH_JVM_CONFIG_FILE && \
sed -i 's/^-Xms/#&/' $ELASTIC_SEARCH_JVM_CONFIG_FILE && \
Expand Down
8 changes: 5 additions & 3 deletions TrafficCapture/trafficCaptureProxyServer/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,17 @@ dependencies {
implementation project(':TrafficCapture:captureKafkaOffloader')
implementation project(':coreUtilities')

implementation group: "com.google.protobuf", name: "protobuf-java"
implementation group: 'com.fasterxml.jackson.core', name: 'jackson-core'
implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind'
implementation group: 'com.lmax', name: 'disruptor'
implementation group: 'io.netty', name: 'netty-all'
implementation group: 'org.apache.logging.log4j', name: 'log4j-api'
implementation group: 'org.apache.logging.log4j', name: 'log4j-core'
implementation group: 'org.apache.logging.log4j', name: 'log4j-slf4j2-impl'
implementation group: 'org.jcommander', name: 'jcommander'
implementation group: 'org.slf4j', name: 'slf4j-api'
implementation group: 'com.lmax', name: 'disruptor'

implementation group: 'org.jcommander', name: 'jcommander'
implementation group: "com.google.protobuf", name: "protobuf-java"

testImplementation project(':TrafficCapture:captureProtobufs')
testImplementation testFixtures(project(path: ':testHelperFixtures'))
Expand Down
48 changes: 29 additions & 19 deletions ...rver/src/main/java/org/opensearch/migrations/trafficcapture/proxyserver/CaptureProxy.java
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package org.opensearch.migrations.trafficcapture.proxyserver;

import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.io.OutputStream;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.time.Duration;
import java.util.ArrayList;
Expand All @@ -18,11 +18,15 @@
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLException;

import com.google.protobuf.CodedOutputStream;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
import org.apache.kafka.clients.CommonClientConfigs;
import org.apache.kafka.clients.producer.KafkaProducer;
import org.apache.kafka.clients.producer.ProducerConfig;
Expand Down Expand Up @@ -69,6 +73,7 @@ public class CaptureProxy {

private static final String HTTPS_CONFIG_PREFIX = "plugins.security.ssl.http.";
public static final String DEFAULT_KAFKA_CLIENT_ID = "HttpCaptureProxyProducer";
public static final String SUPPORTED_TLS_PROTOCOLS_LIST_KEY = "plugins.security.ssl.http.enabled_protocols";

public static class Parameters {
@Parameter(required = false,
Expand Down Expand Up @@ -191,19 +196,26 @@ static Parameters parseArgs(String[] args) {

@SneakyThrows
protected static Settings getSettings(@NonNull String configFile) {
var builder = Settings.builder();
try (var lines = Files.lines(Paths.get(configFile))) {
lines.map(
line -> Optional.of(line.indexOf('#')).filter(i -> i >= 0).map(i -> line.substring(0, i)).orElse(line)
).filter(line -> line.startsWith(HTTPS_CONFIG_PREFIX) && line.contains(":")).forEach(line -> {
var parts = line.split(": *", 2);
builder.put(parts[0], parts[1]);
});
}
builder.put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED, false);
var objectMapper = new ObjectMapper(new YAMLFactory());
var configMap = objectMapper.readValue(new File(configFile), Map.class);

var configParentDirStr = Paths.get(configFile).toAbsolutePath().getParent();
builder.put("path.home", configParentDirStr);
return builder.build();
var httpsSettings =
objectMapper.convertValue(configMap, new TypeReference<Map<String, Object>>(){})
.entrySet().stream()
.filter(kvp -> kvp.getKey().startsWith(HTTPS_CONFIG_PREFIX))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
if (!httpsSettings.containsKey(SUPPORTED_TLS_PROTOCOLS_LIST_KEY)) {
httpsSettings.put(SUPPORTED_TLS_PROTOCOLS_LIST_KEY, List.of("TLSv1.2", "TLSv1.3"));
}

return Settings.builder().loadFromMap(httpsSettings)
// Don't bother with configurations the 'transport' (port 9300), which the plugin that we're using
// will also configure (& fail) otherwise. We only use the plugin to setup security for the 'http'
// port and then move the SSLEngine into our implementation.
.put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED, false)
.put("path.home", configParentDirStr)
.build();
}

protected static IConnectionCaptureFactory<Object> getNullConnectionCaptureFactory() {
Expand Down Expand Up @@ -371,12 +383,10 @@ public static void main(String[] args) throws InterruptedException, IOException
);

var sksOp = Optional.ofNullable(params.sslConfigFilePath)
.map(
sslConfigFile -> new DefaultSecurityKeyStore(
getSettings(sslConfigFile),
Paths.get(sslConfigFile).toAbsolutePath().getParent()
)
);
.map(sslConfigFile -> new DefaultSecurityKeyStore(
getSettings(sslConfigFile),
Paths.get(sslConfigFile).toAbsolutePath().getParent()))
.filter(sks -> sks.sslHTTPProvider != null);

sksOp.ifPresent(DefaultSecurityKeyStore::initHttpSSLConfig);
var proxy = new NettyScanningHttpProxy(params.frontsidePort);
Expand Down
41 changes: 41 additions & 0 deletions ...test/java/org/opensearch/migrations/trafficcapture/proxyserver/CaptureProxySetupTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package org.opensearch.migrations.trafficcapture.proxyserver;

import java.io.IOException;
import java.nio.file.Files;
import java.util.List;
import java.util.Map;
import java.util.Properties;

import org.apache.kafka.clients.CommonClientConfigs;
Expand All @@ -12,6 +15,7 @@
public class CaptureProxySetupTest {

public final static String kafkaBrokerString = "invalid:9092";
public static final String TLS_PROTOCOLS_KEY = "plugins.security.ssl.http.enabled_protocols";

@Test
public void testBuildKafkaPropertiesBaseCase() throws IOException {
Expand Down Expand Up @@ -111,4 +115,41 @@ public void testBuildKafkaPropertiesWithPropertyFile() throws IOException {
// Settings needed for other passed arguments (i.e. --enableMSKAuth) are ignored by property file
Assertions.assertEquals("SASL_SSL", props.get(CommonClientConfigs.SECURITY_PROTOCOL_CONFIG));
}

@Test
public void testTlsParametersAreProperlyRead() throws Exception {
for (var kvp : Map.of(
"[ TLSv1.3, TLSv1.2 ]", List.of("TLSv1.3","TLSv1.2"),
"[ TLSv1.2, TLSv1.3 ]", List.of("TLSv1.2","TLSv1.3"),
"\n - TLSv1.2\n - TLSv1.3", List.of("TLSv1.2","TLSv1.3"),
"\n - TLSv1.2", List.of("TLSv1.2"))
.entrySet())
{
testTlsParametersAreProperlyRead(TLS_PROTOCOLS_KEY + ": " + kvp.getKey(), kvp.getValue());
}
}

@Test
public void testNoProtocolConfigDefaultsToSecureOnesOnly() throws Exception {
testTlsParametersAreProperlyRead("", List.of("TLSv1.2","TLSv1.3"));
}

public void testTlsParametersAreProperlyRead(String protocolsBlockString, List<String> expectedList)
throws Exception
{
var tempFile = Files.createTempFile("captureProxy_tlsConfig", "yaml");
try {
Files.writeString(tempFile, "plugins.security.ssl.http.enabled: true\n" +
"plugins.security.ssl.http.pemcert_filepath: esnode.pem\n" +
"plugins.security.ssl.http.pemkey_filepath: esnode-key.pem\n" +
"plugins.security.ssl.http.pemtrustedcas_filepath: root-ca.pem\n" +
protocolsBlockString);

var settings = CaptureProxy.getSettings(tempFile.toAbsolutePath().toString());
Assertions.assertEquals(String.join(", ", expectedList),
String.join(", ", settings.getAsList(TLS_PROTOCOLS_KEY)));
} finally {
Files.deleteIfExists(tempFile);
}
}
}

0 comments on commit f7d6fb6

Please sign in to comment.