Skip to content

Commit

Permalink
Throw error for oversized inbound noise messages
Browse files Browse the repository at this point in the history
  • Loading branch information
ravi-signal committed Jul 30, 2024
1 parent 3d96d73 commit 3a58272
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 3a58272

Please sign in to comment.