From 4dfc0144b1ba4917987f77fffe2bdc877a68407e 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. 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 | 24 ++++++++++++++++++++++++ source4/dns_server/dns_query.c | 6 ++++++ 2 files changed, 30 insertions(+) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index 1b5b64da3a4..3390a3990c9 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -798,6 +798,30 @@ class TestComplexQueries(DNSTest): self.assertEquals(response.answers[1].name, name2) self.assertEquals(response.answers[1].rdata, name0) + 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, response_packet) =\ + 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 setUp(self): super(TestInvalidQueries, self).setUp() diff --git a/source4/dns_server/dns_query.c b/source4/dns_server/dns_query.c index e8de304c8bb..fafadb6ac6f 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 85e0f5ba1706755f6cf0023a50c32e8e1876b068 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 23 Oct 2018 17:33:46 +1300 Subject: [PATCH 2/4] CVE-2018-16841 heimdal: Fix segfault on PKINIT with mis-matching principal In Heimdal KRB5_KDC_ERR_CLIENT_NAME_MISMATCH is an enum, so we tried to double-free mem_ctx. This was introduced in 9a0263a7c316112caf0265237bfb2cfb3a3d370d for the MIT KDC effort. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13628 Signed-off-by: Andrew Bartlett Reviewed-by: Gary Lockyer --- source4/kdc/db-glue.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index 9ac5a1d38f0..4d7ac333fcc 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -2578,10 +2578,10 @@ samba_kdc_check_pkinit_ms_upn_match(krb5_context context, * comparison */ if (!(orig_sid && target_sid && dom_sid_equal(orig_sid, target_sid))) { talloc_free(mem_ctx); -#ifdef KRB5_KDC_ERR_CLIENT_NAME_MISMATCH /* Heimdal */ - return KRB5_KDC_ERR_CLIENT_NAME_MISMATCH; -#elif defined(KRB5KDC_ERR_CLIENT_NAME_MISMATCH) /* MIT */ +#if defined(KRB5KDC_ERR_CLIENT_NAME_MISMATCH) /* MIT */ return KRB5KDC_ERR_CLIENT_NAME_MISMATCH; +#else /* Heimdal (where this is an enum) */ + return KRB5_KDC_ERR_CLIENT_NAME_MISMATCH; #endif } -- 2.11.0 From cf1777ddf6bfa6a13eaa861a13369df2986a351a Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 24 Oct 2018 15:41:28 +1300 Subject: [PATCH 3/4] CVE-2018-16841 selftest: Check for mismatching principal in certficate compared with principal in AS-REQ BUG: https://bugzilla.samba.org/show_bug.cgi?id=13628 Signed-off-by: Andrew Bartlett --- testprogs/blackbox/test_pkinit_heimdal.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/testprogs/blackbox/test_pkinit_heimdal.sh b/testprogs/blackbox/test_pkinit_heimdal.sh index 0a13aa293e7..0912e0dbfe8 100755 --- a/testprogs/blackbox/test_pkinit_heimdal.sh +++ b/testprogs/blackbox/test_pkinit_heimdal.sh @@ -75,10 +75,18 @@ testit "STEP1 kinit with pkinit (name specified) " $samba4kinit $enctype --reque testit "STEP1 kinit renew ticket (name specified)" $samba4kinit --request-pac -R || failed=`expr $failed + 1` test_smbclient "STEP1 Test login with kerberos ccache (name specified)" 'ls' "$unc" -k yes || failed=`expr $failed + 1` +testit_expect_failure "STEP1 kinit with pkinit (wrong name specified) " $samba4kinit $enctype --request-pac --renewable $PKUSER not$USERNAME@$REALM || failed=`expr $failed + 1` + +testit_expect_failure "STEP1 kinit with pkinit (wrong name specified 2) " $samba4kinit $enctype --request-pac --renewable $PKUSER $SERVER@$REALM || failed=`expr $failed + 1` + testit "STEP1 kinit with pkinit (enterprise name specified)" $samba4kinit $enctype --request-pac --renewable $PKUSER --enterprise $USERNAME@$REALM || failed=`expr $failed + 1` testit "STEP1 kinit renew ticket (enterprise name specified)" $samba4kinit --request-pac -R || failed=`expr $failed + 1` test_smbclient "STEP1 Test login with kerberos ccache (enterprise name specified)" 'ls' "$unc" -k yes || failed=`expr $failed + 1` +testit_expect_failure "STEP1 kinit with pkinit (wrong enterprise name specified) " $samba4kinit $enctype --request-pac --renewable $PKUSER --enterprise not$USERNAME@$REALM || failed=`expr $failed + 1` + +testit_expect_failure "STEP1 kinit with pkinit (wrong enterprise name specified 2) " $samba4kinit $enctype --request-pac --renewable $PKUSER --enterprise $SERVER$@$REALM || failed=`expr $failed + 1` + testit "STEP1 kinit with pkinit (enterprise name in cert)" $samba4kinit $enctype --request-pac --renewable $PKUSER --pk-enterprise || failed=`expr $failed + 1` testit "STEP1 kinit renew ticket (enterprise name in cert)" $samba4kinit --request-pac -R || failed=`expr $failed + 1` test_smbclient "STEP1 Test login with kerberos ccache (enterprise name in cert)" 'ls' "$unc" -k yes || failed=`expr $failed + 1` -- 2.11.0 From 329e48cd9ef9155368d35c3c8b03464162bc199d Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Mon, 5 Nov 2018 16:18:18 +1300 Subject: [PATCH 4/4] CVE-2018-16851 ldap_server: Check ret before manipulating blob In the case of hitting the talloc ~256MB limit, this causes a crash in the server. Note that you would actually need to load >256MB of data into the LDAP. Although there is some generated/hidden data which would help you reach that limit (descriptors and RMD blobs). BUG: https://bugzilla.samba.org/show_bug.cgi?id=13674 Signed-off-by: Garming Sam Reviewed-by: Andrew Bartlett --- source4/ldap_server/ldap_server.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/ldap_server/ldap_server.c b/source4/ldap_server/ldap_server.c index d9f24e0817c..e5e9688ed98 100644 --- a/source4/ldap_server/ldap_server.c +++ b/source4/ldap_server/ldap_server.c @@ -669,13 +669,13 @@ static void ldapsrv_call_writev_start(struct ldapsrv_call *call) ret = data_blob_append(call, &blob, b.data, b.length); data_blob_free(&b); - talloc_set_name_const(blob.data, "Outgoing, encoded LDAP packet"); - if (!ret) { ldapsrv_terminate_connection(conn, "data_blob_append failed"); return; } + talloc_set_name_const(blob.data, "Outgoing, encoded LDAP packet"); + DLIST_REMOVE(call->replies, call->replies); } -- 2.11.0