Attachment 14685 Details for Bug 13600 – Tested patch for 4.5

[patch] Tested patch for 4.5

CVE-2018-14629-v4-5.patch (text/plain), 6.83 KB, created by Andrew Bartlett on 2018-11-23 01:42:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2018-11-23 01:42:28 UTC

Size: 6.83 KB

patch

obsolete

>From 6d3da1eb00a31e7b0a30415d81c1ac4508da2aad Mon Sep 17 00:00:00 2001
>From: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>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 <aaronhaslett@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>---
> 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 <garming@catalyst.net.nz>
>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 <garming@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(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 <garming@catalyst.net.nz>
>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 <garming@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(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 <garming@samba.org>
>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 <garming@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>
>Autobuild-User(master): Garming Sam <garming@samba.org>
>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
>

Flags:

abartlet: review+

Actions: View

Attachments on bug 13600: 14460 | 14537 | 14572 | 14573 | 14574 | 14575 | 14594 | 14631 | 14632 | 14635 | 14636 | 14649 | 14667 | 14668 | 14670 | 14685 | 14691 | 14692 | 14693 | 14694 | 14696 | 14697 | 14704 | 14711 | 14712 | 14717 | 14719 | 14720 | 14724 | 17140