diff --git a/bom/pom.xml b/bom/pom.xml index ac353101cc1..ee59a02391c 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -65,7 +65,7 @@ - 2.0.54.Final + 2.0.55.Final diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java index 8347c8ba0dc..d737fd8f351 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java @@ -2627,7 +2627,7 @@ public int getPeerPort() { @Override public int getPacketBufferSize() { - return maxEncryptedPacketLength(); + return SSL.SSL_MAX_ENCRYPTED_LENGTH; } @Override diff --git a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java index 6a187fded36..b6bdd5c26a2 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java @@ -1645,6 +1645,10 @@ protected void handshake(BufferType type, boolean delegate, SSLEngine clientEngi if (isHandshakeFinished(clientResult)) { clientHandshakeFinished = true; } + + if (clientResult.getStatus() == Status.BUFFER_OVERFLOW) { + cTOs = increaseDstBuffer(clientEngine.getSession().getPacketBufferSize(), type, cTOs); + } } if (!serverHandshakeFinished) { @@ -1656,6 +1660,10 @@ protected void handshake(BufferType type, boolean delegate, SSLEngine clientEngi if (isHandshakeFinished(serverResult)) { serverHandshakeFinished = true; } + + if (serverResult.getStatus() == Status.BUFFER_OVERFLOW) { + sTOc = increaseDstBuffer(serverEngine.getSession().getPacketBufferSize(), type, sTOc); + } } cTOs.flip(); @@ -1674,10 +1682,16 @@ protected void handshake(BufferType type, boolean delegate, SSLEngine clientEngi runDelegatedTasks(delegate, clientResult, clientEngine); assertEquals(sTOc.position() - sTOcPos, clientResult.bytesConsumed()); assertEquals(clientAppReadBuffer.position() - clientAppReadBufferPos, clientResult.bytesProduced()); + assertEquals(0, clientAppReadBuffer.position()); if (isHandshakeFinished(clientResult)) { clientHandshakeFinished = true; } + + if (clientResult.getStatus() == Status.BUFFER_OVERFLOW) { + clientAppReadBuffer = increaseDstBuffer( + clientEngine.getSession().getApplicationBufferSize(), type, clientAppReadBuffer); + } } else { assertEquals(0, sTOc.remaining()); } @@ -1688,19 +1702,33 @@ protected void handshake(BufferType type, boolean delegate, SSLEngine clientEngi runDelegatedTasks(delegate, serverResult, serverEngine); assertEquals(cTOs.position() - cTOsPos, serverResult.bytesConsumed()); assertEquals(serverAppReadBuffer.position() - serverAppReadBufferPos, serverResult.bytesProduced()); + assertEquals(0, serverAppReadBuffer.position()); if (isHandshakeFinished(serverResult)) { serverHandshakeFinished = true; } + + if (serverResult.getStatus() == Status.BUFFER_OVERFLOW) { + serverAppReadBuffer = increaseDstBuffer( + serverEngine.getSession().getApplicationBufferSize(), type, serverAppReadBuffer); + } } else { assertFalse(cTOs.hasRemaining()); } - cTOsHasRemaining = cTOs.hasRemaining(); - sTOcHasRemaining = sTOc.hasRemaining(); + cTOsHasRemaining = compactOrClear(cTOs); + sTOcHasRemaining = compactOrClear(sTOc); + + serverAppReadBuffer.clear(); + clientAppReadBuffer.clear(); - sTOc.compact(); - cTOs.compact(); + if (clientEngine.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING) { + clientHandshakeFinished = true; + } + + if (serverEngine.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING) { + serverHandshakeFinished = true; + } } while (!clientHandshakeFinished || !serverHandshakeFinished || // We need to ensure we feed all the data to the engine to not end up with a corrupted state. // This is especially important with TLS1.3 which may produce sessions after the "main handshake" is @@ -1708,6 +1736,25 @@ protected void handshake(BufferType type, boolean delegate, SSLEngine clientEngi cTOsHasRemaining || sTOcHasRemaining); } + private static boolean compactOrClear(ByteBuffer buffer) { + if (buffer.hasRemaining()) { + buffer.compact(); + return true; + } + buffer.clear(); + return false; + } + + private ByteBuffer increaseDstBuffer(int maxBufferSize, + BufferType type, ByteBuffer dstBuffer) { + assumeFalse(maxBufferSize == dstBuffer.remaining()); + // We need to increase the destination buffer + dstBuffer.flip(); + ByteBuffer tmpBuffer = allocateBuffer(type, maxBufferSize + dstBuffer.remaining()); + tmpBuffer.put(dstBuffer); + return tmpBuffer; + } + private static boolean isHandshakeFinished(SSLEngineResult result) { return result.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.FINISHED; } @@ -4243,6 +4290,40 @@ public void testInvalidSNIIsIgnoredAndNotThrow(SSLEngineTestParam param) throws } } + @MethodSource("newTestParams") + @ParameterizedTest + public void testBufferUnderflowPacketSizeDependency(SSLEngineTestParam param) throws Exception { + SelfSignedCertificate ssc = new SelfSignedCertificate(); + clientSslCtx = wrapContext(param, SslContextBuilder.forClient() + .keyManager(ssc.certificate(), ssc.privateKey()) + .trustManager((TrustManagerFactory) null) + .sslProvider(sslClientProvider()) + .sslContextProvider(clientSslContextProvider()) + .protocols(param.protocols()) + .ciphers(param.ciphers()) + .build()); + serverSslCtx = wrapContext(param, SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) + .sslProvider(sslServerProvider()) + .sslContextProvider(serverSslContextProvider()) + .protocols(param.protocols()) + .ciphers(param.ciphers()) + .clientAuth(ClientAuth.REQUIRE) + .build()); + SSLEngine clientEngine = null; + SSLEngine serverEngine = null; + try { + clientEngine = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); + serverEngine = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); + + handshake(param.type(), param.delegate(), clientEngine, serverEngine); + } catch (SSLHandshakeException expected) { + // Expected + } finally { + cleanupClientSslEngine(clientEngine); + cleanupServerSslEngine(serverEngine); + } + } + protected SSLEngine wrapEngine(SSLEngine engine) { return engine; } diff --git a/pom.xml b/pom.xml index d8f433032cc..e89676d571a 100644 --- a/pom.xml +++ b/pom.xml @@ -598,7 +598,7 @@ fedora,suse,arch netty-tcnative - 2.0.54.Final + 2.0.55.Final ${os.detected.classifier} org.conscrypt conscrypt-openjdk-uber