From 579eb851750ede21b4c43f3eea26f25ee17eb8ff Mon Sep 17 00:00:00 2001 From: Chris Eager Date: Tue, 15 Feb 2022 15:31:52 -0800 Subject: [PATCH] Reject invalid envelope types --- .../controllers/MessageController.java | 46 +++++++++++++------ .../controllers/MessageControllerTest.java | 30 ++++++++++++ ...age_single_device_server_receipt_type.json | 10 ++++ 3 files changed, 73 insertions(+), 13 deletions(-) create mode 100644 service/src/test/resources/fixtures/current_message_single_device_server_receipt_type.json diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java index 9e73b7241..f9b04ac70 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java @@ -126,11 +126,13 @@ public class MessageController { private static final String CONTENT_SIZE_DISTRIBUTION_NAME = name(MessageController.class, "messageContentSize"); private static final String OUTGOING_MESSAGE_LIST_SIZE_BYTES_DISTRIBUTION_NAME = name(MessageController.class, "outgoingMessageListSizeBytes"); private static final String RATE_LIMITED_MESSAGE_COUNTER_NAME = name(MessageController.class, "rateLimitedMessage"); + private static final String REJECT_INVALID_ENVELOPE_TYPE = name(MessageController.class, "rejectInvalidEnvelopeType"); private static final String EPHEMERAL_TAG_NAME = "ephemeral"; private static final String SENDER_TYPE_TAG_NAME = "senderType"; private static final String SENDER_COUNTRY_TAG_NAME = "senderCountry"; private static final String RATE_LIMIT_REASON_TAG_NAME = "rateLimitReason"; + private static final String ENVELOPE_TYPE_TAG_NAME = "envelopeType"; private static final String SENDER_TYPE_IDENTIFIED = "identified"; private static final String SENDER_TYPE_UNIDENTIFIED = "unidentified"; @@ -188,6 +190,7 @@ public class MessageController { } for (final IncomingMessage message : messages.getMessages()) { + int contentLength = 0; if (!Util.isEmpty(message.getContent())) { @@ -198,12 +201,13 @@ public class MessageController { contentLength += message.getBody().length(); } - Metrics.summary(CONTENT_SIZE_DISTRIBUTION_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))).record(contentLength); + Optional maybeResponse = validateContentLength(contentLength, userAgent); - if (contentLength > MAX_MESSAGE_SIZE) { - Metrics.counter(REJECT_OVERSIZE_MESSAGE_COUNTER, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))).increment(); - return Response.status(Response.Status.REQUEST_ENTITY_TOO_LARGE).build(); + if (maybeResponse.isPresent()) { + return maybeResponse.get(); } + + validateEnvelopeType(message.getType(), userAgent); } try { @@ -276,7 +280,7 @@ public class MessageController { @QueryParam("online") boolean online, @QueryParam("ts") long timestamp, @NotNull @Valid IncomingDeviceMessage[] messages) - throws RateLimitExceededException, RateLimitChallengeException { + throws RateLimitExceededException { if (source.isEmpty() && accessKey.isEmpty()) { throw new WebApplicationException(Response.Status.UNAUTHORIZED); @@ -295,14 +299,8 @@ public class MessageController { } for (final IncomingDeviceMessage message : messages) { - int contentLength = message.getContent().length; - - Metrics.summary(CONTENT_SIZE_DISTRIBUTION_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))).record(contentLength); - - if (contentLength > MAX_MESSAGE_SIZE) { - Metrics.counter(REJECT_OVERSIZE_MESSAGE_COUNTER, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))).increment(); - return Response.status(Response.Status.REQUEST_ENTITY_TOO_LARGE).build(); - } + validateContentLength(message.getContent().length, userAgent); + validateEnvelopeType(message.getType(), userAgent); } try { @@ -779,6 +777,28 @@ public class MessageController { } } + private Optional validateContentLength(final int contentLength, final String userAgent) { + Metrics.summary(CONTENT_SIZE_DISTRIBUTION_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))) + .record(contentLength); + + if (contentLength > MAX_MESSAGE_SIZE) { + Metrics.counter(REJECT_OVERSIZE_MESSAGE_COUNTER, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))) + .increment(); + return Optional.of(Response.status(Status.REQUEST_ENTITY_TOO_LARGE).build()); + } + + return Optional.empty(); + } + + private void validateEnvelopeType(final int type, final String userAgent) { + if (type == Type.SERVER_DELIVERY_RECEIPT_VALUE) { + Metrics.counter(REJECT_INVALID_ENVELOPE_TYPE, + Tags.of(UserAgentTagUtil.getPlatformTag(userAgent), Tag.of(ENVELOPE_TYPE_TAG_NAME, String.valueOf(type)))) + .increment(); + throw new BadRequestException("reserved envelope type"); + } + } + private Optional getMessageBody(IncomingMessage message) { if (Util.isEmpty(message.getBody())) return Optional.empty(); 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 2bed1491a..3d78087df 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java @@ -612,6 +612,36 @@ class MessageControllerTest { verify(reportMessageManager).report(senderNumber, messageGuid, AuthHelper.VALID_UUID); } + @ParameterizedTest + @MethodSource + void testValidateEnvelopeType(String payloadFilename, boolean expectOk) throws Exception { + Response response = + resources.getJerseyTest() + .target(String.format("/v1/messages/%s", SINGLE_DEVICE_UUID)) + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .header("User-Agent", "FIXME") + .put(Entity.entity(mapper.readValue(jsonFixture(payloadFilename), IncomingMessageList.class), + MediaType.APPLICATION_JSON_TYPE)); + + if (expectOk) { + assertEquals(200, response.getStatus()); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(Envelope.class); + verify(messageSender).sendMessage(any(Account.class), any(Device.class), captor.capture(), eq(false)); + } else { + assertEquals(400, response.getStatus()); + verify(messageSender, never()).sendMessage(any(), any(), any(), anyBoolean()); + } + } + + private static Stream testValidateEnvelopeType() { + return Stream.of( + Arguments.of("fixtures/current_message_single_device.json", true), + Arguments.of("fixtures/current_message_single_device_server_receipt_type.json", false) + ); + } + static Account mockAccountWithDeviceAndRegId(Object... deviceAndRegistrationIds) { Account account = mock(Account.class); if (deviceAndRegistrationIds.length % 2 != 0) { diff --git a/service/src/test/resources/fixtures/current_message_single_device_server_receipt_type.json b/service/src/test/resources/fixtures/current_message_single_device_server_receipt_type.json new file mode 100644 index 000000000..9496a8ca2 --- /dev/null +++ b/service/src/test/resources/fixtures/current_message_single_device_server_receipt_type.json @@ -0,0 +1,10 @@ +{ + "messages": [ + { + "type": 5, + "destinationDeviceId": 1, + "body": "Zm9vYmFyego", + "timestamp": 1234 + } + ] +}