From c838df90eff72576f666e6f58e4a120022b2b23e Mon Sep 17 00:00:00 2001 From: Chris Eager Date: Wed, 31 Jan 2024 14:05:22 -0600 Subject: [PATCH] Add HttpServletRequestUtil --- .../controllers/ChallengeController.java | 3 +- .../controllers/VerificationController.java | 3 +- .../limits/RateLimitByIpFilter.java | 3 +- .../util/HttpServletRequestUtil.java | 25 ++++++ ...HttpServletRequestUtilIntegrationTest.java | 84 +++++++++++++++++++ .../util/HttpServletRequestUtilTest.java | 29 +++++++ 6 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtil.java create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtilIntegrationTest.java create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtilTest.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ChallengeController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ChallengeController.java index 654fc84a6..6bfa566c1 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ChallengeController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ChallengeController.java @@ -43,6 +43,7 @@ import org.whispersystems.textsecuregcm.spam.FilterSpam; import org.whispersystems.textsecuregcm.spam.PushChallengeConfig; import org.whispersystems.textsecuregcm.spam.ScoreThreshold; import org.whispersystems.textsecuregcm.util.HeaderUtils; +import org.whispersystems.textsecuregcm.util.HttpServletRequestUtil; @Path("/v1/challenge") @Tag(name = "Challenge") @@ -103,7 +104,7 @@ public class ChallengeController { tags = tags.and(CHALLENGE_TYPE_TAG, "recaptcha"); final String remoteAddress = useRemoteAddress - ? request.getRemoteAddr() + ? HttpServletRequestUtil.getRemoteAddress(request) : HeaderUtils.getMostRecentProxy(forwardedFor).orElseThrow(BadRequestException::new); boolean success = rateLimitChallengeManager.answerRecaptchaChallenge( auth.getAccount(), diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java index cdbba86f9..673b2f41b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java @@ -89,6 +89,7 @@ import org.whispersystems.textsecuregcm.storage.RegistrationRecoveryPasswordsMan import org.whispersystems.textsecuregcm.storage.VerificationSessionManager; import org.whispersystems.textsecuregcm.util.ExceptionUtils; import org.whispersystems.textsecuregcm.util.HeaderUtils; +import org.whispersystems.textsecuregcm.util.HttpServletRequestUtil; import org.whispersystems.textsecuregcm.util.Pair; import org.whispersystems.textsecuregcm.util.Util; @@ -212,7 +213,7 @@ public class VerificationController { @NotNull @Extract final SenderOverride senderOverride) { final String sourceHost = useRemoteAddress - ? request.getRemoteAddr() + ? HttpServletRequestUtil.getRemoteAddress(request) : HeaderUtils.getMostRecentProxy(forwardedFor).orElseThrow(); final Pair pushTokenAndType = validateAndExtractPushToken( diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitByIpFilter.java b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitByIpFilter.java index 3ef1093af..0aa244ab2 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitByIpFilter.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitByIpFilter.java @@ -26,6 +26,7 @@ import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper; import org.whispersystems.textsecuregcm.util.HeaderUtils; +import org.whispersystems.textsecuregcm.util.HttpServletRequestUtil; public class RateLimitByIpFilter implements ContainerRequestFilter { @@ -71,7 +72,7 @@ public class RateLimitByIpFilter implements ContainerRequestFilter { try { final String xffHeader = requestContext.getHeaders().getFirst(HttpHeaders.X_FORWARDED_FOR); final Optional remoteAddress = useRemoteAddress - ? Optional.of(httpServletRequestProvider.get().getRemoteAddr()) + ? Optional.of(HttpServletRequestUtil.getRemoteAddress(httpServletRequestProvider.get())) : Optional.ofNullable(xffHeader) .flatMap(HeaderUtils::getMostRecentProxy); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtil.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtil.java new file mode 100644 index 000000000..f799450f8 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtil.java @@ -0,0 +1,25 @@ +/* + * Copyright 2024 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.util; + +import javax.servlet.http.HttpServletRequest; + +public class HttpServletRequestUtil { + + /** + * Returns the remote address of the request, removing bracket ("[…]") host notation from IPv6 addresses present in + * some implementations, notably {@link org.eclipse.jetty.server.HttpChannel}. + */ + public static String getRemoteAddress(final HttpServletRequest request) { + final String remoteAddr = request.getRemoteAddr(); + + if (remoteAddr.startsWith("[")) { + return remoteAddr.substring(1, remoteAddr.length() - 1); + } + + return remoteAddr; + } +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtilIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtilIntegrationTest.java new file mode 100644 index 000000000..a2b3e7430 --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtilIntegrationTest.java @@ -0,0 +1,84 @@ +/* + * Copyright 2024 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import io.dropwizard.core.Application; +import io.dropwizard.core.Configuration; +import io.dropwizard.core.setup.Environment; +import io.dropwizard.testing.junit5.DropwizardAppExtension; +import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; +import java.net.InetAddress; +import java.util.Arrays; +import java.util.Set; +import java.util.stream.Collectors; +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.client.Client; +import javax.ws.rs.core.Context; +import org.eclipse.jetty.util.HostPort; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +@ExtendWith(DropwizardExtensionsSupport.class) +class HttpServletRequestUtilIntegrationTest { + + private static final String PATH = "/test"; + + // The Grizzly test container does not match the Jetty container used in real deployments, and JettyTestContainerFactory + // in jersey-test-framework-provider-jetty doesn’t easily support @Context HttpServletRequest, so this test runs a + // full Jetty server in a separate process + private final DropwizardAppExtension EXTENSION = new DropwizardAppExtension<>( + TestApplication.class); + + @ParameterizedTest + @ValueSource(strings = {"127.0.0.1", "0:0:0:0:0:0:0:1"}) + void test(String ip) throws Exception { + final Set addresses = Arrays.stream(InetAddress.getAllByName("localhost")) + .map(InetAddress::getHostAddress) + .collect(Collectors.toSet()); + + assumeTrue(addresses.contains(ip), String.format("localhost does not resolve to %s", ip)); + + Client client = EXTENSION.client(); + + final TestResponse response = client.target( + String.format("http://%s:%d%s", HostPort.normalizeHost(ip), EXTENSION.getLocalPort(), PATH)) + .request("application/json") + .get(TestResponse.class); + + assertEquals(ip, response.remoteAddress()); + } + + @Path(PATH) + public static class TestController { + + @GET + public TestResponse get(@Context HttpServletRequest request) { + + return new TestResponse(HttpServletRequestUtil.getRemoteAddress(request)); + } + + } + + public record TestResponse(String remoteAddress) { + + } + + public static class TestApplication extends Application { + + @Override + public void run(final TestConfiguration configuration, final Environment environment) throws Exception { + environment.jersey().register(new TestController()); + } + } + + public static class TestConfiguration extends Configuration {} +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtilTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtilTest.java new file mode 100644 index 000000000..cde298cd6 --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/util/HttpServletRequestUtilTest.java @@ -0,0 +1,29 @@ +/* + * Copyright 2024 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import javax.servlet.http.HttpServletRequest; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +class HttpServletRequestUtilTest { + + @ParameterizedTest + @CsvSource({ + "127.0.0.1, 127.0.0.1", + "[0:0:0:0:0:0:0:1], 0:0:0:0:0:0:0:1" + }) + void testGetRemoteAddress(final String remoteAddr, final String expectedRemoteAddr) { + final HttpServletRequest httpServletRequest = mock(HttpServletRequest.class); + when(httpServletRequest.getRemoteAddr()).thenReturn(remoteAddr); + + assertEquals(expectedRemoteAddr, HttpServletRequestUtil.getRemoteAddress(httpServletRequest)); + } +}