fix: Use HttpRequest authority instead of host (hyperledger#6879)

Use HttpRequest authority method to determine the hostname from header instead of using deprecated host method Signed-off-by: Usman Saleem <usman@usmans.info>
macfarla · Apr 26, 2024 · a007d29 · a007d29
1 parent 4bd817d
commit a007d29
Show file tree

Hide file tree

Showing 7 changed files with 79 additions and 125 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@
 ### Deprecations
 
 ### Additions and Improvements
+- Update "host allow list" logic to transition from deprecated `host()` method to suggested `authority()` method.[#6878](https://github.com/hyperledger/besu/issues/6878)
 - `txpool_besuPendingTransactions`change parameter `numResults` to optional parameter [#6708](https://github.com/hyperledger/besu/pull/6708)
 - Extend `Blockchain` service [#6592](https://github.com/hyperledger/besu/pull/6592)
 - Add bft-style `blockperiodseconds` transitions to Clique [#6596](https://github.com/hyperledger/besu/pull/6596)

diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/GraphQLHttpService.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/GraphQLHttpService.java
@@ -15,7 +15,6 @@
 package org.hyperledger.besu.ethereum.api.graphql;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.collect.Streams.stream;
 import static io.vertx.core.http.HttpMethod.GET;
 import static io.vertx.core.http.HttpMethod.POST;
 
@@ -44,8 +43,6 @@
 
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Splitter;
-import com.google.common.collect.Iterables;
 import com.google.common.net.MediaType;
 import graphql.ExecutionInput;
 import graphql.ExecutionResult;
@@ -62,6 +59,7 @@
 import io.vertx.core.json.DecodeException;
 import io.vertx.core.json.Json;
 import io.vertx.core.json.jackson.JacksonCodec;
+import io.vertx.core.net.HostAndPort;
 import io.vertx.ext.web.Router;
 import io.vertx.ext.web.RoutingContext;
 import io.vertx.ext.web.handler.BodyHandler;
@@ -143,7 +141,7 @@ public CompletableFuture<?> start() {
     final Router router = Router.router(vertx);
 
     // Verify Host header to avoid rebind attack.
-    router.route().handler(checkWhitelistHostHeader());
+    router.route().handler(checkAllowlistHostHeader());
 
     router
         .route()
@@ -199,7 +197,7 @@ public CompletableFuture<?> start() {
     return resultFuture;
   }
 
-  private Handler<RoutingContext> checkWhitelistHostHeader() {
+  private Handler<RoutingContext> checkAllowlistHostHeader() {
     return event -> {
       final Optional<String> hostHeader = getAndValidateHostHeader(event);
       if (config.getHostsAllowlist().contains("*")
@@ -218,19 +216,8 @@ private Handler<RoutingContext> checkWhitelistHostHeader() {
   }
 
   private Optional<String> getAndValidateHostHeader(final RoutingContext event) {
-    String hostname =
-        event.request().getHeader(HttpHeaders.HOST) != null
-            ? event.request().getHeader(HttpHeaders.HOST)
-            : event.request().host();
-    final Iterable<String> splitHostHeader = Splitter.on(':').split(hostname);
-    final long hostPieces = stream(splitHostHeader).count();
-    if (hostPieces > 1) {
-      // If the host contains a colon, verify the host is correctly formed - host [ ":" port ]
-      if (hostPieces > 2 || !Iterables.get(splitHostHeader, 1).matches("\\d{1,5}+")) {
-        return Optional.empty();
-      }
-    }
-    return Optional.ofNullable(Iterables.get(splitHostHeader, 0));
+    final HostAndPort hostAndPort = event.request().authority();
+    return Optional.ofNullable(hostAndPort).map(HostAndPort::host);
   }
 
   private boolean hostIsInAllowlist(final String hostHeader) {

diff --git a/...eum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java b/...eum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java
@@ -15,7 +15,6 @@
 package org.hyperledger.besu.ethereum.api.jsonrpc;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.collect.Streams.stream;
 import static org.apache.tuweni.net.tls.VertxTrustOptions.allowlistClients;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.authentication.AuthenticationUtils.truncToken;
 
@@ -63,8 +62,6 @@
 import javax.annotation.Nullable;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Splitter;
-import com.google.common.collect.Iterables;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
 import io.opentelemetry.api.trace.Span;
@@ -83,13 +80,13 @@
 import io.vertx.core.buffer.Buffer;
 import io.vertx.core.http.ClientAuth;
 import io.vertx.core.http.HttpConnection;
-import io.vertx.core.http.HttpHeaders;
 import io.vertx.core.http.HttpMethod;
 import io.vertx.core.http.HttpServer;
 import io.vertx.core.http.HttpServerOptions;
 import io.vertx.core.http.HttpServerRequest;
 import io.vertx.core.http.HttpServerResponse;
 import io.vertx.core.http.ServerWebSocket;
+import io.vertx.core.net.HostAndPort;
 import io.vertx.core.net.PfxOptions;
 import io.vertx.core.net.SocketAddress;
 import io.vertx.ext.auth.User;
@@ -616,19 +613,8 @@ private Handler<RoutingContext> denyRouteToBlockedHost() {
   }
 
   private Optional<String> getAndValidateHostHeader(final RoutingContext event) {
-    String hostname =
-        event.request().getHeader(HttpHeaders.HOST) != null
-            ? event.request().getHeader(HttpHeaders.HOST)
-            : event.request().host();
-    final Iterable<String> splitHostHeader = Splitter.on(':').split(hostname);
-    final long hostPieces = stream(splitHostHeader).count();
-    // If the host contains a colon, verify the host is correctly formed - host [ ":" port ]
-    if (hostPieces > 1) {
-      if (hostPieces > 2 || !Iterables.get(splitHostHeader, 1).matches("\\d{1,5}+")) {
-        return Optional.empty();
-      }
-    }
-    return Optional.ofNullable(Iterables.get(splitHostHeader, 0));
+    final HostAndPort hostAndPort = event.request().authority();
+    return Optional.ofNullable(hostAndPort).map(HostAndPort::host);
   }
 
   private boolean hostIsInAllowlist(final String hostHeader) {

diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java
@@ -15,7 +15,6 @@
 package org.hyperledger.besu.ethereum.api.jsonrpc;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.collect.Streams.stream;
 import static org.apache.tuweni.net.tls.VertxTrustOptions.allowlistClients;
 
 import org.hyperledger.besu.ethereum.api.handlers.HandlerFactory;
@@ -55,8 +54,6 @@
 import javax.annotation.Nullable;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Splitter;
-import com.google.common.collect.Iterables;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
 import io.opentelemetry.api.trace.Span;
@@ -74,12 +71,12 @@
 import io.vertx.core.VertxException;
 import io.vertx.core.http.ClientAuth;
 import io.vertx.core.http.HttpConnection;
-import io.vertx.core.http.HttpHeaders;
 import io.vertx.core.http.HttpMethod;
 import io.vertx.core.http.HttpServer;
 import io.vertx.core.http.HttpServerOptions;
 import io.vertx.core.http.HttpServerRequest;
 import io.vertx.core.http.HttpServerResponse;
+import io.vertx.core.net.HostAndPort;
 import io.vertx.core.net.PfxOptions;
 import io.vertx.core.net.SocketAddress;
 import io.vertx.ext.web.Route;
@@ -507,19 +504,8 @@ private Handler<RoutingContext> checkAllowlistHostHeader() {
   }
 
   private Optional<String> getAndValidateHostHeader(final RoutingContext event) {
-    String hostname =
-        event.request().getHeader(HttpHeaders.HOST) != null
-            ? event.request().getHeader(HttpHeaders.HOST)
-            : event.request().host();
-    final Iterable<String> splitHostHeader = Splitter.on(':').split(hostname);
-    final long hostPieces = stream(splitHostHeader).count();
-    if (hostPieces > 1) {
-      // If the host contains a colon, verify the host is correctly formed - host [ ":" port ]
-      if (hostPieces > 2 || !Iterables.get(splitHostHeader, 1).matches("\\d{1,5}+")) {
-        return Optional.empty();
-      }
-    }
-    return Optional.ofNullable(Iterables.get(splitHostHeader, 0));
+    final HostAndPort hostAndPort = event.request().authority();
+    return Optional.ofNullable(hostAndPort).map(HostAndPort::host);
   }
 
   private boolean hostIsInAllowlist(final String hostHeader) {

diff --git a/...i/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketService.java b/...i/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketService.java
@@ -14,7 +14,6 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.websocket;
 
-import static com.google.common.collect.Streams.stream;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.authentication.AuthenticationUtils.truncToken;
 
 import org.hyperledger.besu.ethereum.api.jsonrpc.authentication.AuthenticationService;
@@ -31,8 +30,6 @@
 import java.util.concurrent.atomic.AtomicInteger;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Splitter;
-import com.google.common.collect.Iterables;
 import io.vertx.core.AsyncResult;
 import io.vertx.core.Handler;
 import io.vertx.core.Vertx;
@@ -43,6 +40,7 @@
 import io.vertx.core.http.HttpServerRequest;
 import io.vertx.core.http.HttpServerResponse;
 import io.vertx.core.http.ServerWebSocket;
+import io.vertx.core.net.HostAndPort;
 import io.vertx.core.net.SocketAddress;
 import io.vertx.ext.web.Router;
 import io.vertx.ext.web.RoutingContext;
@@ -137,7 +135,8 @@ private Handler<ServerWebSocket> websocketHandler() {
             .log();
       }
 
-      if (!hasAllowedHostnameHeader(Optional.ofNullable(websocket.headers().get("Host")))) {
+      if (!checkHostInAllowlist(
+          Optional.ofNullable(websocket.authority()).map(HostAndPort::host))) {
         websocket.reject(403);
       }
 
@@ -294,7 +293,8 @@ private String getAuthToken(final ServerWebSocket websocket) {
 
   private Handler<RoutingContext> checkAllowlistHostHeader() {
     return event -> {
-      if (hasAllowedHostnameHeader(Optional.ofNullable(event.request().host()))) {
+      if (checkHostInAllowlist(
+          Optional.ofNullable(event.request().authority()).map(HostAndPort::host))) {
         event.next();
       } else {
         final HttpServerResponse response = event.response();
@@ -309,29 +309,12 @@ private Handler<RoutingContext> checkAllowlistHostHeader() {
   }
 
   @VisibleForTesting
-  public boolean hasAllowedHostnameHeader(final Optional<String> header) {
+  boolean checkHostInAllowlist(final Optional<String> host) {
     return configuration.getHostsAllowlist().contains("*")
-        || header.map(value -> checkHostInAllowlist(validateHostHeader(value))).orElse(false);
-  }
-
-  private Optional<String> validateHostHeader(final String header) {
-    final Iterable<String> splitHostHeader = Splitter.on(':').split(header);
-    final long hostPieces = stream(splitHostHeader).count();
-    if (hostPieces > 1) {
-      // If the host contains a colon, verify the host is correctly formed - host [ ":" port ]
-      if (hostPieces > 2 || !Iterables.get(splitHostHeader, 1).matches("\\d{1,5}+")) {
-        return Optional.empty();
-      }
-    }
-    return Optional.ofNullable(Iterables.get(splitHostHeader, 0));
-  }
-
-  private boolean checkHostInAllowlist(final Optional<String> hostHeader) {
-    return hostHeader
-        .map(
-            header ->
-                configuration.getHostsAllowlist().stream()
-                    .anyMatch(allowListEntry -> allowListEntry.equalsIgnoreCase(header)))
-        .orElse(false);
+        || host.map(
+                header ->
+                    configuration.getHostsAllowlist().stream()
+                        .anyMatch(allowListEntry -> allowListEntry.equalsIgnoreCase(header)))
+            .orElse(false);
   }
 }