diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/ErrorHandler.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/ErrorHandler.java index 1ad4e9f59..787b091e2 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/ErrorHandler.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/ErrorHandler.java @@ -43,6 +43,7 @@ public void exceptionCaught(final ChannelHandlerContext context, final Throwable case NoiseHandshakeException e -> ApplicationWebSocketCloseReason.NOISE_HANDSHAKE_ERROR.toWebSocketCloseStatus(e.getMessage()); case ClientAuthenticationException ignored -> ApplicationWebSocketCloseReason.CLIENT_AUTHENTICATION_ERROR.toWebSocketCloseStatus("Not authenticated"); case BadPaddingException ignored -> ApplicationWebSocketCloseReason.NOISE_ENCRYPTION_ERROR.toWebSocketCloseStatus("Noise encryption error"); + case NoiseException ignored -> ApplicationWebSocketCloseReason.NOISE_ENCRYPTION_ERROR.toWebSocketCloseStatus("Noise encryption error"); default -> { log.warn("An unexpected exception reached the end of the pipeline", cause); yield WebSocketCloseStatus.INTERNAL_SERVER_ERROR; diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/NoiseException.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/NoiseException.java new file mode 100644 index 000000000..e5d22827f --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/NoiseException.java @@ -0,0 +1,11 @@ +package org.whispersystems.textsecuregcm.grpc.net; + +/** + * Indicates that some problem occurred while processing an encrypted noise message (e.g. an unexpected message size/ + * format or a general encryption error). + */ +class NoiseException extends Exception { + public NoiseException(final String message) { + super(message); + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/NoiseHandler.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/NoiseHandler.java index 48eefac6a..3c2c9379a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/NoiseHandler.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/NoiseHandler.java @@ -83,6 +83,10 @@ abstract CompletableFuture handleHandshakePayload( public void channelRead(final ChannelHandlerContext context, final Object message) throws Exception { try { if (message instanceof BinaryWebSocketFrame frame) { + if (frame.content().readableBytes() > Noise.MAX_PACKET_LEN) { + final String error = "Invalid noise message length " + frame.content().readableBytes(); + throw state == State.HANDSHAKE ? new NoiseHandshakeException(error) : new NoiseException(error); + } // We've read this frame off the wire, and so it's most likely a direct buffer that's not backed by an array. // We'll need to copy it to a heap buffer. handleInboundMessage(context, ByteBufUtil.getBytes(frame.content())); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/net/AbstractNoiseHandlerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/net/AbstractNoiseHandlerTest.java index febeefa33..7c8a40f41 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/net/AbstractNoiseHandlerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/net/AbstractNoiseHandlerTest.java @@ -6,6 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import com.southernstorm.noise.protocol.CipherStatePair; @@ -284,4 +285,11 @@ void writeHugeOutboundMessage(final int plaintextLength) throws Throwable { } + @Test + public void writeHugeInboundMessage() throws Throwable { + doHandshake(); + final byte[] big = TestRandomUtil.nextBytes(Noise.MAX_PACKET_LEN + 1); + embeddedChannel.pipeline().fireChannelRead(new BinaryWebSocketFrame(Unpooled.wrappedBuffer(big))); + assertThrows(NoiseException.class, embeddedChannel::checkException); + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/net/NoiseAuthenticatedHandlerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/net/NoiseAuthenticatedHandlerTest.java index 1aeb0f516..b8c539295 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/net/NoiseAuthenticatedHandlerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/net/NoiseAuthenticatedHandlerTest.java @@ -14,6 +14,7 @@ import com.southernstorm.noise.protocol.CipherStatePair; import com.southernstorm.noise.protocol.HandshakeState; +import com.southernstorm.noise.protocol.Noise; import io.netty.buffer.Unpooled; import io.netty.channel.ChannelFuture; import io.netty.channel.embedded.EmbeddedChannel; @@ -35,6 +36,7 @@ import org.whispersystems.textsecuregcm.auth.grpc.AuthenticatedDevice; import org.whispersystems.textsecuregcm.storage.ClientPublicKeysManager; import org.whispersystems.textsecuregcm.storage.Device; +import org.whispersystems.textsecuregcm.util.TestRandomUtil; import org.whispersystems.textsecuregcm.util.UUIDUtil; class NoiseAuthenticatedHandlerTest extends AbstractNoiseHandlerTest { @@ -219,6 +221,16 @@ void handleInvalidExtraWrites() throws NoSuchAlgorithmException, ShortBufferExce assertNull(embeddedChannel.outboundMessages().poll()); } + @Test + public void handleOversizeHandshakeMessage() { + final EmbeddedChannel embeddedChannel = getEmbeddedChannel(); + final byte[] big = TestRandomUtil.nextBytes(Noise.MAX_PACKET_LEN + 1); + ByteBuffer.wrap(big) + .put(UUIDUtil.toBytes(UUID.randomUUID())) + .put((byte) 0x01); + assertThrows(NoiseHandshakeException.class, () -> doHandshake(big)); + } + private HandshakeState clientHandshakeState() throws NoSuchAlgorithmException { final HandshakeState clientHandshakeState = new HandshakeState(HandshakePattern.IK.protocol(), HandshakeState.INITIATOR);