The Samba-Bugzilla – Attachment 14637 Details for
Bug 13663
[SECURITY] Upcoming 2018 AD Security release
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
combined patch for master (v9) excluding CVE-2018-16857
2018-11-master-CVEs.patch (text/plain), 44.86 KB, created by
Andrew Bartlett
on 2018-11-08 02:04:04 UTC
(
hide
)
Description:
combined patch for master (v9) excluding CVE-2018-16857
Filename:
MIME Type:
Creator:
Andrew Bartlett
Created:
2018-11-08 02:04:04 UTC
Size:
44.86 KB
patch
obsolete
>From 14ae9d8b32c65420b592ff41be690b62967d088a Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Tue, 6 Nov 2018 13:32:05 +1300 >Subject: [PATCH 01/10] CVE-2018-16853 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 <abartlet@samba.org> >Reviewed-by: Gary Lockyer <gary@catalyst.net.nz> >--- > wscript | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > >diff --git a/wscript b/wscript >index c5d8e5bdd7d..e9d8a834aa2 100644 >--- a/wscript >+++ b/wscript >@@ -55,6 +55,14 @@ def 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", >@@ -214,7 +222,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 714e89438d19df310cae3ab53bcb7e05f3fe9786 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Tue, 6 Nov 2018 13:40:48 +1300 >Subject: [PATCH 02/10] CVE-2018-16853 WHATSNEW: The Samba AD DC, when build > with MIT Kerberos is experimental > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13678 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Gary Lockyer <gary@catalyst.net.nz> >--- > WHATSNEW.txt | 10 ++++++++++ > 1 file changed, 10 insertions(+) > >diff --git a/WHATSNEW.txt b/WHATSNEW.txt >index bdc3df78b23..72889c61f2f 100644 >--- a/WHATSNEW.txt >+++ b/WHATSNEW.txt >@@ -31,6 +31,16 @@ the backup and restore to account for the change in domain). > REMOVED FEATURES > ================ > >+MIT Kerberos build of the AD DC >+------------------------------- >+ >+While not removed, the MIT Kerberos build of the Samba AD DC is still >+considered experimental. Because Samba will not issue security >+patches for this configuration, such builds now require the explicit >+configure option: --with-experimental-mit-ad-dc >+ >+For further details see >+https://wiki.samba.org/index.php/Running_a_Samba_AD_DC_with_MIT_Kerberos_KDC > > smb.conf changes > ================ >-- >2.11.0 > > >From 7efd07e29848f06ac9473d36c50c4476bfaae7bb Mon Sep 17 00:00:00 2001 >From: Gary Lockyer <gary@catalyst.net.nz> >Date: Tue, 6 Nov 2018 12:10:07 +1300 >Subject: [PATCH 03/10] 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 > >Signed-off-by: Gary Lockyer <gary@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >--- > 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 <http://www.gnu.org/licenses/>. >+ * >+ */ >+ >+/* >+ * from cmocka.c: >+ * These headers or their equivalents should be included prior to >+ * including >+ * this header file. >+ * >+ * #include <stdarg.h> >+ * #include <stddef.h> >+ * #include <setjmp.h> >+ * >+ * This allows test applications to use custom definitions of C standard >+ * library functions and types. >+ * >+ */ >+ >+#include <stdarg.h> >+#include <stddef.h> >+#include <setjmp.h> >+#include <cmocka.h> >+ >+ >+#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 24817e40fb5..d1289a5c516 100755 >--- a/source4/selftest/tests.py >+++ b/source4/selftest/tests.py >@@ -1231,3 +1231,5 @@ plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit", "none", > [os.path.join(bindir(), "test_group_audit")]) > plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit.errors", "none", > [os.path.join(bindir(), "test_group_audit_errors")]) >+plantestsuite("samba4.dcerpc.dnsserver.dnsutils", "none", >+ [os.path.join(bindir(), "test_rpc_dns_server_dnsutils")]) >-- >2.11.0 > > >From 16a74cc2eb9f5ef5043fd5d54ddb961f18bd19b5 Mon Sep 17 00:00:00 2001 >From: Gary Lockyer <gary@catalyst.net.nz> >Date: Tue, 6 Nov 2018 12:16:30 +1300 >Subject: [PATCH 04/10] 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 > >Signed-off-by: Gary Lockyer <gary@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >--- > selftest/knownfail.d/bug13669 | 4 --- > source4/rpc_server/dnsserver/dnsutils.c | 64 ++++++++++++++++++++++++++++----- > 2 files changed, 56 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 b3d8949f8ab..982b13bc2ac 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,28 @@ 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 = >+ 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 1424bd6ac877f5abb49ef40c6def2cd156bcdd53 Mon Sep 17 00:00:00 2001 >From: Gary Lockyer <gary@catalyst.net.nz> >Date: Wed, 7 Nov 2018 15:08:04 +1300 >Subject: [PATCH 05/10] CVE-2018-16852 dcerpc dnsserver: refactor common > properties handling > >dnsserver_common.c and dnsutils.c both share similar code to process >zone properties. This patch extracts the common code and moves it to >dnsserver_common.c. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13669 > >Signed-off-by: Gary Lockyer <gary@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >--- > source4/dns_server/dnsserver_common.c | 129 +++++++++++++++++++++++--------- > source4/dns_server/dnsserver_common.h | 3 + > source4/rpc_server/dnsserver/dnsutils.c | 107 ++------------------------ > 3 files changed, 104 insertions(+), 135 deletions(-) > >diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c >index 1a032b4aa9f..656d7ca6bff 100644 >--- a/source4/dns_server/dnsserver_common.c >+++ b/source4/dns_server/dnsserver_common.c >@@ -742,6 +742,94 @@ bool dns_name_is_static(struct dnsp_DnssrvRpcRecord *records, > return false; > } > >+/* >+ * 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; >+} >+ >+bool dns_zoneinfo_load_zone_property(struct dnsserver_zoneinfo *zoneinfo, >+ struct dnsp_DnsProperty *prop) >+{ >+ switch (prop->id) { >+ case DSPROPERTY_ZONE_TYPE: >+ zoneinfo->dwZoneType = prop->data.zone_type; >+ break; >+ case DSPROPERTY_ZONE_ALLOW_UPDATE: >+ zoneinfo->fAllowUpdate = prop->data.allow_update_flag; >+ break; >+ case DSPROPERTY_ZONE_NOREFRESH_INTERVAL: >+ zoneinfo->dwNoRefreshInterval = prop->data.norefresh_hours; >+ break; >+ case DSPROPERTY_ZONE_REFRESH_INTERVAL: >+ zoneinfo->dwRefreshInterval = prop->data.refresh_hours; >+ break; >+ case DSPROPERTY_ZONE_AGING_STATE: >+ zoneinfo->fAging = prop->data.aging_enabled; >+ break; >+ case DSPROPERTY_ZONE_SCAVENGING_SERVERS: >+ zoneinfo->aipScavengeServers = copy_ip4_array( >+ zoneinfo, "ZONE_SCAVENGING_SERVERS", prop->data.servers); >+ if (zoneinfo->aipScavengeServers == NULL) { >+ return false; >+ } >+ break; >+ case DSPROPERTY_ZONE_AGING_ENABLED_TIME: >+ zoneinfo->dwAvailForScavengeTime = >+ prop->data.next_scavenging_cycle_hours; >+ break; >+ case DSPROPERTY_ZONE_MASTER_SERVERS: >+ zoneinfo->aipLocalMasters = copy_ip4_array( >+ zoneinfo, "ZONE_MASTER_SERVERS", prop->data.master_servers); >+ if (zoneinfo->aipLocalMasters == NULL) { >+ return false; >+ } >+ break; >+ case DSPROPERTY_ZONE_EMPTY: >+ case DSPROPERTY_ZONE_SECURE_TIME: >+ case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME: >+ case DSPROPERTY_ZONE_AUTO_NS_SERVERS: >+ case DSPROPERTY_ZONE_DCPROMO_CONVERT: >+ case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA: >+ case DSPROPERTY_ZONE_MASTER_SERVERS_DA: >+ case DSPROPERTY_ZONE_NS_SERVERS_DA: >+ case DSPROPERTY_ZONE_NODE_DBFLAGS: >+ break; >+ } >+ return true; >+} > WERROR dns_get_zone_properties(struct ldb_context *samdb, > TALLOC_CTX *mem_ctx, > struct ldb_dn *zone_dn, >@@ -774,6 +862,7 @@ WERROR dns_get_zone_properties(struct ldb_context *samdb, > } > > for (i = 0; i < element->num_values; i++) { >+ bool valid_property; > prop = talloc_zero(mem_ctx, struct dnsp_DnsProperty); > if (prop == NULL) { > return WERR_NOT_ENOUGH_MEMORY; >@@ -787,42 +876,10 @@ WERROR dns_get_zone_properties(struct ldb_context *samdb, > return DNS_ERR(SERVER_FAILURE); > } > >- switch (prop->id) { >- case DSPROPERTY_ZONE_AGING_STATE: >- zoneinfo->fAging = prop->data.aging_enabled; >- break; >- case DSPROPERTY_ZONE_NOREFRESH_INTERVAL: >- zoneinfo->dwNoRefreshInterval = >- prop->data.norefresh_hours; >- break; >- case DSPROPERTY_ZONE_REFRESH_INTERVAL: >- zoneinfo->dwRefreshInterval = prop->data.refresh_hours; >- break; >- case DSPROPERTY_ZONE_ALLOW_UPDATE: >- zoneinfo->fAllowUpdate = prop->data.allow_update_flag; >- break; >- case DSPROPERTY_ZONE_AGING_ENABLED_TIME: >- zoneinfo->dwAvailForScavengeTime = >- prop->data.next_scavenging_cycle_hours; >- break; >- case DSPROPERTY_ZONE_SCAVENGING_SERVERS: >- zoneinfo->aipScavengeServers->AddrCount = >- prop->data.servers.addrCount; >- zoneinfo->aipScavengeServers->AddrArray = >- prop->data.servers.addr; >- break; >- case DSPROPERTY_ZONE_EMPTY: >- case DSPROPERTY_ZONE_TYPE: >- case DSPROPERTY_ZONE_SECURE_TIME: >- case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME: >- case DSPROPERTY_ZONE_MASTER_SERVERS: >- case DSPROPERTY_ZONE_AUTO_NS_SERVERS: >- case DSPROPERTY_ZONE_DCPROMO_CONVERT: >- case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA: >- case DSPROPERTY_ZONE_MASTER_SERVERS_DA: >- case DSPROPERTY_ZONE_NS_SERVERS_DA: >- case DSPROPERTY_ZONE_NODE_DBFLAGS: >- break; >+ valid_property = >+ dns_zoneinfo_load_zone_property(zoneinfo, prop); >+ if (!valid_property) { >+ return DNS_ERR(SERVER_FAILURE); > } > } > >diff --git a/source4/dns_server/dnsserver_common.h b/source4/dns_server/dnsserver_common.h >index 380f61b8dbc..60ecde4fa91 100644 >--- a/source4/dns_server/dnsserver_common.h >+++ b/source4/dns_server/dnsserver_common.h >@@ -87,4 +87,7 @@ NTSTATUS dns_common_zones(struct ldb_context *samdb, > TALLOC_CTX *mem_ctx, > struct ldb_dn *base_dn, > struct dns_server_zone **zones_ret); >+ >+bool dns_zoneinfo_load_zone_property(struct dnsserver_zoneinfo *zoneinfo, >+ struct dnsp_DnsProperty *prop); > #endif /* __DNSSERVER_COMMON_H__ */ >diff --git a/source4/rpc_server/dnsserver/dnsutils.c b/source4/rpc_server/dnsserver/dnsutils.c >index 982b13bc2ac..ea1eca596a1 100644 >--- a/source4/rpc_server/dnsserver/dnsutils.c >+++ b/source4/rpc_server/dnsserver/dnsutils.c >@@ -25,6 +25,7 @@ > #include "dsdb/samdb/samdb.h" > #include "lib/socket/netif.h" > #include "lib/util/util_net.h" >+#include "dnsserver_common.h" > > static struct DNS_ADDR_ARRAY *fill_dns_addr_array(TALLOC_CTX *mem_ctx, > struct loadparm_context *lp_ctx, >@@ -208,47 +209,6 @@ struct dnsserver_serverinfo *dnsserver_init_serverinfo(TALLOC_CTX *mem_ctx, > return serverinfo; > } > >- >-/* >- * 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) > { >@@ -257,8 +217,7 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone, > const char *revzone = "in-addr.arpa"; > const char *revzone6 = "ip6.arpa"; > int len1, len2; >- union dnsPropertyData *prop = NULL; >- int i=0; >+ unsigned int i = 0; > > zoneinfo = talloc_zero(zone, struct dnsserver_zoneinfo); > if (zoneinfo == NULL) { >@@ -326,62 +285,12 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone, > zoneinfo->dwLastXfrResult = 0; > > for(i=0; i<zone->num_props; i++){ >- prop=&(zone->tmp_props[i].data); >- switch (zone->tmp_props[i].id) { >- case DSPROPERTY_ZONE_TYPE: >- zoneinfo->dwZoneType = >- prop->zone_type; >- break; >- case DSPROPERTY_ZONE_ALLOW_UPDATE: >- zoneinfo->fAllowUpdate = >- prop->allow_update_flag; >- break; >- case DSPROPERTY_ZONE_NOREFRESH_INTERVAL: >- zoneinfo->dwNoRefreshInterval = >- prop->norefresh_hours; >- break; >- case DSPROPERTY_ZONE_REFRESH_INTERVAL: >- zoneinfo->dwRefreshInterval = >- prop->refresh_hours; >- break; >- case DSPROPERTY_ZONE_AGING_STATE: >- zoneinfo->fAging = >- prop->aging_enabled; >- break; >- case DSPROPERTY_ZONE_SCAVENGING_SERVERS: >- 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 = >- 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: >- case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME: >- case DSPROPERTY_ZONE_AUTO_NS_SERVERS: >- case DSPROPERTY_ZONE_DCPROMO_CONVERT: >- case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA: >- case DSPROPERTY_ZONE_MASTER_SERVERS_DA: >- case DSPROPERTY_ZONE_NS_SERVERS_DA: >- case DSPROPERTY_ZONE_NODE_DBFLAGS: >- break; >+ bool valid_property; >+ valid_property = dns_zoneinfo_load_zone_property( >+ zoneinfo, &zone->tmp_props[i]); >+ if (!valid_property) { >+ TALLOC_FREE(zoneinfo); >+ return NULL; > } > } > >-- >2.11.0 > > >From 98ef713804a558094a778bb296ac7ca9e6109448 Mon Sep 17 00:00:00 2001 >From: Garming Sam <garming@catalyst.net.nz> >Date: Mon, 5 Nov 2018 16:18:18 +1300 >Subject: [PATCH 06/10] 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 <garming@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >--- > 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 c175ee0c4aab7c545c5e66edb9a20e66b1f241ab Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Tue, 23 Oct 2018 17:33:46 +1300 >Subject: [PATCH 07/10] 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 <abartlet@samba.org> >Reviewed-by: Gary Lockyer <gary@catalyst.net.nz> >--- > 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 acd24ec0c83..969f4f6b556 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 db786f5fdaae0acbfbb3fd4a832c9c13c6b9ac1e Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Wed, 24 Oct 2018 15:41:28 +1300 >Subject: [PATCH 08/10] 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 <abartlet@samba.org> >Reviewed-by: Gary Lockyer <gary@catalyst.net.nz> >--- > 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 3f511e05aa80ec9cbadaef6bdd9bca16073ab280 Mon Sep 17 00:00:00 2001 >From: Aaron Haslett <aaronhaslett@catalyst.net.nz> >Date: Tue, 23 Oct 2018 11:52:07 +1300 >Subject: [PATCH 09/10] dns: prevent self-referencing CNAME > >Stops the user from adding a self-referencing CNAME over RPC, which is an easy >mistake to make with samba-tool. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13600 > >Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Garming Sam <garming@catalyst.net.nz> >--- > python/samba/tests/dns.py | 44 +++++++++++++++++++++++++ > selftest/knownfail.d/dns | 1 + > source4/rpc_server/dnsserver/dcerpc_dnsserver.c | 39 ++++++++++++++++++++++ > 3 files changed, 84 insertions(+) > >diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py >index 12cfb86c254..56609769a4f 100644 >--- a/python/samba/tests/dns.py >+++ b/python/samba/tests/dns.py >@@ -1481,6 +1481,50 @@ class TestRPCRoundtrip(DNSTest): > def tearDown(self): > super(TestRPCRoundtrip, self).tearDown() > >+ def rpc_update(self, fqn=None, data=None, wType=None, delete=False): >+ fqn = fqn or ("rpctestrec." + self.get_dns_domain()) >+ >+ rec = data_to_dns_record(wType, data) >+ add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF() >+ add_rec_buf.rec = rec >+ >+ add_arg = add_rec_buf >+ del_arg = None >+ if delete: >+ add_arg = None >+ del_arg = add_rec_buf >+ >+ self.rpc_conn.DnssrvUpdateRecord2( >+ dnsserver.DNS_CLIENT_VERSION_LONGHORN, >+ 0, >+ self.server_ip, >+ self.get_dns_domain(), >+ fqn, >+ add_arg, >+ del_arg) >+ >+ def test_rpc_self_referencing_cname(self): >+ cname = "cnametest2_unqual_rec_loop" >+ cname_fqn = "%s.%s" % (cname, self.get_dns_domain()) >+ >+ try: >+ self.rpc_update(fqn=cname, data=cname_fqn, >+ wType=dnsp.DNS_TYPE_CNAME, delete=True) >+ except WERRORError as e: >+ if e.args[0] != werror.WERR_DNS_ERROR_RECORD_DOES_NOT_EXIST: >+ self.fail("RPC DNS gaven wrong error on pre-test cleanup " >+ "for self referencing CNAME: %s" % e.args[0]) >+ >+ try: >+ self.rpc_update(fqn=cname, wType=dnsp.DNS_TYPE_CNAME, data=cname_fqn) >+ except WERRORError as e: >+ if e.args[0] != werror.WERR_DNS_ERROR_CNAME_LOOP: >+ self.fail("RPC DNS gaven wrong error on insertion of " >+ "self referencing CNAME: %s" % e.args[0]) >+ return >+ >+ self.fail("RPC DNS allowed insertion of self referencing CNAME") >+ > def test_update_add_txt_rpc_to_dns(self): > prefix, txt = 'rpctextrec', ['"This is a test"'] > >diff --git a/selftest/knownfail.d/dns b/selftest/knownfail.d/dns >index ca18b4334c1..0f6caba8916 100644 >--- a/selftest/knownfail.d/dns >+++ b/selftest/knownfail.d/dns >@@ -13,6 +13,7 @@ samba.tests.dns.__main__.TestRPCRoundtrip.test_update_add_null_char_txt_record\( > samba.tests.dns.__main__.TestRPCRoundtrip.test_update_add_null_padded_txt_record\(rodc:local\) > samba.tests.dns.__main__.TestRPCRoundtrip.test_update_add_slash_txt_record\(rodc:local\) > samba.tests.dns.__main__.TestRPCRoundtrip.test_update_add_two_txt_records\(rodc:local\) >+samba.tests.dns.__main__.TestRPCRoundtrip.test_rpc_self_referencing_cname\(rodc:local\) > samba.tests.dns.__main__.TestDNSUpdates.test_delete_record\(vampire_dc:local\) > samba.tests.dns.__main__.TestDNSUpdates.test_readd_record\(vampire_dc:local\) > samba.tests.dns.__main__.TestDNSUpdates.test_update_add_mx_record\(vampire_dc:local\) >diff --git a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c >index b42d7c549d1..edea2b33f2d 100644 >--- a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c >+++ b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c >@@ -25,6 +25,7 @@ > #include "dsdb/samdb/samdb.h" > #include "lib/util/dlinklist.h" > #include "librpc/gen_ndr/ndr_dnsserver.h" >+#include "dns_server/dnsserver_common.h" > #include "dnsserver.h" > > #define DCESRV_INTERFACE_DNSSERVER_BIND(call, iface) \ >@@ -1868,6 +1869,37 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, > return WERR_OK; > } > >+/* >+ * Check str1 + '.' + str2 = name, for example: >+ * ("dc0", "example.com", "dc0.example.com") = true >+ */ >+static bool cname_self_reference(const char* node_name, >+ const char* zone_name, >+ struct DNS_RPC_NAME name) { >+ size_t node_len, zone_len; >+ >+ if (node_name == NULL || zone_name == NULL) { >+ return false; >+ } >+ >+ node_len = strlen(node_name); >+ zone_len = strlen(zone_name); >+ >+ if (node_len == 0 || >+ zone_len == 0 || >+ (name.len != node_len + zone_len + 1)) { >+ return false; >+ } >+ >+ if (strncmp(node_name, name.str, node_len) == 0 && >+ name.str[node_len] == '.' && >+ strncmp(zone_name, name.str + node_len + 1, zone_len) == 0) { >+ return true; >+ } >+ >+ return false; >+} >+ > /* dnsserver update function */ > > static WERROR dnsserver_update_record(struct dnsserver_state *dsstate, >@@ -1895,6 +1927,13 @@ static WERROR dnsserver_update_record(struct dnsserver_state *dsstate, > } > W_ERROR_HAVE_NO_MEMORY_AND_FREE(name, tmp_ctx); > >+ /* CNAMEs can't point to themselves */ >+ if (add_buf != NULL && add_buf->rec.wType == DNS_TYPE_CNAME) { >+ if (cname_self_reference(node_name, z->name, add_buf->rec.data.name)) { >+ return WERR_DNS_ERROR_CNAME_LOOP; >+ } >+ } >+ > if (add_buf != NULL) { > if (del_buf == NULL) { > /* Add record */ >-- >2.11.0 > > >From cb3d3462d7520dabe0b75feb3fc7e1038c011d75 Mon Sep 17 00:00:00 2001 >From: Aaron Haslett <aaronhaslett@catalyst.net.nz> >Date: Tue, 23 Oct 2018 17:25:51 +1300 >Subject: [PATCH 10/10] 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 <aaronhaslett@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Garming Sam <garming@catalyst.net.nz> >--- > 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 56609769a4f..e35cded7b8c 100644 >--- a/python/samba/tests/dns.py >+++ b/python/samba/tests/dns.py >@@ -846,6 +846,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 0f6caba8916..39b337ed6bb 100644 >--- a/selftest/knownfail.d/dns >+++ b/selftest/knownfail.d/dns >@@ -71,3 +71,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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
dbagnall
:
review+
abartlet
:
review+
Actions:
View
Attachments on
bug 13663
:
14580
|
14581
|
14582
|
14583
|
14585
|
14586
|
14587
|
14588
|
14589
|
14592
|
14593
|
14601
|
14602
|
14608
|
14609
|
14610
|
14611
|
14612
|
14618
|
14619
|
14620
|
14621
|
14622
| 14637 |
14638