From 861dc0d021eb6401044ca86662933d7751769fdf Mon Sep 17 00:00:00 2001 From: Jonathan Klabunde Tomer <125505367+jkt-signal@users.noreply.github.com> Date: Tue, 13 Jun 2023 09:49:50 -0700 Subject: [PATCH] reject message sends that have the same device more than once --- .../entities/IncomingMessageList.java | 23 ++++++++++++ .../entities/MultiRecipientMessage.java | 23 ++++++++++++ .../controllers/MessageControllerTest.java | 37 +++++++++++++++++++ .../current_message_duplicate_device.json | 23 ++++++++++++ 4 files changed, 106 insertions(+) create mode 100644 service/src/test/resources/fixtures/current_message_duplicate_device.json diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/IncomingMessageList.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/IncomingMessageList.java index d5bb2af4e..50113db8a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/IncomingMessageList.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/IncomingMessageList.java @@ -4,15 +4,29 @@ */ package org.whispersystems.textsecuregcm.entities; +import static com.codahale.metrics.MetricRegistry.name; + import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; + +import org.whispersystems.textsecuregcm.controllers.MessageController; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.Metrics; + import java.util.List; import javax.validation.Valid; +import javax.validation.constraints.AssertTrue; import javax.validation.constraints.NotNull; public record IncomingMessageList(@NotNull @Valid List<@NotNull IncomingMessage> messages, boolean online, boolean urgent, long timestamp) { + private static final Counter REJECT_DUPLICATE_RECIPIENT_COUNTER = + Metrics.counter( + name(MessageController.class, "rejectDuplicateRecipients"), + "multiRecipient", "false"); + @JsonCreator public IncomingMessageList(@JsonProperty("messages") @NotNull @Valid List<@NotNull IncomingMessage> messages, @JsonProperty("online") boolean online, @@ -21,4 +35,13 @@ public record IncomingMessageList(@NotNull @Valid List<@NotNull IncomingMessage> this(messages, online, urgent == null || urgent, timestamp); } + + @AssertTrue + public boolean hasNoDuplicateRecipients() { + boolean valid = messages.stream().filter(m -> m != null).map(IncomingMessage::destinationDeviceId).distinct().count() == messages.size(); + if (!valid) { + REJECT_DUPLICATE_RECIPIENT_COUNTER.increment(); + } + return valid; + } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/MultiRecipientMessage.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/MultiRecipientMessage.java index aecba4ac4..4469b6fea 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/MultiRecipientMessage.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/MultiRecipientMessage.java @@ -5,17 +5,31 @@ package org.whispersystems.textsecuregcm.entities; +import static com.codahale.metrics.MetricRegistry.name; + import java.util.Arrays; import java.util.UUID; import javax.validation.Valid; +import javax.validation.constraints.AssertTrue; import javax.validation.constraints.Max; import javax.validation.constraints.Min; import javax.validation.constraints.NotNull; import javax.validation.constraints.Size; + +import org.whispersystems.textsecuregcm.controllers.MessageController; import org.whispersystems.textsecuregcm.providers.MultiRecipientMessageProvider; +import org.whispersystems.textsecuregcm.util.Pair; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.Metrics; public class MultiRecipientMessage { + private static final Counter REJECT_DUPLICATE_RECIPIENT_COUNTER = + Metrics.counter( + name(MessageController.class, "rejectDuplicateRecipients"), + "multiRecipient", "false"); + public static class Recipient { @NotNull @@ -108,4 +122,13 @@ public class MultiRecipientMessage { public byte[] getCommonPayload() { return commonPayload; } + + @AssertTrue + public boolean hasNoDuplicateRecipients() { + boolean valid = Arrays.stream(recipients).map(r -> new Pair<>(r.getUuid(), r.getDeviceId())).distinct().count() == recipients.length; + if (!valid) { + REJECT_DUPLICATE_RECIPIENT_COUNTER.increment(); + } + return valid; + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java index d0018d1e8..9e7c14961 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java @@ -391,6 +391,21 @@ class MessageControllerTest { verifyNoMoreInteractions(messageSender); } + @Test + void testMultiDeviceDuplicate() throws Exception { + Response response = resources.getJerseyTest() + .target(String.format("/v1/messages/%s", MULTI_DEVICE_UUID)) + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.entity(SystemMapper.jsonMapper().readValue(jsonFixture("fixtures/current_message_duplicate_device.json"), + IncomingMessageList.class), + MediaType.APPLICATION_JSON_TYPE)); + + assertThat("Good Response Code", response.getStatus(), is(equalTo(422))); + + verifyNoMoreInteractions(messageSender); + } + @Test void testMultiDevice() throws Exception { Response response = @@ -1041,6 +1056,28 @@ class MessageControllerTest { ); } + @Test + void testMultiRecipientRedisBombProtection() throws Exception { + final List recipients = List.of( + new Recipient(MULTI_DEVICE_UUID, MULTI_DEVICE_ID1, MULTI_DEVICE_REG_ID1, new byte[48]), + new Recipient(MULTI_DEVICE_UUID, MULTI_DEVICE_ID2, MULTI_DEVICE_REG_ID1, new byte[48]), + new Recipient(MULTI_DEVICE_UUID, MULTI_DEVICE_ID1, MULTI_DEVICE_REG_ID1, new byte[48])); + + Response response = resources + .getJerseyTest() + .target("/v1/messages/multi_recipient") + .queryParam("online", true) + .queryParam("ts", 1663798405641L) + .queryParam("story", false) + .queryParam("urgent", false) + .request() + .header(HttpHeaders.USER_AGENT, "cluck cluck, i'm a parrot") + .header(OptionalAccess.UNIDENTIFIED, Base64.getEncoder().encodeToString(UNIDENTIFIED_ACCESS_BYTES)) + .put(Entity.entity(initializeMultiPayload(recipients, new byte[2048]), MultiRecipientMessageProvider.MEDIA_TYPE)); + + checkBadMultiRecipientResponse(response, 422); + } + @Test void testSendStoryToUnknownAccount() throws Exception { String accessBytes = Base64.getEncoder().encodeToString(UNIDENTIFIED_ACCESS_BYTES); diff --git a/service/src/test/resources/fixtures/current_message_duplicate_device.json b/service/src/test/resources/fixtures/current_message_duplicate_device.json new file mode 100644 index 000000000..ec0496486 --- /dev/null +++ b/service/src/test/resources/fixtures/current_message_duplicate_device.json @@ -0,0 +1,23 @@ +{ + "messages" : [{ + "type" : 1, + "destinationDeviceId" : 1, + "destinationRegistrationId" : 222, + "content" : "Zm9vYmFyego", + "timestamp" : 1234 + }, + { + "type" : 1, + "destinationDeviceId" : 2, + "destinationRegistrationId" : 333, + "content" : "Zm9vYmFyego", + "timestamp" : 1234 + }, + { + "type" : 1, + "destinationDeviceId" : 1, + "destinationRegistrationId" : 222, + "content" : "Zm9vYmFyego", + "timestamp" : 1234 + }] +}