fix: Use HttpRequest authority instead of host (#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>
pull/6885/head
Usman Saleem 8 months ago committed by GitHub
parent 1e77a6c419
commit 80c6c04db6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 1
      CHANGELOG.md
  2. 23
      ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/GraphQLHttpService.java
  3. 20
      ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java
  4. 20
      ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java
  5. 39
      ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketService.java
  6. 83
      ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/WebSocketHostAllowlistTest.java
  7. 18
      metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/MetricsHttpService.java

@ -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)

@ -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<RoutingContext> checkWhitelistHostHeader() {
private Handler<RoutingContext> checkAllowlistHostHeader() {
return event -> {
final Optional<String> hostHeader = getAndValidateHostHeader(event);
if (config.getHostsAllowlist().contains("*")
@ -218,19 +216,8 @@ public class GraphQLHttpService {
}
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) {

@ -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<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) {

@ -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<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) {

@ -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<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 @@ public class WebSocketService {
}
@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);
}
}

@ -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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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();
}
}
}

@ -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<String> getAndValidateHostHeader(final RoutingContext event) {
final String hostHeader = event.request().host();
if (hostHeader == null) {
return Optional.empty();
}
final Iterable<String> 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) {

Loading…
Cancel
Save