Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TLS setup harmonization in the classic and async connection operators #540

Merged
merged 4 commits into from
Jan 26, 2024
Merged

TLS setup harmonization in the classic and async connection operators #540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
877aed3
Move plain socket create and connect operations to HttpClientConnecti…
ok2c Jan 21, 2024
c8cc5aa
Standard client TLS strategy implementations to support upgrade of bl…
ok2c Jan 21, 2024
e4ac7c2
Deprecated ConnectionSocketFactory, LayeredConnectionSocketFactory an…
ok2c Jan 22, 2024
fd2961a
Realigned the behavior of TLS upgrade in the classic and async connec…
ok2c Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .../test/java/org/apache/hc/client5/testing/external/CachingHttpClientCompatibilityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHost;
Expand Down Expand Up @@ -76,7 +76,7 @@ public static void main(final String... args) throws Exception {
final SSLContext sslContext = SSLContexts.custom()
.loadTrustMaterial(getClass().getResource("/test-ca.keystore"), "nopassword".toCharArray()).build();
this.connManager = PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(new SSLConnectionSocketFactory(sslContext))
.setTlsSocketStrategy(new DefaultClientTlsStrategy(sslContext))
.build();
this.client = CachingHttpClients.custom()
.setCacheConfig(CacheConfig.custom()
Expand Down
4 changes: 2 additions & 2 deletions ...ing/src/test/java/org/apache/hc/client5/testing/external/HttpClientCompatibilityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HeaderElements;
import org.apache.hc.core5.http.HttpHeaders;
Expand Down Expand Up @@ -102,7 +102,7 @@ public static void main(final String... args) throws Exception {
final SSLContext sslContext = SSLContexts.custom()
.loadTrustMaterial(getClass().getResource("/test-ca.keystore"), "nopassword".toCharArray()).build();
this.connManager = PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(new SSLConnectionSocketFactory(sslContext))
.setTlsSocketStrategy(new DefaultClientTlsStrategy(sslContext))
.build();
this.client = HttpClients.custom()
.setConnectionManager(this.connManager)
Expand Down
164 changes: 61 additions & 103 deletions ...t5/testing/sync/TestSSLSocketFactory.java → ...ng/sync/TestDefaultClientTlsStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,20 @@
import static org.hamcrest.MatcherAssert.assertThat;

import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.security.KeyManagementException;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.util.concurrent.atomic.AtomicBoolean;

import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLSession;
import javax.net.ssl.SSLSocket;

import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactoryBuilder;
import org.apache.hc.client5.http.ssl.TlsSocketStrategy;
import org.apache.hc.client5.testing.SSLTestContexts;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.impl.bootstrap.HttpServer;
Expand All @@ -55,17 +53,15 @@
import org.apache.hc.core5.io.CloseMode;
import org.apache.hc.core5.ssl.SSLContexts;
import org.apache.hc.core5.ssl.TrustStrategy;
import org.apache.hc.core5.util.TimeValue;
import org.apache.hc.core5.util.Timeout;
import org.hamcrest.CoreMatchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

/**
* Unit tests for {@link SSLConnectionSocketFactory}.
* Unit tests for {@link DefaultClientTlsStrategy}.
*/
public class TestSSLSocketFactory {
public class TestDefaultClientTlsStrategy {

private HttpServer server;

Expand Down Expand Up @@ -103,16 +99,14 @@ public void testBasicSSL() throws Exception {

final HttpContext context = new BasicHttpContext();
final TestX509HostnameVerifier hostVerifier = new TestX509HostnameVerifier();
final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(
final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(
SSLTestContexts.createClientSSLContext(), hostVerifier);
try (final Socket socket = socketFactory.createSocket(context)) {
final InetSocketAddress remoteAddress = new InetSocketAddress("localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", this.server.getLocalPort());
try (final SSLSocket sslSocket = (SSLSocket) socketFactory.connectSocket(
TimeValue.ZERO_MILLISECONDS,
final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort());
try (final Socket socket = new Socket(target.getHostName(), target.getPort())) {
try (final SSLSocket sslSocket = tlsStrategy.upgrade(
socket,
target,
remoteAddress,
target.getHostName(),
target.getPort(),
null,
context)) {
final SSLSession sslsession = sslSocket.getSession();
Expand All @@ -123,44 +117,6 @@ public void testBasicSSL() throws Exception {
}
}

@Test
public void testBasicSslConnectOverride() throws Exception {
this.server = ServerBootstrap.bootstrap()
.setSslContext(SSLTestContexts.createServerSSLContext())
.create();
this.server.start();

final HttpContext context = new BasicHttpContext();
final AtomicBoolean connectCalled = new AtomicBoolean();
final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(
SSLTestContexts.createClientSSLContext()) {
@Override
protected void connectSocket(
final Socket sock,
final InetSocketAddress remoteAddress,
final Timeout connectTimeout,
final HttpContext context) throws IOException {
connectCalled.set(true);
super.connectSocket(sock, remoteAddress, connectTimeout, context);
}
};
try (final Socket socket = socketFactory.createSocket(context)) {
final InetSocketAddress remoteAddress = new InetSocketAddress("localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", this.server.getLocalPort());
try (final SSLSocket sslSocket = (SSLSocket) socketFactory.connectSocket(
TimeValue.ZERO_MILLISECONDS,
socket,
target,
remoteAddress,
null,
context)) {
final SSLSession sslsession = sslSocket.getSession();
Assertions.assertNotNull(sslsession);
Assertions.assertTrue(connectCalled.get());
}
}
}

@Test
public void testBasicDefaultHostnameVerifier() throws Exception {
// @formatter:off
Expand All @@ -171,17 +127,13 @@ public void testBasicDefaultHostnameVerifier() throws Exception {
this.server.start();

final HttpContext context = new BasicHttpContext();
final SSLConnectionSocketFactory socketFactory = SSLConnectionSocketFactoryBuilder.create()
.setSslContext(SSLTestContexts.createClientSSLContext())
.build();
try (final Socket socket = socketFactory.createSocket(context)) {
final InetSocketAddress remoteAddress = new InetSocketAddress("localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", this.server.getLocalPort());
try (final SSLSocket sslSocket = (SSLSocket) socketFactory.connectSocket(
TimeValue.ZERO_MILLISECONDS,
final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext());
final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort());
try (final Socket socket = new Socket(target.getHostName(), target.getPort())) {
try (final SSLSocket sslSocket = tlsStrategy.upgrade(
socket,
target,
remoteAddress,
target.getHostName(),
target.getPort(),
null,
context)) {
final SSLSession sslsession = sslSocket.getSession();
Expand All @@ -202,16 +154,14 @@ public void testClientAuthSSL() throws Exception {

final HttpContext context = new BasicHttpContext();
final TestX509HostnameVerifier hostVerifier = new TestX509HostnameVerifier();
final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(
final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(
SSLTestContexts.createClientSSLContext(), hostVerifier);
try (final Socket socket = socketFactory.createSocket(context)) {
final InetSocketAddress remoteAddress = new InetSocketAddress("localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", this.server.getLocalPort());
try (final SSLSocket sslSocket = (SSLSocket) socketFactory.connectSocket(
TimeValue.ZERO_MILLISECONDS,
final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort());
try (final Socket socket = new Socket(target.getHostName(), target.getPort())) {
try (final SSLSocket sslSocket = tlsStrategy.upgrade(
socket,
target,
remoteAddress,
target.getHostName(),
target.getPort(),
null,
context)) {
final SSLSession sslsession = sslSocket.getSession();
Expand All @@ -234,16 +184,15 @@ public void testClientAuthSSLFailure() throws Exception {

final HttpContext context = new BasicHttpContext();
final TestX509HostnameVerifier hostVerifier = new TestX509HostnameVerifier();
final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(
final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(
SSLTestContexts.createClientSSLContext(), hostVerifier);
try (final Socket socket = socketFactory.createSocket(context)) {
final InetSocketAddress remoteAddress = new InetSocketAddress("localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort());
try (final Socket socket = new Socket(target.getHostName(), target.getPort())) {
Assertions.assertThrows(IOException.class, () -> {
try (final SSLSocket sslSocket = (SSLSocket) socketFactory.connectSocket(
TimeValue.ZERO_MILLISECONDS,
socket, target,
remoteAddress,
try (final SSLSocket sslSocket = tlsStrategy.upgrade(
socket,
target.getHostName(),
target.getPort(),
null,
context)) {
final SSLSession sslsession = sslSocket.getSession();
Expand All @@ -269,15 +218,14 @@ public void testSSLTrustVerification() throws Exception {
// Use default SSL context
final SSLContext defaultSslContext = SSLContexts.createDefault();

final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(defaultSslContext,
final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(defaultSslContext,
NoopHostnameVerifier.INSTANCE);

try (final Socket socket = socketFactory.createSocket(context)) {
final InetSocketAddress remoteAddress = new InetSocketAddress("localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort());
try (final Socket socket = new Socket(target.getHostName(), target.getPort())) {
Assertions.assertThrows(SSLException.class, () -> {
try (final SSLSocket sslSocket = (SSLSocket) socketFactory.connectSocket(
TimeValue.ZERO_MILLISECONDS, socket, target, remoteAddress, null, context)) {
try (final SSLSocket sslSocket = tlsStrategy.upgrade(
socket, target.getHostName(), target.getPort(), null, context)) {
// empty for now
}
});
Expand Down Expand Up @@ -306,14 +254,17 @@ private void testSSLTrustVerificationOverride(final TrustStrategy trustStrategy)
.loadTrustMaterial(null, trustStrategy)
.build();
// @formatter:on
final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(sslContext,
final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(sslContext,
NoopHostnameVerifier.INSTANCE);

try (final Socket socket = socketFactory.createSocket(context)) {
final InetSocketAddress remoteAddress = new InetSocketAddress("localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", this.server.getLocalPort());
try (final SSLSocket sslSocket = (SSLSocket) socketFactory.connectSocket(TimeValue.ZERO_MILLISECONDS, socket, target, remoteAddress,
null, context)) {
final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort());
try (final Socket socket = new Socket(target.getHostName(), target.getPort())) {
try (final SSLSocket sslSocket = tlsStrategy.upgrade(
socket,
target.getHostName(),
target.getPort(),
null,
context)) {
// empty for now
}
}
Expand All @@ -330,14 +281,17 @@ public void testSSLDisabledByDefault() throws Exception {
this.server.start();

final HttpContext context = new BasicHttpContext();
final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(
final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(
SSLTestContexts.createClientSSLContext());
try (final Socket socket = socketFactory.createSocket(context)) {
final InetSocketAddress remoteAddress = new InetSocketAddress("localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort());
try (final Socket socket = new Socket(target.getHostName(), target.getPort())) {
Assertions.assertThrows(IOException.class, () ->
socketFactory.connectSocket(
TimeValue.ZERO_MILLISECONDS, socket, target, remoteAddress, null, context));
tlsStrategy.upgrade(
socket,
target.getHostName(),
target.getPort(),
null,
context));
}
}

Expand Down Expand Up @@ -379,12 +333,16 @@ private void testWeakCipherDisabledByDefault(final String cipherSuite) throws Ex
this.server.start();

final HttpContext context = new BasicHttpContext();
final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(
final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(
SSLTestContexts.createClientSSLContext());
try (final Socket socket = socketFactory.createSocket(context)) {
final InetSocketAddress remoteAddress = new InetSocketAddress("localhost", this.server.getLocalPort());
final HttpHost target = new HttpHost("https", "localhost", this.server.getLocalPort());
socketFactory.connectSocket(TimeValue.ZERO_MILLISECONDS, socket, target, remoteAddress, null, context);
final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort());
try (final Socket socket = new Socket(target.getHostName(), target.getPort())) {
tlsStrategy.upgrade(
socket,
target.getHostName(),
target.getPort(),
null,
context);
}
}
}
6 changes: 3 additions & 3 deletions ...sting/src/test/java/org/apache/hc/client5/testing/sync/extension/TestClientResources.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import org.apache.hc.client5.http.impl.classic.MinimalHttpClient;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
import org.apache.hc.client5.testing.SSLTestContexts;
import org.apache.hc.core5.function.Decorator;
import org.apache.hc.core5.http.HttpHost;
Expand Down Expand Up @@ -124,7 +124,7 @@ public CloseableHttpClient startClient(
Assertions.assertNull(client);

final PoolingHttpClientConnectionManagerBuilder connManagerBuilder = PoolingHttpClientConnectionManagerBuilder.create();
connManagerBuilder.setSSLSocketFactory(new SSLConnectionSocketFactory(SSLTestContexts.createClientSSLContext()));
connManagerBuilder.setTlsSocketStrategy(new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext()));
connManagerBuilder.setDefaultSocketConfig(SocketConfig.custom()
.setSoTimeout(timeout)
.build());
Expand All @@ -147,7 +147,7 @@ public MinimalHttpClient startMinimalClient() throws Exception {
Assertions.assertNull(client);

final PoolingHttpClientConnectionManagerBuilder connManagerBuilder = PoolingHttpClientConnectionManagerBuilder.create();
connManagerBuilder.setSSLSocketFactory(new SSLConnectionSocketFactory(SSLTestContexts.createClientSSLContext()));
connManagerBuilder.setTlsSocketStrategy(new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext()));
connManagerBuilder.setDefaultSocketConfig(SocketConfig.custom()
.setSoTimeout(timeout)
.build());
Expand Down
4 changes: 2 additions & 2 deletions httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,9 +843,9 @@ public CloseableHttpAsyncClient build() {
TlsStrategy tlsStrategyCopy = this.tlsStrategy;
if (tlsStrategyCopy == null) {
if (systemProperties) {
tlsStrategyCopy = DefaultClientTlsStrategy.getSystemDefault();
tlsStrategyCopy = DefaultClientTlsStrategy.createSystemDefault();
} else {
tlsStrategyCopy = DefaultClientTlsStrategy.getDefault();
tlsStrategyCopy = DefaultClientTlsStrategy.createDefault();
}
}

Expand Down
2 changes: 1 addition & 1 deletion httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClients.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ public static MinimalH2AsyncClient createHttp2Minimal(
public static MinimalH2AsyncClient createHttp2Minimal(
final H2Config h2Config,
final IOReactorConfig ioReactorConfig) {
return createHttp2Minimal(h2Config, ioReactorConfig, DefaultClientTlsStrategy.getDefault());
return createHttp2Minimal(h2Config, ioReactorConfig, DefaultClientTlsStrategy.createDefault());
}

/**
Expand Down
Loading