>From b0702838dce352728f913d822fb8267b474badac Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 16 Sep 2015 14:17:25 +1200 Subject: [PATCH 01/15] samba-tool sites: use -H to set URL with standard handling samba-tool sites was defaulting to the local database, but we might want to use another URL. This allows that case while defaulting to the old behaviour. Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- python/samba/netcmd/sites.py | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/python/samba/netcmd/sites.py b/python/samba/netcmd/sites.py index 09df55e..8551d35 100644 --- a/python/samba/netcmd/sites.py +++ b/python/samba/netcmd/sites.py @@ -24,7 +24,8 @@ from samba.auth import system_session from samba.netcmd import ( Command, CommandError, - SuperCommand + SuperCommand, + Option, ) @@ -41,14 +42,16 @@ class cmd_sites_create(Command): "credopts": options.CredentialsOptions, } - def run(self, sitename, sambaopts=None, credopts=None, versionopts=None): + takes_options = [ + Option("-H", "--URL", help="LDB URL for database or target server", + type=str, metavar="URL", dest="H"), + ] + + def run(self, sitename, H=None, sambaopts=None, credopts=None, + versionopts=None): lp = sambaopts.get_loadparm() creds = credopts.get_credentials(lp, fallback_machine=True) - url = lp.private_path("sam.ldb") - - if not os.path.exists(url): - raise CommandError("secret database not found at %s " % url) - samdb = SamDB(url=url, session_info=system_session(), + samdb = SamDB(url=H, session_info=system_session(), credentials=creds, lp=lp) samdb.transaction_start() @@ -74,15 +77,17 @@ class cmd_sites_delete(Command): "credopts": options.CredentialsOptions, } - def run(self, sitename, sambaopts=None, credopts=None, versionopts=None): + takes_options = [ + Option("-H", "--URL", help="LDB URL for database or target server", + type=str, metavar="URL", dest="H"), + ] + + def run(self, sitename, H=None, sambaopts=None, credopts=None, + versionopts=None): lp = sambaopts.get_loadparm() creds = credopts.get_credentials(lp, fallback_machine=True) - url = lp.private_path("sam.ldb") - - if not os.path.exists(url): - raise CommandError("secret database not found at %s " % url) - samdb = SamDB(url=url, session_info=system_session(), - credentials=creds, lp=lp) + samdb = SamDB(url=H, session_info=system_session(), + credentials=creds, lp=lp) samdb.transaction_start() try: -- 2.1.4 >From b3744041503543408f59bc58f2762a52fac43f55 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 17 Sep 2015 11:35:55 +1200 Subject: [PATCH 02/15] dsdb.tests.sites: merge interdependent tests The delete test deleted the site made by the create test, which worked because "delete" sorts after "create" alphabetically. By themselves, "delete" would fail and "create" would neglect its duty to clean up. This would be an issue if the order of tests changes, if one of the tests is not run, or if another test appears in between. Everything is fine if they give up the pretense of independence. Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- source4/dsdb/tests/python/sites.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/tests/python/sites.py b/source4/dsdb/tests/python/sites.py index 402d676..4fff77e 100755 --- a/source4/dsdb/tests/python/sites.py +++ b/source4/dsdb/tests/python/sites.py @@ -81,8 +81,8 @@ class SitesBaseTests(samba.tests.TestCase): #tests on sites class SimpleSitesTests(SitesBaseTests): - def test_create(self): - """test creation of 1 site""" + def test_create_and_delete(self): + """test creation and deletion of 1 site""" self.ldb_admin.transaction_start() ok = sites.create_site(self.ldb_admin, self.ldb_admin.get_config_basedn(), @@ -93,9 +93,6 @@ class SimpleSitesTests(SitesBaseTests): sites.create_site, self.ldb_admin, self.ldb_admin.get_config_basedn(), "testsamba") - def test_delete(self): - """test removal of 1 site""" - self.ldb_admin.transaction_start() ok = sites.delete_site(self.ldb_admin, self.ldb_admin.get_config_basedn(), "testsamba") -- 2.1.4 >From e7383afbfd04f95dbf584759a6072ed4b7c6447e Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 17 Sep 2015 18:05:01 +1200 Subject: [PATCH 03/15] samba-tool: add sites subnet subcommands This allows you to add, remove, or shift subnets. Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- python/samba/netcmd/sites.py | 121 +++++++++++++++++++++++++++++++++++++- python/samba/subnets.py | 136 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 python/samba/subnets.py diff --git a/python/samba/netcmd/sites.py b/python/samba/netcmd/sites.py index 8551d35..0baf306 100644 --- a/python/samba/netcmd/sites.py +++ b/python/samba/netcmd/sites.py @@ -17,7 +17,7 @@ # import os -from samba import sites +from samba import sites, subnets from samba.samdb import SamDB import samba.getopt as options from samba.auth import system_session @@ -101,10 +101,127 @@ class cmd_sites_delete(Command): self.outf.write("Site %s removed!\n" % sitename) +class cmd_sites_subnet_create(Command): + """Create a new subnet.""" + synopsis = "%prog [options]" + takes_args = ["subnetname", "site_of_subnet"] + + takes_optiongroups = { + "sambaopts": options.SambaOptions, + "versionopts": options.VersionOptions, + "credopts": options.CredentialsOptions, + } + + takes_options = [ + Option("-H", "--URL", help="LDB URL for database or target server", + type=str, metavar="URL", dest="H"), + ] + + def run(self, subnetname, site_of_subnet, H=None, sambaopts=None, + credopts=None, versionopts=None): + lp = sambaopts.get_loadparm() + creds = credopts.get_credentials(lp) + samdb = SamDB(url=H, session_info=system_session(), + credentials=creds, lp=lp) + + samdb.transaction_start() + try: + subnets.create_subnet(samdb, samdb.get_config_basedn(), subnetname, + site_of_subnet) + samdb.transaction_commit() + except subnets.SubnetException, e: + samdb.transaction_cancel() + raise CommandError("Error while creating subnet %s: %s" % + (subnetname, e)) + + self.outf.write("Subnet %s created !\n" % subnetname) + + +class cmd_sites_subnet_delete(Command): + """Delete an existing subnet.""" + + synopsis = "%prog [options]" + + takes_args = ["subnetname"] + + takes_optiongroups = { + "sambaopts": options.SambaOptions, + "versionopts": options.VersionOptions, + "credopts": options.CredentialsOptions, + } + + takes_options = [ + Option("-H", "--URL", help="LDB URL for database or target server", + type=str, metavar="URL", dest="H"), + ] + + def run(self, subnetname, H=None, sambaopts=None, credopts=None, + versionopts=None): + lp = sambaopts.get_loadparm() + creds = credopts.get_credentials(lp) + samdb = SamDB(url=H, session_info=system_session(), + credentials=creds, lp=lp) + + samdb.transaction_start() + try: + subnets.delete_subnet(samdb, samdb.get_config_basedn(), subnetname) + samdb.transaction_commit() + except subnets.SubnetException, e: + samdb.transaction_cancel() + raise CommandError("Error while removing subnet %s, error: %s" % + (subnetname, e)) + + self.outf.write("Subnet %s removed!\n" % subnetname) + + +class cmd_sites_subnet_set_site(Command): + """Assign a subnet to a site.""" + synopsis = "%prog [options]" + takes_args = ["subnetname", "site_of_subnet"] + + takes_optiongroups = { + "sambaopts": options.SambaOptions, + "versionopts": options.VersionOptions, + "credopts": options.CredentialsOptions, + } + + takes_options = [ + Option("-H", "--URL", help="LDB URL for database or target server", + type=str, metavar="URL", dest="H"), + ] + + def run(self, subnetname, site_of_subnet, H=None, sambaopts=None, + credopts=None, versionopts=None): + lp = sambaopts.get_loadparm() + creds = credopts.get_credentials(lp) + samdb = SamDB(url=H, session_info=system_session(), + credentials=creds, lp=lp) + + samdb.transaction_start() + try: + subnets.set_subnet_site(samdb, samdb.get_config_basedn(), + subnetname, site_of_subnet) + samdb.transaction_commit() + except subnets.SubnetException, e: + samdb.transaction_cancel() + raise CommandError("Error assigning subnet %s to site %s: %s" % + (subnetname, site_of_subnet, e)) + + print >> self.outf, ("Subnet %s shifted to site %s" % + (subnet_name, site_of_subnet)) + + +class cmd_sites_subnet(SuperCommand): + """Subnet management subcommands.""" + subcommands = { + "create": cmd_sites_subnet_create(), + "remove": cmd_sites_subnet_delete(), + "set-site": cmd_sites_subnet_set_site(), + } class cmd_sites(SuperCommand): """Sites management.""" - subcommands = {} subcommands["create"] = cmd_sites_create() subcommands["remove"] = cmd_sites_delete() + subcommands["subnet"] = cmd_sites_subnet() diff --git a/python/samba/subnets.py b/python/samba/subnets.py new file mode 100644 index 0000000..cd9639c --- /dev/null +++ b/python/samba/subnets.py @@ -0,0 +1,136 @@ +# Add/remove subnets to sites. +# +# Copyright Douglas Bagnall 2015 +# Copyright Matthieu Patou 2011 +# +# 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 . +# + +import ldb +from ldb import FLAG_MOD_ADD, FLAG_MOD_REPLACE + + +class SubnetException(Exception): + """Base element for Subnet errors""" + pass + + +class SubnetNotFound(SubnetException): + """The subnet requested does not exist.""" + pass + + +class SubnetAlreadyExists(SubnetException): + """The subnet being added already exists.""" + pass + + +class SiteNotFound(SubnetException): + """The site to be used for the subnet does not exist.""" + pass + + +def create_subnet(samdb, configDn, subnet_name, site_name): + """Create a subnet and associate it with a site. + + :param samdb: A samdb connection + :param configDn: The DN of the configuration partition + :param subnet_name: name of the subnet to create (a CIDR range) + :return: None + :raise SubnetAlreadyExists: if the subnet to be created already exists. + :raise SiteNotFound: if the site does not exist. + """ + # Subnet collisions are checked by exact match only, not + # overlapping range. This won't stop you creating 10.1.1.0/24 when + # there is already 10.1.0.0/16. + ret = samdb.search(base=configDn, scope=ldb.SCOPE_SUBTREE, + expression=('(&(objectclass=subnet)(cn=%s))' % + subnet_name)) + if len(ret) != 0: + raise SubnetAlreadyExists('A subnet with the CIDR %s already exists' + % subnet_name) + + ret = samdb.search(base=configDn, scope=ldb.SCOPE_SUBTREE, + expression='(&(objectclass=Site)(cn=%s))' % site_name) + if len(ret) != 1: + raise SiteNotFound('A site with the name %s does not exist' % + site_name) + + siteDn = str(ret[0].dn) + + m = ldb.Message() + m.dn = ldb.Dn(samdb, "Cn=%s,CN=Subnets,CN=Sites,%s" % (subnet_name, + configDn)) + m["objectclass"] = ldb.MessageElement("subnet", FLAG_MOD_ADD, + "objectclass") + m["siteOjbect"] = ldb.MessageElement(siteDn, FLAG_MOD_ADD, "siteObject") + samdb.add(m) + + +def delete_subnet(samdb, configDn, subnet_name): + """Delete a subnet. + + :param samdb: A samdb connection + :param configDn: The DN of the configuration partition + :param subnet_name: Name of the subnet to delete + :return: None + :raise SubnetNotFound: if the subnet to be deleted does not exist. + """ + dnsubnets = ldb.Dn(samdb, "CN=Subnets,CN=Sites,%s" % configDn) + dnsubnet = ldb.Dn(samdb, "Cn=%s,CN=Subnets,CN=Sites,%s" % (subnet_name, + configDn)) + + ret = samdb.search(base=dnsubnets, scope=ldb.SCOPE_ONELEVEL, + expression='(dn=%s)' % dnsubnet) + if len(ret) != 1: + raise SubnetNotFound('Subnet %s does not exist' % subnet_name) + + samdb.delete(dnsubnet, ["tree_delete:0"]) + + +def set_subnet_site(samdb, configDn, subnet_name, site_name): + """Assign a subnet to a site. + + This dissociates the subnet from its previous site. + + :param samdb: A samdb connection + :param configDn: The DN of the configuration partition + :param subnet_name: Name of the subnet + :param site_name: Name of the site + :return: None + :raise SubnetNotFound: if the subnet does not exist. + :raise SiteNotFound: if the site does not exist. + """ + dnsubnets = ldb.Dn(samdb, "CN=Subnets,CN=Sites,%s" % (configDn,)) + dnsubnet = ldb.Dn(samdb, "Cn=%s,CN=Subnets,CN=Sites,%s" % (subnet_name, + configDn)) + + ret = samdb.search(base=dnsubnets, scope=ldb.SCOPE_ONELEVEL, + expression='(dn=%s)' % dnsubnet) + if len(ret) != 1: + raise SubnetNotFound('The subnet %r does not exist' % subnet_name) + + ret = samdb.search(base=configDn, scope=ldb.SCOPE_SUBTREE, + expression='(&(objectclass=Site)(cn=%s))' % site_name) + if len(ret) != 1: + raise SiteNotFound('The site %r does not exist' % site_name) + + siteDn = str(ret[0].dn) + + m = ldb.Message() + m.dn = ldb.Dn(samdb, "Cn=%s,CN=Subnets,CN=Sites,%s" % (subnet_name, + configDn)) + m["siteObject"] = ldb.MessageElement(siteDn, FLAG_MOD_REPLACE, + "siteObject") + samdb.modify(m) -- 2.1.4 >From 22951a385e1179d828b35a34aa94cf95fa9af969 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 17 Sep 2015 18:07:32 +1200 Subject: [PATCH 04/15] samba.sites: reduce code duplication in Exception classes Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- python/samba/sites.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/python/samba/sites.py b/python/samba/sites.py index 76c57dd..119984a 100644 --- a/python/samba/sites.py +++ b/python/samba/sites.py @@ -28,35 +28,20 @@ class SiteException(Exception): self.value = value def __str__(self): - return "SiteException: " + self.value + return "%s: %s" % (self.__class__.__name__, self.value) class SiteNotFoundException(SiteException): """Raised when the site is not found and it's expected to exists.""" - def __init__(self, value): - self.value = value - - def __str__(self): - return "SiteNotFoundException: " + self.value class SiteAlreadyExistsException(SiteException): """Raised when the site is not found and it's expected not to exists.""" - def __init__(self, value): - self.value = value - - def __str__(self): - return "SiteAlreadyExists: " + self.value class SiteServerNotEmptyException(SiteException): """Raised when the site still has servers attached.""" - def __init__(self, value): - self.value = value - - def __str__(self): - return "SiteServerNotEmpty: " + self.value def create_site(samdb, configDn, siteName): """ -- 2.1.4 >From c45eb19eb263b2f33ab42577f5a2a66ac3b413a0 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 17 Sep 2015 18:10:03 +1200 Subject: [PATCH 05/15] samba.sites: improve grammar in an error message Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- python/samba/sites.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/samba/sites.py b/python/samba/sites.py index 119984a..3b18ff6 100644 --- a/python/samba/sites.py +++ b/python/samba/sites.py @@ -98,7 +98,7 @@ def delete_site(samdb, configDn, siteName): ret = samdb.search(base=dnsites, scope=ldb.SCOPE_ONELEVEL, expression='(dn=%s)' % str(dnsite)) if len(ret) != 1: - raise SiteNotFoundException('Site %s do not exists' % siteName) + raise SiteNotFoundException('Site %s does not exist' % siteName) ret = samdb.search(base=dnserver, scope=ldb.SCOPE_ONELEVEL, expression='(objectclass=server)') -- 2.1.4 >From fd070ef2b581d3c520e2d233aa4da17655e3af3b Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 17 Sep 2015 18:11:59 +1200 Subject: [PATCH 06/15] samba-tool sites: remove unused variables Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- python/samba/netcmd/sites.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/python/samba/netcmd/sites.py b/python/samba/netcmd/sites.py index 0baf306..671ec1e 100644 --- a/python/samba/netcmd/sites.py +++ b/python/samba/netcmd/sites.py @@ -16,7 +16,6 @@ # along with this program. If not, see . # -import os from samba import sites, subnets from samba.samdb import SamDB import samba.getopt as options @@ -56,7 +55,7 @@ class cmd_sites_create(Command): samdb.transaction_start() try: - ok = sites.create_site(samdb, samdb.get_config_basedn(), sitename) + sites.create_site(samdb, samdb.get_config_basedn(), sitename) samdb.transaction_commit() except sites.SiteAlreadyExistsException, e: samdb.transaction_cancel() @@ -91,7 +90,7 @@ class cmd_sites_delete(Command): samdb.transaction_start() try: - ok = sites.delete_site(samdb, samdb.get_config_basedn(), sitename) + sites.delete_site(samdb, samdb.get_config_basedn(), sitename) samdb.transaction_commit() except sites.SiteException, e: samdb.transaction_cancel() -- 2.1.4 >From 3d77d7764eecb8ab814afd5fa4d35a270cdebbda Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 17 Sep 2015 18:16:49 +1200 Subject: [PATCH 07/15] samba-tool tests: Add command line tests for sites and subnets Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- python/samba/tests/samba_tool/sites.py | 132 +++++++++++++++++++++++++++++++++ source4/selftest/tests.py | 2 + 2 files changed, 134 insertions(+) create mode 100644 python/samba/tests/samba_tool/sites.py diff --git a/python/samba/tests/samba_tool/sites.py b/python/samba/tests/samba_tool/sites.py new file mode 100644 index 0000000..31be9e0 --- /dev/null +++ b/python/samba/tests/samba_tool/sites.py @@ -0,0 +1,132 @@ +# Unix SMB/CIFS implementation. +# Copyright (C) Sean Dague 2011 +# +# 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 . +# + +import os +import ldb +from samba.tests.samba_tool.base import SambaToolCmdTest +from samba import sites + + +class BaseSitesCmdTestCase(SambaToolCmdTest): + """Tests for samba-tool sites subnets""" + def setUp(self): + super(BaseSitesCmdTestCase, self).setUp() + self.dburl = "ldap://%s" % os.environ["DC_SERVER"] + self.creds_string = "-U%s%%%s" % (os.environ["DC_USERNAME"], + os.environ["DC_PASSWORD"]) + + self.samdb = self.getSamDB("-H", self.dburl, self.creds_string) + self.config_dn = str(self.samdb.get_config_basedn()) + + +class SitesCmdTestCase(BaseSitesCmdTestCase): + + def test_site_create(self): + sitename = 'new_site' + + result, out, err = self.runsubcmd("sites", "create", sitename, + "-H", self.dburl, self.creds_string) + self.assertCmdSuccess(result) + + dnsites = ldb.Dn(self.samdb, "CN=Sites,%s" % self.config_dn) + dnsite = ldb.Dn(self.samdb, "CN=%s,%s" % (sitename, dnsites)) + + ret = self.samdb.search(base=dnsites, scope=ldb.SCOPE_ONELEVEL, + expression='(dn=%s)' % str(dnsite)) + self.assertEquals(len(ret), 1) + + # now delete it + self.samdb.delete(dnsite, ["tree_delete:0"]) + + +class SitesSubnetCmdTestCase(BaseSitesCmdTestCase): + def setUp(self): + super(SitesSubnetCmdTestCase, self).setUp() + self.sitename = "testsite" + self.sitename2 = "testsite2" + self.samdb.transaction_start() + sites.create_site(self.samdb, self.config_dn, self.sitename) + sites.create_site(self.samdb, self.config_dn, self.sitename2) + self.samdb.transaction_commit() + + def tearDown(self): + self.samdb.transaction_start() + sites.delete_site(self.samdb, self.config_dn, self.sitename) + sites.delete_site(self.samdb, self.config_dn, self.sitename2) + self.samdb.transaction_commit() + super(SitesSubnetCmdTestCase, self).tearDown() + + def test_site_subnet_create(self): + cidrs = (("10.9.8.0/24", self.sitename), + ("50.60.0.0/16", self.sitename2), + ("50.61.0.0/16", self.sitename2), # second subnet on the site + ("50.0.0.0/8", self.sitename), # overlapping subnet, other site + ("50.62.1.2/32", self.sitename), # single IP + ("aaaa:bbbb:cccc:dddd:eeee:ffff:2222:1100/120", + self.sitename2), + ) + + for cidr, sitename in cidrs: + result, out, err = self.runsubcmd("sites", "subnet", "create", + cidr, sitename, + "-H", self.dburl, + self.creds_string) + self.assertCmdSuccess(result) + + ret = self.samdb.search(base=self.config_dn, + scope=ldb.SCOPE_SUBTREE, + expression=('(&(objectclass=subnet)(cn=%s))' + % cidr)) + self.assertIsNotNone(ret) + self.assertEqual(len(ret), 1) + + dnsubnets = ldb.Dn(self.samdb, + "CN=Subnets,CN=Sites,%s" % self.config_dn) + + for cidr, sitename in cidrs: + dnsubnet = ldb.Dn(self.samdb, ("Cn=%s,CN=Subnets,CN=Sites,%s" % + (cidr, self.config_dn))) + + ret = self.samdb.search(base=dnsubnets, scope=ldb.SCOPE_ONELEVEL, + expression='(dn=%s)' % dnsubnet) + self.assertIsNotNone(ret) + self.assertEqual(len(ret), 1) + self.samdb.delete(dnsubnet, ["tree_delete:0"]) + + def test_site_subnet_create_should_fail(self): + cidrs = (("10.9.8.0/33", self.sitename), # mask too big + ("50.60.0.0/8", self.sitename2), # insufficient zeros + ("50.261.0.0/16", self.sitename2), # bad octet + ("7.0.0.0.0/0", self.sitename), # insufficient zeros + ("aaaa:bbbb:cccc:dddd:eeee:ffff:2222:1100/119", + self.sitename), # insufficient zeros + ) + + for cidr, sitename in cidrs: + result, out, err = self.runsubcmd("sites", "subnet", "create", + cidr, sitename, + "-H", self.dburl, + self.creds_string) + self.assertCmdFail(result) + + ret = self.samdb.search(base=self.config_dn, + scope=ldb.SCOPE_SUBTREE, + expression=('(&(objectclass=subnet)(cn=%s))' + % cidr)) + + self.assertIsNotNone(ret) + self.assertEqual(len(ret), 0) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 6c72c34..387fd53 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -478,6 +478,8 @@ planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.samba_tool.user") planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.samba_tool.group") planpythontestsuite("ad_dc:local", "samba.tests.samba_tool.ntacl") +planpythontestsuite("ad_dc:local", "samba.tests.samba_tool.sites") + planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.dcerpc.rpcecho") planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.dcerpc.registry", extra_args=['-U"$USERNAME%$PASSWORD"']) planoldpythontestsuite("ad_dc_ntvfs", "samba.tests.dcerpc.dnsserver", extra_args=['-U"$USERNAME%$PASSWORD"']) -- 2.1.4 >From 7bcfd8b0b4cf3ff0455e9aea66d5b7c5f7568a76 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 17 Sep 2015 18:28:09 +1200 Subject: [PATCH 08/15] dsdb.tests.sites: don't use global database, tidy long lines Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- source4/dsdb/tests/python/sites.py | 45 +++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/source4/dsdb/tests/python/sites.py b/source4/dsdb/tests/python/sites.py index 4fff77e..912dfc4 100755 --- a/source4/dsdb/tests/python/sites.py +++ b/source4/dsdb/tests/python/sites.py @@ -32,7 +32,7 @@ from samba.samdb import SamDB import samba.tests from samba.dcerpc import security -parser = optparse.OptionParser("dirsync.py [options] ") +parser = optparse.OptionParser(__file__ + " [options] ") sambaopts = options.SambaOptions(parser) parser.add_option_group(sambaopts) parser.add_option_group(options.VersionOptions(parser)) @@ -69,10 +69,11 @@ class SitesBaseTests(samba.tests.TestCase): def setUp(self): super(SitesBaseTests, self).setUp() - self.ldb_admin = ldb - self.base_dn = ldb.domain_dn() - self.domain_sid = security.dom_sid(ldb.get_domain_sid()) - self.configuration_dn = self.ldb_admin.get_config_basedn().get_linearized() + self.ldb = SamDB(ldapshost, credentials=creds, + session_info=system_session(lp), lp=lp) + self.base_dn = self.ldb.domain_dn() + self.domain_sid = security.dom_sid(self.ldb.get_domain_sid()) + self.configuration_dn = self.ldb.get_config_basedn().get_linearized() def get_user_dn(self, name): return "CN=%s,CN=Users,%s" % (name, self.base_dn) @@ -84,34 +85,34 @@ class SimpleSitesTests(SitesBaseTests): def test_create_and_delete(self): """test creation and deletion of 1 site""" - self.ldb_admin.transaction_start() - ok = sites.create_site(self.ldb_admin, self.ldb_admin.get_config_basedn(), - "testsamba") - self.ldb_admin.transaction_commit() + self.ldb.transaction_start() + sites.create_site(self.ldb, self.ldb.get_config_basedn(), + "testsamba") + self.ldb.transaction_commit() self.assertRaises(sites.SiteAlreadyExistsException, - sites.create_site, self.ldb_admin, self.ldb_admin.get_config_basedn(), - "testsamba") + sites.create_site, self.ldb, + self.ldb.get_config_basedn(), + "testsamba") - self.ldb_admin.transaction_start() - ok = sites.delete_site(self.ldb_admin, self.ldb_admin.get_config_basedn(), - "testsamba") - - self.ldb_admin.transaction_commit() + self.ldb.transaction_start() + sites.delete_site(self.ldb, self.ldb.get_config_basedn(), + "testsamba") + self.ldb.transaction_commit() self.assertRaises(sites.SiteNotFoundException, - sites.delete_site, self.ldb_admin, self.ldb_admin.get_config_basedn(), - "testsamba") - + sites.delete_site, self.ldb, + self.ldb.get_config_basedn(), + "testsamba") def test_delete_not_empty(self): """test removal of 1 site with servers""" self.assertRaises(sites.SiteServerNotEmptyException, - sites.delete_site, self.ldb_admin, self.ldb_admin.get_config_basedn(), - "Default-First-Site-Name") + sites.delete_site, self.ldb, + self.ldb.get_config_basedn(), + "Default-First-Site-Name") -ldb = SamDB(ldapshost, credentials=creds, session_info=system_session(lp), lp=lp) TestProgram(module=__name__, opts=subunitopts) -- 2.1.4 >From 8bed2d8a098118406cff794bf7fde8781d7a573c Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 17 Sep 2015 18:28:51 +1200 Subject: [PATCH 09/15] dsdb.tests.sites: add basic subnets tests Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- source4/dsdb/tests/python/sites.py | 80 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/source4/dsdb/tests/python/sites.py b/source4/dsdb/tests/python/sites.py index 912dfc4..5cb497f 100755 --- a/source4/dsdb/tests/python/sites.py +++ b/source4/dsdb/tests/python/sites.py @@ -27,10 +27,12 @@ from samba.tests.subunitrun import TestProgram, SubunitOptions import samba.getopt as options from samba import sites +from samba import subnets from samba.auth import system_session from samba.samdb import SamDB import samba.tests from samba.dcerpc import security +from ldb import SCOPE_SUBTREE parser = optparse.OptionParser(__file__ + " [options] ") sambaopts = options.SambaOptions(parser) @@ -114,5 +116,83 @@ class SimpleSitesTests(SitesBaseTests): "Default-First-Site-Name") +# tests for subnets +class SimpleSubnetTests(SitesBaseTests): + + def setUp(self): + super(SimpleSubnetTests, self).setUp() + self.basedn = self.ldb.get_config_basedn() + self.sitename = "testsite" + self.sitename2 = "testsite2" + self.ldb.transaction_start() + sites.create_site(self.ldb, self.basedn, self.sitename) + sites.create_site(self.ldb, self.basedn, self.sitename2) + self.ldb.transaction_commit() + + def tearDown(self): + self.ldb.transaction_start() + sites.delete_site(self.ldb, self.basedn, self.sitename) + sites.delete_site(self.ldb, self.basedn, self.sitename2) + self.ldb.transaction_commit() + super(SimpleSubnetTests, self).tearDown() + + def test_create_delete(self): + """Create a subnet and delete it again.""" + basedn = self.ldb.get_config_basedn() + cidr = "10.11.12.0/24" + + self.ldb.transaction_start() + subnets.create_subnet(self.ldb, basedn, cidr, self.sitename) + self.ldb.transaction_commit() + + self.assertRaises(subnets.SubnetAlreadyExists, + subnets.create_subnet, self.ldb, basedn, cidr, + self.sitename) + + subnets.delete_subnet(self.ldb, basedn, cidr) + + ret = self.ldb.search(base=basedn, scope=SCOPE_SUBTREE, + expression='(&(objectclass=subnet)(cn=%s))' % cidr) + + self.assertEqual(len(ret), 0, 'Failed to delete subnet %s' % cidr) + + def test_create_shift_delete(self): + """Create a subnet, shift it to another site, then delete it.""" + basedn = self.ldb.get_config_basedn() + cidr = "10.11.12.0/24" + + self.ldb.transaction_start() + subnets.create_subnet(self.ldb, basedn, cidr, self.sitename) + self.ldb.transaction_commit() + + subnets.set_subnet_site(self.ldb, basedn, cidr, self.sitename2) + + ret = self.ldb.search(base=basedn, scope=SCOPE_SUBTREE, + expression='(&(objectclass=subnet)(cn=%s))' % cidr) + + sites = ret[0]['siteObject'] + self.assertEqual(len(sites), 1) + self.assertEqual(sites[0], + 'CN=testsite2,CN=Sites,CN=Configuration,DC=samba,DC=example,DC=com') + + self.assertRaises(subnets.SubnetAlreadyExists, + subnets.create_subnet, self.ldb, basedn, cidr, + self.sitename) + + subnets.delete_subnet(self.ldb, basedn, cidr) + + ret = self.ldb.search(base=basedn, scope=SCOPE_SUBTREE, + expression='(&(objectclass=subnet)(cn=%s))' % cidr) + + self.assertEqual(len(ret), 0, 'Failed to delete subnet %s' % cidr) + + def test_delete_subnet_that_does_not_exist(self): + """Ensure we can't delete a site that isn't there.""" + basedn = self.ldb.get_config_basedn() + cidr = "10.15.0.0/16" + + self.assertRaises(subnets.SubnetNotFound, + subnets.delete_subnet, self.ldb, basedn, cidr) + TestProgram(module=__name__, opts=subunitopts) -- 2.1.4 >From 6707c21474255387657e351d45220ac8c5bfdd53 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 17 Sep 2015 18:30:28 +1200 Subject: [PATCH 10/15] sambatool sites: PEP8 improvements We were nearly there, so lets make the jump. Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- python/samba/netcmd/sites.py | 4 +++- python/samba/sites.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/python/samba/netcmd/sites.py b/python/samba/netcmd/sites.py index 671ec1e..f0c792d 100644 --- a/python/samba/netcmd/sites.py +++ b/python/samba/netcmd/sites.py @@ -59,10 +59,12 @@ class cmd_sites_create(Command): samdb.transaction_commit() except sites.SiteAlreadyExistsException, e: samdb.transaction_cancel() - raise CommandError("Error while creating site %s, error: %s" % (sitename, str(e))) + raise CommandError("Error while creating site %s, error: %s" % + (sitename, str(e))) self.outf.write("Site %s created !\n" % sitename) + class cmd_sites_delete(Command): """Delete an existing site.""" diff --git a/python/samba/sites.py b/python/samba/sites.py index 3b18ff6..9bce998 100644 --- a/python/samba/sites.py +++ b/python/samba/sites.py @@ -79,6 +79,7 @@ def create_site(samdb, configDn, siteName): return True + def delete_site(samdb, configDn, siteName): """ Delete a site -- 2.1.4 >From 734330d4a305585e17938f9d9487a5cc35dfd8b0 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 23 Sep 2015 15:10:56 +1200 Subject: [PATCH 11/15] samldb: ensure subnets have proper net ranges A subnet name needs to be a valid CIDR address range -- that's the ones that look like 10.9.8.0/22, where the number after the / determines how many bits are in the address suffix. It can be IPv4 or IPv6. See RFC 4632, or -- hopefully equivalently -- MS-ADTS v20150630 6.1.1.2.2.2.1 "Subnet Object". Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- source4/dsdb/samdb/ldb_modules/samldb.c | 138 ++++++++++++++++++++++++++++++++ source4/dsdb/tests/python/sites.py | 132 ++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+) diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index e3a7db2..07ad21e4 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -42,6 +42,7 @@ #include "ldb_wrap.h" #include "param/param.h" #include "libds/common/flag_mapping.h" +#include "system/network.h" struct samldb_ctx; enum samldb_add_type { @@ -2625,6 +2626,124 @@ static int samldb_fsmo_role_owner_check(struct samldb_ctx *ac) } +static int check_cidr_zero_bits(uint8_t *address, unsigned int len, + unsigned int mask) +{ + /*
is an integer in big-endian form, bits long. All + bits between and must be zero. */ + int i; + unsigned int byte_len; + unsigned int byte_mask; + unsigned int bit_mask; + if (mask > len){ + return -1; + } + if (mask == len){ + /* single address subnet */ + return 0; + } + + byte_len = len / 8; + byte_mask = mask / 8; + + for (i = byte_len - 1; i > byte_mask; i--){ + if (address[i] != 0){ + return -1; + } + } + bit_mask = (1 << (8 - (mask & 7))) - 1; + if (address[byte_mask] & bit_mask){ + return -1; + } + return 0; +} + + +static int verify_cidr(const char *cidr) +{ + /* MS-ADTS v20150630 6.1.1.2.2.2.1 Subnet Object, refers to RFC1166 + and RFC2373. It specifies something seeminglt indistinguishable + from an RFC4632 CIDR address range without saying so explicitly. + Here wqe follow the CIDR spec.*/ + char *address, *slash, *endptr; + bool has_colon, has_dot; + int res, ret; + unsigned long mask; + uint8_t *address_bytes; + unsigned int address_len; + TALLOC_CTX *frame; + frame = talloc_stackframe(); + address = talloc_strdup(frame, cidr); + address_bytes = talloc_size(frame, INET6_ADDRSTRLEN); + /* there must be a '/' */ + slash = strchr(address, '/'); + if (slash == NULL){ + goto error; + } + /* terminate the address for strchr, inet_pton */ + *slash = '\0'; + + mask = strtoul(slash + 1, &endptr, 10); + if (*endptr != '\0' || endptr == slash + 1){ + /* the mask is not a proper integer. */ + goto error; + } + + has_colon = (strchr(address, ':') == NULL) ? false : true; + has_dot = (strchr(address, '.') == NULL) ? false : true; + if (has_dot && has_colon){ + /* This seems to be an IPv4 address embedded in IPv6, which is + icky. We don't support it. */ + goto error; + } else if (has_colon){ /* looks like IPv6 */ + res = inet_pton(AF_INET6, address, address_bytes); + if (res != 1) { + goto error; + } + address_len = 128; + } else if (has_dot) { + /* looks like IPv4 */ + res = inet_pton(AF_INET, address, address_bytes); + if (res != 1) { + goto error; + } + address_len = 32; + } else { + /* This doesn't look like an IP address at all. */ + goto error; + } + + ret = check_cidr_zero_bits(address_bytes, address_len, mask); + talloc_free(frame); + return ret; + error: + talloc_free(frame); + return -1; +} + +static int samldb_verify_subnet(struct samldb_ctx *ac) +{ + struct ldb_context *ldb = ldb_module_get_ctx(ac->module); + const char *cidr; + const struct ldb_val *rdn_value; + + rdn_value = ldb_dn_get_rdn_val(ac->msg->dn); + cidr = ldb_dn_escape_value(ac, *rdn_value); + if (cidr == NULL) { + ldb_set_errstring(ldb, + "samldb: adding an empty subnet cidr seems wrong"); + return LDB_ERR_UNWILLING_TO_PERFORM; + } + + if (verify_cidr(cidr)){ + ldb_set_errstring(ldb, + "samldb: subnet value is invalid"); + return LDB_ERR_CONSTRAINT_VIOLATION; + } + return LDB_SUCCESS; +} + + /* add */ static int samldb_add(struct ldb_module *module, struct ldb_request *req) { @@ -2732,6 +2851,16 @@ static int samldb_add(struct ldb_module *module, struct ldb_request *req) return samldb_fill_object(ac); } + if (samdb_find_attribute(ldb, ac->msg, + "objectclass", "subnet") != NULL) { + ret = samldb_verify_subnet(ac); + if (ret != LDB_SUCCESS) { + talloc_free(ac); + return ret; + } + /* No auto-generation, fall through */ + } + talloc_free(ac); /* nothing matched, go on */ @@ -3079,6 +3208,15 @@ static int check_rename_constraints(struct ldb_message *msg, return LDB_ERR_UNWILLING_TO_PERFORM; } + /* subnet objects */ + if (samdb_find_attribute(ldb, msg, "objectclass", "subnet") != NULL) { + ret = samldb_verify_subnet(ac); + if (ret != LDB_SUCCESS) { + talloc_free(ac); + return ret; + } + } + /* systemFlags */ dn1 = ldb_dn_get_parent(ac, olddn); diff --git a/source4/dsdb/tests/python/sites.py b/source4/dsdb/tests/python/sites.py index 5cb497f..cc38aed 100755 --- a/source4/dsdb/tests/python/sites.py +++ b/source4/dsdb/tests/python/sites.py @@ -194,5 +194,137 @@ class SimpleSubnetTests(SitesBaseTests): self.assertRaises(subnets.SubnetNotFound, subnets.delete_subnet, self.ldb, basedn, cidr) + def test_create_bad_ranges(self): + """These CIDR ranges all have something wrong with them, and they + should all fail.""" + basedn = self.ldb.get_config_basedn() + + cidrs = [ + # IPv4 + # insufficient zeros + "10.11.12.0/14", + "110.0.0.0/6", + "1.0.0.0/0", + "10.11.13.1/24", + "1.2.3.4/29", + "10.11.12.0/21", + # out of range mask + "110.0.0.0/33", + "110.0.0.0/-1", + "0.0.0.0/111", + # out of range address + "310.0.0.0/24", + "10.0.0.256/32", + "1.1.-20.0/24", + # badly formed + "1.0.0.0/1e", + "1.0.0.0/24.0", + "1.0.0.0/1/1", + "1.0.0.0", + "1.c.0.0/24", + "1.2.0.0.0/27", + "1.23.0/24", + "1.23.0.0/0x10", + # IPv6 insufficient zeros -- this could be a subtle one + # due to the vagaries of endianness in the 16 bit groups. + "aaaa:bbbb:cccc:dddd:eeee:ffff:2222:1100/119", + "aaaa:bbbb::/31", + "a:b::/31", + "c000::/1", + "a::b00/119", + "1::1/127", + "1::2/126", + "1::100/119", + "1::8000/112", + # out of range mask + "a:b::/130", + "a:b::/-1", + "::/129", + # badly formed + "a::b::0/120", + "a::abcdf:0/120", + "a::g:0/120", + "aaaa:bbbb:cccc:dddd:eeee:ffff:2222:1111:0000/128", + # IP4 embedded - rejected + "a::10.0.0.0/120", + "a:::5:0/120", + # completely wrong + None, + "bob", + 3.1415, + False + ] + + for cidr in cidrs: + #print >> sys.stderr, "\nstarting CIDR %s" % (cidr,) + self.ldb.transaction_start() + self.assertRaises(subnets.SubnetInvalid, subnets.create_subnet, + self.ldb, basedn, cidr, self.sitename) + + # subnets.delete_subnet(self.ldb, basedn, cidr) + self.ldb.transaction_commit() + #print >> sys.stderr, "%s fails properly" % (cidr,) + + def test_create_good_ranges(self): + """All of these CIDRs are good, and the subnet creation should + succeed.""" + basedn = self.ldb.get_config_basedn() + + cidrs = [ + # IPv4 + # insufficient zeros + "10.11.12.0/24", + "10.11.12.0/23", + "10.11.12.0/25", + "110.0.0.0/7", + "128.0.0.0/1", + "1.0.0.0/32", + "0.0.0.0/0", + "0.0.0.0/8", + "10.11.13.0/32", + "10.11.13.1/32", + "99.0.97.0/24", + "1.2.3.4/30", + "10.11.12.0/22", + # IPv6 + "aaaa:bbbb:cccc:dddd:eeee:ffff:2222:1100/120", + "aaaa:bbbb:cccc:dddd:eeee:ffff:2222:11f0/124", + "aaaa:bbbb:cccc:dddd:eeee:ffff:2222:11fc/126", + "9876::ab00/120", + "9876::abf0/124", + "9876::abfc/126", + "aaaa:bbbb::/32", + "aaaa:bbba::/31", + "aaaa:ba00::/23", + "aaaa:bb00::/24", + "aaaa:bb00::/77", + "a:b::/32", + "c000::/2", + "a::b00/120", + "1::2/127", + "::1000/116", + "::8000/113", + # some unicode for good measure + u"192.168.1.0/24", + u"10.11.12.0/22", + ] + + for cidr in cidrs: + #print >> sys.stderr, "\nstarting CIDR %s" % (cidr,) + self.ldb.transaction_start() + subnets.create_subnet(self.ldb, basedn, cidr, self.sitename) + self.ldb.transaction_commit() + + ret = self.ldb.search(base=basedn, scope=SCOPE_SUBTREE, + expression='(&(objectclass=subnet)(cn=%s))' % cidr) + + self.assertEqual(len(ret), 1, msg="%s was not created" % cidr) + + self.ldb.transaction_start() + subnets.delete_subnet(self.ldb, basedn, cidr) + self.ldb.transaction_commit() + + #print >> sys.stderr, "%s created and deleted" % (cidr,) + TestProgram(module=__name__, opts=subunitopts) -- 2.1.4 >From a9f5e005eecbf60e1c447933da670e366a1f7adb Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 1 Oct 2015 17:27:48 +1300 Subject: [PATCH 12/15] samba.subnets: raise an exception on badly formed subnets Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- python/samba/subnets.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/python/samba/subnets.py b/python/samba/subnets.py index cd9639c..7073097 100644 --- a/python/samba/subnets.py +++ b/python/samba/subnets.py @@ -36,6 +36,11 @@ class SubnetAlreadyExists(SubnetException): pass +class SubnetInvalid(SubnetException): + """The subnet CIDR is invalid.""" + pass + + class SiteNotFound(SubnetException): """The site to be used for the subnet does not exist.""" pass @@ -68,14 +73,17 @@ def create_subnet(samdb, configDn, subnet_name, site_name): site_name) siteDn = str(ret[0].dn) - - m = ldb.Message() - m.dn = ldb.Dn(samdb, "Cn=%s,CN=Subnets,CN=Sites,%s" % (subnet_name, - configDn)) - m["objectclass"] = ldb.MessageElement("subnet", FLAG_MOD_ADD, - "objectclass") - m["siteOjbect"] = ldb.MessageElement(siteDn, FLAG_MOD_ADD, "siteObject") - samdb.add(m) + try: + m = ldb.Message() + m.dn = ldb.Dn(samdb, "Cn=%s,CN=Subnets,CN=Sites,%s" % (subnet_name, + configDn)) + m["objectclass"] = ldb.MessageElement("subnet", FLAG_MOD_ADD, + "objectclass") + m["siteObject"] = ldb.MessageElement(siteDn, FLAG_MOD_ADD, + "siteObject") + samdb.add(m) + except ldb.LdbError: + raise SubnetInvalid("%s is not a valid subnet" % subnet_name) def delete_subnet(samdb, configDn, subnet_name): -- 2.1.4 >From 25a9062f31e1023b8643d6e204bfebc9f381f4e8 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 1 Oct 2015 17:55:46 +1300 Subject: [PATCH 13/15] samldb: add debug messages to the subnet path Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- source4/dsdb/samdb/ldb_modules/samldb.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index 07ad21e4..0894aee 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -2635,7 +2635,22 @@ static int check_cidr_zero_bits(uint8_t *address, unsigned int len, unsigned int byte_len; unsigned int byte_mask; unsigned int bit_mask; + if (len == 32) { + DBG_INFO("Looking at address %02x%02x%02x%02x, mask %u\n", + address[0], address[1], address[2], address[3], + mask); + } else if (len == 128){ + DBG_INFO("Looking at address %02x%02x%02x%02x-%02x%02x%02x%02x-" + "%02x%02x%02x%02x-%02x%02x%02x%02x, mask %u\n", + address[0], address[1], address[2], address[3], + address[4], address[5], address[6], address[7], + address[8], address[9], address[10], address[11], + address[12], address[13], address[14], address[15], + mask); + } + if (mask > len){ + DBG_INFO("mask %u is too big (> %u)\n", mask, len); return -1; } if (mask == len){ @@ -2647,11 +2662,14 @@ static int check_cidr_zero_bits(uint8_t *address, unsigned int len, byte_mask = mask / 8; for (i = byte_len - 1; i > byte_mask; i--){ + DBG_INFO("checking byte %d %02x\n", i, address[i]); if (address[i] != 0){ return -1; } } bit_mask = (1 << (8 - (mask & 7))) - 1; + DBG_INFO("checking bitmask %02x & %02x overlap %02x\n", bit_mask, address[byte_mask], + bit_mask & address[byte_mask]); if (address[byte_mask] & bit_mask){ return -1; } @@ -2672,6 +2690,7 @@ static int verify_cidr(const char *cidr) uint8_t *address_bytes; unsigned int address_len; TALLOC_CTX *frame; + DBG_INFO("CIDR is %s\n", cidr); frame = talloc_stackframe(); address = talloc_strdup(frame, cidr); address_bytes = talloc_size(frame, INET6_ADDRSTRLEN); @@ -2688,12 +2707,14 @@ static int verify_cidr(const char *cidr) /* the mask is not a proper integer. */ goto error; } - + DBG_INFO("found address %s, mask %lu\n", address, mask); has_colon = (strchr(address, ':') == NULL) ? false : true; has_dot = (strchr(address, '.') == NULL) ? false : true; if (has_dot && has_colon){ /* This seems to be an IPv4 address embedded in IPv6, which is icky. We don't support it. */ + DBG_INFO("Refusing to consider cidr '%s' with dots and colons", + cidr); goto error; } else if (has_colon){ /* looks like IPv6 */ res = inet_pton(AF_INET6, address, address_bytes); @@ -2729,6 +2750,7 @@ static int samldb_verify_subnet(struct samldb_ctx *ac) rdn_value = ldb_dn_get_rdn_val(ac->msg->dn); cidr = ldb_dn_escape_value(ac, *rdn_value); + DBG_INFO("looking at cidr '%s'\n", cidr); if (cidr == NULL) { ldb_set_errstring(ldb, "samldb: adding an empty subnet cidr seems wrong"); -- 2.1.4 >From 6ec59648646af32dcbafb16a57d0fc000f158cd0 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 23 Oct 2015 16:21:05 +1300 Subject: [PATCH 14/15] samldb: clarify documentation of CIDR validation code Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- source4/dsdb/samdb/ldb_modules/samldb.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index 0894aee..6a4647c 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -2625,7 +2625,11 @@ static int samldb_fsmo_role_owner_check(struct samldb_ctx *ac) return LDB_SUCCESS; } - +/* + * Return zero if the number of zero bits in the address (looking from low to + * high) is equal to or greater than the length minus the mask. Otherwise it + * returns -1. + */ static int check_cidr_zero_bits(uint8_t *address, unsigned int len, unsigned int mask) { @@ -2676,13 +2680,14 @@ static int check_cidr_zero_bits(uint8_t *address, unsigned int len, return 0; } - +/* + * MS-ADTS v20150630 6.1.1.2.2.2.1 Subnet Object, refers to RFC1166 and + * RFC2373. It specifies something seemingly indistinguishable from an RFC4632 + * CIDR address range without saying so explicitly. Here we follow the CIDR + * spec. + */ static int verify_cidr(const char *cidr) { - /* MS-ADTS v20150630 6.1.1.2.2.2.1 Subnet Object, refers to RFC1166 - and RFC2373. It specifies something seeminglt indistinguishable - from an RFC4632 CIDR address range without saying so explicitly. - Here wqe follow the CIDR spec.*/ char *address, *slash, *endptr; bool has_colon, has_dot; int res, ret; @@ -2880,7 +2885,8 @@ static int samldb_add(struct ldb_module *module, struct ldb_request *req) talloc_free(ac); return ret; } - /* No auto-generation, fall through */ + /* We are just checking the value is valid, and there are no + values to fill in. */ } talloc_free(ac); -- 2.1.4 >From 125e6951119bac5414470e6d0a59977d0ac52478 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 23 Oct 2015 16:22:08 +1300 Subject: [PATCH 15/15] samldb: initialise pointers to NULL in CIDR checks While I was working they changed the coding standard. Signed-off-by: Douglas Bagnall Reviewed-by: Garming Sam --- source4/dsdb/samdb/ldb_modules/samldb.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index 6a4647c..beb790a 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -2688,13 +2688,14 @@ static int check_cidr_zero_bits(uint8_t *address, unsigned int len, */ static int verify_cidr(const char *cidr) { - char *address, *slash, *endptr; + char *address = NULL, *slash = NULL, *endptr = NULL; bool has_colon, has_dot; int res, ret; unsigned long mask; - uint8_t *address_bytes; + uint8_t *address_bytes = NULL; unsigned int address_len; - TALLOC_CTX *frame; + TALLOC_CTX *frame = NULL; + DBG_INFO("CIDR is %s\n", cidr); frame = talloc_stackframe(); address = talloc_strdup(frame, cidr); @@ -2750,8 +2751,8 @@ static int verify_cidr(const char *cidr) static int samldb_verify_subnet(struct samldb_ctx *ac) { struct ldb_context *ldb = ldb_module_get_ctx(ac->module); - const char *cidr; - const struct ldb_val *rdn_value; + const char *cidr = NULL; + const struct ldb_val *rdn_value = NULL; rdn_value = ldb_dn_get_rdn_val(ac->msg->dn); cidr = ldb_dn_escape_value(ac, *rdn_value); -- 2.1.4