diff options
author | Bjørn Christian Seime <bjorn.christian@seime.no> | 2020-10-15 10:21:51 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-10-15 10:21:51 +0200 |
commit | 62cf53967da366797a54bb1d43c3825d4afae7a8 (patch) | |
tree | 5980a188ee158ab4fd87084521e76dfa3df85258 /jdisc_http_service | |
parent | c8602587ed7282e742cb0e174fe145b7f6041b18 (diff) |
Revert "Revert "Bjorncs/health check proxy https""
Diffstat (limited to 'jdisc_http_service')
4 files changed, 41 insertions, 20 deletions
diff --git a/jdisc_http_service/abi-spec.json b/jdisc_http_service/abi-spec.json index 3f68009cd42..43f68274c2e 100644 --- a/jdisc_http_service/abi-spec.json +++ b/jdisc_http_service/abi-spec.json @@ -38,6 +38,7 @@ "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)", @@ -357,6 +358,7 @@ "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 94c08212706..ef166bae999 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,7 +18,6 @@ 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; /** @@ -42,19 +41,25 @@ 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 (!sslEnabled || tlsMixedModeEnabled) { + if (!isSslEffectivelyEnabled(config) || 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; } @@ -72,7 +77,7 @@ public class ConnectorFactory { private List<ConnectionFactory> createConnectionFactories(Metric metric) { HttpConnectionFactory httpFactory = newHttpConnectionFactory(); - if (connectorConfig.healthCheckProxy().enable() || connectorConfig.secureRedirect().enabled()) { + if (!isSslEffectivelyEnabled(connectorConfig)) { return List.of(httpFactory); } else if (connectorConfig.ssl().enabled()) { return connectionFactoriesForHttps(metric, httpFactory); @@ -114,7 +119,7 @@ public class ConnectorFactory { httpConfig.setOutputBufferSize(connectorConfig.outputBufferSize()); httpConfig.setRequestHeaderSize(connectorConfig.requestHeaderSize()); httpConfig.setResponseHeaderSize(connectorConfig.responseHeaderSize()); - if (connectorConfig.ssl().enabled() || TransportSecurityUtils.isTransportSecurityEnabled()) { // TODO Cleanup once mixed mode is gone + if (isSslEffectivelyEnabled(connectorConfig)) { httpConfig.addCustomizer(new SecureRequestCustomizer()); } return new HttpConnectionFactory(httpConfig); @@ -127,4 +132,9 @@ 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 75c1c9aa440..2722c21bce3 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,16 +3,18 @@ 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; @@ -25,7 +27,6 @@ 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; @@ -167,21 +168,25 @@ 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. 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(); + 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(); } else { log.info(String.format( - "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); - } + "Port %d does not require a client certificate - client will not provide a certificate", port)); + return new SslContextBuilder() + .withTrustManager(new TrustAllX509TrustManager()) + .build(); } } 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 4c86c8b9bb6..055e5ad62d2 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,6 +53,10 @@ 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 |