From 6dad9426c16cc5a2e95247d8e170fab07baa862e Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Tue, 18 Jul 2023 13:18:37 +0200 Subject: Make headers controlling client address connector specific --- .../model/container/http/ConnectorFactory.java | 4 + .../model/container/http/JettyHttpServer.java | 9 --- .../http/ssl/HostedSslConnectorFactory.java | 15 +++- container-core/abi-spec.json | 90 +++++++++++----------- .../http/server/jetty/AccessLogRequestLog.java | 17 ++-- .../jdisc/http/server/jetty/JettyHttpServer.java | 2 +- .../jdisc.http.jdisc.http.connector.def | 6 ++ .../jdisc.http.jdisc.http.server.def | 6 -- .../http/server/jetty/AccessLogRequestLogTest.java | 7 +- .../http/server/jetty/JettyMockRequestBuilder.java | 6 +- 10 files changed, 82 insertions(+), 80 deletions(-) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ConnectorFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ConnectorFactory.java index 697cfc95039..4929c09d561 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ConnectorFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ConnectorFactory.java @@ -8,6 +8,7 @@ import com.yahoo.vespa.model.container.component.SimpleComponent; import com.yahoo.vespa.model.container.http.ssl.DefaultSslProvider; import com.yahoo.vespa.model.container.http.ssl.SslProvider; +import java.util.List; import java.util.Optional; /** @@ -40,6 +41,9 @@ public class ConnectorFactory extends SimpleComponent implements ConnectorConfig public void getConfig(ConnectorConfig.Builder connectorBuilder) { connectorBuilder.listenPort(listenPort); connectorBuilder.name(name); + connectorBuilder.accessLog(new ConnectorConfig.AccessLog.Builder() + .remoteAddressHeaders(List.of("x-forwarded-for")) + .remotePortHeaders(List.of("X-Forwarded-Port"))); sslProviderComponent.amendConnectorConfig(connectorBuilder); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java index 6a2d9685a33..0388230fa6a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java @@ -63,17 +63,8 @@ public class JettyHttpServer extends SimpleComponent implements ServerConfig.Pro .searchHandlerPaths(List.of("/search")) ); if (isHostedVespa) { - // Proxy-protocol v1/v2 is used in hosted Vespa for remote address/port - builder.accessLog(new ServerConfig.AccessLog.Builder() - .remoteAddressHeaders(List.of()) - .remotePortHeaders(List.of())); - // Enable connection log hosted Vespa builder.connectionLog(new ServerConfig.ConnectionLog.Builder().enabled(true)); - } else { - builder.accessLog(new ServerConfig.AccessLog.Builder() - .remoteAddressHeaders(List.of("x-forwarded-for")) - .remotePortHeaders(List.of("X-Forwarded-Port"))); } configureJettyThreadpool(builder); builder.stopTimeout(300); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java index 2b13cd21e99..243d14a006f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java @@ -7,6 +7,7 @@ import com.yahoo.security.tls.TlsContext; import com.yahoo.vespa.model.container.http.ConnectorFactory; import java.time.Duration; +import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -22,6 +23,8 @@ public class HostedSslConnectorFactory extends ConnectorFactory { private final boolean proxyProtocolEnabled; private final boolean proxyProtocolMixedMode; private final Duration endpointConnectionTtl; + private final List remoteAddressHeaders; + private final List remotePortHeaders; public static Builder builder(String name, int listenPort) { return new Builder(name, listenPort); } @@ -32,6 +35,8 @@ public class HostedSslConnectorFactory extends ConnectorFactory { this.proxyProtocolEnabled = builder.proxyProtocolEnabled; this.proxyProtocolMixedMode = builder.proxyProtocolMixedMode; this.endpointConnectionTtl = builder.endpointConnectionTtl; + this.remoteAddressHeaders = List.copyOf(builder.remoteAddressHeaders); + this.remotePortHeaders = List.copyOf(builder.remotePortHeaders); } private static SslProvider createSslProvider(Builder builder) { @@ -62,13 +67,19 @@ public class HostedSslConnectorFactory extends ConnectorFactory { .proxyProtocol(new ConnectorConfig.ProxyProtocol.Builder() .enabled(proxyProtocolEnabled).mixedMode(proxyProtocolMixedMode)) .idleTimeout(Duration.ofSeconds(30).toSeconds()) - .maxConnectionLife(endpointConnectionTtl != null ? endpointConnectionTtl.toSeconds() : 0); + .maxConnectionLife(endpointConnectionTtl != null ? endpointConnectionTtl.toSeconds() : 0) + .accessLog(new ConnectorConfig.AccessLog.Builder() + .remoteAddressHeaders(remoteAddressHeaders) + .remotePortHeaders(remotePortHeaders)); + } public enum SslClientAuth { WANT, NEED, WANT_WITH_ENFORCER } public static class Builder { final String name; final int port; + final List remoteAddressHeaders = new ArrayList<>(); + final List remotePortHeaders = new ArrayList<>(); SslClientAuth clientAuth; List tlsCiphersOverride; boolean proxyProtocolEnabled; @@ -88,6 +99,8 @@ public class HostedSslConnectorFactory extends ConnectorFactory { public Builder tlsCaCertificatesPath(String path) { this.tlsCaCertificatesPath = path; return this; } public Builder tlsCaCertificatesPem(String pem) { this.tlsCaCertificatesPem = pem; return this; } public Builder tokenEndpoint(boolean enable) { this.tokenEndpoint = enable; return this; } + public Builder remoteAddressHeader(String header) { this.remoteAddressHeaders.add(header); return this; } + public Builder remotePortHeader(String header) { this.remotePortHeaders.add(header); return this; } public HostedSslConnectorFactory build() { return new HostedSslConnectorFactory(this); } } diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index 757afeb64e2..6d7e3c86351 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -1027,6 +1027,45 @@ ], "fields" : [ ] }, + "com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder" : { + "superClass" : "java.lang.Object", + "interfaces" : [ + "com.yahoo.config.ConfigBuilder" + ], + "attributes" : [ + "public", + "final" + ], + "methods" : [ + "public void ()", + "public void (com.yahoo.jdisc.http.ConnectorConfig$AccessLog)", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder remoteAddressHeaders(java.lang.String)", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder remoteAddressHeaders(java.util.Collection)", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder remotePortHeaders(java.lang.String)", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder remotePortHeaders(java.util.Collection)", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog build()" + ], + "fields" : [ + "public java.util.List remoteAddressHeaders", + "public java.util.List remotePortHeaders" + ] + }, + "com.yahoo.jdisc.http.ConnectorConfig$AccessLog" : { + "superClass" : "com.yahoo.config.InnerNode", + "interfaces" : [ ], + "attributes" : [ + "public", + "final" + ], + "methods" : [ + "public void (com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder)", + "public java.util.List remoteAddressHeaders()", + "public java.lang.String remoteAddressHeaders(int)", + "public java.util.List remotePortHeaders()", + "public java.lang.String remotePortHeaders(int)" + ], + "fields" : [ ] + }, "com.yahoo.jdisc.http.ConnectorConfig$Builder" : { "superClass" : "java.lang.Object", "interfaces" : [ @@ -1069,6 +1108,8 @@ "public com.yahoo.jdisc.http.ConnectorConfig$Builder http2(java.util.function.Consumer)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder serverName(com.yahoo.jdisc.http.ConnectorConfig$ServerName$Builder)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder serverName(java.util.function.Consumer)", + "public com.yahoo.jdisc.http.ConnectorConfig$Builder accessLog(com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder)", + "public com.yahoo.jdisc.http.ConnectorConfig$Builder accessLog(java.util.function.Consumer)", "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)", "public final java.lang.String getDefMd5()", "public final java.lang.String getDefName()", @@ -1084,7 +1125,8 @@ "public com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder healthCheckProxy", "public com.yahoo.jdisc.http.ConnectorConfig$ProxyProtocol$Builder proxyProtocol", "public com.yahoo.jdisc.http.ConnectorConfig$Http2$Builder http2", - "public com.yahoo.jdisc.http.ConnectorConfig$ServerName$Builder serverName" + "public com.yahoo.jdisc.http.ConnectorConfig$ServerName$Builder serverName", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder accessLog" ] }, "com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder" : { @@ -1438,7 +1480,8 @@ "public double maxConnectionLife()", "public boolean http2Enabled()", "public com.yahoo.jdisc.http.ConnectorConfig$Http2 http2()", - "public com.yahoo.jdisc.http.ConnectorConfig$ServerName serverName()" + "public com.yahoo.jdisc.http.ConnectorConfig$ServerName serverName()", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog accessLog()" ], "fields" : [ "public static final java.lang.String CONFIG_DEF_MD5", @@ -1771,45 +1814,6 @@ ], "fields" : [ ] }, - "com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder" : { - "superClass" : "java.lang.Object", - "interfaces" : [ - "com.yahoo.config.ConfigBuilder" - ], - "attributes" : [ - "public", - "final" - ], - "methods" : [ - "public void ()", - "public void (com.yahoo.jdisc.http.ServerConfig$AccessLog)", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remoteAddressHeaders(java.lang.String)", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remoteAddressHeaders(java.util.Collection)", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remotePortHeaders(java.lang.String)", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remotePortHeaders(java.util.Collection)", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog build()" - ], - "fields" : [ - "public java.util.List remoteAddressHeaders", - "public java.util.List remotePortHeaders" - ] - }, - "com.yahoo.jdisc.http.ServerConfig$AccessLog" : { - "superClass" : "com.yahoo.config.InnerNode", - "interfaces" : [ ], - "attributes" : [ - "public", - "final" - ], - "methods" : [ - "public void (com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder)", - "public java.util.List remoteAddressHeaders()", - "public java.lang.String remoteAddressHeaders(int)", - "public java.util.List remotePortHeaders()", - "public java.lang.String remotePortHeaders(int)" - ], - "fields" : [ ] - }, "com.yahoo.jdisc.http.ServerConfig$Builder" : { "superClass" : "java.lang.Object", "interfaces" : [ @@ -1839,8 +1843,6 @@ "public com.yahoo.jdisc.http.ServerConfig$Builder jmx(java.util.function.Consumer)", "public com.yahoo.jdisc.http.ServerConfig$Builder metric(com.yahoo.jdisc.http.ServerConfig$Metric$Builder)", "public com.yahoo.jdisc.http.ServerConfig$Builder metric(java.util.function.Consumer)", - "public com.yahoo.jdisc.http.ServerConfig$Builder accessLog(com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder)", - "public com.yahoo.jdisc.http.ServerConfig$Builder accessLog(java.util.function.Consumer)", "public com.yahoo.jdisc.http.ServerConfig$Builder connectionLog(com.yahoo.jdisc.http.ServerConfig$ConnectionLog$Builder)", "public com.yahoo.jdisc.http.ServerConfig$Builder connectionLog(java.util.function.Consumer)", "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)", @@ -1856,7 +1858,6 @@ "public java.util.List defaultFilters", "public com.yahoo.jdisc.http.ServerConfig$Jmx$Builder jmx", "public com.yahoo.jdisc.http.ServerConfig$Metric$Builder metric", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder accessLog", "public com.yahoo.jdisc.http.ServerConfig$ConnectionLog$Builder connectionLog" ] }, @@ -2070,7 +2071,6 @@ "public double stopTimeout()", "public com.yahoo.jdisc.http.ServerConfig$Jmx jmx()", "public com.yahoo.jdisc.http.ServerConfig$Metric metric()", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog accessLog()", "public com.yahoo.jdisc.http.ServerConfig$ConnectionLog connectionLog()" ], "fields" : [ diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java index 5b51eeee7d6..7a305c23ba3 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java @@ -7,8 +7,6 @@ import com.yahoo.container.logging.AccessLogEntry; import com.yahoo.container.logging.RequestLog; import com.yahoo.container.logging.RequestLogEntry; import com.yahoo.jdisc.http.HttpRequest; -import com.yahoo.jdisc.http.ServerConfig; -import jakarta.servlet.http.HttpServletRequest; import org.eclipse.jetty.http2.HTTP2Stream; import org.eclipse.jetty.http2.server.HttpTransportOverHTTP2; import org.eclipse.jetty.server.HttpChannel; @@ -27,6 +25,7 @@ import java.util.function.BiConsumer; import java.util.logging.Level; import java.util.logging.Logger; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnectorLocalPort; /** @@ -44,13 +43,9 @@ class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty private static final List LOGGED_REQUEST_HEADERS = List.of("Vespa-Client-Version"); private final RequestLog requestLog; - private final List remoteAddressHeaders; - private final List remotePortHeaders; - AccessLogRequestLog(RequestLog requestLog, ServerConfig.AccessLog config) { + AccessLogRequestLog(RequestLog requestLog) { this.requestLog = requestLog; - this.remoteAddressHeaders = config.remoteAddressHeaders(); - this.remotePortHeaders = config.remotePortHeaders(); } @Override @@ -144,16 +139,16 @@ class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty } } - private String getRemoteAddress(HttpServletRequest request) { - for (String header : remoteAddressHeaders) { + private String getRemoteAddress(Request request) { + for (String header : getConnector(request).connectorConfig().accessLog().remoteAddressHeaders()) { String value = request.getHeader(header); if (value != null) return value; } return request.getRemoteAddr(); } - private int getRemotePort(HttpServletRequest request) { - for (String header : remotePortHeaders) { + private int getRemotePort(Request request) { + for (String header : getConnector(request).connectorConfig().accessLog().remotePortHeaders()) { String value = request.getHeader(header); if (value != null) { OptionalInt maybePort = parsePort(value); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index 3ebb65e7979..7d84ee6f8a3 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -68,7 +68,7 @@ public class JettyHttpServer extends AbstractServerProvider { server = new Server(); server.setStopTimeout((long)(serverConfig.stopTimeout() * 1000.0)); - server.setRequestLog(new AccessLogRequestLog(requestLog, serverConfig.accessLog())); + server.setRequestLog(new AccessLogRequestLog(requestLog)); setupJmx(server, serverConfig); configureJettyThreadpool(server, serverConfig); JettyConnectionLogger connectionLogger = new JettyConnectionLogger(serverConfig.connectionLog(), connectionLog); diff --git a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def index 3c01012fd9e..5a2bad63682 100644 --- a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def +++ b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def @@ -138,3 +138,9 @@ serverName.fallback string default="" # The list of accepted server names. Empty list to accept any. Elements follows format of 'serverName.default'. serverName.allowed[] string + +# HTTP request headers that contain remote address +accessLog.remoteAddressHeaders[] string + +# HTTP request headers that contain remote port +accessLog.remotePortHeaders[] string diff --git a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def index c15cb6b2cc4..a85641f61e9 100644 --- a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def +++ b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def @@ -52,11 +52,5 @@ metric.searchHandlerPaths[] string # User-agent names to ignore wrt statistics (crawlers etc) metric.ignoredUserAgents[] string -# HTTP request headers that contain remote address -accessLog.remoteAddressHeaders[] string - -# HTTP request headers that contain remote port -accessLog.remotePortHeaders[] string - # Whether to enable jdisc connection log connectionLog.enabled bool default=false diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java index 766c7918882..122db0f765d 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java @@ -4,7 +4,6 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.container.logging.AccessLogEntry; import com.yahoo.container.logging.RequestLog; import com.yahoo.container.logging.RequestLogEntry; -import com.yahoo.jdisc.http.ServerConfig; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.junit.jupiter.api.Test; @@ -117,11 +116,7 @@ public class AccessLogRequestLogTest { } private void doAccessLoggingOfRequest(RequestLog requestLog, Request jettyRequest) { - ServerConfig.AccessLog config = new ServerConfig.AccessLog( - new ServerConfig.AccessLog.Builder() - .remoteAddressHeaders(List.of("x-forwarded-for", "y-ra")) - .remotePortHeaders(List.of("X-Forwarded-Port", "y-rp"))); - new AccessLogRequestLog(requestLog, config).log(jettyRequest, createResponseMock()); + new AccessLogRequestLog(requestLog).log(jettyRequest, createResponseMock()); } private static JettyMockRequestBuilder createRequestBuilder() { diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockRequestBuilder.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockRequestBuilder.java index e62825fc2a8..8b13f30bcd7 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockRequestBuilder.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockRequestBuilder.java @@ -85,7 +85,11 @@ public class JettyMockRequestBuilder { HttpChannel channel = mock(HttpChannel.class); HttpConnection connection = mock(HttpConnection.class); JDiscServerConnector connector = mock(JDiscServerConnector.class); - when(connector.connectorConfig()).thenReturn(new ConnectorConfig(new ConnectorConfig.Builder().listenPort(localPort))); + when(connector.connectorConfig()).thenReturn(new ConnectorConfig( + new ConnectorConfig.Builder().listenPort(localPort) + .accessLog(new ConnectorConfig.AccessLog.Builder() + .remoteAddressHeaders(List.of("x-forwarded-for", "y-ra")) + .remotePortHeaders(List.of("X-Forwarded-Port", "y-rp"))))); when(connector.getLocalPort()).thenReturn(localPort); when(connection.getCreatedTimeStamp()).thenReturn(System.currentTimeMillis()); when(connection.getConnector()).thenReturn(connector); -- cgit v1.2.3