From 13a10d43141c29dad61868b451c0c1dca82360de Mon Sep 17 00:00:00 2001 From: Nadezhda Ivanova Date: Mon, 14 Oct 2013 12:38:10 +0300 Subject: s4-samldb: Do not allow deletion of objects with RID < 1000 According to [MS-SAMR] 3.1.5.7 Delete Pattern we should not allow deletion of security objects with RID < 1000. This patch will prevent deletion of well-known accounts and groups. Signed-off-by: Nadezhda Ivanova Reviewed-by: Andrew Bartlett Autobuild-User(master): Nadezhda Ivanova Autobuild-Date(master): Mon Oct 14 13:31:50 CEST 2013 on sn-devel-104 --- python/samba/tests/samba3sam.py | 12 +++++------ source4/dsdb/samdb/ldb_modules/samldb.c | 5 +++++ source4/dsdb/samdb/samdb.h | 1 + source4/dsdb/tests/python/sam.py | 37 ++++++++++++++++++++++++++++++--- testdata/samba3/samba3.ldif | 4 ++-- 5 files changed, 48 insertions(+), 11 deletions(-) diff --git a/python/samba/tests/samba3sam.py b/python/samba/tests/samba3sam.py index 9c017fb79c..7cd656670a 100644 --- a/python/samba/tests/samba3sam.py +++ b/python/samba/tests/samba3sam.py @@ -172,7 +172,7 @@ class Samba3SamTestCase(MapBaseTestCase): self.assertEquals(str(msg[0].dn), "cn=Replicator,ou=Groups,dc=vernstok,dc=nl") self.assertTrue("objectSid" in msg[0]) - self.assertSidEquals("S-1-5-21-4231626423-2410014848-2360679739-552", + self.assertSidEquals("S-1-5-21-4231626423-2410014848-2360679739-1052", msg[0]["objectSid"]) oc = set(msg[0]["objectClass"]) self.assertEquals(oc, set(["group"])) @@ -345,7 +345,7 @@ dnsHostName: x nextRid: y lastLogon: x description: x -objectSid: S-1-5-21-4231626423-2410014848-2360679739-552 +objectSid: S-1-5-21-4231626423-2410014848-2360679739-1052 """) self.ldb.add({ @@ -380,7 +380,7 @@ objectSid: S-1-5-21-4231626423-2410014848-2360679739-552 "sambaBadPasswordCount": "x", "sambaLogonTime": "x", "description": "x", - "sambaSID": "S-1-5-21-4231626423-2410014848-2360679739-552", + "sambaSID": "S-1-5-21-4231626423-2410014848-2360679739-1052", "sambaPrimaryGroupSID": "S-1-5-21-4231626423-2410014848-2360679739-512"}) self.samba3.db.add({ @@ -483,20 +483,20 @@ objectSid: S-1-5-21-4231626423-2410014848-2360679739-552 # TODO: # Using the SID directly in the parse tree leads to conversion # errors, letting the search fail with no results. - #res = self.ldb.search("(objectSid=S-1-5-21-4231626423-2410014848-2360679739-552)", scope=SCOPE_DEFAULT, attrs) + #res = self.ldb.search("(objectSid=S-1-5-21-4231626423-2410014848-2360679739-1052)", scope=SCOPE_DEFAULT, attrs) res = self.ldb.search(expression="(objectSid=*)", base=None, scope=SCOPE_DEFAULT, attrs=["dnsHostName", "lastLogon", "objectSid"]) self.assertEquals(len(res), 4) res = sorted(res, key=attrgetter('dn')) self.assertEquals(str(res[1].dn), self.samba4.dn("cn=X")) self.assertEquals(str(res[1]["dnsHostName"]), "x") self.assertEquals(str(res[1]["lastLogon"]), "x") - self.assertSidEquals("S-1-5-21-4231626423-2410014848-2360679739-552", + self.assertSidEquals("S-1-5-21-4231626423-2410014848-2360679739-1052", res[1]["objectSid"]) self.assertTrue("objectSid" in res[1]) self.assertEquals(str(res[0].dn), self.samba4.dn("cn=A")) self.assertTrue(not "dnsHostName" in res[0]) self.assertEquals(str(res[0]["lastLogon"]), "x") - self.assertSidEquals("S-1-5-21-4231626423-2410014848-2360679739-552", + self.assertSidEquals("S-1-5-21-4231626423-2410014848-2360679739-1052", res[0]["objectSid"]) self.assertTrue("objectSid" in res[0]) diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index 603370fd62..b79810279c 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -2552,6 +2552,11 @@ static int samldb_prim_group_users_check(struct samldb_ctx *ac) /* Special object (security principal?) */ return LDB_SUCCESS; } + /* do not allow deletion of well-known sids */ + if (rid < DSDB_SAMDB_MINIMUM_ALLOWED_RID && + (ldb_request_get_control(ac->req, LDB_CONTROL_RELAX_OID) == NULL)) { + return LDB_ERR_OTHER; + } /* Deny delete requests from groups which are primary ones */ ret = dsdb_module_search(ac->module, ac, &res, diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h index 7605c65cdd..7f77d4e382 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -244,6 +244,7 @@ struct dsdb_extended_sec_desc_propagation_op { }; #define DSDB_ACL_CHECKS_DIRSYNC_FLAG 0x1 +#define DSDB_SAMDB_MINIMUM_ALLOWED_RID 1000 #define DSDB_METADATA_SCHEMA_SEQ_NUM "SCHEMA_SEQ_NUM" #endif /* __SAMDB_H__ */ diff --git a/source4/dsdb/tests/python/sam.py b/source4/dsdb/tests/python/sam.py index 754096a015..b2d4d4920f 100755 --- a/source4/dsdb/tests/python/sam.py +++ b/source4/dsdb/tests/python/sam.py @@ -586,7 +586,7 @@ class SamTests(samba.tests.TestCase): def test_sam_attributes(self): """Test the behaviour of special attributes of SAM objects""" - print "Testing the behaviour of special attributes of SAM objects\n""" + print "Testing the behaviour of special attributes of SAM objects\n" ldb.add({ "dn": "cn=ldaptestuser,cn=users," + self.base_dn, @@ -2604,7 +2604,7 @@ class SamTests(samba.tests.TestCase): def test_sam_description_attribute(self): """Test SAM description attribute""" - print "Test SAM description attribute""" + print "Test SAM description attribute" self.ldb.add({ "dn": "cn=ldaptestgroup,cn=users," + self.base_dn, @@ -2772,7 +2772,7 @@ class SamTests(samba.tests.TestCase): def test_fSMORoleOwner_attribute(self): """Test fSMORoleOwner attribute""" - print "Test fSMORoleOwner attribute""" + print "Test fSMORoleOwner attribute" ds_service_name = self.ldb.get_dsServiceName() @@ -2846,6 +2846,37 @@ class SamTests(samba.tests.TestCase): delete_force(self.ldb, "cn=ldaptestgroup,cn=users," + self.base_dn) + def test_protected_sid_objects(self): + """Test deletion of objects with RID < 1000""" + self.ldb.create_ou("ou=ldaptestou," + self.base_dn) + # a list of some well-known sids + # objects in Builtin are aready covered by objectclass + protected_list = [ + ["CN=Domain Admins","CN=Users,"], + ["CN=Schema Admins","CN=Users,"], + ["CN=Enterprise Admins","CN=Users,"], + ["CN=Administrator","CN=Users,"], + ["CN=Domain Controllers","CN=Users,"], + ] + + + + for pr_object in protected_list: + try: + self.ldb.delete(pr_object[0] + "," + pr_object[1] + self.base_dn) + except LdbError, (num, _): + self.assertEquals(num, ERR_OTHER) + else: + self.fail("Deleted " + pr_object[0]) + + try: + self.ldb.rename(pr_object[0] + "," + pr_object[1] + self.base_dn, + pr_object[0] + "2," + pr_object[1] + self.base_dn) + except LdbError, (num, _): + self.fail("Could not rename " + pr_object[0]) + + self.ldb.rename(pr_object[0] + "2," + pr_object[1] + self.base_dn, + pr_object[0] + "," + pr_object[1] + self.base_dn) if not "://" in host: if os.path.isfile(host): diff --git a/testdata/samba3/samba3.ldif b/testdata/samba3/samba3.ldif index 76792d09e5..3978777e27 100644 --- a/testdata/samba3/samba3.ldif +++ b/testdata/samba3/samba3.ldif @@ -94,10 +94,10 @@ displayName: Backup Operators dn: cn=Replicator,ou=Groups,sambaDomainName=TESTS,${BASEDN} objectClass: posixGroup objectClass: sambaGroupMapping -gidNumber: 552 +gidNumber: 1052 cn: Replicator description: Netbios Domain Supports file replication in a sambaDomainName -sambaSID: S-1-5-21-4231626423-2410014848-2360679739-552 +sambaSID: S-1-5-21-4231626423-2410014848-2360679739-1052 sambaGroupType: 2 displayName: Replicator -- cgit