Skip to content

Commit

Permalink
Slice token before pass it to QuicTokenHandler (#743)
Browse files Browse the repository at this point in the history
Motivation:

As the QuicTokenHandler implementation might modify the readerIndex of
the token we need to ensure we slice it before pass it to to the
handler. Otherwise we will run into issues that might fail the
connection establishment

Modifications:

- Slice token before pass it to the QuicTokenHandler
- Add unit test

Result:

No more connection issues due of token readerIndex adjustments. Fixes
#742
  • Loading branch information
normanmaurer authored Aug 20, 2024
1 parent 6b4591d commit 11b2b4f
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,10 @@ private QuicheQuicChannel handleServer(ChannelHandlerContext ctx, InetSocketAddr
offset = 0;
noToken = true;
} else {
offset = tokenHandler.validateToken(token, sender);
// Slice the token before pass it ot the QuicTokenHandler as the implementation might modify
// the readerIndex.
// See https://github.com/netty/netty-incubator-codec-quic/issues/742
offset = tokenHandler.validateToken(token.slice(), sender);
if (offset == -1) {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("invalid token: {}", token.toString(CharsetUtil.US_ASCII));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,84 @@ public void testConnectAlreadyConnected(Executor executor) throws Throwable {
}
}


@ParameterizedTest
@MethodSource("newSslTaskExecutors")
public void testConnectWithTokenValidation(Executor executor) throws Throwable {
int numBytes = 8;
ChannelActiveVerifyHandler serverQuicChannelHandler = new ChannelActiveVerifyHandler();
CountDownLatch serverLatch = new CountDownLatch(1);
CountDownLatch clientLatch = new CountDownLatch(1);

// Disable token validation
Channel server = QuicTestUtils.newServer(executor, new QuicTokenHandler() {
@Override
public boolean writeToken(ByteBuf out, ByteBuf dcid, InetSocketAddress address) {
out.writeInt(0)
.writeBytes(dcid, dcid.readerIndex(), dcid.readableBytes());
return true;
}

@Override
public int validateToken(ByteBuf token, InetSocketAddress address) {
// Use readInt() so we adjust the readerIndex of the token.
assertEquals(0, token.readInt());
return token.readerIndex();
}

@Override
public int maxTokenLength() {
return 96;
}
},
serverQuicChannelHandler, new BytesCountingHandler(serverLatch, numBytes));
InetSocketAddress address = (InetSocketAddress) server.localAddress();
Channel channel = QuicTestUtils.newClient(executor);
try {
ChannelActiveVerifyHandler clientQuicChannelHandler = new ChannelActiveVerifyHandler();
QuicChannel quicChannel = QuicTestUtils.newQuicChannelBootstrap(channel)
.handler(clientQuicChannelHandler)
.streamHandler(new ChannelInboundHandlerAdapter())
.remoteAddress(address)
.connect()
.get();
QuicConnectionAddress localAddress = (QuicConnectionAddress) quicChannel.localAddress();
QuicConnectionAddress remoteAddress = (QuicConnectionAddress) quicChannel.remoteAddress();
assertNotNull(localAddress);
assertNotNull(remoteAddress);

QuicStreamChannel stream = quicChannel.createStream(QuicStreamType.BIDIRECTIONAL,
new BytesCountingHandler(clientLatch, numBytes)).get();
stream.writeAndFlush(Unpooled.directBuffer().writeZero(numBytes)).sync();
clientLatch.await();

QuicheQuicSslEngine quicheQuicSslEngine = (QuicheQuicSslEngine) quicChannel.sslEngine();
assertNotNull(quicheQuicSslEngine);
assertEquals(QuicTestUtils.PROTOS[0],
// Just do the cast as getApplicationProtocol() only exists in SSLEngine itself since Java9+ and
// we may run on an earlier version
quicheQuicSslEngine.getApplicationProtocol());
stream.close().sync();
quicChannel.close().sync();
ChannelFuture closeFuture = quicChannel.closeFuture().await();
assertTrue(closeFuture.isSuccess());

clientQuicChannelHandler.assertState();
serverQuicChannelHandler.assertState();

assertEquals(serverQuicChannelHandler.localAddress(), remoteAddress);
assertEquals(serverQuicChannelHandler.remoteAddress(), localAddress);
} finally {
serverLatch.await();

server.close().sync();
// Close the parent Datagram channel as well.
channel.close().sync();

shutdown(executor);
}
}

@ParameterizedTest
@MethodSource("newSslTaskExecutors")
public void testConnectWithoutTokenValidation(Executor executor) throws Throwable {
Expand Down

0 comments on commit 11b2b4f

Please sign in to comment.