From 3fb5f5b31d4359cffe7642ce243693af7a543544 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 6 Nov 2018 13:32:05 +1300 Subject: [PATCH 1/7] build: The Samba AD DC, when build with MIT Kerberos is experimental This matches https://wiki.samba.org/index.php/Running_a_Samba_AD_DC_with_MIT_Kerberos_KDC BUG: https://bugzilla.samba.org/show_bug.cgi?id=13678 Signed-off-by: Andrew Bartlett --- wscript | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/wscript b/wscript index 19fc6d12118..7c265e7befb 100644 --- a/wscript +++ b/wscript @@ -56,6 +56,14 @@ def set_options(opt): help='build Samba with system MIT Kerberos. ' + 'You may specify list of paths where Kerberos is installed (e.g. /usr/local /usr/kerberos) to search krb5-config', action='callback', callback=system_mitkrb5_callback, dest='with_system_mitkrb5', default=False) + + opt.add_option('--with-experimental-mit-ad-dc', + help='Enable the experimental MIT Kerberos-backed AD DC. ' + + 'Note that security patches are not issued for this configuration', + action='store_true', + dest='with_experimental_mit_ad_dc', + default=False) + opt.add_option('--with-system-mitkdc', help=('Specify the path to the krb5kdc binary from MIT Kerberos'), type="string", @@ -210,7 +218,16 @@ def configure(conf): conf.DEFINE('AD_DC_BUILD_IS_ENABLED', 1) if Options.options.with_system_mitkrb5: + if not Options.options.with_experimental_mit_ad_dc and \ + not Options.options.without_ad_dc: + raise Utils.WafError('The MIT Kerberos build of Samba as an AD DC ' + + 'is experimental. Therefore ' + '--with-system-mitkrb5 requires either ' + + '--with-experimental-mit-ad-dc or ' + + '--without-ad-dc') + conf.PROCESS_SEPARATE_RULE('system_mitkrb5') + if not (Options.options.without_ad_dc or Options.options.with_system_mitkrb5): conf.DEFINE('AD_DC_BUILD_IS_ENABLED', 1) -- 2.11.0 From d34d85075006d8d1d3728d1c4c55b43579c0a849 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Tue, 6 Nov 2018 12:10:07 +1300 Subject: [PATCH 2/7] CVE-2018-16852 dcerpc dnsserver: Verification tests Tests to verify Bug 13669 - (CVE-2018-16852) NULL pointer de-reference in Samba AD DC DNS management The presence of the ZONE_MASTER_SERVERS property or the ZONE_SCAVENGING_SERVERS property in a zone record causes the server to follow a null pointer and terminate. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13669 Reviewed-by: Andrew Bartlett Signed-off-by: Gary Lockyer --- selftest/knownfail.d/bug13669 | 4 + .../tests/rpc_dns_server_dnsutils_test.c | 304 +++++++++++++++++++++ source4/rpc_server/wscript_build | 17 +- source4/selftest/tests.py | 2 + 4 files changed, 325 insertions(+), 2 deletions(-) create mode 100644 selftest/knownfail.d/bug13669 create mode 100644 source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c diff --git a/selftest/knownfail.d/bug13669 b/selftest/knownfail.d/bug13669 new file mode 100644 index 00000000000..74c8c130674 --- /dev/null +++ b/selftest/knownfail.d/bug13669 @@ -0,0 +1,4 @@ +^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers_empty +^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers +^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers_empty +^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers diff --git a/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c b/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c new file mode 100644 index 00000000000..89721135658 --- /dev/null +++ b/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c @@ -0,0 +1,304 @@ +/* + * Unit tests for source4/rpc_server/dnsserver/dnsutils.c + * + * Copyright (C) Catalyst.NET Ltd 2018 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +/* + * from cmocka.c: + * These headers or their equivalents should be included prior to + * including + * this header file. + * + * #include + * #include + * #include + * + * This allows test applications to use custom definitions of C standard + * library functions and types. + * + */ + +#include +#include +#include +#include + + +#include "../dnsserver/dnsutils.c" + + +/* + * Test setting of an empty ZONE_MASTER_SERVERS property + */ +static void test_dnsserver_init_zoneinfo_master_servers_empty(void **state) +{ + struct dnsserver_zone *zone = NULL; + struct dnsserver_serverinfo *serverinfo = NULL; + struct dnsserver_zoneinfo *zoneinfo = NULL; + struct dnsp_DnsProperty *property = NULL; + + TALLOC_CTX *ctx = talloc_new(NULL); + + /* + * Setup the zone data + */ + zone = talloc_zero(ctx, struct dnsserver_zone); + assert_non_null(zone); + zone->name = "test"; + + /* + * Set up an empty ZONE_MASTER_SERVERS property + */ + property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1); + assert_non_null(property); + property->id = DSPROPERTY_ZONE_MASTER_SERVERS; + property->data.master_servers.addrCount = 0; + property->data.master_servers.addr = NULL; + + zone->tmp_props = property; + zone->num_props = 1; + + + /* + * Setup the server info + */ + serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo); + assert_non_null(serverinfo); + + /* + * call dnsserver_init_zoneinfo + */ + zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo); + + /* + * Check results + */ + assert_non_null(zoneinfo); + assert_non_null(zoneinfo->aipLocalMasters); + assert_int_equal(zoneinfo->aipLocalMasters->AddrCount, 0); + assert_null(zoneinfo->aipLocalMasters->AddrArray); + + TALLOC_FREE(ctx); +} + +/* + * Test setting of a non empty ZONE_MASTER_SERVERS property + */ +static void test_dnsserver_init_zoneinfo_master_servers(void **state) +{ + struct dnsserver_zone *zone = NULL; + struct dnsserver_serverinfo *serverinfo = NULL; + struct dnsserver_zoneinfo *zoneinfo = NULL; + struct dnsp_DnsProperty *property = NULL; + + TALLOC_CTX *ctx = talloc_new(NULL); + + /* + * Setup the zone data + */ + zone = talloc_zero(ctx, struct dnsserver_zone); + assert_non_null(zone); + zone->name = "test"; + + /* + * Set up an empty ZONE_MASTER_SERVERS property + */ + property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1); + assert_non_null(property); + property->id = DSPROPERTY_ZONE_MASTER_SERVERS; + property->data.master_servers.addrCount = 4; + property->data.master_servers.addr = + talloc_zero_array(ctx, uint32_t, 4); + property->data.master_servers.addr[0] = 1000; + property->data.master_servers.addr[1] = 1001; + property->data.master_servers.addr[2] = 1002; + property->data.master_servers.addr[3] = 1003; + + zone->tmp_props = property; + zone->num_props = 1; + + + /* + * Setup the server info + */ + serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo); + assert_non_null(serverinfo); + + /* + * call dnsserver_init_zoneinfo + */ + zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo); + + /* + * Check results + */ + assert_non_null(zoneinfo); + assert_non_null(zoneinfo->aipLocalMasters); + assert_int_equal(zoneinfo->aipLocalMasters->AddrCount, 4); + assert_non_null(zoneinfo->aipLocalMasters->AddrArray); + assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[0], 1000); + assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[1], 1001); + assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[2], 1002); + assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[3], 1003); + + /* + * Ensure that we're working with a copy of the property data + * and not a reference. + * The pointers should be different, and we should be able to change + * the values in the property without affecting the zoneinfo data + */ + assert_true(zoneinfo->aipLocalMasters->AddrArray != + property->data.master_servers.addr); + property->data.master_servers.addr[0] = 0; + property->data.master_servers.addr[1] = 1; + property->data.master_servers.addr[2] = 2; + property->data.master_servers.addr[3] = 3; + assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[0], 1000); + assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[1], 1001); + assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[2], 1002); + assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[3], 1003); + + TALLOC_FREE(ctx); +} + +/* + * Test setting of an empty ZONE_SCAVENGING_SERVERS property + */ +static void test_dnsserver_init_zoneinfo_scavenging_servers_empty(void **state) +{ + struct dnsserver_zone *zone = NULL; + struct dnsserver_serverinfo *serverinfo = NULL; + struct dnsserver_zoneinfo *zoneinfo = NULL; + struct dnsp_DnsProperty *property = NULL; + + TALLOC_CTX *ctx = talloc_new(NULL); + + /* + * Setup the zone data + */ + zone = talloc_zero(ctx, struct dnsserver_zone); + assert_non_null(zone); + zone->name = "test"; + + property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1); + assert_non_null(property); + property->id = DSPROPERTY_ZONE_SCAVENGING_SERVERS; + property->data.servers.addrCount = 0; + property->data.servers.addr = NULL; + + zone->tmp_props = property; + zone->num_props = 1; + + + serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo); + assert_non_null(serverinfo); + + zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo); + + assert_non_null(zoneinfo); + assert_non_null(zoneinfo->aipScavengeServers); + assert_int_equal(zoneinfo->aipScavengeServers->AddrCount, 0); + assert_null(zoneinfo->aipScavengeServers->AddrArray); + + TALLOC_FREE(ctx); +} + +/* + * Test setting of a non empty ZONE_SCAVENGING_SERVERS property + */ +static void test_dnsserver_init_zoneinfo_scavenging_servers(void **state) +{ + struct dnsserver_zone *zone = NULL; + struct dnsserver_serverinfo *serverinfo = NULL; + struct dnsserver_zoneinfo *zoneinfo = NULL; + struct dnsp_DnsProperty *property = NULL; + + TALLOC_CTX *ctx = talloc_new(NULL); + + /* + * Setup the zone data + */ + zone = talloc_zero(ctx, struct dnsserver_zone); + assert_non_null(zone); + zone->name = "test"; + + property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1); + assert_non_null(property); + property->id = DSPROPERTY_ZONE_SCAVENGING_SERVERS; + property->data.servers.addrCount = 4; + property->data.servers.addr = talloc_zero_array(ctx, uint32_t, 4); + property->data.servers.addr[0] = 1000; + property->data.servers.addr[1] = 1001; + property->data.servers.addr[2] = 1002; + property->data.servers.addr[3] = 1003; + + zone->tmp_props = property; + zone->num_props = 1; + + + serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo); + assert_non_null(serverinfo); + + zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo); + + assert_non_null(zoneinfo); + assert_non_null(zoneinfo->aipScavengeServers); + assert_int_equal(zoneinfo->aipScavengeServers->AddrCount, 4); + assert_non_null(zoneinfo->aipScavengeServers->AddrArray); + assert_non_null(zoneinfo->aipScavengeServers->AddrArray); + assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[0], 1000); + assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[1], 1001); + assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[2], 1002); + assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[3], 1003); + + /* + * Ensure that we're working with a copy of the property data + * and not a reference. + * The pointers should be different, and we should be able to change + * the values in the property without affecting the zoneinfo data + */ + assert_true(zoneinfo->aipScavengeServers->AddrArray != + property->data.servers.addr); + property->data.servers.addr[0] = 0; + property->data.servers.addr[1] = 1; + property->data.servers.addr[2] = 2; + property->data.servers.addr[3] = 3; + assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[0], 1000); + assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[1], 1001); + assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[2], 1002); + assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[3], 1003); + + + TALLOC_FREE(ctx); +} +int main(int argc, const char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test( + test_dnsserver_init_zoneinfo_master_servers_empty), + cmocka_unit_test( + test_dnsserver_init_zoneinfo_master_servers), + cmocka_unit_test( + test_dnsserver_init_zoneinfo_scavenging_servers_empty), + cmocka_unit_test( + test_dnsserver_init_zoneinfo_scavenging_servers), + }; + + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/source4/rpc_server/wscript_build b/source4/rpc_server/wscript_build index 8e05eb8a0c3..14b68c7ce0f 100644 --- a/source4/rpc_server/wscript_build +++ b/source4/rpc_server/wscript_build @@ -3,7 +3,7 @@ bld.SAMBA_SUBSYSTEM('DCERPC_SHARE', source='common/share_info.c', autoproto='common/share.h', - deps='ldb', + deps='ldb share', enabled=bld.CONFIG_SET('WITH_NTVFS_FILESERVER'), ) @@ -163,7 +163,7 @@ bld.SAMBA_MODULE('dcerpc_dnsserver', source='dnsserver/dcerpc_dnsserver.c dnsserver/dnsutils.c dnsserver/dnsdata.c dnsserver/dnsdb.c', subsystem='dcerpc_server', init_function='dcerpc_server_dnsserver_init', - deps='DCERPC_COMMON dnsserver_common' + deps='DCERPC_COMMON dnsserver_common netif' ) @@ -176,3 +176,16 @@ bld.SAMBA_MODULE('service_dcerpc', deps='dcerpc_server' ) +if bld.CONFIG_GET('ENABLE_SELFTEST'): + bld.SAMBA_BINARY( + 'test_rpc_dns_server_dnsutils', + source='tests/rpc_dns_server_dnsutils_test.c', + deps=''' + dnsserver_common + DCERPC_COMMON + cmocka + talloc + ''', + install=False, + enabled=bld.AD_DC_BUILD_IS_ENABLED() + ) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 3a07eee4750..13f50df2c15 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1139,3 +1139,5 @@ plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_util", "none", [os.path.join(bindir(), "test_audit_util")]) plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_log", "none", [os.path.join(bindir(), "test_audit_log")]) +plantestsuite("samba4.dcerpc.dnsserver.dnsutils", "none", + [os.path.join(bindir(), "test_rpc_dns_server_dnsutils")]) -- 2.11.0 From bb49a4ea4519a5b1859d1ae66a56820eab375673 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Tue, 6 Nov 2018 12:16:30 +1300 Subject: [PATCH 3/7] CVE-2018-16852 dcerpc dnsserver: Ensure properties are handled correctly Fixes for Bug 13669 - (CVE-2018-16852) NULL pointer de-reference in Samba AD DC DNS management The presence of the ZONE_MASTER_SERVERS property or the ZONE_SCAVENGING_SERVERS property in a zone record causes the server to follow a null pointer and terminate. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13669 Reviewed-by: Andrew Bartlett Signed-off-by: Gary Lockyer --- selftest/knownfail.d/bug13669 | 4 -- source4/rpc_server/dnsserver/dnsutils.c | 65 +++++++++++++++++++++++++++++---- 2 files changed, 57 insertions(+), 12 deletions(-) delete mode 100644 selftest/knownfail.d/bug13669 diff --git a/selftest/knownfail.d/bug13669 b/selftest/knownfail.d/bug13669 deleted file mode 100644 index 74c8c130674..00000000000 --- a/selftest/knownfail.d/bug13669 +++ /dev/null @@ -1,4 +0,0 @@ -^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers_empty -^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers -^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers_empty -^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers diff --git a/source4/rpc_server/dnsserver/dnsutils.c b/source4/rpc_server/dnsserver/dnsutils.c index a1c749074af..13b01deca64 100644 --- a/source4/rpc_server/dnsserver/dnsutils.c +++ b/source4/rpc_server/dnsserver/dnsutils.c @@ -209,6 +209,46 @@ struct dnsserver_serverinfo *dnsserver_init_serverinfo(TALLOC_CTX *mem_ctx, } +/* + * Helper function to copy a dnsp_ip4_array struct to an IP4_ARRAY struct. + * The new structure and it's data are allocated on the supplied talloc context + */ +static struct IP4_ARRAY *copy_ip4_array( + TALLOC_CTX *ctx, + const char *name, + struct dnsp_ip4_array array) { + + struct IP4_ARRAY *ip4_array = NULL; + unsigned int i; + + ip4_array = talloc_zero(ctx, struct IP4_ARRAY); + if (ip4_array == NULL) { + DBG_ERR("Out of memory copying property [%s]\n", + name); + return NULL; + } + + ip4_array->AddrCount = array.addrCount; + if (ip4_array->AddrCount == 0) { + return ip4_array; + } + + ip4_array->AddrArray = talloc_array(ip4_array, uint32_t, + ip4_array->AddrCount); + if (ip4_array->AddrArray == NULL) { + TALLOC_FREE(ip4_array); + DBG_ERR("Out of memory copying property [%s] values\n", + name); + return NULL; + } + + for (i = 0; i < ip4_array->AddrCount; i++) { + ip4_array->AddrArray[i] = array.addr[i]; + } + + return ip4_array; +} + struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone, struct dnsserver_serverinfo *serverinfo) { @@ -309,20 +349,29 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone, prop->aging_enabled; break; case DSPROPERTY_ZONE_SCAVENGING_SERVERS: - zoneinfo->aipScavengeServers->AddrCount = - prop->servers.addrCount; - zoneinfo->aipScavengeServers->AddrArray = - prop->servers.addr; + zoneinfo->aipScavengeServers = NULL; + zoneinfo->aipScavengeServers = + copy_ip4_array(zoneinfo, + "ZONE_SCAVENGING_SERVERS", + prop->servers); + if (zoneinfo->aipScavengeServers == NULL) { + TALLOC_FREE(zoneinfo); + return NULL; + } break; case DSPROPERTY_ZONE_AGING_ENABLED_TIME: zoneinfo->dwAvailForScavengeTime = prop->next_scavenging_cycle_hours; break; case DSPROPERTY_ZONE_MASTER_SERVERS: - zoneinfo->aipLocalMasters->AddrCount = - prop->master_servers.addrCount; - zoneinfo->aipLocalMasters->AddrArray = - prop->master_servers.addr; + zoneinfo->aipLocalMasters = + copy_ip4_array(zoneinfo, + "ZONE_MASTER_SERVERS", + prop->master_servers); + if (zoneinfo->aipLocalMasters == NULL) { + TALLOC_FREE(zoneinfo); + return NULL; + } break; case DSPROPERTY_ZONE_EMPTY: case DSPROPERTY_ZONE_SECURE_TIME: -- 2.11.0 From b1305f9279cae9428d14d7e8940fec1413ca5af5 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Mon, 5 Nov 2018 16:18:18 +1300 Subject: [PATCH 4/7] 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 b5251e3623e..bc2f54bc146 100644 --- a/source4/ldap_server/ldap_server.c +++ b/source4/ldap_server/ldap_server.c @@ -690,13 +690,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 From 48ec1aa95ae9063a2643a4534d1c06bf00456d3b Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 23 Oct 2018 17:33:46 +1300 Subject: [PATCH 5/7] 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 bb428e45cbc..f4f3ba63299 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -2610,10 +2610,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 6c945010a2a81de3858be32ecbfc54282c6f1561 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 24 Oct 2018 15:41:28 +1300 Subject: [PATCH 6/7] 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 Reviewed-by: Gary Lockyer --- 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 e32ffaab283aeeef9cf3422119f9fc89fd81da21 Mon Sep 17 00:00:00 2001 From: Aaron Haslett Date: Tue, 23 Oct 2018 17:25:51 +1300 Subject: [PATCH 7/7] 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 | 22 ++++++++++++++++++++++ selftest/knownfail.d/dns | 6 ++++++ source4/dns_server/dns_query.c | 6 ++++++ 3 files changed, 34 insertions(+) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index 6771e3bb8c4..3e6306e2be8 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -844,6 +844,28 @@ 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): diff --git a/selftest/knownfail.d/dns b/selftest/knownfail.d/dns index a5176654cc2..a248432aafa 100644 --- a/selftest/knownfail.d/dns +++ b/selftest/knownfail.d/dns @@ -69,3 +69,9 @@ samba.tests.dns.__main__.TestSimpleQueries.test_qtype_all_query\(rodc:local\) # The SOA override should not pass against the RODC, it must not overstamp 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 +^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 923f7233eb9..65faeac3b6a 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