Revert "Bjorncs/health check proxy https"

5 files changed, 20 insertions, 68 deletions

diff --git a/jdisc_http_service/abi-spec.json b/jdisc_http_service/abi-spec.json
index 43f68274c2e..3f68009cd42 100644
--- a/jdisc_http_service/abi-spec.json
+++ b/jdisc_http_service/abi-spec.json
@@ -38,7 +38,6 @@
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder tcpKeepAliveEnabled(boolean)",
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder tcpNoDelay(boolean)",
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder throttling(com.yahoo.jdisc.http.ConnectorConfig$Throttling$Builder)",
-      "public com.yahoo.jdisc.http.ConnectorConfig$Builder implicitTlsEnabled(boolean)",
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder ssl(com.yahoo.jdisc.http.ConnectorConfig$Ssl$Builder)",
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder tlsClientAuthEnforcer(com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder)",
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder healthCheckProxy(com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder)",
@@ -358,7 +357,6 @@
       "public boolean tcpKeepAliveEnabled()",
       "public boolean tcpNoDelay()",
       "public com.yahoo.jdisc.http.ConnectorConfig$Throttling throttling()",
-      "public boolean implicitTlsEnabled()",
       "public com.yahoo.jdisc.http.ConnectorConfig$Ssl ssl()",
       "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer tlsClientAuthEnforcer()",
       "public com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy healthCheckProxy()",
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
index ef166bae999..94c08212706 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
@@ -18,6 +18,7 @@ import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.server.SslConnectionFactory;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 
+import java.nio.channels.ServerSocketChannel;
 import java.util.List;
 
 /**
@@ -41,25 +42,19 @@ public class ConnectorFactory {
     // e.g. due to TLS configuration through environment variables.
     private static void runtimeConnectorConfigValidation(ConnectorConfig config) {
         validateProxyProtocolConfiguration(config);
-        validateSecureRedirectConfig(config);
     }
 
     private static void validateProxyProtocolConfiguration(ConnectorConfig config) {
         ConnectorConfig.ProxyProtocol proxyProtocolConfig = config.proxyProtocol();
         if (proxyProtocolConfig.enabled()) {
+            boolean sslEnabled = config.ssl().enabled() || TransportSecurityUtils.isTransportSecurityEnabled();
             boolean tlsMixedModeEnabled = TransportSecurityUtils.getInsecureMixedMode() != MixedMode.DISABLED;
-            if (!isSslEffectivelyEnabled(config) || tlsMixedModeEnabled) {
+            if (!sslEnabled || tlsMixedModeEnabled) {
                 throw new IllegalArgumentException("Proxy protocol can only be enabled if connector is effectively HTTPS only");
             }
         }
     }
 
-    private static void validateSecureRedirectConfig(ConnectorConfig config) {
-        if (config.secureRedirect().enabled() && isSslEffectivelyEnabled(config)) {
-            throw new IllegalArgumentException("Secure redirect can only be enabled on connectors without HTTPS");
-        }
-    }
-
     public ConnectorConfig getConnectorConfig() {
         return connectorConfig;
     }
@@ -77,7 +72,7 @@ public class ConnectorFactory {
 
     private List<ConnectionFactory> createConnectionFactories(Metric metric) {
         HttpConnectionFactory httpFactory = newHttpConnectionFactory();
-        if (!isSslEffectivelyEnabled(connectorConfig)) {
+        if (connectorConfig.healthCheckProxy().enable() || connectorConfig.secureRedirect().enabled()) {
             return List.of(httpFactory);
         } else if (connectorConfig.ssl().enabled()) {
             return connectionFactoriesForHttps(metric, httpFactory);
@@ -119,7 +114,7 @@ public class ConnectorFactory {
         httpConfig.setOutputBufferSize(connectorConfig.outputBufferSize());
         httpConfig.setRequestHeaderSize(connectorConfig.requestHeaderSize());
         httpConfig.setResponseHeaderSize(connectorConfig.responseHeaderSize());
-        if (isSslEffectivelyEnabled(connectorConfig)) {
+        if (connectorConfig.ssl().enabled() || TransportSecurityUtils.isTransportSecurityEnabled()) { // TODO Cleanup once mixed mode is gone
             httpConfig.addCustomizer(new SecureRequestCustomizer());
         }
         return new HttpConnectionFactory(httpConfig);
@@ -132,9 +127,4 @@ public class ConnectorFactory {
         return connectionFactory;
     }
 
-    private static boolean isSslEffectivelyEnabled(ConnectorConfig config) {
-        return config.ssl().enabled()
-                || (config.implicitTlsEnabled() && TransportSecurityUtils.isTransportSecurityEnabled());
-    }
-
 }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java
index 2722c21bce3..75c1c9aa440 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java
@@ -3,18 +3,16 @@ package com.yahoo.jdisc.http.server.jetty;
 
 import com.yahoo.jdisc.Response;
 import com.yahoo.jdisc.http.ConnectorConfig;
-import com.yahoo.security.SslContextBuilder;
-import com.yahoo.security.tls.TransportSecurityOptions;
-import com.yahoo.security.tls.TransportSecurityUtils;
-import com.yahoo.security.tls.TrustAllX509TrustManager;
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
 import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.conn.ssl.TrustAllStrategy;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.ssl.SSLContexts;
 import org.eclipse.jetty.server.DetectorConnectionFactory;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.SslConnectionFactory;
@@ -27,6 +25,7 @@ import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
+import java.security.GeneralSecurityException;
 import java.time.Duration;
 import java.util.HashMap;
 import java.util.List;
@@ -168,25 +167,21 @@ class HealthCheckProxyHandler extends HandlerWrapper {
         }
 
         private SSLContext getSslContext(SslContextFactory.Server sslContextFactory) {
-            // A client certificate is only required if the server connector's ssl context factory is configured with "need-auth".
             if (sslContextFactory.getNeedClientAuth()) {
-                log.info(String.format("Port %d requires client certificate - client will provide its node certificate", port));
-                // We should ideally specify the client certificate through connector config, but the model has currently no knowledge of node certificate location on disk.
-                // Instead we assume that the server connector will accept its own node certificate. This will work for the current hosted use-case.
-                // The Vespa TLS config will provide us the location of certificate and key.
-                TransportSecurityOptions options = TransportSecurityUtils.getOptions()
-                        .orElseThrow(() ->
-                                new IllegalStateException("Vespa TLS configuration is required when using health check proxy to a port with client auth 'need'"));
-                return new SslContextBuilder()
-                        .withKeyStore(options.getPrivateKeyFile().get(), options.getCertificatesFile().get())
-                        .withTrustManager(new TrustAllX509TrustManager())
-                        .build();
+                log.info(String.format("Port %d requires client certificate. HTTPS client will use the target server connector's ssl context.", port));
+                // A client certificate is only required if the server connector's ssl context factory is configured with "need-auth".
+                // We use the server's ssl context (truststore + keystore) if a client certificate is required.
+                // This will only work if the server certificate's CA is in the truststore.
+                return sslContextFactory.getSslContext();
             } else {
                 log.info(String.format(
-                        "Port %d does not require a client certificate - client will not provide a certificate", port));
-                return new SslContextBuilder()
-                        .withTrustManager(new TrustAllX509TrustManager())
-                        .build();
+                        "Port %d does not require a client certificate. HTTPS client will use a custom ssl context accepting all certificates.", port));
+                // No client certificate required. The client is configured with a trust manager that accepts all certificates.
+                try {
+                    return SSLContexts.custom().loadTrustMaterial(new TrustAllStrategy()).build();
+                } catch (GeneralSecurityException e) {
+                    throw new RuntimeException(e);
+                }
             }
         }
 
diff --git a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def
index 055e5ad62d2..4c86c8b9bb6 100644
--- a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def
+++ b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def
@@ -53,10 +53,6 @@ throttling.maxAcceptRate            int      default=-1
 # Idle timeout in seconds applied to endpoints when a threshold is exceeded.
 throttling.idleTimeout              double   default=-1.0
 
-# Whether to enable TLS on connector when Vespa is configured with TLS.
-# The connector will implicitly enable TLS if set to 'true' and Vespa TLS is enabled.
-implicitTlsEnabled                  bool     default=true
-
 # Whether to enable SSL for this connector.
 ssl.enabled                         bool     default=false
 
diff --git a/security-utils/src/main/java/com/yahoo/security/tls/TrustAllX509TrustManager.java b/security-utils/src/main/java/com/yahoo/security/tls/TrustAllX509TrustManager.java
deleted file mode 100644
index d163366e686..00000000000
--- a/security-utils/src/main/java/com/yahoo/security/tls/TrustAllX509TrustManager.java
+++ /dev/null
@@ -1,27 +0,0 @@
-// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.security.tls;
-
-import javax.net.ssl.SSLEngine;
-import javax.net.ssl.X509ExtendedTrustManager;
-import java.net.Socket;
-import java.security.cert.X509Certificate;
-
-/**
- * A {@link X509ExtendedTrustManager} that accepts all server certificates.
- *
- * @author bjorncs
- */
-public class TrustAllX509TrustManager extends X509ExtendedTrustManager {
-    @Override public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) { failWhenUsedOnServer(); }
-    @Override public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) { failWhenUsedOnServer(); }
-    @Override public void checkClientTrusted(X509Certificate[] chain, String authType) { failWhenUsedOnServer(); }
-
-    @Override public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) {}
-    @Override public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
-    @Override public void checkServerTrusted(X509Certificate[] chain, String authType) {}
-    @Override public X509Certificate[] getAcceptedIssuers() { return new X509Certificate[0]; }
-
-    private static void failWhenUsedOnServer() {
-        throw new IllegalStateException("TrustAllX509TrustManager cannot be used on server, only client");
-    }
-}