From 156ce3e06c1a3cd93e147718f83e02b7e891aded Mon Sep 17 00:00:00 2001 From: Maxim Karpov Date: Tue, 2 Jul 2024 15:01:55 +0200 Subject: [PATCH] QuicTokenHandler: expose max connection id length, fix endianness (#732) Motivation: The ByteBuf that is passed into the `writeToken(...)` method should use `BIG_ENDIAN` while at the moment it might use `LITTLE_ENDIAN`. Modifications: * Ensures that all buffers passed to the `QuicTokenHandler` are now big endian * Expose max connection it via static field in Quic class. Result: Fixes #731. --------- Co-authored-by: Norman Maurer --- .../codec/quic/InsecureQuicTokenHandler.java | 2 +- .../io/netty/incubator/codec/quic/Quic.java | 6 +++ .../codec/quic/QuicHeaderParser.java | 7 ++- .../codec/quic/QuicTokenHandler.java | 3 +- .../codec/quic/QuicheQuicServerCodec.java | 3 +- .../codec/quic/QuicChannelConnectTest.java | 20 ++++---- .../codec/quic/TestQuicTokenHandler.java | 48 +++++++++++++++++++ 7 files changed, 72 insertions(+), 17 deletions(-) create mode 100644 codec-native-quic/src/test/java/io/netty/incubator/codec/quic/TestQuicTokenHandler.java diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/InsecureQuicTokenHandler.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/InsecureQuicTokenHandler.java index 2cb2f9c41..059d4f981 100644 --- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/InsecureQuicTokenHandler.java +++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/InsecureQuicTokenHandler.java @@ -36,7 +36,7 @@ public final class InsecureQuicTokenHandler implements QuicTokenHandler { Unpooled.wrappedBuffer(SERVER_NAME_BYTES)).asReadOnly(); // Just package-private for unit tests - static final int MAX_TOKEN_LEN = Quiche.QUICHE_MAX_CONN_ID_LEN + + static final int MAX_TOKEN_LEN = Quic.MAX_CONN_ID_LEN + NetUtil.LOCALHOST6.getAddress().length + SERVER_NAME_BYTES.length; private InsecureQuicTokenHandler() { diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/Quic.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/Quic.java index 120ccc280..22923683c 100644 --- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/Quic.java +++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/Quic.java @@ -52,6 +52,12 @@ public final class Quic { UNAVAILABILITY_CAUSE = cause; } + /** + * The maximum length of the connection id. + * + */ + public static final int MAX_CONN_ID_LEN = 20; + /** * Return if the given QUIC version is supported. * diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicHeaderParser.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicHeaderParser.java index 4f124af1b..0f7f14866 100644 --- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicHeaderParser.java +++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicHeaderParser.java @@ -34,8 +34,7 @@ public final class QuicHeaderParser implements AutoCloseable { // See https://datatracker.ietf.org/doc/rfc7714/ private static final int AES_128_GCM_TAG_LENGTH = 16; - // https://www.rfc-editor.org/rfc/rfc9000.html#section-17.2 - private static final int MAX_CONN_ID_LEN = 20; + private final int localConnectionIdLength; private boolean closed; @@ -146,8 +145,8 @@ public void parse(InetSocketAddress sender, InetSocketAddress recipient, ByteBuf // Check if the connection id is not longer then 20. This is what is the maximum for QUIC version 1. // See https://www.rfc-editor.org/rfc/rfc9000.html#section-17.2 private void checkCidLength(int length) throws QuicException{ - if (length > MAX_CONN_ID_LEN) { - throw new QuicException("connection id to large: " + length + " > " + MAX_CONN_ID_LEN, + if (length > Quic.MAX_CONN_ID_LEN) { + throw new QuicException("connection id to large: " + length + " > " + Quic.MAX_CONN_ID_LEN, QuicTransportError.PROTOCOL_VIOLATION); } } diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicTokenHandler.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicTokenHandler.java index 07baa4ff9..4c3691921 100644 --- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicTokenHandler.java +++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicTokenHandler.java @@ -30,7 +30,8 @@ public interface QuicTokenHandler { * {@code false}. * * @param out {@link ByteBuf} into which the token will be written. - * @param dcid the destination connection id. + * @param dcid the destination connection id. The {@link ByteBuf#readableBytes()} will be at most + * {@link Quic#MAX_CONN_ID_LEN}. * @param address the {@link InetSocketAddress} of the sender. * @return {@code true} if a token was written and so validation should happen, {@code false} otherwise. */ diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicServerCodec.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicServerCodec.java index 628f81794..fec68b60a 100644 --- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicServerCodec.java +++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicServerCodec.java @@ -16,6 +16,7 @@ package io.netty.incubator.codec.quic; import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelOption; @@ -87,7 +88,7 @@ final class QuicheQuicServerCodec extends QuicheQuicCodec { @Override protected void handlerAdded(ChannelHandlerContext ctx, int localConnIdLength) { connIdBuffer = Quiche.allocateNativeOrder(localConnIdLength); - mintTokenBuffer = allocateNativeOrder(tokenHandler.maxTokenLength()); + mintTokenBuffer = Unpooled.directBuffer(tokenHandler.maxTokenLength()); } @Override diff --git a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java index e3270cbb1..54b5096f1 100644 --- a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java +++ b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java @@ -276,7 +276,7 @@ private static void testConnectWithCustomIdLength(Executor executor, int clientI ChannelStateVerifyHandler serverQuicStreamHandler = new ChannelStateVerifyHandler(); Channel server = QuicTestUtils.newServer(QuicTestUtils.newQuicServerBuilder(executor) .localConnectionIdLength(serverIdLength), - InsecureQuicTokenHandler.INSTANCE, serverQuicChannelHandler, serverQuicStreamHandler); + TestQuicTokenHandler.INSTANCE, serverQuicChannelHandler, serverQuicStreamHandler); InetSocketAddress address = (InetSocketAddress) server.localAddress(); Channel channel = QuicTestUtils.newClient(QuicTestUtils.newQuicClientBuilder(executor) .localConnectionIdLength(clientIdLength)); @@ -662,7 +662,7 @@ public void testConnectWith0RTT(Executor executor) throws Throwable { .applicationProtocols(QuicTestUtils.PROTOS) .earlyData(true) .build()), - InsecureQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { + TestQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { @Override public boolean isSharable() { return true; @@ -938,7 +938,7 @@ public void testALPNProtocolMissmatch(Executor executor) throws Throwable { QuicTestUtils.SELF_SIGNED_CERTIFICATE.privateKey(), null, QuicTestUtils.SELF_SIGNED_CERTIFICATE.certificate()) .applicationProtocols("my-protocol").build()), - InsecureQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { + TestQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { @Override public void userEventTriggered(ChannelHandlerContext ctx, Object evt) { @@ -1008,7 +1008,7 @@ public void testConnectSuccessWhenTrustManagerBuildFromSameCert(Executor executo QuicTestUtils.SELF_SIGNED_CERTIFICATE.privateKey(), null, QuicTestUtils.SELF_SIGNED_CERTIFICATE.certificate()) .applicationProtocols(QuicTestUtils.PROTOS).clientAuth(ClientAuth.NONE).build()), - InsecureQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter(), + TestQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter(), new ChannelInboundHandlerAdapter()); InetSocketAddress address = (InetSocketAddress) server.localAddress(); @@ -1070,7 +1070,7 @@ private void testConnectMutualAuthSuccess(Executor executor, MutalAuthTestMode m .applicationProtocols(QuicTestUtils.PROTOS) .clientAuth(mode == MutalAuthTestMode.REQUIRED ? ClientAuth.REQUIRE : ClientAuth.OPTIONAL).build()), - InsecureQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter(), + TestQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter(), new ChannelInboundHandlerAdapter()); InetSocketAddress address = (InetSocketAddress) server.localAddress(); @@ -1166,7 +1166,7 @@ public void testConnectMutualAuthFailsIfClientNotSendCertificate(Executor execut QuicTestUtils.SELF_SIGNED_CERTIFICATE.certificate()) .trustManager(InsecureTrustManagerFactory.INSTANCE) .applicationProtocols(QuicTestUtils.PROTOS).clientAuth(ClientAuth.REQUIRE).build()), - InsecureQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { + TestQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { causeRef.compareAndSet(null, cause); @@ -1230,7 +1230,7 @@ public void testSniMatch(Executor executor) throws Throwable { .add(hostname, sniServerSslContext).build()); Channel server = QuicTestUtils.newServer(QuicTestUtils.newQuicServerBuilder(executor, serverSslContext), - InsecureQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { + TestQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { @Override public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception { if (evt instanceof SniCompletionEvent) { @@ -1306,7 +1306,7 @@ private void testSniFallbackToDefault(Executor executor, boolean sendSni) throws .add("quic.netty.io", sniServerSslContext).build()); Channel server = QuicTestUtils.newServer(QuicTestUtils.newQuicServerBuilder(executor, serverSslContext), - InsecureQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter(), + TestQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter(), new ChannelInboundHandlerAdapter()); InetSocketAddress address = (InetSocketAddress) server.localAddress(); @@ -1403,7 +1403,7 @@ public Future decrypt(SSLEngine engine, byte[] input) { Channel server = QuicTestUtils.newServer(QuicTestUtils.newQuicServerBuilder(executor, QuicSslContextBuilder.forServer(factory, null) .applicationProtocols(QuicTestUtils.PROTOS).clientAuth(ClientAuth.NONE).build()), - InsecureQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { + TestQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) { causeRef.set(cause); @@ -1475,7 +1475,7 @@ private static void testSessionReuse(Executor executor, boolean ticketKey) throw } CountDownLatch serverSslCompletionEventLatch = new CountDownLatch(2); Channel server = QuicTestUtils.newServer(QuicTestUtils.newQuicServerBuilder(executor, sslServerCtx), - InsecureQuicTokenHandler.INSTANCE, + TestQuicTokenHandler.INSTANCE, new ChannelInboundHandlerAdapter() { @Override public boolean isSharable() { diff --git a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/TestQuicTokenHandler.java b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/TestQuicTokenHandler.java new file mode 100644 index 000000000..174c5d663 --- /dev/null +++ b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/TestQuicTokenHandler.java @@ -0,0 +1,48 @@ +/* + * Copyright 2024 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.incubator.codec.quic; + +import io.netty.buffer.ByteBuf; + +import java.net.InetSocketAddress; +import java.nio.ByteOrder; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +final class TestQuicTokenHandler implements QuicTokenHandler { + public static final QuicTokenHandler INSTANCE = new TestQuicTokenHandler(); + + @Override + @SuppressWarnings("deprecation") + public boolean writeToken(ByteBuf out, ByteBuf dcid, InetSocketAddress address) { + assertEquals(ByteOrder.BIG_ENDIAN, out.order()); + return InsecureQuicTokenHandler.INSTANCE.writeToken(out, dcid, address); + } + + @Override + @SuppressWarnings("deprecation") + public int validateToken(ByteBuf token, InetSocketAddress address) { + assertEquals(ByteOrder.BIG_ENDIAN, token.order()); + return InsecureQuicTokenHandler.INSTANCE.validateToken(token, address); + } + + @Override + public int maxTokenLength() { + return InsecureQuicTokenHandler.INSTANCE.maxTokenLength(); + } + + private TestQuicTokenHandler() { } +}