From 678888b41bd07898399b5a66739796b04dbdf33a Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 21 Oct 2019 12:12:10 +1300 Subject: CVE-2019-14861: s4-rpc_server: Remove special case for @ in dns_build_tree() BUG: https://bugzilla.samba.org/show_bug.cgi?id=14138 Signed-off-by: Andrew Bartlett --- source4/rpc_server/dnsserver/dnsdata.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'source4') diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c index 2dc098a64a0..0739ea3f476 100644 --- a/source4/rpc_server/dnsserver/dnsdata.c +++ b/source4/rpc_server/dnsserver/dnsdata.c @@ -801,10 +801,11 @@ struct dns_tree *dns_build_tree(TALLOC_CTX *mem_ctx, const char *name, struct ld goto failed; } - if (strcmp(ptr, "@") == 0) { - base->data = res->msgs[i]; - continue; - } else if (strcasecmp(ptr, name) == 0) { + /* + * This might be the sub-domain in the zone being + * requested, or @ for the root of the zone + */ + if (strcasecmp(ptr, name) == 0) { base->data = res->msgs[i]; continue; } -- cgit v1.2.1 From 2318a4a7233d63a262d3e095dd2ea2b87b047bd6 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 29 Oct 2019 14:15:36 +1300 Subject: CVE-2019-14861: s4-rpc/dnsserver: Avoid crash in ldb_qsort() via dcesrv_DnssrvEnumRecords) dns_name_compare() had logic to put @ and the top record in the tree being enumerated first, but if a domain had both then this would break the older qsort() implementation in ldb_qsort() and cause a read of memory before the base pointer. By removing this special case (not required as the base pointer is already seperatly located, no matter were it is in the returned records) the crash is avoided. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14138 Signed-off-by: Andrew Bartlett --- source4/rpc_server/dnsserver/dcerpc_dnsserver.c | 21 +++++++++++++-------- source4/rpc_server/dnsserver/dnsdata.c | 19 ++----------------- source4/rpc_server/dnsserver/dnsserver.h | 4 ++-- 3 files changed, 17 insertions(+), 27 deletions(-) (limited to 'source4') diff --git a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c index 993e5dc4e56..b6389f2328a 100644 --- a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c +++ b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c @@ -1763,6 +1763,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, struct DNS_RPC_RECORDS_ARRAY *recs; char **add_names = NULL; char *rname; + const char *preference_name = NULL; int add_count = 0; int i, ret, len; WERROR status; @@ -1779,6 +1780,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, ret = ldb_search(dsstate->samdb, tmp_ctx, &res, z->zone_dn, LDB_SCOPE_ONELEVEL, attrs, "(&(objectClass=dnsNode)(!(dNSTombstoned=TRUE)))"); + preference_name = "@"; } else { char *encoded_name = ldb_binary_encode_string(tmp_ctx, name); @@ -1786,6 +1788,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, LDB_SCOPE_ONELEVEL, attrs, "(&(objectClass=dnsNode)(|(name=%s)(name=*.%s))(!(dNSTombstoned=TRUE)))", encoded_name, encoded_name); + preference_name = name; } if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); @@ -1799,16 +1802,18 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, recs = talloc_zero(mem_ctx, struct DNS_RPC_RECORDS_ARRAY); W_ERROR_HAVE_NO_MEMORY_AND_FREE(recs, tmp_ctx); - /* Sort the names, so that the first record is the parent record */ - ldb_qsort(res->msgs, res->count, sizeof(struct ldb_message *), name, - (ldb_qsort_cmp_fn_t)dns_name_compare); + /* + * Sort the names, so that the records are in order by the child + * component below "name". + * + * A full tree sort is not required, so we pass in "name" so + * we know which level to sort, as only direct children are + * eventually returned + */ + LDB_TYPESAFE_QSORT(res->msgs, res->count, name, dns_name_compare); /* Build a tree of name components from dns name */ - if (strcasecmp(name, z->name) == 0) { - tree = dns_build_tree(tmp_ctx, "@", res); - } else { - tree = dns_build_tree(tmp_ctx, name, res); - } + tree = dns_build_tree(tmp_ctx, preference_name, res); W_ERROR_HAVE_NO_MEMORY_AND_FREE(tree, tmp_ctx); /* Find the parent record in the tree */ diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c index 0739ea3f476..47d6f5d5c88 100644 --- a/source4/rpc_server/dnsserver/dnsdata.c +++ b/source4/rpc_server/dnsserver/dnsdata.c @@ -1066,8 +1066,8 @@ WERROR dns_fill_records_array(TALLOC_CTX *mem_ctx, } -int dns_name_compare(const struct ldb_message **m1, const struct ldb_message **m2, - char *search_name) +int dns_name_compare(struct ldb_message * const *m1, struct ldb_message * const *m2, + const char *search_name) { const char *name1, *name2; const char *ptr1, *ptr2; @@ -1078,21 +1078,6 @@ int dns_name_compare(const struct ldb_message **m1, const struct ldb_message **m return 0; } - /* '@' record and the search_name record gets preference */ - if (name1[0] == '@') { - return -1; - } - if (search_name && strcasecmp(name1, search_name) == 0) { - return -1; - } - - if (name2[0] == '@') { - return 1; - } - if (search_name && strcasecmp(name2, search_name) == 0) { - return 1; - } - /* Compare the last components of names. * If search_name is not NULL, compare the second last components of names */ ptr1 = strrchr(name1, '.'); diff --git a/source4/rpc_server/dnsserver/dnsserver.h b/source4/rpc_server/dnsserver/dnsserver.h index a8307ef836a..2e46e7c66a4 100644 --- a/source4/rpc_server/dnsserver/dnsserver.h +++ b/source4/rpc_server/dnsserver/dnsserver.h @@ -188,8 +188,8 @@ struct DNS_ADDR_ARRAY *dns_addr_array_copy(TALLOC_CTX *mem_ctx, struct DNS_ADDR_ int dns_split_name_components(TALLOC_CTX *mem_ctx, const char *name, char ***components); char *dns_split_node_name(TALLOC_CTX *mem_ctx, const char *node_name, const char *zone_name); -int dns_name_compare(const struct ldb_message **m1, const struct ldb_message **m2, - char *search_name); +int dns_name_compare(struct ldb_message * const *m1, struct ldb_message * const *m2, + const char *search_name); bool dns_record_match(struct dnsp_DnssrvRpcRecord *rec1, struct dnsp_DnssrvRpcRecord *rec2); void dnsp_to_dns_copy(TALLOC_CTX *mem_ctx, struct dnsp_DnssrvRpcRecord *dnsp, -- cgit v1.2.1 From b69ee283de5de1f560a73ad63c10f7974afeb9f8 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 1 Nov 2019 06:53:56 +1300 Subject: s4-torture: Reduce flapping in SambaToolDrsTests.test_samba_tool_replicate_local This test often flaps in Samba 4.9 (where more tests and DCs run in the environment) with obj_1 being 3. This is quite OK, we just need to see some changes get replicated, not 0 changes. Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall (cherry picked from commit 4ae0f9ce0f5ada99cf1d236377e5a1234c879ae3) --- source4/torture/drs/python/samba_tool_drs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'source4') diff --git a/source4/torture/drs/python/samba_tool_drs.py b/source4/torture/drs/python/samba_tool_drs.py index 76cc86f832e..988f1dc7a3c 100644 --- a/source4/torture/drs/python/samba_tool_drs.py +++ b/source4/torture/drs/python/samba_tool_drs.py @@ -210,6 +210,7 @@ class SambaToolDrsTests(drs_base.DrsBaseTestCase): self._disable_inbound_repl(self.dnsname_dc1) self._disable_inbound_repl(self.dnsname_dc2) + self._net_drs_replicate(DC=self.dnsname_dc2, fromDC=self.dnsname_dc1) self._net_drs_replicate(DC=self.dnsname_dc1, fromDC=self.dnsname_dc2) # add an object with link on dc1 @@ -232,7 +233,7 @@ class SambaToolDrsTests(drs_base.DrsBaseTestCase): (obj_1, link_1) = get_num_obj_links(out) - self.assertEqual(obj_1, 2) + self.assertGreaterEqual(obj_1, 2) self.assertEqual(link_1, 1) # pull that change with --local into local db from dc2: shouldn't send link or object -- cgit v1.2.1 From fc0127db4b9f2fb21cb72b6f4cddd8de6167f555 Mon Sep 17 00:00:00 2001 From: Isaac Boukris Date: Wed, 30 Oct 2019 15:59:16 +0100 Subject: CVE-2019-14870: heimdal: add S4U test for delegation_not_allowed Signed-off-by: Isaac Boukris --- source4/selftest/tests.py | 1 + 1 file changed, 1 insertion(+) (limited to 'source4') diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 1772611eb53..2bc4561b87a 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -455,6 +455,7 @@ if have_heimdal_support: plantestsuite("samba4.blackbox.kinit_trust(fl2003dc:local)", "fl2003dc:local", [os.path.join(bbdir, "test_kinit_trusts_heimdal.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$TRUST_SERVER', '$TRUST_USERNAME', '$TRUST_PASSWORD', '$TRUST_REALM', '$TRUST_DOMAIN', '$PREFIX', "external", "arcfour-hmac-md5"]) plantestsuite("samba4.blackbox.export.keytab(ad_dc_ntvfs:local)", "ad_dc_ntvfs:local", [os.path.join(bbdir, "test_export_keytab_heimdal.sh"), '$SERVER', '$USERNAME', '$REALM', '$DOMAIN', "$PREFIX", smbclient4]) plantestsuite("samba4.blackbox.kpasswd(ad_dc_ntvfs:local)", "ad_dc_ntvfs:local", [os.path.join(bbdir, "test_kpasswd_heimdal.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', "$PREFIX/ad_dc_ntvfs"]) + plantestsuite("samba4.blackbox.krb5.s4u", "fl2008r2dc:local", [os.path.join(bbdir, "test_s4u_heimdal.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$PREFIX', configuration]) else: plantestsuite("samba4.blackbox.kinit(ad_dc_ntvfs:local)", "ad_dc_ntvfs:local", [os.path.join(bbdir, "test_kinit_mit.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$PREFIX', smbclient4, configuration]) plantestsuite("samba4.blackbox.kinit(fl2000dc:local)", "fl2000dc:local", [os.path.join(bbdir, "test_kinit_mit.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$PREFIX', smbclient4, configuration]) -- cgit v1.2.1 From fbc1f000cf76f2172d63c9cdf4889fd83a087b14 Mon Sep 17 00:00:00 2001 From: Isaac Boukris Date: Mon, 28 Oct 2019 02:54:09 +0200 Subject: CVE-2019-14870: heimdal: enforce delegation_not_allowed in S4U2Self Signed-off-by: Isaac Boukris --- source4/heimdal/kdc/krb5tgs.c | 58 +++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 22 deletions(-) (limited to 'source4') diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c index ff7d93138c0..ee3ac3d8f53 100644 --- a/source4/heimdal/kdc/krb5tgs.c +++ b/source4/heimdal/kdc/krb5tgs.c @@ -1975,30 +1975,42 @@ server_lookup: if (ret) goto out; + ret = _kdc_db_fetch(context, config, tp, HDB_F_GET_CLIENT | flags, + NULL, &s4u2self_impersonated_clientdb, + &s4u2self_impersonated_client); + if (ret) { + const char *msg; + + /* + * If the client belongs to the same realm as our krbtgt, it + * should exist in the local database. + * + */ + + if (ret == HDB_ERR_NOENTRY) + ret = KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN; + msg = krb5_get_error_message(context, ret); + kdc_log(context, config, 1, + "S2U4Self principal to impersonate %s not found in database: %s", + tpn, msg); + krb5_free_error_message(context, msg); + goto out; + } + + /* Ignore pw_end attributes (as Windows does), + * since S4U2Self is not password authentication. */ + free(s4u2self_impersonated_client->entry.pw_end); + s4u2self_impersonated_client->entry.pw_end = NULL; + + ret = kdc_check_flags(context, config, s4u2self_impersonated_client, tpn, + NULL, NULL, FALSE); + if (ret) + goto out; + /* If we were about to put a PAC into the ticket, we better fix it to be the right PAC */ if(rspac.data) { krb5_pac p = NULL; krb5_data_free(&rspac); - ret = _kdc_db_fetch(context, config, tp, HDB_F_GET_CLIENT | flags, - NULL, &s4u2self_impersonated_clientdb, &s4u2self_impersonated_client); - if (ret) { - const char *msg; - - /* - * If the client belongs to the same realm as our krbtgt, it - * should exist in the local database. - * - */ - - if (ret == HDB_ERR_NOENTRY) - ret = KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN; - msg = krb5_get_error_message(context, ret); - kdc_log(context, config, 1, - "S2U4Self principal to impersonate %s not found in database: %s", - tpn, msg); - krb5_free_error_message(context, msg); - goto out; - } ret = _kdc_pac_generate(context, s4u2self_impersonated_client, NULL, &p); if (ret) { kdc_log(context, config, 0, "PAC generation failed for -- %s", @@ -2034,10 +2046,12 @@ server_lookup: /* * If the service isn't trusted for authentication to - * delegation, remove the forward flag. + * delegation or if the impersonate client is disallowed + * forwardable, remove the forwardable flag. */ - if (client->entry.flags.trusted_for_delegation) { + if (client->entry.flags.trusted_for_delegation && + s4u2self_impersonated_client->entry.flags.forwardable) { str = "[forwardable]"; } else { b->kdc_options.forwardable = 0; -- cgit v1.2.1 From 1ccab20c59b651173e76918a6b84290a5be4a27d Mon Sep 17 00:00:00 2001 From: Isaac Boukris Date: Thu, 21 Nov 2019 11:12:48 +0100 Subject: CVE-2019-14870: mit-kdc: enforce delegation_not_allowed flag Signed-off-by: Isaac Boukris --- source4/kdc/mit_samba.c | 5 +++++ source4/kdc/sdb_to_kdb.c | 17 ++++++----------- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'source4') diff --git a/source4/kdc/mit_samba.c b/source4/kdc/mit_samba.c index 54dcd545ea1..5a4f6e73e97 100644 --- a/source4/kdc/mit_samba.c +++ b/source4/kdc/mit_samba.c @@ -304,6 +304,11 @@ fetch_referral_principal: sdb_free_entry(&sentry); + if ((kflags & KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY) == 0) { + kentry->attributes &= ~KRB5_KDB_DISALLOW_FORWARDABLE; + kentry->attributes &= ~KRB5_KDB_DISALLOW_PROXIABLE; + } + done: krb5_free_principal(ctx->context, referral_principal); referral_principal = NULL; diff --git a/source4/kdc/sdb_to_kdb.c b/source4/kdc/sdb_to_kdb.c index 1411b0f5f66..2981f180333 100644 --- a/source4/kdc/sdb_to_kdb.c +++ b/source4/kdc/sdb_to_kdb.c @@ -36,18 +36,13 @@ static int SDBFlags_to_kflags(const struct SDBFlags *s, if (s->initial) { *k |= KRB5_KDB_DISALLOW_TGT_BASED; } - /* - * Do not set any disallow rules for forwardable, proxiable, - * renewable, postdate and server. - * - * The KDC will take care setting the flags based on the incoming - * ticket. - */ - if (s->forwardable) { - ; + /* The forwardable and proxiable flags are set according to client and + * server attributes. */ + if (!s->forwardable) { + *k |= KRB5_KDB_DISALLOW_FORWARDABLE; } - if (s->proxiable) { - ; + if (!s->proxiable) { + *k |= KRB5_KDB_DISALLOW_PROXIABLE; } if (s->renewable) { ; -- cgit v1.2.1