Throw error for oversized inbound noise messages

Author: ravi-signal

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;

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);
+  }
+}

service/src/main/java/org/whispersystems/textsecuregcm/grpc/net/NoiseHandler.java

@@ -83,6 +83,10 @@ abstract CompletableFuture<HandshakeResult> 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()));

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);
+  }
 }

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);