Skip to content

Commit

Permalink
Pre-decompressed DNS record RData that may contain compression pointe…
Browse files Browse the repository at this point in the history
…rs (netty#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 netty#8962
  • Loading branch information
qeesung authored and normanmaurer committed Jul 2, 2019
1 parent deea51e commit 18e4121
Show file tree
Hide file tree
Showing 4 changed files with 216 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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));
}
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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) {
Expand Down
132 changes: 132 additions & 0 deletions codec-dns/src/main/java/io/netty/handler/codec/dns/DnsCodecUtil.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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]
Expand Down

0 comments on commit 18e4121

Please sign in to comment.