Add option to not add X-Forwarded headers

Co-authored-by: Manfred Moser <[email protected]>

docs/installation.md

@@ -63,7 +63,8 @@ From a users perspective Trino Gateway acts as a transparent proxy for one
 or more Trino clusters. The following Trino configuration tips should be 
 taken into account for all clusters behind the Trino Gateway.
 
-Process forwarded HTTP headers must be enabled:
+If all client and server communication is routed through Trino Gateway, 
+then process forwarded HTTP headers must be enabled:
 
 ```commandline
 http-server.process-forwarded=true
@@ -73,16 +74,22 @@ Without this setting, first requests go from the user to Trino Gateway and then
 to Trino correctly. However, the URL for subsequent next URIs for more results
 in a query provided by Trino is then using the local URL of the Trino cluster,
 and not the URL of the Trino Gateway. This circumvents the Trino Gateway for all
-these requests, and is contrary to the purpose of using the Trino Gateway. In
-scenarios, where the local URL of the Trino cluster is private to the Trino
-cluster on the DNS/network level, these following calls might not work at all
-for users.
+these requests. In scenarios, where the local URL of the Trino cluster is private 
+to the Trino cluster on the network level, these following calls do not work
+at all for users.
 
 This setting is also required for Trino to authenticate in the case TLS is 
 terminated at the Trino Gateway. Normally it refuses to authenticate plain HTTP 
 requests, but if `http-server.process-forwarded=true` it authenticates over 
 HTTP if the request includes `X-Forwarded-Proto: HTTPS`.
 
+To prevent Trino Gateway from sending `X-Forwarded-*` headers, add the following configuration:
+
+```yaml
+routing:
+  addXForwardedHeaders: false
+```
+
 Find more information in [the related Trino documentation](https://trino.io/docs/current/security/tls.html#use-a-load-balancer-to-terminate-tls-https).
 
 ## Configuration

gateway-ha/src/main/java/io/trino/gateway/ha/config/RoutingConfiguration.java

@@ -21,6 +21,8 @@ public class RoutingConfiguration
 {
     private Duration asyncTimeout = new Duration(2, MINUTES);
 
+    private boolean addXForwardedHeaders = true;
+
     public Duration getAsyncTimeout()
     {
         return asyncTimeout;
@@ -30,4 +32,14 @@ public void setAsyncTimeout(Duration asyncTimeout)
     {
         this.asyncTimeout = asyncTimeout;
     }
+
+    public boolean isAddXForwardedHeaders()
+    {
+        return addXForwardedHeaders;
+    }
+
+    public void setAddXForwardedHeaders(boolean addXForwardedHeaders)
+    {
+        this.addXForwardedHeaders = addXForwardedHeaders;
+    }
 }

gateway-ha/src/main/java/io/trino/gateway/proxyserver/ProxyRequestHandler.java

@@ -84,6 +84,7 @@ public class ProxyRequestHandler
     private final RoutingManager routingManager;
     private final QueryHistoryManager queryHistoryManager;
     private final boolean cookiesEnabled;
+    private final boolean addXForwardedHeaders;
 
     @Inject
     public ProxyRequestHandler(
@@ -97,6 +98,7 @@ public ProxyRequestHandler(
         this.queryHistoryManager = requireNonNull(queryHistoryManager, "queryHistoryManager is null");
         cookiesEnabled = GatewayCookieConfigurationPropertiesProvider.getInstance().isEnabled();
         asyncTimeout = haGatewayConfiguration.getRouting().getAsyncTimeout();
+        addXForwardedHeaders = haGatewayConfiguration.getRouting().isAddXForwardedHeaders();
     }
 
     @PreDestroy
@@ -145,13 +147,17 @@ private void performRequest(
         for (String name : list(servletRequest.getHeaderNames())) {
             for (String value : list(servletRequest.getHeaders(name))) {
                 // TODO: decide what should and shouldn't be forwarded
-                if (!name.equalsIgnoreCase("Accept-Encoding") && !name.equalsIgnoreCase("Host")) {
+                if (!name.equalsIgnoreCase("Accept-Encoding")
+                        && !name.equalsIgnoreCase("Host")
+                        && (addXForwardedHeaders || !name.startsWith("X-Forwarded"))) {
                     requestBuilder.addHeader(name, value);
                 }
             }
         }
         requestBuilder.addHeader(VIA, format("%s TrinoGateway", servletRequest.getProtocol()));
-        addXForwardedHeaders(servletRequest, requestBuilder);
+        if (addXForwardedHeaders) {
+            addXForwardedHeaders(servletRequest, requestBuilder);
+        }
 
         ImmutableList<NewCookie> oauth2GatewayCookie = getOAuth2GatewayCookie(remoteUri, servletRequest);
 

gateway-ha/src/test/java/io/trino/gateway/ha/TestNoXForwarded.java

@@ -0,0 +1,87 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.trino.gateway.ha;
+
+import io.airlift.json.JsonCodec;
+import io.trino.client.QueryResults;
+import okhttp3.MediaType;
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.RequestBody;
+import okhttp3.Response;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.TestInstance.Lifecycle;
+import org.testcontainers.containers.TrinoContainer;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.testcontainers.utility.MountableFile.forClasspathResource;
+
+@TestInstance(Lifecycle.PER_CLASS)
+public class TestNoXForwarded
+{
+    private final OkHttpClient httpClient = new OkHttpClient();
+    private TrinoContainer trino;
+    int routerPort = 21001 + (int) (Math.random() * 1000);
+    int backendPort;
+
+    @BeforeAll
+    public void setup()
+            throws Exception
+    {
+        trino = new TrinoContainer("trinodb/trino");
+        trino.withCopyFileToContainer(forClasspathResource("trino-config.properties"), "/etc/trino/config.properties");
+        trino.start();
+
+        backendPort = trino.getMappedPort(8080);
+
+        // seed database
+        HaGatewayTestUtils.TestConfig testConfig =
+                HaGatewayTestUtils.buildGatewayConfigAndSeedDb(routerPort, "test-config-without-x-forwarded-template.yml");
+        // Start Gateway
+        String[] args = {testConfig.configFilePath()};
+        HaGatewayLauncher.main(args);
+        // Now populate the backend
+        HaGatewayTestUtils.setUpBackend(
+                "trino1", "http://localhost:" + backendPort, "externalUrl", true, "adhoc", routerPort);
+    }
+
+    @Test
+    public void testRequestDelivery()
+            throws Exception
+    {
+        RequestBody requestBody =
+                RequestBody.create(MediaType.parse("application/json; charset=utf-8"), "SELECT 1");
+        Request request =
+                new Request.Builder()
+                        .url("http://localhost:" + routerPort + "/v1/statement")
+                        .addHeader("X-Trino-User", "test")
+                        .post(requestBody)
+                        .build();
+        Response response = httpClient.newCall(request).execute();
+        JsonCodec<QueryResults> responseCodec = JsonCodec.jsonCodec(QueryResults.class);
+        QueryResults queryResults = responseCodec.fromJson(response.body().string());
+
+        assertThat(queryResults.getNextUri().getHost()).isEqualTo("localhost");
+        assertThat(queryResults.getNextUri().getPort()).isEqualTo(backendPort);
+    }
+
+    @AfterAll
+    public void cleanup()
+    {
+        trino.close();
+    }
+}

gateway-ha/src/test/resources/test-config-without-x-forwarded-template.yml

@@ -0,0 +1,27 @@
+serverConfig:
+  node.environment: test
+  http-server.http.port: REQUEST_ROUTER_PORT
+
+dataStore:
+  jdbcUrl: jdbc:h2:DB_FILE_PATH
+  user: sa
+  password: sa
+  driver: org.h2.Driver
+
+modules:
+  - io.trino.gateway.ha.module.HaGatewayProviderModule
+
+extraWhitelistPaths:
+  - '/v1/custom.*'
+  - '/custom/logout.*'
+
+gatewayCookieConfiguration:
+  enabled: true
+  cookieSigningSecret: "kjlhbfrewbyuo452cds3dc1234ancdsjh"
+
+oauth2GatewayCookieConfiguration:
+  deletePaths:
+    - "/custom/logout"
+
+routing:
+  addXForwardedHeaders: false