diff --git a/CHANGELOG.md b/CHANGELOG.md index 526aa34773..c9b670f3f7 100644 --- 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 index a9a12d6179..abbcdbf945 100644 --- 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 java.util.concurrent.TimeUnit; 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.http.HttpServerResponse; 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 class GraphQLHttpService { 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 class GraphQLHttpService { return resultFuture; } - private Handler checkWhitelistHostHeader() { + private Handler checkAllowlistHostHeader() { return event -> { final Optional hostHeader = getAndValidateHostHeader(event); if (config.getHostsAllowlist().contains("*") @@ -218,19 +216,8 @@ public class GraphQLHttpService { } private Optional getAndValidateHostHeader(final RoutingContext event) { - String hostname = - event.request().getHeader(HttpHeaders.HOST) != null - ? event.request().getHeader(HttpHeaders.HOST) - : event.request().host(); - final Iterable 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/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java index 2e0fd4064d..567a8d3341 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java +++ b/ethereum/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.Nonnull; 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.VertxException; 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 @@ public class EngineJsonRpcService { } private Optional getAndValidateHostHeader(final RoutingContext event) { - String hostname = - event.request().getHeader(HttpHeaders.HOST) != null - ? event.request().getHeader(HttpHeaders.HOST) - : event.request().host(); - final Iterable 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 index e86bbfdb8a..1c8d7a2baf 100644 --- 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 java.util.concurrent.atomic.AtomicInteger; 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.Vertx; 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 @@ public class JsonRpcHttpService { } private Optional getAndValidateHostHeader(final RoutingContext event) { - String hostname = - event.request().getHeader(HttpHeaders.HOST) != null - ? event.request().getHeader(HttpHeaders.HOST) - : event.request().host(); - final Iterable 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/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketService.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketService.java index c040464ddd..59742836ba 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketService.java +++ b/ethereum/api/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.CompletableFuture; 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.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.SocketAddress; import io.vertx.ext.web.Router; import io.vertx.ext.web.RoutingContext; @@ -137,7 +135,8 @@ public class WebSocketService { .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 @@ public class WebSocketService { private Handler 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 @@ public class WebSocketService { } @VisibleForTesting - public boolean hasAllowedHostnameHeader(final Optional header) { + boolean checkHostInAllowlist(final Optional host) { return configuration.getHostsAllowlist().contains("*") - || header.map(value -> checkHostInAllowlist(validateHostHeader(value))).orElse(false); - } - - private Optional validateHostHeader(final String header) { - final Iterable 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 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); } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketHostAllowlistTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketHostAllowlistTest.java index 879788b26a..a53ef3d142 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketHostAllowlistTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketHostAllowlistTest.java @@ -41,12 +41,15 @@ import io.vertx.core.Vertx; import io.vertx.core.http.HttpClient; import io.vertx.core.http.HttpClientOptions; import io.vertx.core.http.HttpMethod; +import io.vertx.core.net.HostAndPort; import io.vertx.junit5.VertxExtension; import io.vertx.junit5.VertxTestContext; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; @ExtendWith(VertxExtension.class) public class WebSocketHostAllowlistTest { @@ -104,49 +107,60 @@ public class WebSocketHostAllowlistTest { @Test public void websocketRequestWithDefaultHeaderAndDefaultConfigIsAccepted() { - boolean result = websocketService.hasAllowedHostnameHeader(Optional.of("localhost:50012")); + final Optional host = + Optional.of(HostAndPort.parseAuthority("localhost", 50012)).map(HostAndPort::host); + boolean result = websocketService.checkHostInAllowlist(host); assertThat(result).isTrue(); } @Test - public void httpRequestWithDefaultHeaderAndDefaultConfigIsAccepted() throws InterruptedException { + public void httpRequestWithDefaultHeaderAndDefaultConfigIsAccepted() throws Throwable { doHttpRequestAndVerify(testContext, "localhost:50012", 400); } @Test public void websocketRequestWithEmptyHeaderAndDefaultConfigIsRejected() { - assertThat(websocketService.hasAllowedHostnameHeader(Optional.of(""))).isFalse(); + final Optional host = + Optional.ofNullable(HostAndPort.parseAuthority("", 50012)).map(HostAndPort::host); + boolean result = websocketService.checkHostInAllowlist(host); + assertThat(result).isFalse(); } @Test - public void httpRequestWithEmptyHeaderAndDefaultConfigIsRejected() throws InterruptedException { + public void httpRequestWithEmptyHeaderAndDefaultConfigIsRejected() throws Throwable { doHttpRequestAndVerify(testContext, "", 403); } - @Test - public void websocketRequestWithAnyHostnameAndWildcardConfigIsAccepted() { + @ParameterizedTest + @ValueSource(strings = {"ally", "foe"}) + public void websocketRequestWithAnyHostnameAndWildcardConfigIsAccepted(final String hostname) { webSocketConfiguration.setHostsAllowlist(Collections.singletonList("*")); - assertThat(websocketService.hasAllowedHostnameHeader(Optional.of("ally"))).isTrue(); - assertThat(websocketService.hasAllowedHostnameHeader(Optional.of("foe"))).isTrue(); + + final Optional host = + Optional.ofNullable(HostAndPort.parseAuthority(hostname, -1)).map(HostAndPort::host); + boolean result = websocketService.checkHostInAllowlist(host); + assertThat(result).isTrue(); } @Test - public void httpRequestWithAnyHostnameAndWildcardConfigIsAccepted() throws InterruptedException { + public void httpRequestWithAnyHostnameAndWildcardConfigIsAccepted() throws Throwable { webSocketConfiguration.setHostsAllowlist(Collections.singletonList("*")); doHttpRequestAndVerify(testContext, "ally", 400); doHttpRequestAndVerify(testContext, "foe", 400); } - @Test - public void websocketRequestWithAllowedHostIsAccepted() { + @ParameterizedTest + @ValueSource(strings = {"ally", "ally:12345", "friend"}) + public void websocketRequestWithAllowedHostIsAccepted(final String hostname) { webSocketConfiguration.setHostsAllowlist(hostsAllowlist); - assertThat(websocketService.hasAllowedHostnameHeader(Optional.of("ally"))).isTrue(); - assertThat(websocketService.hasAllowedHostnameHeader(Optional.of("ally:12345"))).isTrue(); - assertThat(websocketService.hasAllowedHostnameHeader(Optional.of("friend"))).isTrue(); + final Optional host = + Optional.ofNullable(HostAndPort.parseAuthority(hostname, -1)).map(HostAndPort::host); + boolean result = websocketService.checkHostInAllowlist(host); + assertThat(result).isTrue(); } @Test - public void httpRequestWithAllowedHostIsAccepted() throws InterruptedException { + public void httpRequestWithAllowedHostIsAccepted() throws Throwable { webSocketConfiguration.setHostsAllowlist(hostsAllowlist); doHttpRequestAndVerify(testContext, "ally", 400); doHttpRequestAndVerify(testContext, "ally:12345", 400); @@ -156,37 +170,40 @@ public class WebSocketHostAllowlistTest { @Test public void websocketRequestWithUnknownHostIsRejected() { webSocketConfiguration.setHostsAllowlist(hostsAllowlist); - assertThat(websocketService.hasAllowedHostnameHeader(Optional.of("foe"))).isFalse(); + final Optional host = + Optional.ofNullable(HostAndPort.parseAuthority("foe", -1)).map(HostAndPort::host); + assertThat(websocketService.checkHostInAllowlist(host)).isFalse(); } @Test - public void httpRequestWithUnknownHostIsRejected() throws InterruptedException { + public void httpRequestWithUnknownHostIsRejected() throws Throwable { webSocketConfiguration.setHostsAllowlist(hostsAllowlist); doHttpRequestAndVerify(testContext, "foe", 403); } - @Test - public void websocketRequestWithMalformedHostIsRejected() { + @ParameterizedTest + @ValueSource(strings = {"ally:friend", "ally:123456", "ally:friend:1234"}) + public void websocketRequestWithMalformedHostIsRejected(final String hostname) { webSocketConfiguration.setAuthenticationEnabled(false); webSocketConfiguration.setHostsAllowlist(hostsAllowlist); - assertThat(websocketService.hasAllowedHostnameHeader(Optional.of("ally:friend"))).isFalse(); - assertThat(websocketService.hasAllowedHostnameHeader(Optional.of("ally:123456"))).isFalse(); - assertThat(websocketService.hasAllowedHostnameHeader(Optional.of("ally:friend:1234"))) - .isFalse(); + final Optional host = + Optional.ofNullable(HostAndPort.parseAuthority(hostname, -1)).map(HostAndPort::host); + final boolean result = websocketService.checkHostInAllowlist(host); + assertThat(result).isFalse(); } @Test - public void httpRequestWithMalformedHostIsRejected() throws InterruptedException { + public void httpRequestWithMalformedHostIsRejected() throws Throwable { webSocketConfiguration.setAuthenticationEnabled(false); webSocketConfiguration.setHostsAllowlist(hostsAllowlist); - doHttpRequestAndVerify(testContext, "ally:friend", 400); + doHttpRequestAndVerify(testContext, "ally:friend", 403); doHttpRequestAndVerify(testContext, "ally:123456", 403); doHttpRequestAndVerify(testContext, "ally:friend:1234", 403); } private void doHttpRequestAndVerify( final VertxTestContext testContext, final String hostname, final int expectedResponse) - throws InterruptedException { + throws Throwable { httpClient.request( HttpMethod.POST, @@ -200,10 +217,18 @@ public class WebSocketHostAllowlistTest { .result() .send( response -> { - assertThat(response.result().statusCode()).isEqualTo(expectedResponse); - testContext.completeNow(); + if (response.succeeded()) { + assertThat(response.result().statusCode()).isEqualTo(expectedResponse); + testContext.completeNow(); + } else { + testContext.failNow(response.cause()); + } }); }); - testContext.awaitCompletion(VERTX_AWAIT_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS); + assertThat(testContext.awaitCompletion(VERTX_AWAIT_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS)) + .isTrue(); + if (testContext.failed()) { + throw testContext.causeOfFailure(); + } } } diff --git a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/MetricsHttpService.java b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/MetricsHttpService.java index c547b3d4cd..94a198dc0e 100644 --- a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/MetricsHttpService.java +++ b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/MetricsHttpService.java @@ -15,7 +15,6 @@ package org.hyperledger.besu.metrics.prometheus; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.collect.Streams.stream; import org.hyperledger.besu.metrics.MetricsService; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -32,8 +31,6 @@ import java.util.Set; import java.util.TreeSet; import java.util.concurrent.CompletableFuture; -import com.google.common.base.Splitter; -import com.google.common.collect.Iterables; import io.netty.handler.codec.http.HttpResponseStatus; import io.prometheus.client.exporter.common.TextFormat; import io.vertx.core.Handler; @@ -42,6 +39,7 @@ import io.vertx.core.http.HttpMethod; import io.vertx.core.http.HttpServer; import io.vertx.core.http.HttpServerOptions; import io.vertx.core.http.HttpServerResponse; +import io.vertx.core.net.HostAndPort; import io.vertx.ext.web.Router; import io.vertx.ext.web.RoutingContext; import org.slf4j.Logger; @@ -158,19 +156,7 @@ public class MetricsHttpService implements MetricsService { } private Optional getAndValidateHostHeader(final RoutingContext event) { - final String hostHeader = event.request().host(); - if (hostHeader == null) { - return Optional.empty(); - } - final Iterable splitHostHeader = Splitter.on(':').split(hostHeader); - 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)); + return Optional.ofNullable(event.request().authority()).map(HostAndPort::host); } private boolean hostIsInAllowlist(final String hostHeader) {