From 18e412195256190f3d4a1d88c0f44608d4fce6ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A7=A6=E4=B8=96=E6=88=90?= Date: Wed, 3 Jul 2019 01:38:51 +0800 Subject: [PATCH] Pre-decompressed DNS record RData that may contain compression pointers (#9311) Motivation: When decoding DnsRecord, if the record contains compression pointers, and not all compression pointers are decompressed, but part of the pointers are decompressed. Then when encoding the record, the compressed pointer will point to the wrong location, resulting in bad label problem. Modification: Pre-decompressed record RData that may contain compression pointers. Result: Fixes #8962 --- .../codec/dns/DefaultDnsRecordDecoder.java | 72 +--------- .../codec/dns/DefaultDnsRecordEncoder.java | 23 +-- .../netty/handler/codec/dns/DnsCodecUtil.java | 132 ++++++++++++++++++ .../dns/DefaultDnsRecordDecoderTest.java | 78 ++++++++++- 4 files changed, 216 insertions(+), 89 deletions(-) create mode 100644 codec-dns/src/main/java/io/netty/handler/codec/dns/DnsCodecUtil.java diff --git a/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoder.java b/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoder.java index b4e50ff402ed..e61d46cc204a 100644 --- a/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoder.java +++ b/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoder.java @@ -16,8 +16,6 @@ package io.netty.handler.codec.dns; import io.netty.buffer.ByteBuf; -import io.netty.handler.codec.CorruptedFrameException; -import io.netty.util.CharsetUtil; import io.netty.util.internal.UnstableApi; /** @@ -98,6 +96,11 @@ protected DnsRecord decodeRecord( return new DefaultDnsPtrRecord( name, dnsClass, timeToLive, decodeName0(in.duplicate().setIndex(offset, offset + length))); } + if (type == DnsRecordType.CNAME || type == DnsRecordType.NS) { + return new DefaultDnsRawRecord(name, type, dnsClass, timeToLive, + DnsCodecUtil.decompressDomainName( + in.duplicate().setIndex(offset, offset + length))); + } return new DefaultDnsRawRecord( name, type, dnsClass, timeToLive, in.retainedDuplicate().setIndex(offset, offset + length)); } @@ -123,69 +126,6 @@ protected String decodeName0(ByteBuf in) { * @return the domain name for an entry */ public static String decodeName(ByteBuf in) { - int position = -1; - int checked = 0; - final int end = in.writerIndex(); - final int readable = in.readableBytes(); - - // Looking at the spec we should always have at least enough readable bytes to read a byte here but it seems - // some servers do not respect this for empty names. So just workaround this and return an empty name in this - // case. - // - // See: - // - https://github.com/netty/netty/issues/5014 - // - https://www.ietf.org/rfc/rfc1035.txt , Section 3.1 - if (readable == 0) { - return ROOT; - } - - final StringBuilder name = new StringBuilder(readable << 1); - while (in.isReadable()) { - final int len = in.readUnsignedByte(); - final boolean pointer = (len & 0xc0) == 0xc0; - if (pointer) { - if (position == -1) { - position = in.readerIndex() + 1; - } - - if (!in.isReadable()) { - throw new CorruptedFrameException("truncated pointer in a name"); - } - - final int next = (len & 0x3f) << 8 | in.readUnsignedByte(); - if (next >= end) { - throw new CorruptedFrameException("name has an out-of-range pointer"); - } - in.readerIndex(next); - - // check for loops - checked += 2; - if (checked >= end) { - throw new CorruptedFrameException("name contains a loop."); - } - } else if (len != 0) { - if (!in.isReadable(len)) { - throw new CorruptedFrameException("truncated label in a name"); - } - name.append(in.toString(in.readerIndex(), len, CharsetUtil.UTF_8)).append('.'); - in.skipBytes(len); - } else { // len == 0 - break; - } - } - - if (position != -1) { - in.readerIndex(position); - } - - if (name.length() == 0) { - return ROOT; - } - - if (name.charAt(name.length() - 1) != '.') { - name.append('.'); - } - - return name.toString(); + return DnsCodecUtil.decodeDomainName(in); } } diff --git a/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoder.java b/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoder.java index 48f60bcc9517..45914ea06560 100644 --- a/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoder.java +++ b/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoder.java @@ -16,14 +16,11 @@ package io.netty.handler.codec.dns; import io.netty.buffer.ByteBuf; -import io.netty.buffer.ByteBufUtil; import io.netty.channel.socket.InternetProtocolFamily; import io.netty.handler.codec.UnsupportedMessageTypeException; import io.netty.util.internal.StringUtil; import io.netty.util.internal.UnstableApi; -import static io.netty.handler.codec.dns.DefaultDnsRecordDecoder.ROOT; - /** * The default {@link DnsRecordEncoder} implementation. * @@ -141,25 +138,7 @@ private void encodeRawRecord(DnsRawRecord record, ByteBuf out) throws Exception } protected void encodeName(String name, ByteBuf buf) throws Exception { - if (ROOT.equals(name)) { - // Root domain - buf.writeByte(0); - return; - } - - final String[] labels = name.split("\\."); - for (String label : labels) { - final int labelLen = label.length(); - if (labelLen == 0) { - // zero-length label means the end of the name. - break; - } - - buf.writeByte(labelLen); - ByteBufUtil.writeAscii(buf, label); - } - - buf.writeByte(0); // marks end of name field + DnsCodecUtil.encodeDomainName(name, buf); } private static byte padWithZeros(byte b, int lowOrderBitsToPreserve) { diff --git a/codec-dns/src/main/java/io/netty/handler/codec/dns/DnsCodecUtil.java b/codec-dns/src/main/java/io/netty/handler/codec/dns/DnsCodecUtil.java new file mode 100644 index 000000000000..8804cf748e4f --- /dev/null +++ b/codec-dns/src/main/java/io/netty/handler/codec/dns/DnsCodecUtil.java @@ -0,0 +1,132 @@ +/* + * Copyright 2019 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.codec.dns; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufUtil; +import io.netty.buffer.Unpooled; +import io.netty.handler.codec.CorruptedFrameException; +import io.netty.util.CharsetUtil; + +import static io.netty.handler.codec.dns.DefaultDnsRecordDecoder.*; + +final class DnsCodecUtil { + private DnsCodecUtil() { + // Util class + } + + static void encodeDomainName(String name, ByteBuf buf) { + if (ROOT.equals(name)) { + // Root domain + buf.writeByte(0); + return; + } + + final String[] labels = name.split("\\."); + for (String label : labels) { + final int labelLen = label.length(); + if (labelLen == 0) { + // zero-length label means the end of the name. + break; + } + + buf.writeByte(labelLen); + ByteBufUtil.writeAscii(buf, label); + } + + buf.writeByte(0); // marks end of name field + } + + static String decodeDomainName(ByteBuf in) { + int position = -1; + int checked = 0; + final int end = in.writerIndex(); + final int readable = in.readableBytes(); + + // Looking at the spec we should always have at least enough readable bytes to read a byte here but it seems + // some servers do not respect this for empty names. So just workaround this and return an empty name in this + // case. + // + // See: + // - https://github.com/netty/netty/issues/5014 + // - https://www.ietf.org/rfc/rfc1035.txt , Section 3.1 + if (readable == 0) { + return ROOT; + } + + final StringBuilder name = new StringBuilder(readable << 1); + while (in.isReadable()) { + final int len = in.readUnsignedByte(); + final boolean pointer = (len & 0xc0) == 0xc0; + if (pointer) { + if (position == -1) { + position = in.readerIndex() + 1; + } + + if (!in.isReadable()) { + throw new CorruptedFrameException("truncated pointer in a name"); + } + + final int next = (len & 0x3f) << 8 | in.readUnsignedByte(); + if (next >= end) { + throw new CorruptedFrameException("name has an out-of-range pointer"); + } + in.readerIndex(next); + + // check for loops + checked += 2; + if (checked >= end) { + throw new CorruptedFrameException("name contains a loop."); + } + } else if (len != 0) { + if (!in.isReadable(len)) { + throw new CorruptedFrameException("truncated label in a name"); + } + name.append(in.toString(in.readerIndex(), len, CharsetUtil.UTF_8)).append('.'); + in.skipBytes(len); + } else { // len == 0 + break; + } + } + + if (position != -1) { + in.readerIndex(position); + } + + if (name.length() == 0) { + return ROOT; + } + + if (name.charAt(name.length() - 1) != '.') { + name.append('.'); + } + + return name.toString(); + } + + /** + * Decompress pointer data. + * @param compression comporession data + * @return decompressed data + */ + static ByteBuf decompressDomainName(ByteBuf compression) { + String domainName = decodeDomainName(compression); + ByteBuf result = compression.alloc().buffer(domainName.length() << 1); + encodeDomainName(domainName, result); + return result; + } +} diff --git a/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoderTest.java b/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoderTest.java index 6de6ce5d7246..a90acaa2833f 100644 --- a/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoderTest.java +++ b/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoderTest.java @@ -16,10 +16,11 @@ package io.netty.handler.codec.dns; import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufUtil; import io.netty.buffer.Unpooled; import org.junit.Test; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; public class DefaultDnsRecordDecoderTest { @@ -89,6 +90,81 @@ public void testDecodePtrRecord() throws Exception { } } + @Test + public void testdecompressCompressPointer() { + byte[] compressionPointer = { + 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0, + (byte) 0xC0, 0 + }; + ByteBuf buffer = Unpooled.wrappedBuffer(compressionPointer); + ByteBuf uncompressed = null; + try { + uncompressed = DnsCodecUtil.decompressDomainName(buffer.duplicate().setIndex(10, 12)); + assertEquals(0, ByteBufUtil.compare(buffer.duplicate().setIndex(0, 10), uncompressed)); + } finally { + buffer.release(); + if (uncompressed != null) { + uncompressed.release(); + } + } + } + + @Test + public void testdecompressNestedCompressionPointer() { + byte[] nestedCompressionPointer = { + 6, 'g', 'i', 't', 'h', 'u', 'b', 2, 'i', 'o', 0, // github.io + 5, 'n', 'e', 't', 't', 'y', (byte) 0xC0, 0, // netty.github.io + (byte) 0xC0, 11, // netty.github.io + }; + ByteBuf buffer = Unpooled.wrappedBuffer(nestedCompressionPointer); + ByteBuf uncompressed = null; + try { + uncompressed = DnsCodecUtil.decompressDomainName(buffer.duplicate().setIndex(19, 21)); + assertEquals(0, ByteBufUtil.compare( + Unpooled.wrappedBuffer(new byte[] { + 5, 'n', 'e', 't', 't', 'y', 6, 'g', 'i', 't', 'h', 'u', 'b', 2, 'i', 'o', 0 + }), uncompressed)); + } finally { + buffer.release(); + if (uncompressed != null) { + uncompressed.release(); + } + } + } + + @Test + public void testDecodeCompressionRDataPointer() throws Exception { + DefaultDnsRecordDecoder decoder = new DefaultDnsRecordDecoder(); + byte[] compressionPointer = { + 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0, + (byte) 0xC0, 0 + }; + ByteBuf buffer = Unpooled.wrappedBuffer(compressionPointer); + DefaultDnsRawRecord cnameRecord = null; + DefaultDnsRawRecord nsRecord = null; + try { + cnameRecord = (DefaultDnsRawRecord) decoder.decodeRecord( + "netty.github.io", DnsRecordType.CNAME, DnsRecord.CLASS_IN, 60, buffer, 10, 2); + assertEquals("The rdata of CNAME-type record should be decompressed in advance", + 0, ByteBufUtil.compare(buffer.duplicate().setIndex(0, 10), cnameRecord.content())); + assertEquals("netty.io.", DnsCodecUtil.decodeDomainName(cnameRecord.content())); + nsRecord = (DefaultDnsRawRecord) decoder.decodeRecord( + "netty.github.io", DnsRecordType.NS, DnsRecord.CLASS_IN, 60, buffer, 10, 2); + assertEquals("The rdata of NS-type record should be decompressed in advance", + 0, ByteBufUtil.compare(buffer.duplicate().setIndex(0, 10), nsRecord.content())); + assertEquals("netty.io.", DnsCodecUtil.decodeDomainName(nsRecord.content())); + } finally { + buffer.release(); + if (cnameRecord != null) { + cnameRecord.release(); + } + + if (nsRecord != null) { + nsRecord.release(); + } + } + } + @Test public void testDecodeMessageCompression() throws Exception { // See https://www.ietf.org/rfc/rfc1035 [4.1.4. Message compression]