[SCM] Samba Shared Repository - branch master updated
Andrew Bartlett
abartlet at samba.org
Tue Dec 4 07:53:03 UTC 2018
The branch, master has been updated
via 34f4491d79b CVE-2018-14629 dns: fix CNAME loop prevention using counter regression
via 14399fd818b CVE-2018-14629: Tests to expose regression from dns cname loop fix
from 2b2edccb5a9 s3:lib: Fix undefined behavior in tdb_unpack()
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 34f4491d79b47b2fe2457b8882f11644cf773bc4
Author: Stefan Metzmacher <metze at samba.org>
Date: Wed Nov 28 15:21:56 2018 +0100
CVE-2018-14629 dns: fix CNAME loop prevention using counter regression
The loop prevention should only be done for CNAME records!
Otherwise we truncate the answer records for A, AAAA or
SRV queries, which is a bad idea if you have more than 20 DCs.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13600
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
Autobuild-Date(master): Tue Dec 4 08:52:29 CET 2018 on sn-devel-144
commit 14399fd818b130a6347eec860460929c292d5996
Author: Aaron Haslett <aaronhaslett at catalyst.net.nz>
Date: Fri Nov 30 18:37:27 2018 +1300
CVE-2018-14629: Tests to expose regression from dns cname loop fix
These tests expose the regression described by Stefan Metzmacher in
discussion on the bugzilla paged linked below.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13600
Signed-off-by: Aaron Haslett <aaronhaslett at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
-----------------------------------------------------------------------
Summary of changes:
python/samba/tests/dns.py | 101 +++++++++++++++++++++++++++++++++++++++++
selftest/knownfail.d/dns | 14 +++++-
source4/dns_server/dns_query.c | 29 ++++++++----
3 files changed, 133 insertions(+), 11 deletions(-)
Changeset truncated at 500 lines:
diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py
index e35cded7b8c..1bf57e2aad0 100644
--- a/python/samba/tests/dns.py
+++ b/python/samba/tests/dns.py
@@ -869,6 +869,107 @@ class TestComplexQueries(DNSTest):
max_recursion_depth = 20
self.assertEquals(len(response.answers), max_recursion_depth)
+ # Make sure cname limit doesn't count other records. This is a generic
+ # test called in tests below
+ def max_rec_test(self, rtype, rec_gen):
+ name = "limittestrec{0}.{1}".format(rtype, self.get_dns_domain())
+ limit = 20
+ num_recs_to_enter = limit + 5
+
+ for i in range(1, num_recs_to_enter+1):
+ ip = rec_gen(i)
+ self.make_dns_update(name, ip, rtype)
+
+ p = self.make_name_packet(dns.DNS_OPCODE_QUERY)
+ questions = []
+
+ q = self.make_name_question(name,
+ rtype,
+ dns.DNS_QCLASS_IN)
+ questions.append(q)
+ self.finish_name_packet(p, questions)
+
+ (response, response_packet) =\
+ self.dns_transaction_udp(p, host=self.server_ip)
+
+ self.assertEqual(len(response.answers), num_recs_to_enter)
+
+ def test_record_limit_A(self):
+ def ip4_gen(i):
+ return "127.0.0." + str(i)
+ self.max_rec_test(rtype=dns.DNS_QTYPE_A, rec_gen=ip4_gen)
+
+ def test_record_limit_AAAA(self):
+ def ip6_gen(i):
+ return "AAAA:0:0:0:0:0:0:" + str(i)
+ self.max_rec_test(rtype=dns.DNS_QTYPE_AAAA, rec_gen=ip6_gen)
+
+ def test_record_limit_SRV(self):
+ def srv_gen(i):
+ rec = dns.srv_record()
+ rec.priority = 1
+ rec.weight = 1
+ rec.port = 92
+ rec.target = "srvtestrec" + str(i)
+ return rec
+ self.max_rec_test(rtype=dns.DNS_QTYPE_SRV, rec_gen=srv_gen)
+
+ # Same as test_record_limit_A but with a preceding CNAME follow
+ def test_cname_limit(self):
+ cname1 = "cnamelimittestrec." + self.get_dns_domain()
+ cname2 = "cnamelimittestrec2." + self.get_dns_domain()
+ cname3 = "cnamelimittestrec3." + self.get_dns_domain()
+ ip_prefix = '127.0.0.'
+ limit = 20
+ num_recs_to_enter = limit + 5
+
+ self.make_dns_update(cname1, cname2, dnsp.DNS_TYPE_CNAME)
+ self.make_dns_update(cname2, cname3, dnsp.DNS_TYPE_CNAME)
+ num_arecs_to_enter = num_recs_to_enter - 2
+ for i in range(1, num_arecs_to_enter+1):
+ ip = ip_prefix + str(i)
+ self.make_dns_update(cname3, ip, dns.DNS_QTYPE_A)
+
+ 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, response_packet) =\
+ self.dns_transaction_udp(p, host=self.server_ip)
+
+ self.assertEqual(len(response.answers), num_recs_to_enter)
+
+ # ANY query on cname record shouldn't follow the link
+ def test_cname_any_query(self):
+ cname1 = "cnameanytestrec." + self.get_dns_domain()
+ cname2 = "cnameanytestrec2." + self.get_dns_domain()
+ cname3 = "cnameanytestrec3." + self.get_dns_domain()
+
+ self.make_dns_update(cname1, cname2, dnsp.DNS_TYPE_CNAME)
+ self.make_dns_update(cname2, cname3, dnsp.DNS_TYPE_CNAME)
+
+ p = self.make_name_packet(dns.DNS_OPCODE_QUERY)
+ questions = []
+
+ q = self.make_name_question(cname1,
+ dns.DNS_QTYPE_ALL,
+ dns.DNS_QCLASS_IN)
+ questions.append(q)
+ self.finish_name_packet(p, questions)
+
+ (response, response_packet) =\
+ self.dns_transaction_udp(p, host=self.server_ip)
+
+ self.assertEqual(len(response.answers), 1)
+ self.assertEqual(response.answers[0].name, cname1)
+ self.assertEqual(response.answers[0].rdata, cname2)
+
+
class TestInvalidQueries(DNSTest):
def setUp(self):
super(TestInvalidQueries, self).setUp()
diff --git a/selftest/knownfail.d/dns b/selftest/knownfail.d/dns
index 39b337ed6bb..ec345372046 100644
--- a/selftest/knownfail.d/dns
+++ b/selftest/knownfail.d/dns
@@ -73,7 +73,17 @@ samba.tests.dns.__main__.TestSimpleQueries.test_qtype_all_query\(rodc:local\)
samba.tests.dns.__main__.TestSimpleQueries.test_one_SOA_query\(rodc:local\)
#
-# rodc and vampire_dc require signed dns updates, so the test setup
-# fails, but the test does run on fl2003dc
+# rodc and vampire_dc require signed dns updates, so these tests' setups
+# fail, but they pass on fl2003dc
^samba.tests.dns.__main__.TestComplexQueries.test_cname_loop\(rodc:local\)
^samba.tests.dns.__main__.TestComplexQueries.test_cname_loop\(vampire_dc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_record_limit_A\(rodc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_record_limit_A\(vampire_dc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_record_limit_AAAA\(rodc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_record_limit_AAAA\(vampire_dc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_record_limit_SRV\(rodc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_record_limit_SRV\(vampire_dc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_cname_limit\(vampire_dc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_cname_limit\(rodc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_cname_any_query\(vampire_dc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_cname_any_query\(rodc:local\)
diff --git a/source4/dns_server/dns_query.c b/source4/dns_server/dns_query.c
index 65faeac3b6a..b75fabe7e82 100644
--- a/source4/dns_server/dns_query.c
+++ b/source4/dns_server/dns_query.c
@@ -388,7 +388,8 @@ static struct tevent_req *handle_authoritative_send(
TALLOC_CTX *mem_ctx, struct tevent_context *ev,
struct dns_server *dns, const char *forwarder,
struct dns_name_question *question,
- struct dns_res_rec **answers, struct dns_res_rec **nsrecs);
+ struct dns_res_rec **answers, struct dns_res_rec **nsrecs,
+ size_t cname_depth);
static WERROR handle_authoritative_recv(struct tevent_req *req);
struct handle_dnsrpcrec_state {
@@ -404,7 +405,8 @@ static struct tevent_req *handle_dnsrpcrec_send(
struct dns_server *dns, const char *forwarder,
const struct dns_name_question *question,
struct dnsp_DnssrvRpcRecord *rec,
- struct dns_res_rec **answers, struct dns_res_rec **nsrecs)
+ struct dns_res_rec **answers, struct dns_res_rec **nsrecs,
+ size_t cname_depth)
{
struct tevent_req *req, *subreq;
struct handle_dnsrpcrec_state *state;
@@ -420,7 +422,7 @@ static struct tevent_req *handle_dnsrpcrec_send(
state->answers = answers;
state->nsrecs = nsrecs;
- if (talloc_array_length(*answers) >= MAX_Q_RECURSION_DEPTH) {
+ if (cname_depth >= MAX_Q_RECURSION_DEPTH) {
tevent_req_done(req);
return tevent_req_post(req, ev);
}
@@ -465,7 +467,8 @@ static struct tevent_req *handle_dnsrpcrec_send(
if (dns_authoritative_for_zone(dns, new_q->name)) {
subreq = handle_authoritative_send(
state, ev, dns, forwarder, new_q,
- state->answers, state->nsrecs);
+ state->answers, state->nsrecs,
+ cname_depth + 1);
if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
}
@@ -549,6 +552,8 @@ struct handle_authoritative_state {
struct dns_res_rec **answers;
struct dns_res_rec **nsrecs;
+
+ size_t cname_depth;
};
static void handle_authoritative_done(struct tevent_req *subreq);
@@ -557,7 +562,8 @@ static struct tevent_req *handle_authoritative_send(
TALLOC_CTX *mem_ctx, struct tevent_context *ev,
struct dns_server *dns, const char *forwarder,
struct dns_name_question *question,
- struct dns_res_rec **answers, struct dns_res_rec **nsrecs)
+ struct dns_res_rec **answers, struct dns_res_rec **nsrecs,
+ size_t cname_depth)
{
struct tevent_req *req, *subreq;
struct handle_authoritative_state *state;
@@ -575,6 +581,7 @@ static struct tevent_req *handle_authoritative_send(
state->forwarder = forwarder;
state->answers = answers;
state->nsrecs = nsrecs;
+ state->cname_depth = cname_depth;
werr = dns_name2dn(dns, state, question->name, &dn);
if (tevent_req_werror(req, werr)) {
@@ -595,7 +602,8 @@ static struct tevent_req *handle_authoritative_send(
subreq = handle_dnsrpcrec_send(
state, state->ev, state->dns, state->forwarder,
state->question, &state->recs[state->recs_done],
- state->answers, state->nsrecs);
+ state->answers, state->nsrecs,
+ state->cname_depth);
if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
}
@@ -627,7 +635,8 @@ static void handle_authoritative_done(struct tevent_req *subreq)
subreq = handle_dnsrpcrec_send(
state, state->ev, state->dns, state->forwarder,
state->question, &state->recs[state->recs_done],
- state->answers, state->nsrecs);
+ state->answers, state->nsrecs,
+ state->cname_depth);
if (tevent_req_nomem(subreq, req)) {
return;
}
@@ -1000,7 +1009,8 @@ struct tevent_req *dns_server_process_query_send(
subreq = handle_authoritative_send(
state, ev, dns, (forwarders == NULL ? NULL : forwarders[0]),
- &in->questions[0], &state->answers, &state->nsrecs);
+ &in->questions[0], &state->answers, &state->nsrecs,
+ 0); /* cname_depth */
if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
}
@@ -1102,7 +1112,8 @@ static void dns_server_process_query_got_auth(struct tevent_req *subreq)
subreq = handle_authoritative_send(state, state->ev, state->dns,
state->forwarders->forwarder,
state->question, &state->answers,
- &state->nsrecs);
+ &state->nsrecs,
+ 0); /* cname_depth */
if (tevent_req_nomem(subreq, req)) {
return;
--
Samba Shared Repository
More information about the samba-cvs
mailing list