Skip to content

Commit 1f37ae2

Browse files
dlvenabledlvenable
committed
Require full TLS trust in OpenSearch plugins by default unless insecure is configured (opensearch-project#6165)
Require full TLS trust in OpenSearch plugins by default unless insecure is configured. Update the integration tests and end-to-end tests to set the insecure flag. Co-authored-by: Jeremy Michael <jsusanto@amazon.com> Signed-off-by: David Venable <dlv@amazon.com> Signed-off-by: Jeremy Michael <jsusanto@amazon.com> (cherry picked from commit 98fcf0d)
1 parent b82ea06 commit 1f37ae2

25 files changed

Lines changed: 284 additions & 22 deletions

File tree

‎data-prepper-plugins/opensearch/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ dependencies {
4646
testImplementation 'net.bytebuddy:byte-buddy:1.15.11'
4747
testImplementation 'net.bytebuddy:byte-buddy-agent:1.15.11'
4848
testImplementation testLibs.slf4j.simple
49+
testImplementation 'org.wiremock:wiremock:3.10.0'
4950
}
5051

5152
sourceSets {

‎data-prepper-plugins/opensearch/src/integrationTest/java/org/opensearch/dataprepper/plugins/sink/opensearch/OpenSearchIT.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public void testOpenSearchConnection() throws IOException {
3232
builder.withUsername(user);
3333
builder.withPassword(password);
3434
}
35+
builder.withInsecure(true);
3536
final AwsCredentialsSupplier awsCredentialsSupplier = mock(AwsCredentialsSupplier.class);
3637
final RestHighLevelClient client = builder.build().createClient(awsCredentialsSupplier);
3738

‎data-prepper-plugins/opensearch/src/integrationTest/java/org/opensearch/dataprepper/plugins/sink/opensearch/OpenSearchSinkIT.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1691,6 +1691,7 @@ private Map<String, Object> initializeConfigurationMetadata(final String indexTy
16911691
metadata.put(IndexConfiguration.INDEX_ALIAS, indexAlias);
16921692
metadata.put(IndexConfiguration.TEMPLATE_FILE, templateFilePath);
16931693
metadata.put(IndexConfiguration.FLUSH_TIMEOUT, -1);
1694+
metadata.put("insecure", true);
16941695
final String user = System.getProperty("tests.opensearch.user");
16951696
final String password = System.getProperty("tests.opensearch.password");
16961697
if (user != null) {

‎data-prepper-plugins/opensearch/src/main/java/org/opensearch/dataprepper/plugins/sink/opensearch/ConnectionConfiguration.java‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,18 @@ private void checkProxyPort(final int port) {
384384
}
385385

386386
private void attachSSLContext(final HttpAsyncClientBuilder httpClientBuilder) {
387-
final SSLContext sslContext = certPath != null ? getCAStrategy(certPath) : getTrustAllStrategy();
388-
httpClientBuilder.setSSLContext(sslContext);
387+
final SSLContext sslContext;
388+
if(certPath != null) {
389+
sslContext = getCAStrategy(certPath);
390+
} else if(this.insecure) {
391+
sslContext = getTrustAllStrategy();
392+
} else {
393+
sslContext = null;
394+
}
395+
if(sslContext != null) {
396+
httpClientBuilder.setSSLContext(sslContext);
397+
}
398+
389399
if (this.insecure) {
390400
httpClientBuilder.setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE);
391401
}
@@ -439,7 +449,7 @@ private OpenSearchTransport createOpenSearchTransport(final RestHighLevelClient
439449
transportOptions.setRequestCompressionSize(Integer.MAX_VALUE);
440450
}
441451

442-
return new AwsSdk2Transport(createSdkHttpClient(), HttpHost.create(hosts.get(0)).getHostName(),
452+
return new AwsSdk2Transport(createSdkHttpClient(), HttpHost.create(hosts.get(0)).toHostString(),
443453
serviceName, Region.of(awsRegion), transportOptions.build());
444454
} else {
445455
return new RestClientTransport(
@@ -461,11 +471,13 @@ private SdkHttpClient createSdkHttpClient() {
461471
}
462472

463473
private void attachSSLContext(final ApacheHttpClient.Builder apacheHttpClientBuilder) {
464-
TrustManager[] trustManagers = createTrustManagers(certPath);
465-
apacheHttpClientBuilder.tlsTrustManagersProvider(() -> trustManagers);
474+
TrustManager[] trustManagers = createTrustManagers(certPath, insecure);
475+
if(trustManagers.length > 0) {
476+
apacheHttpClientBuilder.tlsTrustManagersProvider(() -> trustManagers);
477+
}
466478
}
467479

468-
private static TrustManager[] createTrustManagers(final Path certPath) {
480+
private static TrustManager[] createTrustManagers(final Path certPath, final boolean insecure) {
469481
if (certPath != null) {
470482
LOG.info("Using the cert provided in the config.");
471483
try (InputStream certificateInputStream = Files.newInputStream(certPath)) {
@@ -481,8 +493,11 @@ private static TrustManager[] createTrustManagers(final Path certPath) {
481493
} catch (Exception ex) {
482494
throw new RuntimeException(ex.getMessage(), ex);
483495
}
484-
} else {
496+
} else if(insecure) {
497+
LOG.info("Using the trust all strategy");
485498
return new TrustManager[] { new X509TrustAllManager() };
499+
} else {
500+
return new TrustManager[0];
486501
}
487502
}
488503

‎data-prepper-plugins/opensearch/src/main/java/org/opensearch/dataprepper/plugins/source/opensearch/worker/client/OpenSearchClientFactory.java‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,9 @@ private void setConnectAndSocketTimeout(final org.elasticsearch.client.RestClien
271271

272272
private void attachSSLContext(final NettyNioAsyncHttpClient.Builder asyncClientBuilder, final OpenSearchSourceConfiguration openSearchSourceConfiguration) {
273273
TrustManager[] trustManagers = createTrustManagers(openSearchSourceConfiguration.getConnectionConfiguration());
274-
asyncClientBuilder.tlsTrustManagersProvider(() -> trustManagers);
274+
if (trustManagers.length > 0) {
275+
asyncClientBuilder.tlsTrustManagersProvider(() -> trustManagers);
276+
}
275277
}
276278

277279
private void attachSSLContext(final HttpAsyncClientBuilder httpClientBuilder, final OpenSearchSourceConfiguration openSearchSourceConfiguration) {
@@ -287,31 +289,37 @@ private void attachSSLContext(final HttpAsyncClientBuilder httpClientBuilder, fi
287289

288290
private TrustManager[] createTrustManagers(final ConnectionConfiguration connectionConfiguration) {
289291
final Path certPath = connectionConfiguration.getCertPath();
290-
if (Objects.nonNull(certPath)) {
292+
final String certificate = connectionConfiguration.getCertificate();
293+
if (certPath != null) {
291294
return TrustStoreProvider.createTrustManager(certPath);
292-
} else if (Objects.nonNull(connectionConfiguration.getCertificate())) {
293-
if (PemObjectValidator.isPemObject(connectionConfiguration.getCertificate())) {
294-
return TrustStoreProvider.createTrustManager(connectionConfiguration.getCertificate());
295+
} else if (certificate != null) {
296+
if (PemObjectValidator.isPemObject(certificate)) {
297+
return TrustStoreProvider.createTrustManager(certificate);
295298
} else {
296-
return TrustStoreProvider.createTrustManager(Path.of(connectionConfiguration.getCertificate()));
297-
}
298-
} else {
299+
return TrustStoreProvider.createTrustManager(Path.of(certificate));}
300+
} else if (connectionConfiguration.isInsecure()) {
299301
return TrustStoreProvider.createTrustAllManager();
302+
303+
} else {
304+
return new TrustManager[0];
300305
}
301306
}
302307

303308
private SSLContext getCAStrategy(final ConnectionConfiguration connectionConfiguration) {
304309
final Path certPath = connectionConfiguration.getCertPath();
305-
if (Objects.nonNull(certPath)) {
310+
final String certificate = connectionConfiguration.getCertificate();
311+
if (certPath != null) {
306312
return TrustStoreProvider.createSSLContext(certPath);
307-
} else if (Objects.nonNull(connectionConfiguration.getCertificate())) {
308-
if (PemObjectValidator.isPemObject(connectionConfiguration.getCertificate())) {
309-
return TrustStoreProvider.createSSLContext(connectionConfiguration.getCertificate());
313+
} else if (certificate != null) {
314+
if (PemObjectValidator.isPemObject(certificate)) {
315+
return TrustStoreProvider.createSSLContext(certificate);
310316
} else {
311317
return TrustStoreProvider.createSSLContext(Path.of(connectionConfiguration.getCertificate()));
312318
}
319+
} else if (connectionConfiguration.isInsecure()) {
320+
return TrustStoreProvider.createSSLContextWithTrustAllStrategy();
313321
} else {
314-
return TrustStoreProvider.createSSLContextWithTrustAllStrategy();
322+
return null;
315323
}
316324
}
317325
}

‎data-prepper-plugins/opensearch/src/test/java/org/opensearch/dataprepper/plugins/sink/opensearch/ConnectionConfigurationTests.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,6 @@ void testCreateOpenSearchClientAwsServerlessDefault() throws IOException {
150150
when(awsCredentialsSupplier.getProvider(any())).thenReturn(awsCredentialsProvider);
151151

152152
final RestHighLevelClient client = connectionConfiguration.createClient(awsCredentialsSupplier);
153-
when(apacheHttpClientBuilder.tlsTrustManagersProvider(any())).thenReturn(apacheHttpClientBuilder);
154153
when(apacheHttpClientBuilder.build()).thenReturn(apacheHttpClient);
155154
final OpenSearchClient openSearchClient;
156155
try (final MockedStatic<ApacheHttpClient> apacheHttpClientMockedStatic = mockStatic(ApacheHttpClient.class)) {
@@ -160,7 +159,6 @@ void testCreateOpenSearchClientAwsServerlessDefault() throws IOException {
160159
assertNotNull(openSearchClient);
161160
assertThat(openSearchClient._transport(), instanceOf(AwsSdk2Transport.class));
162161
assertThat(openSearchClient._transport().jsonpMapper(), instanceOf(PreSerializedJsonpMapper.class));
163-
verify(apacheHttpClientBuilder).tlsTrustManagersProvider(any());
164162
verify(apacheHttpClientBuilder).build();
165163
openSearchClient.shutdown();
166164
client.close();

‎data-prepper-plugins/opensearch/src/test/java/org/opensearch/dataprepper/plugins/sink/opensearch/ConnectionConfiguration_ServerTest.java‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.dataprepper.plugins.sink.opensearch;
7+
8+
import com.github.tomakehurst.wiremock.WireMockServer;
9+
import org.junit.jupiter.api.AfterAll;
10+
import org.junit.jupiter.api.BeforeAll;
11+
import org.junit.jupiter.api.BeforeEach;
12+
import org.junit.jupiter.api.Nested;
13+
import org.junit.jupiter.api.Test;
14+
import org.junit.jupiter.api.extension.ExtendWith;
15+
import org.mockito.Mock;
16+
import org.mockito.junit.jupiter.MockitoExtension;
17+
import org.opensearch.client.RequestOptions;
18+
import org.opensearch.client.RestHighLevelClient;
19+
import org.opensearch.client.core.MainResponse;
20+
import org.opensearch.client.opensearch.OpenSearchClient;
21+
import org.opensearch.client.opensearch.core.InfoResponse;
22+
import org.opensearch.dataprepper.aws.api.AwsCredentialsSupplier;
23+
import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider;
24+
25+
import javax.net.ssl.SSLHandshakeException;
26+
import java.io.IOException;
27+
import java.util.Collections;
28+
import java.util.Map;
29+
import java.util.UUID;
30+
31+
import static com.github.tomakehurst.wiremock.client.WireMock.get;
32+
import static com.github.tomakehurst.wiremock.client.WireMock.jsonResponse;
33+
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
34+
import static org.hamcrest.CoreMatchers.equalTo;
35+
import static org.hamcrest.CoreMatchers.notNullValue;
36+
import static org.hamcrest.MatcherAssert.assertThat;
37+
import static org.junit.jupiter.api.Assertions.assertThrows;
38+
import static org.mockito.ArgumentMatchers.any;
39+
import static org.mockito.Mockito.when;
40+
41+
@ExtendWith(MockitoExtension.class)
42+
class ConnectionConfiguration_ServerTest {
43+
private static WireMockServer wireMockServer;
44+
45+
@Mock
46+
private AwsCredentialsSupplier awsCredentialsSupplier;
47+
48+
private String host;
49+
50+
private String clusterUuid;
51+
52+
@BeforeAll
53+
static void setUpAll() {
54+
wireMockServer = new WireMockServer(options()
55+
.httpDisabled(true)
56+
.dynamicHttpsPort()
57+
.keystorePath("src/test/resources/test_keystore.jks")
58+
.keystorePassword("password")
59+
.keyManagerPassword("password")
60+
);
61+
62+
wireMockServer.start();
63+
}
64+
65+
@AfterAll
66+
static void tearDownAll() {
67+
wireMockServer.stop();
68+
}
69+
70+
@BeforeEach
71+
void setUp() {
72+
host = "https://localhost:" + wireMockServer.httpsPort();
73+
74+
clusterUuid = UUID.randomUUID().toString();
75+
final Map<String, Object> responseBody = Map.of(
76+
"name", "opensearch",
77+
"cluster_name", "opensearch",
78+
"cluster_uuid", clusterUuid,
79+
"version", Map.of(
80+
"number", "2.10.0",
81+
"build_hash", "abcdefg",
82+
"build_date", "20241212",
83+
"build_type", "testing",
84+
"distribution", "datapreppertesting",
85+
"build_snapshot", "false",
86+
"lucene_version", "8",
87+
"minimum_wire_compatibility_version", "2.10.0",
88+
"minimum_index_compatibility_version", "2.10.0"
89+
),
90+
"tagline", "You Know, for Search"
91+
);
92+
wireMockServer.stubFor(get("/").willReturn(jsonResponse(responseBody, 200)));
93+
}
94+
95+
@Nested
96+
class DefaultConfiguration {
97+
private ConnectionConfiguration createObjectUnderTest() {
98+
return new ConnectionConfiguration.Builder(Collections.singletonList(host))
99+
.build();
100+
}
101+
102+
@Test
103+
void createClient_will_not_trust_self_signed_certificates_by_default() {
104+
final RestHighLevelClient client = createObjectUnderTest().createClient(awsCredentialsSupplier);
105+
assertThat(client, notNullValue());
106+
107+
assertThrows(SSLHandshakeException.class, () -> client.info(RequestOptions.DEFAULT));
108+
}
109+
110+
@Test
111+
void createOpenSearchClient_will_not_trust_self_signed_certificates_by_default() {
112+
final ConnectionConfiguration objectUnderTest = createObjectUnderTest();
113+
final OpenSearchClient openSearchClient = objectUnderTest.createOpenSearchClient(objectUnderTest.createClient(awsCredentialsSupplier), awsCredentialsSupplier);
114+
assertThat(openSearchClient, notNullValue());
115+
116+
assertThrows(SSLHandshakeException.class, openSearchClient::info);
117+
}
118+
}
119+
120+
@Nested
121+
class DefaultSigV4Configuration {
122+
@BeforeEach
123+
void setUp() {
124+
when(awsCredentialsSupplier.getProvider(any())).thenReturn(AnonymousCredentialsProvider.create());
125+
}
126+
127+
private ConnectionConfiguration createObjectUnderTest() {
128+
return new ConnectionConfiguration.Builder(Collections.singletonList(host))
129+
.withAwsSigv4(true)
130+
.withAwsRegion("us-east-1")
131+
.build();
132+
}
133+
134+
@Test
135+
void createClient_will_not_trust_self_signed_certificates_by_default() {
136+
final RestHighLevelClient client = createObjectUnderTest().createClient(awsCredentialsSupplier);
137+
assertThat(client, notNullValue());
138+
139+
assertThrows(SSLHandshakeException.class, () -> client.info(RequestOptions.DEFAULT));
140+
}
141+
142+
@Test
143+
void createOpenSearchClient_will_not_trust_self_signed_certificates_by_default() {
144+
final ConnectionConfiguration objectUnderTest = createObjectUnderTest();
145+
final OpenSearchClient openSearchClient = objectUnderTest.createOpenSearchClient(objectUnderTest.createClient(awsCredentialsSupplier), awsCredentialsSupplier);
146+
assertThat(openSearchClient, notNullValue());
147+
148+
assertThrows(SSLHandshakeException.class, openSearchClient::info);
149+
}
150+
}
151+
152+
@Nested
153+
class InsecureConfiguration {
154+
private ConnectionConfiguration createObjectUnderTest() {
155+
return new ConnectionConfiguration.Builder(Collections.singletonList(host))
156+
.withInsecure(true)
157+
.build();
158+
}
159+
160+
@Test
161+
void createClient_will_trust_self_signed_certificates_if_insecure() throws IOException {
162+
final RestHighLevelClient client = createObjectUnderTest().createClient(awsCredentialsSupplier);
163+
assertThat(client, notNullValue());
164+
165+
final MainResponse infoResponse = client.info(RequestOptions.DEFAULT);
166+
167+
assertThat(infoResponse, notNullValue());
168+
assertThat(infoResponse.getClusterName(), equalTo("opensearch"));
169+
assertThat(infoResponse.getClusterUuid(), equalTo(clusterUuid));
170+
}
171+
172+
173+
@Test
174+
void createOpenSearchClient_will_trust_self_signed_certificates_if_insecure() throws IOException {
175+
final ConnectionConfiguration objectUnderTest = createObjectUnderTest();
176+
final OpenSearchClient openSearchClient = objectUnderTest.createOpenSearchClient(objectUnderTest.createClient(awsCredentialsSupplier), awsCredentialsSupplier);
177+
assertThat(openSearchClient, notNullValue());
178+
179+
final InfoResponse infoResponse = openSearchClient.info();
180+
181+
assertThat(infoResponse, notNullValue());
182+
assertThat(infoResponse.clusterName(), equalTo("opensearch"));
183+
assertThat(infoResponse.clusterUuid(), equalTo(clusterUuid));
184+
}
185+
}
186+
}

0 commit comments

Comments
 (0)