DNS RR encodes 8-bit rname incorrectly

[arnout]

Brief description

DNS should support 8-bit characters in names. However, DNSRR does not convert a correctly UTF-8 encoded rname correctly into a DNS path.

Environment

• Scapy version: 5c9a7789

How to reproduce

Create a DNSRR with an 8-bit character in the rrname (e.g. a valid non-ASCII UTF-8 character).

bytes(DNSRR(rrname='a\u0080b.'))

Actual result

b'a\xc2\x80b.\x00\x01\x00\x01\x00\x00\x00\x00\x00\x00'
  ^ Not correctly path/length encoded

Expected result

b'\x04a\xc2\x80b\x00\x00\x01\x00\x01\x00\x00\x00\x00\x00\x00'
       ^^^^^^^^ UTF-8 encoding of U+0080
  ^^^^ 4 bytes in path component

Proposed fix

I believe the following patch solves the issue.

diff --git a/scapy/layers/dns.py b/scapy/layers/dns.py
index 48453867..31b897b4 100755
--- a/scapy/layers/dns.py
+++ b/scapy/layers/dns.py
@@ -194,7 +194,7 @@ class DNSStrField(StrField):
         return x
 
     def i2m(self, pkt, x):
-        if any((orb(y) >= 0xc0) for y in x):
+        if b'.' not in x and x[-1] == b"\x00":
             # The value has already been processed. Do not process it again
             return x

The check was introduced by fdae9df "DNS improvements & compression". Compression is indeed indicated by a length byte with a value >= 0xc0. However, such characters may actually occur in the name so it's not a good check. Since the conversion removes all dots (path splitting), it is a much more reliable marker.

However, I think the check might even be removed entirely. AFAIU, the result of i2m is not saved.

[gpotter2]

Thanks for the report, your fix seems better to me.

We have the check in order to allow wrongly built pointers not be fixed. It’s useful when building packets that can, for instance, trigger a DNS decompression loop

[gpotter2]

Could you check if #1968 fixes it for you ?
I had to modify the code a bit to handle strings ending with a pointer (no ending \x00)

[arnout]

There are still cases where it doesn't work correctly, but I'm not sure if it's supposed to be supported. When the rrname doesn't have the final dot (so it has no dots at all), a 2-byte UTF-8 character at the end will be misdetected.

>>> raw(DNSRR(rrname='a\u0080'))
b'a\xc2\x80\x00\x01\x00\x01\x00\x00\x00\x00\x00\x00'
>>> raw(DNSRR(rrname='a\u0080.'))
b'\x03a\xc2\x80\x00\x00\x01\x00\x01\x00\x00\x00\x00\x00\x00'

However, I don't see how it would ever be possible to distinguish those situations.

Would it be possible to instead make sure that the rrname gets a final dot by adding an any2i() method to StrField that adds a final dot?

Note BTW that I'm abusing the lack of any2m to generate incorrect labels, e.g. `DNSRR(rrname=b'\x04ab\x00') to test against buffer overflows in a DNS implementation, so I do like the current situation :-).

[gpotter2]

The issue is that we convert Unicode to bytes before processing anything,

>>> raw(DNSRR(rrname='a\u0080'))
b'a\xc2\x80\x00\x01\x00\x01\x00\x00\x00\x00\x00\x00'
>>> raw(DNSRR(rrname=b'a\x80'))
b'\x02a\x80\x00\x00\x01\x00\x01\x00\x00\x00\x00\x00\x00'
>>> 'a\u0080'.encode()
b'a\xc2\x80'

We use a generic conversion function to get Python 2-3 compatibility. We could add the dot for strings only, so that it still allows to make custom bytes, and works with strings

[gpotter2]

@arnout I’ve updated #1968 to match with your proposition. (See the added tests: it should handle Unicode)

What do you think ?

[guedou]

Could you point out to the RFC that specify this encoding?

[arnout]

@guedou:

Could you point out to the RFC that specify this encoding?

https://tools.ietf.org/html/rfc6762#section-16

AFAIU the DNS RFCs don't specify anything about encoding, which would imply that they should be character set agnostic. The label encoding allows any character, it only limits label length (between the dots) to 63 bytes.

[guedou]

Thanks!
Do you have live captures that show that kind of encoding?

[arnout]

Do you have live captures that show that kind of encoding?

No, I'm testing an mDNS implementation and I use scapy to throw valid and invalid DNS packets at it. But I believe that it is possible to set avahi-daemon's host-name setting to any UTF-8 encoded string.

[guedou]

I am not sure that this encoding is valid. I think that the correct way to encode non-ascii character is defined in RFC5891. We should implement this in Scapy.

[gpotter2]

Encoding non ASCII -> utf8 is apparently called IDNA 2008

Apparently you can perform pretty much all DNS-related encoding witching Python:

• "test".encode("punycode")
• "test".encode("idna")

[arnout]

IDNA (RFC5891) is for unicast DNS. AFAIU the labels are still transmitted in punycode, so 7-bit ASCII. I would think that transforming from/to punycode is not scapy's responsibility.

mDNS, however, uses the same packet format as unicast DNS, but specifies UTF-8.

Except if I missed some RFC that overrides RFC 6762.

[guedou]

Thanks, I now understand what you want to do. I think that Scapy default behavior should be to convert UTF-8 to Unicode (like done in regular DNS) instead of packing it (like done in mDNS). You could try to implement this behavior with a conf.mdns_encoding flag.