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/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 @@ public class DefaultDnsRecordDecoder implements DnsRecordDecoder {
             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 @@ public class DefaultDnsRecordDecoder implements DnsRecordDecoder {
      * @return the domain name for an entry
      */
     public static String decodeName(ByteBuf in) {
-        int position = -1;
+        return DnsCodecUtil.decodeDomainName(in);
-        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();
     }
 }

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 @@ public class DefaultDnsRecordEncoder implements DnsRecordEncoder {
     }
 
     protected void encodeName(String name, ByteBuf buf) throws Exception {
-        if (ROOT.equals(name)) {
+        DnsCodecUtil.encodeDomainName(name, buf);
-            // 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
     }
 
     private static byte padWithZeros(byte b, int lowOrderBitsToPreserve) {

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;
+    }
+}

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 class DefaultDnsRecordDecoderTest {
         }
     }
 
+    @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]