Skip to content

Commit

Permalink
Fix tests after Apache HttpClient5 / HttpCore5 update since those use…
Browse files Browse the repository at this point in the history 
… deprecated APIs

Signed-off-by: Andriy Redko <[email protected]>
  • Loading branch information
@reta
reta committed Jan 17, 2025
1 parent 48be2af commit e953819
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 57 deletions.
3 changes: 2 additions & 1 deletion src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/KeySetRetriever.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.hc.client5.http.impl.classic.HttpClients;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
import org.apache.hc.client5.http.io.HttpClientConnectionManager;
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -223,7 +224,7 @@ private CloseableHttpClient createHttpClient(HttpCacheStorage httpCacheStorage)

if (sslConfig != null) {
final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(sslConfig.toSSLConnectionSocketFactory())
.setTlsSocketStrategy(new DefaultClientTlsStrategy(sslConfig.getSslContext()))
.build();

builder.setConnectionManager(cm);
Expand Down
18 changes: 13 additions & 5 deletions src/main/java/org/opensearch/security/auditlog/sink/WebhookSink.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@
import org.apache.hc.client5.http.impl.classic.HttpClients;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
import org.apache.hc.client5.http.io.HttpClientConnectionManager;
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.io.SocketConfig;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
import org.apache.hc.core5.ssl.SSLContextBuilder;
import org.apache.hc.core5.ssl.TrustStrategy;
import org.apache.http.HttpStatus;
Expand Down Expand Up @@ -380,15 +381,16 @@ public boolean isTrusted(X509Certificate[] chain, String authType) {
HttpClientBuilder hcb = HttpClients.custom().setDefaultRequestConfig(config);
if (!verifySSL) {
SSLContext sslContext = SSLContextBuilder.create().loadTrustMaterial(trustAllStrategy).build();
final SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory(
final DefaultClientTlsStrategy sslsf = new DefaultClientTlsStrategy(
sslContext,
null,
null,
SSLBufferMode.STATIC,
NoopHostnameVerifier.INSTANCE
);

final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(sslsf)
.setTlsSocketStrategy(sslsf)
.setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(timeout, TimeUnit.SECONDS).build())
.build();
hcb.setConnectionManager(cm);
Expand All @@ -399,10 +401,16 @@ public boolean isTrusted(X509Certificate[] chain, String authType) {
return HttpClients.custom().setDefaultRequestConfig(config).build();
}
SSLContext sslContext = SSLContextBuilder.create().loadTrustMaterial(effectiveTruststore, null).build();
final SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory(sslContext, null, null, new DefaultHostnameVerifier());
final DefaultClientTlsStrategy sslsf = new DefaultClientTlsStrategy(
sslContext,
null,
null,
SSLBufferMode.STATIC,
new DefaultHostnameVerifier()
);

final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(sslsf)
.setTlsSocketStrategy(sslsf)
.setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(timeout, TimeUnit.SECONDS).build())
.build();
hcb.setConnectionManager(cm);
Expand Down
26 changes: 9 additions & 17 deletions src/main/java/org/opensearch/security/httpclient/HttpClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,21 @@
import java.util.stream.Collectors;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;

import com.google.common.collect.Lists;
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder;
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
import org.apache.hc.client5.http.nio.AsyncClientConnectionManager;
import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder;
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.core5.function.Factory;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.message.BasicHeader;
import org.apache.hc.core5.http.nio.ssl.TlsStrategy;
import org.apache.hc.core5.reactor.ssl.TlsDetails;
import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
import org.apache.hc.core5.ssl.PrivateKeyDetails;
import org.apache.hc.core5.ssl.PrivateKeyStrategy;
import org.apache.hc.core5.ssl.SSLContextBuilder;
Expand Down Expand Up @@ -280,19 +278,13 @@ public String chooseAlias(Map<String, PrivateKeyDetails> aliases, SSLParameters
final HostnameVerifier hnv = verifyHostnames ? new DefaultHostnameVerifier() : NoopHostnameVerifier.INSTANCE;

final SSLContext sslContext = sslContextBuilder.build();
TlsStrategy tlsStrategy = ClientTlsStrategyBuilder.create()
.setSslContext(sslContext)
.setTlsVersions(supportedProtocols)
.setCiphers(supportedCipherSuites)
.setHostnameVerifier(hnv)
// See please https://issues.apache.org/jira/browse/HTTPCLIENT-2219
.setTlsDetailsFactory(new Factory<SSLEngine, TlsDetails>() {
@Override
public TlsDetails create(final SSLEngine sslEngine) {
return new TlsDetails(sslEngine.getSession(), sslEngine.getApplicationProtocol());
}
})
.build();
final TlsStrategy tlsStrategy = new DefaultClientTlsStrategy(
sslContext,
supportedProtocols,
supportedCipherSuites,
SSLBufferMode.STATIC,
hnv
);

final AsyncClientConnectionManager cm = PoolingAsyncClientConnectionManagerBuilder.create().setTlsStrategy(tlsStrategy).build();
httpClientBuilder.setConnectionManager(cm);
Expand Down
39 changes: 21 additions & 18 deletions src/test/java/com/amazon/dlic/auth/http/jwt/keybyoidc/MockIpdServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,27 @@ class MockIpdServer implements Closeable {
this.ssl = ssl;
this.jwks = jwks;

ServerBootstrap serverBootstrap = ServerBootstrap.bootstrap()
.setListenerPort(port)
.register(CTX_DISCOVER, new HttpRequestHandler() {

@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleDiscoverRequest(request, response, context);
}
})
.register(CTX_KEYS, new HttpRequestHandler() {

@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleKeysRequest(request, response, context);
}
});
ServerBootstrap serverBootstrap = ServerBootstrap.bootstrap().setListenerPort(port).setRequestRouter((request, context) -> {
if (request.getRequestUri().equals(CTX_DISCOVER)) {
return new HttpRequestHandler() {
@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleDiscoverRequest(request, response, context);
}
};
} else if (request.getRequestUri().equals(CTX_KEYS)) {
return new HttpRequestHandler() {
@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleKeysRequest(request, response, context);
}
};
} else {
return null;
}
});

