From a9e9cafdd8c6b75a18031c7d9e005721ff378477 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 f1facc83125..07cde80a258 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; @@ -419,6 +420,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 c3378d2d469194e8f1d8ebb8e7ed2f890ce5172b 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 8ccc34cd665..519060a5641 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -2606,10 +2606,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 cf7b086a819a97b9d62f65fde0c24d496eacca3e 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 fb553544286510b20e5a1e7b4f13a47ec239b149 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 5f7efe90bba..d754c4dae78 100644 --- a/source4/ldap_server/ldap_server.c +++ b/source4/ldap_server/ldap_server.c @@ -675,13 +675,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