s4-samldb: Do not allow deletion of objects with RID < 1000
authorNadezhda Ivanova <nivanova@symas.com>
Mon, 14 Oct 2013 09:38:10 +0000 (12:38 +0300)
committerNadezhda Ivanova <nivanova@samba.org>
Mon, 14 Oct 2013 11:31:50 +0000 (13:31 +0200)
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 <nivanova@symas.com>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Autobuild-User(master): Nadezhda Ivanova <nivanova@samba.org>
Autobuild-Date(master): Mon Oct 14 13:31:50 CEST 2013 on sn-devel-104

python/samba/tests/samba3sam.py
source4/dsdb/samdb/ldb_modules/samldb.c
source4/dsdb/samdb/samdb.h
source4/dsdb/tests/python/sam.py
testdata/samba3/samba3.ldif

index 9c017fb..7cd6566 100644 (file)
@@ -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])
 
index 603370f..b798102 100644 (file)
@@ -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,
index 7605c65..7f77d4e 100644 (file)
@@ -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__ */
index 754096a..b2d4d49 100755 (executable)
@@ -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):
index 76792d0..3978777 100644 (file)
@@ -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