Do not throw IndexOutOfBoundsException on an invalid SSL record

Motivation:

When an SSL record contains an invalid extension data, SniHandler
currently throws an IndexOutOfBoundsException, which is not optimal.

Modifications:

- Do strict index range checks

Result:

No more unnecessary instantiation of exceptions and their stack traces
This commit is contained in:
Trustin Lee 2016-01-29 14:02:21 +09:00 committed by Norman Maurer
parent 3616d9e814
commit c3e5604f59

View File

@ -108,19 +108,25 @@ public class SniHandler extends ByteToMessageDecoder implements ChannelOutboundH
@Override @Override
protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception { protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
if (!suppressRead && !handshakeFailed && in.readableBytes() >= SslUtils.SSL_RECORD_HEADER_LENGTH) { if (!suppressRead && !handshakeFailed) {
int writerIndex = in.writerIndex(); final int writerIndex = in.writerIndex();
int readerIndex = in.readerIndex();
try { try {
loop: loop:
for (int i = 0; i < MAX_SSL_RECORDS; i++) { for (int i = 0; i < MAX_SSL_RECORDS; i++) {
int command = in.getUnsignedByte(readerIndex); final int readerIndex = in.readerIndex();
final int readableBytes = writerIndex - readerIndex;
if (readableBytes < SslUtils.SSL_RECORD_HEADER_LENGTH) {
// Not enough data to determine the record type and length.
return;
}
final int command = in.getUnsignedByte(readerIndex);
// tls, but not handshake command // tls, but not handshake command
switch (command) { switch (command) {
case SslUtils.SSL_CONTENT_TYPE_CHANGE_CIPHER_SPEC: case SslUtils.SSL_CONTENT_TYPE_CHANGE_CIPHER_SPEC:
case SslUtils.SSL_CONTENT_TYPE_ALERT: case SslUtils.SSL_CONTENT_TYPE_ALERT:
int len = SslUtils.getEncryptedPacketLength(in, readerIndex); final int len = SslUtils.getEncryptedPacketLength(in, readerIndex);
// Not an SSL/TLS packet // Not an SSL/TLS packet
if (len == -1) { if (len == -1) {
@ -138,20 +144,21 @@ public class SniHandler extends ByteToMessageDecoder implements ChannelOutboundH
return; return;
} }
// increase readerIndex and try again. // increase readerIndex and try again.
readerIndex += len; in.skipBytes(len);
continue; continue;
case SslUtils.SSL_CONTENT_TYPE_HANDSHAKE: case SslUtils.SSL_CONTENT_TYPE_HANDSHAKE:
int majorVersion = in.getUnsignedByte(readerIndex + 1); final int majorVersion = in.getUnsignedByte(readerIndex + 1);
// SSLv3 or TLS // SSLv3 or TLS
if (majorVersion == 3) { if (majorVersion == 3) {
int packetLength = in.getUnsignedShort(readerIndex + 3) final int packetLength = in.getUnsignedShort(readerIndex + 3) +
+ SslUtils.SSL_RECORD_HEADER_LENGTH; SslUtils.SSL_RECORD_HEADER_LENGTH;
if (in.readableBytes() < packetLength) { if (readableBytes < packetLength) {
// client hello incomplete try again to decode once more data is ready. // client hello incomplete; try again to decode once more data is ready.
return; return;
} }
// See https://tools.ietf.org/html/rfc5246#section-7.4.1.2 // See https://tools.ietf.org/html/rfc5246#section-7.4.1.2
// //
// Decode the ssl client hello packet. // Decode the ssl client hello packet.
@ -171,35 +178,67 @@ public class SniHandler extends ByteToMessageDecoder implements ChannelOutboundH
// }; // };
// } ClientHello; // } ClientHello;
// //
final int endOffset = readerIndex + packetLength;
int offset = readerIndex + 43; int offset = readerIndex + 43;
int sessionIdLength = in.getUnsignedByte(offset); if (endOffset - offset < 6) {
break loop;
}
final int sessionIdLength = in.getUnsignedByte(offset);
offset += sessionIdLength + 1; offset += sessionIdLength + 1;
int cipherSuitesLength = in.getUnsignedShort(offset); final int cipherSuitesLength = in.getUnsignedShort(offset);
offset += cipherSuitesLength + 2; offset += cipherSuitesLength + 2;
int compressionMethodLength = in.getUnsignedByte(offset); final int compressionMethodLength = in.getUnsignedByte(offset);
offset += compressionMethodLength + 1; offset += compressionMethodLength + 1;
int extensionsLength = in.getUnsignedShort(offset); final int extensionsLength = in.getUnsignedShort(offset);
offset += 2; offset += 2;
int extensionsLimit = offset + extensionsLength; final int extensionsLimit = offset + extensionsLength;
while (offset < extensionsLimit) { if (extensionsLimit > endOffset) {
int extensionType = in.getUnsignedShort(offset); // Extensions should never exceed the record boundary.
break loop;
}
for (;;) {
if (extensionsLimit - offset < 4) {
break loop;
}
final int extensionType = in.getUnsignedShort(offset);
offset += 2; offset += 2;
int extensionLength = in.getUnsignedShort(offset); final int extensionLength = in.getUnsignedShort(offset);
offset += 2; offset += 2;
if (extensionsLimit - offset < extensionLength) {
break loop;
}
// SNI // SNI
// See https://tools.ietf.org/html/rfc6066#page-6 // See https://tools.ietf.org/html/rfc6066#page-6
if (extensionType == 0) { if (extensionType == 0) {
int serverNameType = in.getUnsignedByte(offset + 2); offset += 2;
if (extensionsLimit - offset < 3) {
break loop;
}
final int serverNameType = in.getUnsignedByte(offset);
offset++;
if (serverNameType == 0) { if (serverNameType == 0) {
int serverNameLength = in.getUnsignedShort(offset + 3); final int serverNameLength = in.getUnsignedShort(offset);
String hostname = in.toString(offset + 5, serverNameLength, offset += 2;
if (extensionsLimit - offset < serverNameLength) {
break loop;
}
final String hostname = in.toString(offset, serverNameLength,
CharsetUtil.UTF_8); CharsetUtil.UTF_8);
select(ctx, IDN.toASCII(hostname, select(ctx, IDN.toASCII(hostname,