From 165c7155b7a51f11a9b304ca81fbdb4743bc49cc Mon Sep 17 00:00:00 2001 From: Usman Saleem Date: Mon, 8 Apr 2024 12:22:51 +1000 Subject: [PATCH] fix!: Fix Host Allow List Handler (#985) * fix!: Fix Host Allow List Handler * Handle case of empty Host header and its value --- CHANGELOG.md | 1 + .../dsl/lotus/FilecoinJsonRpcEndpoint.java | 3 +- .../support/TlsEnabledHttpServerFactory.java | 2 +- .../HttpHostAllowListAcceptanceTest.java | 47 +++++++++++++++++++ .../service/http/HostAllowListHandler.java | 16 ++----- gradle/versions.gradle | 2 +- 6 files changed, 54 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 009da0150..9f2ec21fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Bugs fixed - Update Teku libraries to 24.3.1 - Update Vert.x to 4.5.7 (which include fixes for CVE-2024-1023) +- Fix Host Allow List handler to handle empty host header - Update Postgresql JDBC driver to fix CVE-2024-1597 - Fix cached gvr to be thread-safe during first boot. [#978](https://github.com/Consensys/web3signer/issues/978) diff --git a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/lotus/FilecoinJsonRpcEndpoint.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/lotus/FilecoinJsonRpcEndpoint.java index 0c3b8aced..aa1bd7a10 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/lotus/FilecoinJsonRpcEndpoint.java +++ b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/lotus/FilecoinJsonRpcEndpoint.java @@ -24,7 +24,6 @@ import com.fasterxml.jackson.databind.json.JsonMapper; import com.github.arteam.simplejsonrpc.client.JsonRpcClient; import com.google.common.net.MediaType; -import org.apache.commons.codec.Charsets; import org.apache.http.HttpHeaders; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpPost; @@ -122,7 +121,7 @@ public String executeRawJsonRpcRequest(final String request) throws IOException AUTH_TOKEN.ifPresent(token -> post.setHeader("Authorization", "Bearer " + token)); try (final CloseableHttpClient httpClient = HttpClients.createDefault(); final CloseableHttpResponse httpResponse = httpClient.execute(post)) { - return EntityUtils.toString(httpResponse.getEntity(), Charsets.UTF_8); + return EntityUtils.toString(httpResponse.getEntity(), StandardCharsets.UTF_8); } } diff --git a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/tls/support/TlsEnabledHttpServerFactory.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/tls/support/TlsEnabledHttpServerFactory.java index 1277ba44f..f1434b670 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/tls/support/TlsEnabledHttpServerFactory.java +++ b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/tls/support/TlsEnabledHttpServerFactory.java @@ -75,7 +75,7 @@ public HttpServer create( web3HttpServerOptions.setTrustOptions( VertxTrustOptions.allowlistClients(serverFingerprintFile)); web3HttpServerOptions.setPort(0); - web3HttpServerOptions.setPfxKeyCertOptions( + web3HttpServerOptions.setKeyCertOptions( new PfxOptions() .setPath(serverCert.getPkcs12File().toString()) .setPassword(serverCert.getPassword())); diff --git a/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/HttpHostAllowListAcceptanceTest.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/HttpHostAllowListAcceptanceTest.java index 044425fa1..d8b9d1c5d 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/HttpHostAllowListAcceptanceTest.java +++ b/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/HttpHostAllowListAcceptanceTest.java @@ -13,14 +13,26 @@ package tech.pegasys.web3signer.tests; import static io.restassured.RestAssured.given; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import tech.pegasys.web3signer.dsl.signer.SignerConfiguration; import tech.pegasys.web3signer.dsl.signer.SignerConfigurationBuilder; +import java.io.BufferedReader; +import java.io.InputStreamReader; +import java.io.OutputStreamWriter; +import java.io.PrintWriter; +import java.net.InetAddress; +import java.net.Socket; +import java.net.URI; import java.util.Collections; +import java.util.stream.Collectors; import io.restassured.http.ContentType; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class HttpHostAllowListAcceptanceTest extends AcceptanceTestBase { private static final String UPCHECK_ENDPOINT = "/upcheck"; @@ -80,4 +92,39 @@ void httpEndpointForNonAllowedHostRespondsWithForbiddenResponse() { .assertThat() .statusCode(403); } + + @ParameterizedTest + @ValueSource(strings = {"Host: \r\n", ""}) + void httpEndpointWithoutHostHeaderRespondsWithForbiddenResponse(final String rawHeader) + throws Exception { + final SignerConfiguration signerConfiguration = + new SignerConfigurationBuilder() + .withHttpAllowHostList(Collections.singletonList("127.0.0.1")) + .withMode("eth2") + .build(); + startSigner(signerConfiguration); + + // raw request without Host header + final URI uri = URI.create(signer.getUrl()); + try (final Socket s = new Socket(InetAddress.getLoopbackAddress(), uri.getPort()); + final PrintWriter writer = + new PrintWriter(new OutputStreamWriter(s.getOutputStream(), UTF_8), true); + final BufferedReader reader = + new BufferedReader(new InputStreamReader(s.getInputStream(), UTF_8))) { + final String req = + "GET " + + UPCHECK_ENDPOINT + + " HTTP/1.1\r\n" + + "Connection: close\r\n" // signals server to close the connection + + rawHeader + + "\r\n"; // end of headers section + writer.write(req); + writer.flush(); + + final String response = reader.lines().collect(Collectors.joining("\n")); + + assertThat(response).startsWith("HTTP/1.1 403 Forbidden"); + assertThat(response).contains("{\"message\":\"Host not authorized.\"}"); + } + } } diff --git a/core/src/main/java/tech/pegasys/web3signer/core/service/http/HostAllowListHandler.java b/core/src/main/java/tech/pegasys/web3signer/core/service/http/HostAllowListHandler.java index 5e9bfe2da..7bda8fab3 100644 --- a/core/src/main/java/tech/pegasys/web3signer/core/service/http/HostAllowListHandler.java +++ b/core/src/main/java/tech/pegasys/web3signer/core/service/http/HostAllowListHandler.java @@ -12,15 +12,12 @@ */ package tech.pegasys.web3signer.core.service.http; -import static com.google.common.collect.Streams.stream; - import java.util.List; import java.util.Optional; -import com.google.common.base.Splitter; -import com.google.common.collect.Iterables; import io.vertx.core.Handler; import io.vertx.core.http.HttpServerResponse; +import io.vertx.core.net.HostAndPort; import io.vertx.ext.web.RoutingContext; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -51,15 +48,8 @@ public void handle(final RoutingContext event) { } private Optional getAndValidateHostHeader(final RoutingContext event) { - final Iterable splitHostHeader = Splitter.on(':').split(event.request().host()); - 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/gradle/versions.gradle b/gradle/versions.gradle index d842b173b..25903c659 100644 --- a/gradle/versions.gradle +++ b/gradle/versions.gradle @@ -110,7 +110,7 @@ dependencyManagement { dependency 'tech.pegasys:jblst:0.3.8' - dependency 'io.rest-assured:rest-assured:4.4.0' + dependency 'io.rest-assured:rest-assured:5.4.0' dependency 'org.zeroturnaround:zt-exec:1.12' dependencySet(group: 'org.web3j', version: '4.10.2') { entry 'besu'