From 4bb0999694a314b8daebe179db39c1fe48cca21d Mon Sep 17 00:00:00 2001 From: Andreas Eriksen Date: Fri, 6 Jan 2023 17:16:35 +0100 Subject: Revert "Ensure that HTTPS clients only use allowed ciphers and protocol versions" (#25436) --- .../util/http/AcceptAllHostnamesVerifier.java | 21 -------- .../util/http/hc4/SslConnectionSocketFactory.java | 54 -------------------- .../util/http/hc4/VespaHttpClientBuilder.java | 7 ++- .../util/http/hc5/DefaultHttpClientBuilder.java | 15 ++++-- .../util/http/hc5/SslConnectionSocketFactory.java | 57 ---------------------- .../util/http/hc5/VespaHttpClientBuilder.java | 7 ++- 6 files changed, 21 insertions(+), 140 deletions(-) delete mode 100644 http-utils/src/main/java/ai/vespa/util/http/AcceptAllHostnamesVerifier.java delete mode 100644 http-utils/src/main/java/ai/vespa/util/http/hc4/SslConnectionSocketFactory.java delete mode 100644 http-utils/src/main/java/ai/vespa/util/http/hc5/SslConnectionSocketFactory.java (limited to 'http-utils') diff --git a/http-utils/src/main/java/ai/vespa/util/http/AcceptAllHostnamesVerifier.java b/http-utils/src/main/java/ai/vespa/util/http/AcceptAllHostnamesVerifier.java deleted file mode 100644 index 77d718bccb3..00000000000 --- a/http-utils/src/main/java/ai/vespa/util/http/AcceptAllHostnamesVerifier.java +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package ai.vespa.util.http; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLSession; - -/** - * @author bjorncs - */ -public class AcceptAllHostnamesVerifier implements HostnameVerifier { - - private static final AcceptAllHostnamesVerifier INSTANCE = new AcceptAllHostnamesVerifier(); - - public static AcceptAllHostnamesVerifier instance() { return INSTANCE; } - - private AcceptAllHostnamesVerifier() {} - - @Override public boolean verify(String hostname, SSLSession session) { return true; } - -} - diff --git a/http-utils/src/main/java/ai/vespa/util/http/hc4/SslConnectionSocketFactory.java b/http-utils/src/main/java/ai/vespa/util/http/hc4/SslConnectionSocketFactory.java deleted file mode 100644 index 16449a72524..00000000000 --- a/http-utils/src/main/java/ai/vespa/util/http/hc4/SslConnectionSocketFactory.java +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package ai.vespa.util.http.hc4; - -import ai.vespa.util.http.AcceptAllHostnamesVerifier; -import com.yahoo.security.tls.TlsContext; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLSocketFactory; -import java.util.Collection; - -import static com.yahoo.security.tls.TlsContext.getAllowedCipherSuites; -import static com.yahoo.security.tls.TlsContext.getAllowedProtocols; - -/** - * Provides {@link SSLConnectionSocketFactory} that applies protocol restrictions from {@link TlsContext}. - * - * @author bjorncs - */ -public class SslConnectionSocketFactory { - private SslConnectionSocketFactory() {} - - public static SSLConnectionSocketFactory of(SSLContext ctx, HostnameVerifier verifier) { - return new SSLConnectionSocketFactory(ctx, protocols(ctx), cipherSuites(ctx), verifier); - } - - public static SSLConnectionSocketFactory of(SSLContext ctx) { return of(ctx, defaultVerifier()); } - - public static SSLConnectionSocketFactory of(TlsContext ctx, HostnameVerifier verifier) { - return new SSLConnectionSocketFactory( - ctx.context(), ctx.parameters().getProtocols(), ctx.parameters().getCipherSuites(), verifier); - } - - public static SSLConnectionSocketFactory of(SSLSocketFactory fac, HostnameVerifier verifier) { - return new SSLConnectionSocketFactory(fac, protocols(), cipherSuites(), verifier); - } - - public static SSLConnectionSocketFactory of() { - return new SSLConnectionSocketFactory(TlsContext.defaultSslContext(), protocols(), cipherSuites(), defaultVerifier()); - } - - public static SSLConnectionSocketFactory of(TlsContext ctx) { return of(ctx, defaultVerifier()); } - - public static HostnameVerifier defaultVerifier() { return SSLConnectionSocketFactory.getDefaultHostnameVerifier(); } - - public static HostnameVerifier noopVerifier() { return AcceptAllHostnamesVerifier.instance(); } - - private static String[] cipherSuites(SSLContext ctx) { return array(getAllowedCipherSuites(ctx)); } - private static String[] protocols(SSLContext ctx) { return array(getAllowedProtocols(ctx)); } - private static String[] cipherSuites() { return array(getAllowedCipherSuites()); } - private static String[] protocols() { return array(getAllowedProtocols()); } - private static String[] array(Collection c) { return c.toArray(String[]::new); } -} diff --git a/http-utils/src/main/java/ai/vespa/util/http/hc4/VespaHttpClientBuilder.java b/http-utils/src/main/java/ai/vespa/util/http/hc4/VespaHttpClientBuilder.java index af01b123a27..953abcb04bc 100644 --- a/http-utils/src/main/java/ai/vespa/util/http/hc4/VespaHttpClientBuilder.java +++ b/http-utils/src/main/java/ai/vespa/util/http/hc4/VespaHttpClientBuilder.java @@ -29,8 +29,6 @@ import java.net.InetAddress; import java.util.logging.Level; import java.util.logging.Logger; -import static ai.vespa.util.http.hc4.SslConnectionSocketFactory.noopVerifier; - /** * Http client builder for internal Vespa communications over http/https. * @@ -103,8 +101,9 @@ public class VespaHttpClientBuilder { } } - private static SSLConnectionSocketFactory createSslSocketFactory(TlsContext ctx) { - return SslConnectionSocketFactory.of(ctx, noopVerifier()); + private static SSLConnectionSocketFactory createSslSocketFactory(TlsContext tlsContext) { + SSLParameters parameters = tlsContext.parameters(); + return new SSLConnectionSocketFactory(tlsContext.context(), parameters.getProtocols(), parameters.getCipherSuites(), new NoopHostnameVerifier()); } private static Registry createRegistry(SSLConnectionSocketFactory sslSocketFactory) { diff --git a/http-utils/src/main/java/ai/vespa/util/http/hc5/DefaultHttpClientBuilder.java b/http-utils/src/main/java/ai/vespa/util/http/hc5/DefaultHttpClientBuilder.java index 8575bc16ee8..8866d67fd60 100644 --- a/http-utils/src/main/java/ai/vespa/util/http/hc5/DefaultHttpClientBuilder.java +++ b/http-utils/src/main/java/ai/vespa/util/http/hc5/DefaultHttpClientBuilder.java @@ -1,12 +1,19 @@ package ai.vespa.util.http.hc5; +import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; +import org.apache.hc.client5.http.impl.classic.HttpClients; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier; +import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactoryBuilder; +import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.util.Timeout; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; import java.time.Duration; +import java.util.Map; import java.util.function.Supplier; /** @@ -27,12 +34,14 @@ public class DefaultHttpClientBuilder { /** Creates an HTTP client builder with the given SSL context, and using the provided timeouts for requests where config is not overridden. */ public static HttpClientBuilder create(Supplier sslContext, HostnameVerifier verifier, String userAgent) { - SSLContext ctx = sslContext.get(); - var factory = ctx == null ? SslConnectionSocketFactory.of(verifier) : SslConnectionSocketFactory.of(ctx, verifier); return HttpClientBuilder.create() .setConnectionManager(PoolingHttpClientConnectionManagerBuilder .create() - .setSSLSocketFactory(factory) + .setSSLSocketFactory(SSLConnectionSocketFactoryBuilder + .create() + .setSslContext(sslContext.get()) + .setHostnameVerifier(verifier) + .build()) .build()) .setUserAgent(userAgent) .disableCookieManagement() diff --git a/http-utils/src/main/java/ai/vespa/util/http/hc5/SslConnectionSocketFactory.java b/http-utils/src/main/java/ai/vespa/util/http/hc5/SslConnectionSocketFactory.java deleted file mode 100644 index 7ba408c260b..00000000000 --- a/http-utils/src/main/java/ai/vespa/util/http/hc5/SslConnectionSocketFactory.java +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package ai.vespa.util.http.hc5; - -import ai.vespa.util.http.AcceptAllHostnamesVerifier; -import com.yahoo.security.tls.TlsContext; -import org.apache.hc.client5.http.ssl.HttpsSupport; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLSocketFactory; - -import java.util.Collection; - -import static com.yahoo.security.tls.TlsContext.getAllowedCipherSuites; -import static com.yahoo.security.tls.TlsContext.getAllowedProtocols; - -/** - * Provides {@link SSLConnectionSocketFactory} that applies protocol restrictions from {@link TlsContext}. - * - * @author bjorncs - */ -public class SslConnectionSocketFactory { - private SslConnectionSocketFactory() {} - - public static SSLConnectionSocketFactory of(SSLContext ctx, HostnameVerifier verifier) { - return new SSLConnectionSocketFactory(ctx, protocols(ctx), cipherSuites(ctx), verifier); - } - - public static SSLConnectionSocketFactory of(SSLContext ctx) { return of(ctx, defaultVerifier()); } - - public static SSLConnectionSocketFactory of(TlsContext ctx, HostnameVerifier verifier) { - return new SSLConnectionSocketFactory( - ctx.context(), ctx.parameters().getProtocols(), ctx.parameters().getCipherSuites(), verifier); - } - - public static SSLConnectionSocketFactory of(TlsContext ctx) { return of(ctx, defaultVerifier()); } - - public static SSLConnectionSocketFactory of(SSLSocketFactory fac, HostnameVerifier verifier) { - return new SSLConnectionSocketFactory(fac, protocols(), cipherSuites(), verifier); - } - - public static SSLConnectionSocketFactory of(HostnameVerifier verifier) { - return of(TlsContext.defaultSslContext(), verifier); - } - - public static HostnameVerifier defaultVerifier() { return HttpsSupport.getDefaultHostnameVerifier(); } - - public static HostnameVerifier noopVerifier() { return AcceptAllHostnamesVerifier.instance(); } - - private static String[] cipherSuites(SSLContext ctx) { return array(getAllowedCipherSuites(ctx)); } - private static String[] protocols(SSLContext ctx) { return array(getAllowedProtocols(ctx)); } - private static String[] cipherSuites() { return array(getAllowedCipherSuites()); } - private static String[] protocols() { return array(getAllowedProtocols()); } - private static String[] array(Collection c) { return c.toArray(String[]::new); } - -} diff --git a/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java b/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java index a33c4c119c2..52f7ad9b56b 100644 --- a/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java +++ b/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java @@ -12,6 +12,7 @@ import org.apache.hc.core5.http.config.Registry; import org.apache.hc.core5.http.config.RegistryBuilder; import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLParameters; import static com.yahoo.security.tls.MixedMode.PLAINTEXT_CLIENT_MIXED_SERVER; import static com.yahoo.security.tls.TransportSecurityUtils.getInsecureMixedMode; @@ -64,7 +65,11 @@ public class VespaHttpClientBuilder { private static void addSslSocketFactory(HttpClientBuilder builder, HttpClientConnectionManagerFactory connectionManagerFactory, HostnameVerifier hostnameVerifier) { getSystemTlsContext().ifPresent(tlsContext -> { - SSLConnectionSocketFactory socketFactory = SslConnectionSocketFactory.of(tlsContext, hostnameVerifier); + SSLParameters parameters = tlsContext.parameters(); + SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(tlsContext.context(), + parameters.getProtocols(), + parameters.getCipherSuites(), + hostnameVerifier); builder.setConnectionManager(connectionManagerFactory.create(createRegistry(socketFactory))); // Workaround that allows re-using https connections, see https://stackoverflow.com/a/42112034/1615280 for details. // Proper solution would be to add a request interceptor that adds a x500 principal as user token, -- cgit v1.2.3