From c9bffa67b5eade12ab03e392a44f84cd30e7ae42 Mon Sep 17 00:00:00 2001 From: arithmetic1728 Date: Thu, 12 Nov 2020 23:44:26 -0800 Subject: [PATCH 1/2] fix: socketfactory not registered for apache --- .../apache/v2/GoogleApacheHttpTransport.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/google-api-client/src/main/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransport.java b/google-api-client/src/main/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransport.java index f5f41fddf..2c0a44a82 100644 --- a/google-api-client/src/main/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransport.java +++ b/google-api-client/src/main/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransport.java @@ -28,6 +28,9 @@ import java.util.concurrent.TimeUnit; import javax.net.ssl.SSLContext; import org.apache.http.client.HttpClient; +import org.apache.http.config.Registry; +import org.apache.http.config.RegistryBuilder; +import org.apache.http.conn.socket.ConnectionSocketFactory; import org.apache.http.conn.socket.LayeredConnectionSocketFactory; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.impl.client.HttpClientBuilder; @@ -71,12 +74,6 @@ public static ApacheHttpTransport newTrustedTransport(MtlsProvider mtlsProvider) mtlsKeyStorePassword = mtlsProvider.getKeyStorePassword(); } - PoolingHttpClientConnectionManager connectionManager = - new PoolingHttpClientConnectionManager(-1, TimeUnit.MILLISECONDS); - - // Disable the stale connection check (previously configured in the HttpConnectionParams - connectionManager.setValidateAfterInactivity(-1); - // Use the included trust store KeyStore trustStore = GoogleUtils.getCertificateTrustStore(); SSLContext sslContext = SslUtils.getTlsSslContext(); @@ -96,6 +93,16 @@ public static ApacheHttpTransport newTrustedTransport(MtlsProvider mtlsProvider) } LayeredConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(sslContext); + Registry socketFactoryRegistry = + RegistryBuilder.create().register("https", socketFactory).build(); + PoolingHttpClientConnectionManager connectionManager = + new PoolingHttpClientConnectionManager( + socketFactoryRegistry, null, null, null, -1, TimeUnit.MILLISECONDS); + + // Disable the stale connection check (previously configured in the + // HttpConnectionParams + connectionManager.setValidateAfterInactivity(-1); + HttpClient client = HttpClientBuilder.create() .useSystemProperties() From 0be40aa37437b188c6108f916268895bb7e9dd5a Mon Sep 17 00:00:00 2001 From: arithmetic1728 Date: Fri, 13 Nov 2020 14:04:27 -0800 Subject: [PATCH 2/2] refactor code and add test --- .../apache/v2/GoogleApacheHttpTransport.java | 85 ++++++++++++------- .../v2/GoogleApacheHttpTransportTest.java | 14 +++ .../mtls/MtlsTransportBaseTest.java | 2 +- 3 files changed, 69 insertions(+), 32 deletions(-) diff --git a/google-api-client/src/main/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransport.java b/google-api-client/src/main/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransport.java index 2c0a44a82..93347cd3e 100644 --- a/google-api-client/src/main/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransport.java +++ b/google-api-client/src/main/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransport.java @@ -21,6 +21,7 @@ import com.google.api.client.http.apache.v2.ApacheHttpTransport; import com.google.api.client.util.Beta; import com.google.api.client.util.SslUtils; +import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.net.ProxySelector; import java.security.GeneralSecurityException; @@ -32,6 +33,7 @@ import org.apache.http.config.RegistryBuilder; import org.apache.http.conn.socket.ConnectionSocketFactory; import org.apache.http.conn.socket.LayeredConnectionSocketFactory; +import org.apache.http.conn.socket.PlainConnectionSocketFactory; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; @@ -67,37 +69,10 @@ public static ApacheHttpTransport newTrustedTransport() @Beta public static ApacheHttpTransport newTrustedTransport(MtlsProvider mtlsProvider) throws GeneralSecurityException, IOException { - KeyStore mtlsKeyStore = null; - String mtlsKeyStorePassword = null; - if (mtlsProvider.useMtlsClientCertificate()) { - mtlsKeyStore = mtlsProvider.getKeyStore(); - mtlsKeyStorePassword = mtlsProvider.getKeyStorePassword(); - } - - // Use the included trust store - KeyStore trustStore = GoogleUtils.getCertificateTrustStore(); - SSLContext sslContext = SslUtils.getTlsSslContext(); - - boolean isMtls = false; - if (mtlsKeyStore != null && mtlsKeyStorePassword != null) { - isMtls = true; - SslUtils.initSslContext( - sslContext, - trustStore, - SslUtils.getPkixTrustManagerFactory(), - mtlsKeyStore, - mtlsKeyStorePassword, - SslUtils.getDefaultKeyManagerFactory()); - } else { - SslUtils.initSslContext(sslContext, trustStore, SslUtils.getPkixTrustManagerFactory()); - } - LayeredConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(sslContext); - - Registry socketFactoryRegistry = - RegistryBuilder.create().register("https", socketFactory).build(); + SocketFactoryRegistryHandler handler = new SocketFactoryRegistryHandler(mtlsProvider); PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager( - socketFactoryRegistry, null, null, null, -1, TimeUnit.MILLISECONDS); + handler.getSocketFactoryRegistry(), null, null, null, -1, TimeUnit.MILLISECONDS); // Disable the stale connection check (previously configured in the // HttpConnectionParams @@ -106,7 +81,6 @@ public static ApacheHttpTransport newTrustedTransport(MtlsProvider mtlsProvider) HttpClient client = HttpClientBuilder.create() .useSystemProperties() - .setSSLSocketFactory(socketFactory) .setMaxConnTotal(200) .setMaxConnPerRoute(20) .setRoutePlanner(new SystemDefaultRoutePlanner(ProxySelector.getDefault())) @@ -114,7 +88,56 @@ public static ApacheHttpTransport newTrustedTransport(MtlsProvider mtlsProvider) .disableRedirectHandling() .disableAutomaticRetries() .build(); - return new ApacheHttpTransport(client, isMtls); + return new ApacheHttpTransport(client, handler.isMtls()); + } + + @VisibleForTesting + static class SocketFactoryRegistryHandler { + private final Registry socketFactoryRegistry; + private final boolean isMtls; + + public SocketFactoryRegistryHandler(MtlsProvider mtlsProvider) + throws GeneralSecurityException, IOException { + KeyStore mtlsKeyStore = null; + String mtlsKeyStorePassword = null; + if (mtlsProvider.useMtlsClientCertificate()) { + mtlsKeyStore = mtlsProvider.getKeyStore(); + mtlsKeyStorePassword = mtlsProvider.getKeyStorePassword(); + } + + // Use the included trust store + KeyStore trustStore = GoogleUtils.getCertificateTrustStore(); + SSLContext sslContext = SslUtils.getTlsSslContext(); + + if (mtlsKeyStore != null && mtlsKeyStorePassword != null) { + this.isMtls = true; + SslUtils.initSslContext( + sslContext, + trustStore, + SslUtils.getPkixTrustManagerFactory(), + mtlsKeyStore, + mtlsKeyStorePassword, + SslUtils.getDefaultKeyManagerFactory()); + } else { + this.isMtls = false; + SslUtils.initSslContext(sslContext, trustStore, SslUtils.getPkixTrustManagerFactory()); + } + LayeredConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(sslContext); + + this.socketFactoryRegistry = + RegistryBuilder.create() + .register("http", PlainConnectionSocketFactory.getSocketFactory()) + .register("https", socketFactory) + .build(); + } + + public Registry getSocketFactoryRegistry() { + return this.socketFactoryRegistry; + } + + public boolean isMtls() { + return this.isMtls; + } } private GoogleApacheHttpTransport() {} diff --git a/google-api-client/src/test/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransportTest.java b/google-api-client/src/test/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransportTest.java index af7ecea86..18d504287 100644 --- a/google-api-client/src/test/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransportTest.java +++ b/google-api-client/src/test/java/com/google/api/client/googleapis/apache/v2/GoogleApacheHttpTransportTest.java @@ -14,11 +14,16 @@ package com.google.api.client.googleapis.apache.v2; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import com.google.api.client.googleapis.apache.v2.GoogleApacheHttpTransport.SocketFactoryRegistryHandler; import com.google.api.client.googleapis.mtls.MtlsProvider; import com.google.api.client.googleapis.mtls.MtlsTransportBaseTest; import com.google.api.client.http.HttpTransport; import java.io.IOException; import java.security.GeneralSecurityException; +import org.junit.Test; public class GoogleApacheHttpTransportTest extends MtlsTransportBaseTest { @Override @@ -26,4 +31,13 @@ protected HttpTransport buildTrustedTransport(MtlsProvider mtlsProvider) throws GeneralSecurityException, IOException { return GoogleApacheHttpTransport.newTrustedTransport(mtlsProvider); } + + @Test + public void socketFactoryRegistryHandlerTest() throws GeneralSecurityException, IOException { + MtlsProvider mtlsProvider = new TestMtlsProvider(true, createTestMtlsKeyStore(), "", false); + SocketFactoryRegistryHandler handler = new SocketFactoryRegistryHandler(mtlsProvider); + assertNotNull(handler.getSocketFactoryRegistry().lookup("http")); + assertNotNull(handler.getSocketFactoryRegistry().lookup("https")); + assertTrue(handler.isMtls()); + } } diff --git a/google-api-client/src/test/java/com/google/api/client/googleapis/mtls/MtlsTransportBaseTest.java b/google-api-client/src/test/java/com/google/api/client/googleapis/mtls/MtlsTransportBaseTest.java index d795d24cf..096ff4abf 100644 --- a/google-api-client/src/test/java/com/google/api/client/googleapis/mtls/MtlsTransportBaseTest.java +++ b/google-api-client/src/test/java/com/google/api/client/googleapis/mtls/MtlsTransportBaseTest.java @@ -41,7 +41,7 @@ protected static class TestMtlsProvider implements MtlsProvider { private String keyStorePassword; private boolean throwExceptionForGetKeyStore; - TestMtlsProvider( + public TestMtlsProvider( boolean useClientCertificate, KeyStore keystore, String keyStorePassword,