Skip to content

Commit

Permalink
fix!: Fix Host Allow List Handler (Consensys#985)
Browse files Browse the repository at this point in the history
* fix!: Fix Host Allow List Handler
* Handle case of empty Host header and its value
  • Loading branch information
usmansaleem authored Apr 8, 2024
1 parent 76cf179 commit 165c715
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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.\"}");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -51,15 +48,8 @@ public void handle(final RoutingContext event) {
}

private Optional<String> getAndValidateHostHeader(final RoutingContext event) {
final Iterable<String> 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) {
Expand Down
2 changes: 1 addition & 1 deletion gradle/versions.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down

0 comments on commit 165c715

Please sign in to comment.