From e886efaa384bac106f2e5efcc3fb59901c251505 Mon Sep 17 00:00:00 2001 From: arithmetic1728 <58957152+arithmetic1728@users.noreply.github.com> Date: Mon, 16 Nov 2020 09:27:11 -0800 Subject: [PATCH] fix: socketfactory not registered for apache (#1637) * fix: socketfactory not registered for apache * refactor code and add test --- .../apache/v2/GoogleApacheHttpTransport.java | 90 ++++++++++++------- .../v2/GoogleApacheHttpTransportTest.java | 14 +++ .../mtls/MtlsTransportBaseTest.java | 2 +- 3 files changed, 75 insertions(+), 31 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..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; @@ -28,7 +29,11 @@ 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.socket.PlainConnectionSocketFactory; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; @@ -64,42 +69,18 @@ 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(); - } - + SocketFactoryRegistryHandler handler = new SocketFactoryRegistryHandler(mtlsProvider); PoolingHttpClientConnectionManager connectionManager = - new PoolingHttpClientConnectionManager(-1, TimeUnit.MILLISECONDS); + new PoolingHttpClientConnectionManager( + handler.getSocketFactoryRegistry(), null, null, null, -1, TimeUnit.MILLISECONDS); - // Disable the stale connection check (previously configured in the HttpConnectionParams + // 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(); - - 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); - HttpClient client = HttpClientBuilder.create() .useSystemProperties() - .setSSLSocketFactory(socketFactory) .setMaxConnTotal(200) .setMaxConnPerRoute(20) .setRoutePlanner(new SystemDefaultRoutePlanner(ProxySelector.getDefault())) @@ -107,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,