if (ssl) {
serverBootstrap = serverBootstrap.setSslContext(createSSLContext()).setSslSetupHandler(new Callback<SSLParameters>() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/opensearch/security/InitializationIntegrationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public void testWhoAmIForceHttp1() throws Exception {
Response whoAmIRes = restHighLevelClient.getLowLevelClient().performRequest(new Request("GET", "/_plugins/_security/whoami"));
assertThat(200, is(whoAmIRes.getStatusLine().getStatusCode()));
// The HTTP/1.1 is forced and should be used instead
assertThat(HttpVersion.HTTP_1_1, is(whoAmIRes.getStatusLine().getProtocolVersion()));
assertThat(whoAmIRes.getStatusLine().getProtocolVersion(), is(HttpVersion.HTTP_1_1));
JsonNode whoAmIResNode = DefaultObjectMapper.objectMapper.readTree(whoAmIRes.getEntity().getContent());
String whoAmIResponsePayload = whoAmIResNode.toPrettyString();
assertThat(whoAmIResponsePayload, whoAmIResNode.get("dn").asText(), is("CN=spock,OU=client,O=client,L=Test,C=DE"));
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/opensearch/security/auditlog/sink/SinkProviderTLSTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void testTlsConfigurationNoFallback() throws Exception {
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.setSslContext(createSSLContext())
.register("*", handler)
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/org/opensearch/security/auditlog/sink/WebhookAuditLogTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ public void postGetHttpTest() throws Exception {
server = ServerBootstrap.bootstrap()
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.register("*", handler)
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down Expand Up @@ -355,7 +355,7 @@ public void httpsTestWithoutTLSServer() throws Exception {
server = ServerBootstrap.bootstrap()
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.register("*", handler)
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down Expand Up @@ -394,7 +394,7 @@ public void httpsTest() throws Exception {
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.setSslContext(createSSLContext())
.register("*", handler)
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down Expand Up @@ -482,7 +482,7 @@ public void httpsTestPemDefault() throws Exception {
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.setSslContext(createSSLContext())
.register("*", handler)
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down Expand Up @@ -611,7 +611,7 @@ public void httpsTestPemEndpoint() throws Exception {
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.setSslContext(createSSLContext())
.register("*", handler)
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down Expand Up @@ -718,7 +718,7 @@ public void httpsTestPemContentEndpoint() throws Exception {
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.setSslContext(createSSLContext())
.register("*", handler)
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down
13 changes: 6 additions & 7 deletions src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope.Scope;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import org.apache.hc.client5.http.config.TlsConfig;
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
import org.apache.hc.client5.http.nio.AsyncClientConnectionManager;
import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.core5.function.Factory;
Expand Down Expand Up @@ -197,14 +197,13 @@ public TlsDetails create(final SSLEngine sslEngine) {
})
.build();

final AsyncClientConnectionManager cm = PoolingAsyncClientConnectionManagerBuilder.create()
.setTlsStrategy(tlsStrategy)
.build();
builder.setConnectionManager(cm);
final PoolingAsyncClientConnectionManagerBuilder cm = PoolingAsyncClientConnectionManagerBuilder.create()
.setTlsStrategy(tlsStrategy);

if (httpVersionPolicy != null) {
builder.setVersionPolicy(httpVersionPolicy);
cm.setDefaultTlsConfig(TlsConfig.custom().setVersionPolicy(httpVersionPolicy).build());
}
return builder;
return builder.setConnectionManager(cm.build());
});
return new RestHighLevelClient(restClientBuilder);
} catch (Exception e) {
Expand Down
4 changes: 3 additions & 1 deletion src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,9 @@ public TlsDetails create(final SSLEngine sslEngine) {
hcb.setConnectionManager(cm);
}

final RequestConfig.Builder requestConfigBuilder = RequestConfig.custom().setResponseTimeout(Timeout.ofSeconds(60));
final RequestConfig.Builder requestConfigBuilder = RequestConfig.custom()
.setResponseTimeout(Timeout.ofSeconds(60))
.setProtocolUpgradeEnabled(false);

return hcb.setDefaultRequestConfig(requestConfigBuilder.build()).disableAutomaticRetries().build();
}
Expand Down

0 comments on commit e953819

Please sign in to comment.