From a0fbd9ab5612756b3bee9c6ae0e37cb865cc4651 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 11 Jun 2020 17:38:51 +1200 Subject: [PATCH 1/6] CVE-2020-10745: pytests: hand-rolled invalid dns/nbt packet tests The client libraries don't allow us to make packets that are broken in certain ways, so we need to construct them as byte strings. These tests all fail at present, proving the server is rendered unresponsive, which is the crux of CVE-2020-10745. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 Signed-off-by: Douglas Bagnall (backported from patch for master) [abartlet@samba.org: f"" strings are not in Python 3.4 and bytes cannot be formatted in python 3.4] [gary@catalyst.net.nz - cmocka tests not ported as cmocka is not in 4.5 - fixed tests to run in python 2.7 - knownfail.d does not exist in samba 4.5 ] Signed-off-by: Andrew Bartlett --- python/samba/tests/dns_packet.py | 219 +++++++++++++++++++++++++++++++ selftest/knownfail | 7 + source4/selftest/tests.py | 10 ++ 3 files changed, 236 insertions(+) create mode 100644 python/samba/tests/dns_packet.py diff --git a/python/samba/tests/dns_packet.py b/python/samba/tests/dns_packet.py new file mode 100644 index 00000000000..c8ff2f96926 --- /dev/null +++ b/python/samba/tests/dns_packet.py @@ -0,0 +1,219 @@ +# Tests of malformed DNS packets +# Copyright (C) Catalyst.NET ltd +# +# written by Douglas Bagnall +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Sanity tests for DNS and NBT server parsing. + +We don't use a proper client library so we can make improper packets. +""" + +import os +import struct +import socket +import select +from samba.dcerpc import dns, nbt + +from samba.tests import TestCase + +def _msg_id(): + while True: + for i in range(1, 0xffff): + yield i + + +SERVER = os.environ['SERVER_IP'] +SERVER_NAME = os.environ['SERVER'] + "." + os.environ['REALM'] +TIMEOUT = 0.5 + + +def encode_netbios_bytes(chars): + """Even RFC 1002 uses distancing quotes when calling this "compression".""" + out = [] + chars = (chars + b' ')[:16] + for c in chars: + out.append((ord(c) >> 4) + 65) + out.append((ord(c) & 15) + 65) + return "".join(chr(x) for x in out) + + +class TestDnsPacketBase(TestCase): + msg_id = _msg_id() + + def tearDown(self): + # we need to ensure the DNS server is responsive before + # continuing. + for i in range(40): + ok = self._known_good_query() + if ok: + return + print("the server is STILL unresponsive after %d seconds" % (40 * TIMEOUT)) + + def decode_reply(self, data): + header = data[:12] + id, flags, n_q, n_a, n_rec, n_exta = struct.unpack('!6H', + header) + return { + 'rcode': flags & 0xf + } + + def construct_query(self, names): + """Create a query packet containing one query record. + + *names* is either a single string name in the usual dotted + form, or a list of names. In the latter case, each name can + be a dotted string or a list of byte components, which allows + dots in components. Where I say list, I mean non-string + iterable. + + Examples: + + # these 3 are all the same + "example.com" + ["example.com"] + [[b"example", b"com"]] + + # this is three names in the same request + ["example.com", + [b"example", b"com", b"..!"], + (b"first component", b" 2nd component")] + """ + header = struct.pack('!6H', + next(self.msg_id), + 0x0100, # query, with recursion + len(names), # number of queries + 0x0000, # no answers + 0x0000, # no records + 0x0000, # no extra records + ) + tail = struct.pack('!BHH', + 0x00, # root node + self.qtype, + 0x0001, # class IN-ternet + ) + encoded_bits = [] + for name in names: + if isinstance(name, str): + bits = name.split(b'.') + else: + bits = name + + for b in bits: + encoded_bits.append(chr(len(b)) + b) + encoded_bits.append(tail) + + return header + b''.join(encoded_bits) + + def _test_query(self, names=(), expected_rcode=None): + + if isinstance(names, str): + names = [names] + + packet = self.construct_query(names) + s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + s.sendto(packet, self.server) + r, _, _ = select.select([s], [], [], TIMEOUT) + s.close() + # It is reasonable to not reply to these packets (Windows + # doesn't), but it is not reasonable to render the server + # unresponsive. + if r != [s]: + ok = self._known_good_query() + self.assertTrue(ok, "the server is unresponsive") + + def _known_good_query(self): + if self.server[1] == 53: + name = SERVER_NAME + expected_rcode = dns.DNS_RCODE_OK + else: + name = [encode_netbios_bytes(b'nxdomain'), b'nxdomain'] + expected_rcode = nbt.NBT_RCODE_NAM + + packet = self.construct_query([name]) + s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + s.sendto(packet, self.server) + r, _, _ = select.select([s], [], [], TIMEOUT) + if r != [s]: + s.close() + return False + + data, addr = s.recvfrom(4096) + s.close() + rcode = self.decode_reply(data)['rcode'] + + return expected_rcode == rcode + + +class TestDnsPackets(TestDnsPacketBase): + server = (SERVER, 53) + qtype = 1 # dns type A + + def _test_many_repeated_components(self, label, n, expected_rcode=None): + name = [label] * n + self._test_query([name], + expected_rcode=expected_rcode) + + def test_127_very_dotty_components(self): + label = b'.' * 63 + self._test_many_repeated_components(label, 127) + + def test_127_half_dotty_components(self): + label = b'x.' * 31 + b'x' + self._test_many_repeated_components(label, 127) + + def test_known_good(self): + ok = self._known_good_query() + self.assertTrue(ok, "the server is unresponsive") + +class TestNbtPackets(TestDnsPacketBase): + server = (SERVER, 137) + qtype = 0x20 # NBT_QTYPE_NETBIOS + + def _test_nbt_encode_query(self, names, *args, **kwargs): + if isinstance(names, str): + names = [names] + + nbt_names = [] + for name in names: + if isinstance(name, str): + bits = name.encode('utf8').split(b'.') + else: + bits = name + + encoded = [encode_netbios_bytes(bits[0])] + encoded.extend(bits[1:]) + nbt_names.append(encoded) + + self._test_query(nbt_names, *args, **kwargs) + + def _test_many_repeated_components(self, label, n, expected_rcode=None): + name = [label] * n + name[0] = encode_netbios_bytes(label) + self._test_query([name], + expected_rcode=expected_rcode) + + def test_127_very_dotty_components(self): + label = b'.' * 63 + self._test_many_repeated_components(label, 127) + + def test_127_half_dotty_components(self): + label = b'x.' * 31 + b'x' + self._test_many_repeated_components(label, 127) + + def test_known_good(self): + ok = self._known_good_query() + self.assertTrue(ok, "the server is unresponsive") + diff --git a/selftest/knownfail b/selftest/knownfail index 4cd6a068c5e..e2617ba2928 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -293,3 +293,10 @@ ^samba4.ldap.vlv.python.*test_vlv_paged ^samba4.asq.python.*asq_vlv_paged + +# +# +samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_very_dotty_components +samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_half_dotty_components +samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components +samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_half_dotty_components diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index ee76c561b0e..0e0da4ec47d 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -363,6 +363,16 @@ plantestsuite_loadlist("samba.tests.dns_forwarder", "fl2003dc:local", [python, o plantestsuite_loadlist("samba.tests.dns_tkey", "fl2008r2dc", [python, os.path.join(srcdir(), "python/samba/tests/dns_tkey.py"), '$SERVER', '$SERVER_IP', '--machine-pass', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) +plantestsuite_loadlist("samba.tests.dns_packet", + "ad_dc", + [python, + '-msamba.subunit.run', + '$LOADLIST', + "$LISTOPT" + "samba.tests.dns_packet" + ]) + + for t in smbtorture4_testsuites("dns_internal."): plansmbtorture4testsuite(t, "ad_dc_ntvfs:local", '//$SERVER/whavever') -- 2.17.1 From 6366a818a8e3a650f37699e2a57c744dc52d81a7 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Sat, 25 Apr 2020 11:02:08 +1200 Subject: [PATCH 2/6] CVE-2020-10745: ndr_dns: move ndr_push_dns_string core into sharable function This is because ndr_nbt.c does almost exactly the same thing with almost exactly the same code, and they both do it wrong. Soon they will both be using the better version that this will become. Though in this patch we just move the code, not fix it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 Signed-off-by: Douglas Bagnall (backported from patch for master) [gary@catalyst.net.nz - manually add changes to ndr_push_string_list signature from commit 70923b7521786d59b76e651d566bbd61fea024cc ] --- librpc/ndr/ndr_dns.c | 79 +++------------------------------- librpc/ndr/ndr_dns_utils.c | 88 ++++++++++++++++++++++++++++++++++++++ librpc/ndr/ndr_dns_utils.h | 5 +++ librpc/wscript_build | 2 +- 4 files changed, 99 insertions(+), 75 deletions(-) create mode 100644 librpc/ndr/ndr_dns_utils.c create mode 100644 librpc/ndr/ndr_dns_utils.h diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c index 4e4c556e20d..85525ed301e 100644 --- a/librpc/ndr/ndr_dns.c +++ b/librpc/ndr/ndr_dns.c @@ -33,6 +33,7 @@ #include "librpc/gen_ndr/ndr_dnsp.h" #include "system/locale.h" #include "lib/util/util_net.h" +#include "ndr_dns_utils.h" /* don't allow an unlimited number of name components */ #define MAX_COMPONENTS 128 @@ -159,80 +160,10 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr, int ndr_flags, const char *s) { - if (!(ndr_flags & NDR_SCALARS)) { - return NDR_ERR_SUCCESS; - } - - while (s && *s) { - enum ndr_err_code ndr_err; - char *compname; - size_t complen; - uint32_t offset; - - if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { - /* see if we have pushed the remaining string already, - * if so we use a label pointer to this string - */ - ndr_err = ndr_token_retrieve_cmp_fn(&ndr->dns_string_list, s, - &offset, - (comparison_fn_t)strcmp, - false); - if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - uint8_t b[2]; - - if (offset > 0x3FFF) { - return ndr_push_error(ndr, NDR_ERR_STRING, - "offset for dns string " \ - "label pointer " \ - "%u[%08X] > 0x00003FFF", - offset, offset); - } - - b[0] = 0xC0 | (offset>>8); - b[1] = (offset & 0xFF); - - return ndr_push_bytes(ndr, b, 2); - } - } - - complen = strcspn(s, "."); - - /* we need to make sure the length fits into 6 bytes */ - if (complen > 0x3F) { - return ndr_push_error(ndr, NDR_ERR_STRING, - "component length %u[%08X] > " \ - "0x0000003F", - (unsigned)complen, - (unsigned)complen); - } - - compname = talloc_asprintf(ndr, "%c%*.*s", - (unsigned char)complen, - (unsigned char)complen, - (unsigned char)complen, s); - NDR_ERR_HAVE_NO_MEMORY(compname); - - /* remember the current component + the rest of the string - * so it can be reused later - */ - if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { - NDR_CHECK(ndr_token_store(ndr, &ndr->dns_string_list, s, - ndr->offset)); - } - - /* push just this component into the blob */ - NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, - complen+1)); - talloc_free(compname); - - s += complen; - if (*s == '.') s++; - } - - /* if we reach the end of the string and have pushed the last component - * without using a label pointer, we need to terminate the string - */ - return ndr_push_bytes(ndr, (const uint8_t *)"", 1); + return ndr_push_dns_string_list(ndr, + &ndr->dns_string_list, + ndr_flags, + s); } _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r) diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c new file mode 100644 index 00000000000..9a0f50ad6b0 --- /dev/null +++ b/librpc/ndr/ndr_dns_utils.c @@ -0,0 +1,88 @@ +#include "includes.h" +#include "../librpc/ndr/libndr.h" +#include "ndr_dns_utils.h" + + +/** + push a dns/nbt string list to the wire +*/ +enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, + struct ndr_token_list **string_list, + int ndr_flags, + const char *s) +{ + if (!(ndr_flags & NDR_SCALARS)) { + return NDR_ERR_SUCCESS; + } + + while (s && *s) { + enum ndr_err_code ndr_err; + char *compname; + size_t complen; + uint32_t offset; + + if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { + /* see if we have pushed the remaining string already, + * if so we use a label pointer to this string + */ + ndr_err = ndr_token_retrieve_cmp_fn(string_list, s, + &offset, + (comparison_fn_t)strcmp, + false); + if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + uint8_t b[2]; + + if (offset > 0x3FFF) { + return ndr_push_error(ndr, NDR_ERR_STRING, + "offset for dns string " \ + "label pointer " \ + "%u[%08X] > 0x00003FFF", + offset, offset); + } + + b[0] = 0xC0 | (offset>>8); + b[1] = (offset & 0xFF); + + return ndr_push_bytes(ndr, b, 2); + } + } + + complen = strcspn(s, "."); + + /* we need to make sure the length fits into 6 bytes */ + if (complen > 0x3F) { + return ndr_push_error(ndr, NDR_ERR_STRING, + "component length %u[%08X] > " \ + "0x0000003F", + (unsigned)complen, + (unsigned)complen); + } + + compname = talloc_asprintf(ndr, "%c%*.*s", + (unsigned char)complen, + (unsigned char)complen, + (unsigned char)complen, s); + NDR_ERR_HAVE_NO_MEMORY(compname); + + /* remember the current component + the rest of the string + * so it can be reused later + */ + if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { + NDR_CHECK(ndr_token_store(ndr, string_list, s, + ndr->offset)); + } + + /* push just this component into the blob */ + NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, + complen+1)); + talloc_free(compname); + + s += complen; + if (*s == '.') s++; + } + + /* if we reach the end of the string and have pushed the last component + * without using a label pointer, we need to terminate the string + */ + return ndr_push_bytes(ndr, (const uint8_t *)"", 1); +} diff --git a/librpc/ndr/ndr_dns_utils.h b/librpc/ndr/ndr_dns_utils.h new file mode 100644 index 00000000000..9e80db507bc --- /dev/null +++ b/librpc/ndr/ndr_dns_utils.h @@ -0,0 +1,5 @@ + +enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, + struct ndr_token_list **string_list, + int ndr_flags, + const char *s); diff --git a/librpc/wscript_build b/librpc/wscript_build index d8fbf5d5005..6db272345f2 100644 --- a/librpc/wscript_build +++ b/librpc/wscript_build @@ -31,7 +31,7 @@ bld.SAMBA_SUBSYSTEM('NDR_DNSSERVER', ) bld.SAMBA_SUBSYSTEM('NDR_DNS', - source='gen_ndr/ndr_dns.c ndr/ndr_dns.c', + source='gen_ndr/ndr_dns.c ndr/ndr_dns.c ndr/ndr_dns_utils.c', public_deps='ndr NDR_DNSP' ) -- 2.17.1 From 4a283fa1726a85ccff056f4f2325c00b428fdb6b Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Sat, 25 Apr 2020 11:03:30 +1200 Subject: [PATCH 3/6] CVE-2020-10745: ndr/dns_utils: correct a comment BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 Signed-off-by: Douglas Bagnall --- librpc/ndr/ndr_dns_utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c index 9a0f50ad6b0..9210ffc5ef4 100644 --- a/librpc/ndr/ndr_dns_utils.c +++ b/librpc/ndr/ndr_dns_utils.c @@ -49,7 +49,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, complen = strcspn(s, "."); - /* we need to make sure the length fits into 6 bytes */ + /* the length must fit into 6 bits (i.e. <= 63) */ if (complen > 0x3F) { return ndr_push_error(ndr, NDR_ERR_STRING, "component length %u[%08X] > " \ -- 2.17.1 From 1a6c6e63a337d2a3d79e84fe33f6150cfe35e51a Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Sat, 25 Apr 2020 11:10:18 +1200 Subject: [PATCH 4/6] CVE-2020-10745: ndr_dns: do not allow consecutive dots The empty subdomain component is reserved for the root domain, which we should only (and always) see at the end of the list. That is, we expect "example.com.", but never "example..com". BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 Signed-off-by: Douglas Bagnall --- librpc/ndr/ndr_dns_utils.c | 6 ++++++ selftest/knownfail | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c index 9210ffc5ef4..ef8e4d22230 100644 --- a/librpc/ndr/ndr_dns_utils.c +++ b/librpc/ndr/ndr_dns_utils.c @@ -58,6 +58,12 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, (unsigned)complen); } + if (complen == 0 && s[complen] == '.') { + return ndr_push_error(ndr, NDR_ERR_STRING, + "component length is 0 " + "(consecutive dots)"); + } + compname = talloc_asprintf(ndr, "%c%*.*s", (unsigned char)complen, (unsigned char)complen, diff --git a/selftest/knownfail b/selftest/knownfail index e2617ba2928..c36e254c1ba 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -296,7 +296,6 @@ # # -samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_very_dotty_components samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_half_dotty_components samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_half_dotty_components -- 2.17.1 From 5893beac13478a8ade80e45483f3d47b24ba7bb1 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 15 May 2020 00:06:08 +1200 Subject: [PATCH 5/6] CVE-2020-10745: dns_util/push: forbid names longer than 255 bytes As per RFC 1035. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 Signed-off-by: Douglas Bagnall --- librpc/ndr/ndr_dns_utils.c | 10 +++++++++- selftest/knownfail | 1 - 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c index ef8e4d22230..4a8d28ae242 100644 --- a/librpc/ndr/ndr_dns_utils.c +++ b/librpc/ndr/ndr_dns_utils.c @@ -11,6 +11,8 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, int ndr_flags, const char *s) { + const char *start = s; + if (!(ndr_flags & NDR_SCALARS)) { return NDR_ERR_SUCCESS; } @@ -84,7 +86,13 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, talloc_free(compname); s += complen; - if (*s == '.') s++; + if (*s == '.') { + s++; + } + if (s - start > 255) { + return ndr_push_error(ndr, NDR_ERR_STRING, + "name > 255 character long"); + } } /* if we reach the end of the string and have pushed the last component diff --git a/selftest/knownfail b/selftest/knownfail index c36e254c1ba..6bb290d1ea5 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -296,6 +296,5 @@ # # -samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_half_dotty_components samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_half_dotty_components -- 2.17.1 From 3460a0d065f32a83ea610b09257cdb24ffef84e7 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 15 May 2020 10:52:45 +1200 Subject: [PATCH 6/6] CVE-2020-10745: ndr/dns-utils: prepare for NBT compatibility NBT has a funny thing where it sometimes needs to send a trailing dot as part of the last component, because the string representation is a user name. In DNS, "example.com", and "example.com." are the same, both having three components ("example", "com", ""); in NBT, we want to treat them differently, with the second form having the three components ("example", "com.", ""). This retains the logic of e6e2ec0001fe3c010445e26cc0efddbc1f73416b. Also DNS compression cannot be turned off for NBT. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 Signed-off-by: Douglas Bagnall --- librpc/ndr/ndr_dns.c | 3 +- librpc/ndr/ndr_dns_utils.c | 42 +++++++++++++++++++--- librpc/ndr/ndr_dns_utils.h | 3 +- librpc/ndr/ndr_nbt.c | 72 ++++---------------------------------- librpc/wscript_build | 2 +- selftest/knownfail | 4 --- 6 files changed, 49 insertions(+), 77 deletions(-) diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c index 85525ed301e..5629f25c287 100644 --- a/librpc/ndr/ndr_dns.c +++ b/librpc/ndr/ndr_dns.c @@ -163,7 +163,8 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr, return ndr_push_dns_string_list(ndr, &ndr->dns_string_list, ndr_flags, - s); + s, + false); } _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r) diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c index 4a8d28ae242..538a0a1eb1a 100644 --- a/librpc/ndr/ndr_dns_utils.c +++ b/librpc/ndr/ndr_dns_utils.c @@ -9,9 +9,32 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, struct ndr_token_list **string_list, int ndr_flags, - const char *s) + const char *s, + bool is_nbt) { const char *start = s; + bool use_compression; + size_t max_length; + if (is_nbt) { + use_compression = true; + /* + * Max length is longer in NBT/Wins, because Windows counts + * the semi-decompressed size of the netbios name (16 bytes) + * rather than the wire size of 32, which is what you'd expect + * if it followed RFC1002 (it uses the short form in + * [MS-WINSRA]). In other words the maximum size of the + * "scope" is 237, not 221. + * + * We make the size limit slightly larger than 255 + 16, + * because the 237 scope limit is already enforced in the + * winsserver code with a specific return value; bailing out + * here would muck with that. + */ + max_length = 274; + } else { + use_compression = !(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION); + max_length = 255; + } if (!(ndr_flags & NDR_SCALARS)) { return NDR_ERR_SUCCESS; @@ -23,7 +46,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, size_t complen; uint32_t offset; - if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { + if (use_compression) { /* see if we have pushed the remaining string already, * if so we use a label pointer to this string */ @@ -66,6 +89,14 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, "(consecutive dots)"); } + if (is_nbt && s[complen] == '.' && s[complen + 1] == '\0') { + /* nbt names are sometimes usernames, and we need to + * keep a trailing dot to ensure it is byte-identical, + * (not just semantically identical given DNS + * semantics). */ + complen++; + } + compname = talloc_asprintf(ndr, "%c%*.*s", (unsigned char)complen, (unsigned char)complen, @@ -75,7 +106,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, /* remember the current component + the rest of the string * so it can be reused later */ - if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { + if (use_compression) { NDR_CHECK(ndr_token_store(ndr, string_list, s, ndr->offset)); } @@ -89,9 +120,10 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, if (*s == '.') { s++; } - if (s - start > 255) { + if (s - start > max_length) { return ndr_push_error(ndr, NDR_ERR_STRING, - "name > 255 character long"); + "name > %zu character long", + max_length); } } diff --git a/librpc/ndr/ndr_dns_utils.h b/librpc/ndr/ndr_dns_utils.h index 9e80db507bc..de41b6130c0 100644 --- a/librpc/ndr/ndr_dns_utils.h +++ b/librpc/ndr/ndr_dns_utils.h @@ -2,4 +2,5 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, struct ndr_token_list **string_list, int ndr_flags, - const char *s); + const char *s, + bool is_nbt); diff --git a/librpc/ndr/ndr_nbt.c b/librpc/ndr/ndr_nbt.c index 838f947a168..e8dd7549a53 100644 --- a/librpc/ndr/ndr_nbt.c +++ b/librpc/ndr/ndr_nbt.c @@ -25,6 +25,8 @@ #include "includes.h" #include "../libcli/nbt/libnbt.h" #include "../libcli/netlogon/netlogon.h" +#include "ndr_dns_utils.h" + /* don't allow an unlimited number of name components */ #define MAX_COMPONENTS 128 @@ -141,71 +143,11 @@ _PUBLIC_ enum ndr_err_code ndr_pull_nbt_string(struct ndr_pull *ndr, int ndr_fla */ _PUBLIC_ enum ndr_err_code ndr_push_nbt_string(struct ndr_push *ndr, int ndr_flags, const char *s) { - if (!(ndr_flags & NDR_SCALARS)) { - return NDR_ERR_SUCCESS; - } - - while (s && *s) { - enum ndr_err_code ndr_err; - char *compname; - size_t complen; - uint32_t offset; - - /* see if we have pushed the remaining string already, - * if so we use a label pointer to this string - */ - ndr_err = ndr_token_retrieve_cmp_fn(&ndr->nbt_string_list, s, &offset, (comparison_fn_t)strcmp, false); - if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - uint8_t b[2]; - - if (offset > 0x3FFF) { - return ndr_push_error(ndr, NDR_ERR_STRING, - "offset for nbt string label pointer %u[%08X] > 0x00003FFF", - offset, offset); - } - - b[0] = 0xC0 | (offset>>8); - b[1] = (offset & 0xFF); - - return ndr_push_bytes(ndr, b, 2); - } - - complen = strcspn(s, "."); - - /* we need to make sure the length fits into 6 bytes */ - if (complen > 0x3F) { - return ndr_push_error(ndr, NDR_ERR_STRING, - "component length %u[%08X] > 0x0000003F", - (unsigned)complen, (unsigned)complen); - } - - if (s[complen] == '.' && s[complen+1] == '\0') { - complen++; - } - - compname = talloc_asprintf(ndr, "%c%*.*s", - (unsigned char)complen, - (unsigned char)complen, - (unsigned char)complen, s); - NDR_ERR_HAVE_NO_MEMORY(compname); - - /* remember the current componemt + the rest of the string - * so it can be reused later - */ - NDR_CHECK(ndr_token_store(ndr, &ndr->nbt_string_list, s, ndr->offset)); - - /* push just this component into the blob */ - NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, complen+1)); - talloc_free(compname); - - s += complen; - if (*s == '.') s++; - } - - /* if we reach the end of the string and have pushed the last component - * without using a label pointer, we need to terminate the string - */ - return ndr_push_bytes(ndr, (const uint8_t *)"", 1); + return ndr_push_dns_string_list(ndr, + &ndr->dns_string_list, + ndr_flags, + s, + true); } diff --git a/librpc/wscript_build b/librpc/wscript_build index 6db272345f2..bc43a0906bb 100644 --- a/librpc/wscript_build +++ b/librpc/wscript_build @@ -401,7 +401,7 @@ bld.SAMBA_SUBSYSTEM('NDR_SCHANNEL', bld.SAMBA_LIBRARY('ndr_nbt', source='gen_ndr/ndr_nbt.c ndr/ndr_nbt.c', - public_deps='ndr NDR_NBT_BUF NDR_SECURITY', + public_deps='ndr NDR_NBT_BUF NDR_SECURITY NDR_DNS', public_headers='gen_ndr/nbt.h gen_ndr/ndr_nbt.h ndr/ndr_nbt.h', header_path=[ ('gen_ndr*', 'gen_ndr'), ('ndr*', 'ndr')], pc_files='ndr_nbt.pc', diff --git a/selftest/knownfail b/selftest/knownfail index 6bb290d1ea5..56a2961b1ae 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -294,7 +294,3 @@ ^samba4.ldap.vlv.python.*test_vlv_paged ^samba4.asq.python.*asq_vlv_paged -# -# -samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components -samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_half_dotty_components -- 2.17.1