From 6d3da1eb00a31e7b0a30415d81c1ac4508da2aad Mon Sep 17 00:00:00 2001 From: Aaron Haslett Date: Tue, 23 Oct 2018 17:25:51 +1300 Subject: [PATCH 1/4] CVE-2018-14629 dns: CNAME loop prevention using counter Count number of answers generated by internal DNS query routine and stop at 20 to match Microsoft's loop prevention mechanism. (backport to Samba 4.5) BUG: https://bugzilla.samba.org/show_bug.cgi?id=13600 Signed-off-by: Aaron Haslett Reviewed-by: Andrew Bartlett Reviewed-by: Garming Sam --- python/samba/tests/dns.py | 23 +++++++++++++++++++++++ selftest/knownfail.d/dns | 5 +++++ source4/dns_server/dns_query.c | 6 ++++++ 3 files changed, 34 insertions(+) create mode 100644 selftest/knownfail.d/dns diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index babf898afd7..102269c7156 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -896,6 +896,29 @@ class TestComplexQueries(DNSTest): self.assertEquals(response.answers[1].name, name2) self.assertEquals(response.answers[1].rdata, name3) + def test_cname_loop(self): + cname1 = "cnamelooptestrec." + self.get_dns_domain() + cname2 = "cnamelooptestrec2." + self.get_dns_domain() + cname3 = "cnamelooptestrec3." + self.get_dns_domain() + self.make_dns_update(cname1, cname2, dnsp.DNS_TYPE_CNAME) + self.make_dns_update(cname2, cname3, dnsp.DNS_TYPE_CNAME) + self.make_dns_update(cname3, cname1, dnsp.DNS_TYPE_CNAME) + + p = self.make_name_packet(dns.DNS_OPCODE_QUERY) + questions = [] + + q = self.make_name_question(cname1, + dns.DNS_QTYPE_A, + dns.DNS_QCLASS_IN) + questions.append(q) + self.finish_name_packet(p, questions) + + response = self.dns_transaction_udp(p, host=self.server_ip) + + max_recursion_depth = 20 + self.assertEquals(len(response.answers), max_recursion_depth) + + class TestInvalidQueries(DNSTest): def test_one_a_query(self): diff --git a/selftest/knownfail.d/dns b/selftest/knownfail.d/dns new file mode 100644 index 00000000000..916afc1af85 --- /dev/null +++ b/selftest/knownfail.d/dns @@ -0,0 +1,5 @@ +# +# rodc and vampire_dc require signed dns updates, so the test setup +# fails, but the test does run on fl2003dc +^samba.tests.dns.__main__.TestComplexQueries.test_cname_loop\(rodc:local\) +^samba.tests.dns.__main__.TestComplexQueries.test_cname_loop\(vampire_dc:local\) diff --git a/source4/dns_server/dns_query.c b/source4/dns_server/dns_query.c index 3e9359ee666..0c26f9f8fb5 100644 --- a/source4/dns_server/dns_query.c +++ b/source4/dns_server/dns_query.c @@ -40,6 +40,7 @@ #undef DBGC_CLASS #define DBGC_CLASS DBGC_DNS +#define MAX_Q_RECURSION_DEPTH 20 struct forwarder_string { const char *forwarder; @@ -470,6 +471,11 @@ static struct tevent_req *handle_dnsrpcrec_send( state->answers = answers; state->nsrecs = nsrecs; + if (talloc_array_length(*answers) >= MAX_Q_RECURSION_DEPTH) { + tevent_req_done(req); + return tevent_req_post(req, ev); + } + resolve_cname = ((rec->wType == DNS_TYPE_CNAME) && ((question->question_type == DNS_QTYPE_A) || (question->question_type == DNS_QTYPE_AAAA))); -- 2.11.0 From 5104d30c53be7161d03c853f480223b24115874a Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Tue, 6 Sep 2016 10:48:57 +1200 Subject: [PATCH 2/4] tests/dns_forwarder: Wait for port for 15 seconds Signed-off-by: Garming Sam Reviewed-by: Andrew Bartlett (cherry picked from commit 668e4e4a436756d73d64790fd0a7e79fa4769ffe) --- python/samba/tests/dns_forwarder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/samba/tests/dns_forwarder.py b/python/samba/tests/dns_forwarder.py index de36de9c979..1d598662a83 100644 --- a/python/samba/tests/dns_forwarder.py +++ b/python/samba/tests/dns_forwarder.py @@ -185,8 +185,8 @@ class TestDnsForwarding(DNSTest): host, str(port), id]) self.subprocesses.append(p) s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0) - for i in xrange(30): - time.sleep(0.01) + for i in xrange(300): + time.sleep(0.05) s.connect((host, port)) try: s.send('timeout 0', 0) -- 2.11.0 From c66c90469efe920d8273df041c1c46698ffb1a21 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Thu, 7 Jul 2016 16:58:27 +1200 Subject: [PATCH 3/4] tests/dns_forwarder: Check that the subprocess is still living Signed-off-by: Garming Sam Reviewed-by: Andrew Bartlett (cherry picked from commit ad3b3e978ebf0692580166f9deba0368a922362d) --- python/samba/tests/dns_forwarder.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/python/samba/tests/dns_forwarder.py b/python/samba/tests/dns_forwarder.py index 1d598662a83..bef21f6bdaf 100644 --- a/python/samba/tests/dns_forwarder.py +++ b/python/samba/tests/dns_forwarder.py @@ -193,6 +193,10 @@ class TestDnsForwarding(DNSTest): except socket.error, e: if e.errno in (errno.ECONNREFUSED, errno.EHOSTUNREACH): continue + + if p.returncode is not None: + self.fail("Toy server has managed to die already!") + return s def tearDown(self): -- 2.11.0 From dec2a328b3262b131f147c339fa17112f998ca39 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Thu, 9 Jun 2016 03:52:38 +0200 Subject: [PATCH 4/4] tests/dns_forwarder: Fail out with an assertion instead OOB error Signed-off-by: Garming Sam Reviewed-by: Andrew Bartlett Autobuild-User(master): Garming Sam Autobuild-Date(master): Tue Sep 6 15:41:54 CEST 2016 on sn-devel-144 (cherry picked from commit 451907739cc14717c12875b88fbbe63a53e9cbec) --- python/samba/tests/dns_forwarder.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/samba/tests/dns_forwarder.py b/python/samba/tests/dns_forwarder.py index bef21f6bdaf..51a86198b54 100644 --- a/python/samba/tests/dns_forwarder.py +++ b/python/samba/tests/dns_forwarder.py @@ -466,8 +466,9 @@ class TestDnsForwarding(DNSTest): try: data = ad.recv(0xffff + 2, 0) data = ndr.ndr_unpack(dns.name_packet, data) - self.assertEqual('forwarder1', data.answers[0].rdata) self.assert_dns_rcode_equals(data, dns.DNS_RCODE_OK) + self.assertEqual(len(data.answers), 1) + self.assertEqual('forwarder1', data.answers[0].rdata) except socket.timeout: self.fail("DNS server is too slow (timeout %s)" % timeout) -- 2.11.0