From 732948b3c4be68c2f5e4b3d3f0dc595363e18abe Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 8 May 2017 05:54:37 +0200 Subject: [PATCH] Ensure SslUtils and so SslHandler works when using with Little-Endian buffers. Motivation: We not correctly handle LE buffers when try to read the packet length out of the buffer and just assume it always is a BE buffer. Modifications: Correctly account for the endianess of the buffer when reading the packet lenght. Result: Fixes [#6709]. --- .../java/io/netty/handler/ssl/SslUtils.java | 45 +++++++++---- .../io/netty/handler/ssl/SslUtilsTest.java | 66 +++++++++++++++++++ 2 files changed, 97 insertions(+), 14 deletions(-) create mode 100644 handler/src/test/java/io/netty/handler/ssl/SslUtilsTest.java diff --git a/handler/src/main/java/io/netty/handler/ssl/SslUtils.java b/handler/src/main/java/io/netty/handler/ssl/SslUtils.java index dbb22609ee..bdf6567248 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslUtils.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslUtils.java @@ -17,11 +17,13 @@ package io.netty.handler.ssl; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufAllocator; +import io.netty.buffer.ByteBufUtil; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.base64.Base64; import io.netty.handler.codec.base64.Base64Dialect; import java.nio.ByteBuffer; +import java.nio.ByteOrder; import javax.net.ssl.SSLHandshakeException; /** @@ -120,7 +122,7 @@ final class SslUtils { int majorVersion = buffer.getUnsignedByte(offset + 1); if (majorVersion == 3) { // SSLv3 or TLS - packetLength = buffer.getUnsignedShort(offset + 3) + SSL_RECORD_HEADER_LENGTH; + packetLength = unsignedShortBE(buffer, offset + 3) + SSL_RECORD_HEADER_LENGTH; if (packetLength <= SSL_RECORD_HEADER_LENGTH) { // Neither SSLv3 or TLSv1 (i.e. SSLv2 or bad data) tls = false; @@ -137,11 +139,8 @@ final class SslUtils { int majorVersion = buffer.getUnsignedByte(offset + headerLength + 1); if (majorVersion == 2 || majorVersion == 3) { // SSLv2 - if (headerLength == 2) { - packetLength = (buffer.getShort(offset) & 0x7FFF) + 2; - } else { - packetLength = (buffer.getShort(offset) & 0x3FFF) + 3; - } + packetLength = headerLength == 2 ? + (shortBE(buffer, offset) & 0x7FFF) + 2 : (shortBE(buffer, offset) & 0x3FFF) + 3; if (packetLength <= headerLength) { return NOT_ENOUGH_DATA; } @@ -152,12 +151,33 @@ final class SslUtils { return packetLength; } + // Reads a big-endian unsigned short integer from the buffer + @SuppressWarnings("deprecation") + private static int unsignedShortBE(ByteBuf buffer, int offset) { + return buffer.order() == ByteOrder.BIG_ENDIAN ? + buffer.getUnsignedShort(offset) : buffer.getUnsignedShortLE(offset); + } + + // Reads a big-endian short integer from the buffer + @SuppressWarnings("deprecation") + private static short shortBE(ByteBuf buffer, int offset) { + return buffer.order() == ByteOrder.BIG_ENDIAN ? + buffer.getShort(offset) : buffer.getShortLE(offset); + } + private static short unsignedByte(byte b) { return (short) (b & 0xFF); } - private static int unsignedShort(short s) { - return s & 0xFFFF; + // Reads a big-endian unsigned short integer from the buffer + private static int unsignedShortBE(ByteBuffer buffer, int offset) { + return shortBE(buffer, offset) & 0xFFFF; + } + + // Reads a big-endian short integer from the buffer + private static short shortBE(ByteBuffer buffer, int offset) { + return buffer.order() == ByteOrder.BIG_ENDIAN ? + buffer.getShort(offset) : ByteBufUtil.swapShort(buffer.getShort(offset)); } static int getEncryptedPacketLength(ByteBuffer[] buffers, int offset) { @@ -207,7 +227,7 @@ final class SslUtils { int majorVersion = unsignedByte(buffer.get(pos + 1)); if (majorVersion == 3) { // SSLv3 or TLS - packetLength = unsignedShort(buffer.getShort(pos + 3)) + SSL_RECORD_HEADER_LENGTH; + packetLength = unsignedShortBE(buffer, pos + 3) + SSL_RECORD_HEADER_LENGTH; if (packetLength <= SSL_RECORD_HEADER_LENGTH) { // Neither SSLv3 or TLSv1 (i.e. SSLv2 or bad data) tls = false; @@ -224,11 +244,8 @@ final class SslUtils { int majorVersion = unsignedByte(buffer.get(pos + headerLength + 1)); if (majorVersion == 2 || majorVersion == 3) { // SSLv2 - if (headerLength == 2) { - packetLength = (buffer.getShort(pos) & 0x7FFF) + 2; - } else { - packetLength = (buffer.getShort(pos) & 0x3FFF) + 3; - } + packetLength = headerLength == 2 ? + (shortBE(buffer, pos) & 0x7FFF) + 2 : (shortBE(buffer, pos) & 0x3FFF) + 3; if (packetLength <= headerLength) { return NOT_ENOUGH_DATA; } diff --git a/handler/src/test/java/io/netty/handler/ssl/SslUtilsTest.java b/handler/src/test/java/io/netty/handler/ssl/SslUtilsTest.java new file mode 100644 index 0000000000..c1de33dd6e --- /dev/null +++ b/handler/src/test/java/io/netty/handler/ssl/SslUtilsTest.java @@ -0,0 +1,66 @@ +/* + * Copyright 2017 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: + * + * http://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.handler.ssl; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import org.junit.Test; + +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLException; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.security.NoSuchAlgorithmException; + +import static io.netty.handler.ssl.SslUtils.getEncryptedPacketLength; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class SslUtilsTest { + + @SuppressWarnings("deprecation") + @Test + public void testPacketLength() throws SSLException, NoSuchAlgorithmException { + SSLEngine engineLE = newEngine(); + SSLEngine engineBE = newEngine(); + + ByteBuffer empty = ByteBuffer.allocate(0); + ByteBuffer cTOsLE = ByteBuffer.allocate(17 * 1024).order(ByteOrder.LITTLE_ENDIAN); + ByteBuffer cTOsBE = ByteBuffer.allocate(17 * 1024); + + assertTrue(engineLE.wrap(empty, cTOsLE).bytesProduced() > 0); + cTOsLE.flip(); + + assertTrue(engineBE.wrap(empty, cTOsBE).bytesProduced() > 0); + cTOsBE.flip(); + + ByteBuf bufferLE = Unpooled.buffer().order(ByteOrder.LITTLE_ENDIAN).writeBytes(cTOsLE); + ByteBuf bufferBE = Unpooled.buffer().writeBytes(cTOsBE); + + // Test that the packet-length for BE and LE is the same + assertEquals(getEncryptedPacketLength(bufferBE, 0), getEncryptedPacketLength(bufferLE, 0)); + assertEquals(getEncryptedPacketLength(new ByteBuffer[] { bufferBE.nioBuffer() }, 0), + getEncryptedPacketLength(new ByteBuffer[] { bufferLE.nioBuffer().order(ByteOrder.LITTLE_ENDIAN) }, 0)); + } + + private static SSLEngine newEngine() throws SSLException, NoSuchAlgorithmException { + SSLEngine engine = SSLContext.getDefault().createSSLEngine(); + engine.setUseClientMode(true); + engine.beginHandshake(); + return engine; + } +}