CVE-2020-25720: s4-acl: Owner no longer has implicit Write DACL

The implicit right of an object's owner to modify its security
descriptor no longer exists, according to the new access rules. However,
we continue to grant this implicit right for fileserver access checks.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14810

Signed-off-by: Nadezhda Ivanova <nivanova@symas.com>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index f5051b0fa93d67a222f5d089aa633b0f9fd6c6dd..7d8eca74c43c4cc7121bcc4969f518657e43addf 100644 (file)
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -106,7 +106,8 @@ void se_map_standard(uint32_t *access_mask, const struct standard_mapping *mappi
   perform a SEC_FLAG_MAXIMUM_ALLOWED access check
 */
 static uint32_t access_check_max_allowed(const struct security_descriptor *sd,
-                                       const struct security_token *token)
+                                        const struct security_token *token,
+                                        enum implicit_owner_rights implicit_owner_rights)
 {
        uint32_t denied = 0, granted = 0;
        bool am_owner = false;
@@ -115,7 +116,14 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd,
 
        if (sd->dacl == NULL) {
                if (security_token_has_sid(token, sd->owner_sid)) {
-                       granted |= SEC_STD_WRITE_DAC | SEC_STD_READ_CONTROL;
+                       switch (implicit_owner_rights) {
+                       case IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS:
+                               granted |= SEC_STD_WRITE_DAC;
+                               FALL_THROUGH;
+                       case IMPLICIT_OWNER_READ_CONTROL_RIGHTS:
+                               granted |= SEC_STD_READ_CONTROL;
+                               break;
+                       }
                }
                return granted;
        }
@@ -145,7 +153,14 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd,
        }
 
        if (am_owner && !have_owner_rights_ace) {
-               granted |= SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL;
+               switch (implicit_owner_rights) {
+               case IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS:
+                       granted |= SEC_STD_WRITE_DAC;
+                       FALL_THROUGH;
+               case IMPLICIT_OWNER_READ_CONTROL_RIGHTS:
+                       granted |= SEC_STD_READ_CONTROL;
+                       break;
+               }
        }
 
        for (i = 0;i<sd->dacl->num_aces; i++) {
@@ -183,15 +198,11 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd,
        return granted & ~denied;
 }
 
-/*
-  The main entry point for access checking. If returning ACCESS_DENIED
-  this function returns the denied bits in the uint32_t pointed
-  to by the access_granted pointer.
-*/
-NTSTATUS se_access_check(const struct security_descriptor *sd,
-                         const struct security_token *token,
-                         uint32_t access_desired,
-                         uint32_t *access_granted)
+static NTSTATUS se_access_check_implicit_owner(const struct security_descriptor *sd,
+                                              const struct security_token *token,
+                                              uint32_t access_desired,
+                                              uint32_t *access_granted,
+                                              enum implicit_owner_rights implicit_owner_rights)
 {
        uint32_t i;
        uint32_t bits_remaining;
@@ -206,7 +217,7 @@ NTSTATUS se_access_check(const struct security_descriptor *sd,
        if (access_desired & SEC_FLAG_MAXIMUM_ALLOWED) {
                uint32_t orig_access_desired = access_desired;
 
-               access_desired |= access_check_max_allowed(sd, token);
+               access_desired |= access_check_max_allowed(sd, token, implicit_owner_rights);
                access_desired &= ~SEC_FLAG_MAXIMUM_ALLOWED;
                *access_granted = access_desired;
                bits_remaining = access_desired;
@@ -251,7 +262,14 @@ NTSTATUS se_access_check(const struct security_descriptor *sd,
                }
        }
        if (am_owner && !have_owner_rights_ace) {
-               bits_remaining &= ~(SEC_STD_WRITE_DAC | SEC_STD_READ_CONTROL);
+               switch (implicit_owner_rights) {
+               case IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS:
+                       bits_remaining &= ~SEC_STD_WRITE_DAC;
+                       FALL_THROUGH;
+               case IMPLICIT_OWNER_READ_CONTROL_RIGHTS:
+                       bits_remaining &= ~SEC_STD_READ_CONTROL;
+                       break;
+               }
        }
 
        /* check each ace in turn. */
@@ -317,6 +335,23 @@ done:
        return NT_STATUS_OK;
 }
 
+/*
+  The main entry point for access checking. If returning ACCESS_DENIED
+  this function returns the denied bits in the uint32_t pointed
+  to by the access_granted pointer.
+*/
+NTSTATUS se_access_check(const struct security_descriptor *sd,
+                        const struct security_token *token,
+                        uint32_t access_desired,
+                        uint32_t *access_granted)
+{
+       return se_access_check_implicit_owner(sd,
+                                             token,
+                                             access_desired,
+                                             access_granted,
+                                             IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS);
+}
+
 /*
   The main entry point for access checking FOR THE FILE SERVER ONLY !
   If returning ACCESS_DENIED this function returns the denied bits in
@@ -333,10 +368,11 @@ NTSTATUS se_file_access_check(const struct security_descriptor *sd,
 
        if (!priv_open_requested) {
                /* Fall back to generic se_access_check(). */
-               return se_access_check(sd,
-                               token,
-                               access_desired,
-                               access_granted);
+               return se_access_check_implicit_owner(sd,
+                                                     token,
+                                                     access_desired,
+                                                     access_granted,
+                                                     IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS);
        }
 
        /*
@@ -349,7 +385,7 @@ NTSTATUS se_file_access_check(const struct security_descriptor *sd,
        if (access_desired & SEC_FLAG_MAXIMUM_ALLOWED) {
                uint32_t orig_access_desired = access_desired;
 
-               access_desired |= access_check_max_allowed(sd, token);
+               access_desired |= access_check_max_allowed(sd, token, true);
                access_desired &= ~SEC_FLAG_MAXIMUM_ALLOWED;
 
                if (security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
@@ -366,10 +402,11 @@ NTSTATUS se_file_access_check(const struct security_descriptor *sd,
                        access_desired));
        }
 
-       status = se_access_check(sd,
-                               token,
-                               access_desired,
-                               access_granted);
+       status = se_access_check_implicit_owner(sd,
+                                               token,
+                                               access_desired,
+                                               access_granted,
+                                               IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS);
 
        if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
                return status;
@@ -478,34 +515,13 @@ static NTSTATUS check_object_specific_access(struct security_ace *ace,
        return NT_STATUS_OK;
 }
 
-/**
- * @brief Perform directoryservice (DS) related access checks for a given user
- *
- * Perform DS access checks for the user represented by its security_token, on
- * the provided security descriptor. If an tree associating GUID and access
- * required is provided then object access (OA) are checked as well. *
- * @param[in]   sd             The security descritor against which the required
- *                             access are requested
- *
- * @param[in]   token          The security_token associated with the user to
- *                             test
- *
- * @param[in]   access_desired A bitfield of rights that must be granted for the
- *                             given user in the specified SD.
- *
- * If one
- * of the entry in the tree grants all the requested rights for the given GUID
- * FIXME
- * tree can be null if not null it's the
- * Lots of code duplication, it will be united in just one
- * function eventually */
-
-NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
-                            const struct security_token *token,
-                            uint32_t access_desired,
-                            uint32_t *access_granted,
-                            struct object_tree *tree,
-                            struct dom_sid *replace_sid)
+NTSTATUS sec_access_check_ds_implicit_owner(const struct security_descriptor *sd,
+                                           const struct security_token *token,
+                                           uint32_t access_desired,
+                                           uint32_t *access_granted,
+                                           struct object_tree *tree,
+                                           struct dom_sid *replace_sid,
+                                           enum implicit_owner_rights implicit_owner_rights)
 {
        uint32_t i;
        uint32_t bits_remaining;
@@ -518,7 +534,7 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
 
        /* handle the maximum allowed flag */
        if (access_desired & SEC_FLAG_MAXIMUM_ALLOWED) {
-               access_desired |= access_check_max_allowed(sd, token);
+               access_desired |= access_check_max_allowed(sd, token, implicit_owner_rights);
                access_desired &= ~SEC_FLAG_MAXIMUM_ALLOWED;
                *access_granted = access_desired;
                bits_remaining = access_desired;
@@ -535,7 +551,14 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
        /* the owner always gets SEC_STD_WRITE_DAC and SEC_STD_READ_CONTROL */
        if ((bits_remaining & (SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL)) &&
            security_token_has_sid(token, sd->owner_sid)) {
-               bits_remaining &= ~(SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL);
+               switch (implicit_owner_rights) {
+               case IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS:
+                       bits_remaining &= ~SEC_STD_WRITE_DAC;
+                       FALL_THROUGH;
+               case IMPLICIT_OWNER_READ_CONTROL_RIGHTS:
+                       bits_remaining &= ~SEC_STD_READ_CONTROL;
+                       break;
+               }
        }
 
        /* SEC_PRIV_TAKE_OWNERSHIP grants SEC_STD_WRITE_OWNER */
@@ -613,3 +636,41 @@ done:
 
        return NT_STATUS_OK;
 }
+
+/**
+ * @brief Perform directoryservice (DS) related access checks for a given user
+ *
+ * Perform DS access checks for the user represented by its security_token, on
+ * the provided security descriptor. If an tree associating GUID and access
+ * required is provided then object access (OA) are checked as well. *
+ * @param[in]   sd             The security descritor against which the required
+ *                             access are requested
+ *
+ * @param[in]   token          The security_token associated with the user to
+ *                             test
+ *
+ * @param[in]   access_desired A bitfield of rights that must be granted for the
+ *                             given user in the specified SD.
+ *
+ * If one
+ * of the entry in the tree grants all the requested rights for the given GUID
+ * FIXME
+ * tree can be null if not null it's the
+ * Lots of code duplication, it will be united in just one
+ * function eventually */
+
+NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
+                            const struct security_token *token,
+                            uint32_t access_desired,
+                            uint32_t *access_granted,
+                            struct object_tree *tree,
+                            struct dom_sid *replace_sid)
+{
+       return sec_access_check_ds_implicit_owner(sd,
+                                                 token,
+                                                 access_desired,
+                                                 access_granted,
+                                                 tree,
+                                                 replace_sid,
+                                                 IMPLICIT_OWNER_READ_CONTROL_RIGHTS);
+}

diff --git a/libcli/security/access_check.h b/libcli/security/access_check.h
index 96e33c6624f550f3bcc6e07667ab5ebf7d6abf8e..e7150914524f77d4ce1f6fa5258f473e9402d512 100644 (file)
--- a/libcli/security/access_check.h
+++ b/libcli/security/access_check.h
@@ -65,6 +65,14 @@ NTSTATUS se_file_access_check(const struct security_descriptor *sd,
                         uint32_t access_desired,
                         uint32_t *access_granted);
 
+NTSTATUS sec_access_check_ds_implicit_owner(const struct security_descriptor *sd,
+                                           const struct security_token *token,
+                                           uint32_t access_desired,
+                                           uint32_t *access_granted,
+                                           struct object_tree *tree,
+                                           struct dom_sid *replace_sid,
+                                           enum implicit_owner_rights implicit_owner_rights);
+
 /* modified access check for the purposes of DS security
  * Lots of code duplication, it will be united in just one
  * function eventually */

diff --git a/librpc/idl/security.idl b/librpc/idl/security.idl
index d05e3c3e1b7a4d52af74c7ff6731be5d5a336c04..2ef341704794ecf1aa2a2f835862744b667c34a1 100644 (file)
--- a/librpc/idl/security.idl
+++ b/librpc/idl/security.idl
@@ -206,6 +206,15 @@ interface security
                 SEC_ADS_GENERIC_READ           |
                 SEC_ADS_GENERIC_ALL_DS);
 
+       /*
+        * Rights implicitly granted to a user who is an owner of the security
+        * descriptor being processed.
+        */
+       typedef enum {
+               IMPLICIT_OWNER_READ_CONTROL_RIGHTS,
+               IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS
+       } implicit_owner_rights;
+
        /***************************************************************/
        /* WELL KNOWN SIDS */
 

diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index 38aa465a8d52c9d924838392f6bb3c91844ea760..d433122d145f54ee7c269ab6c873ad8891f2f001 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -502,13 +502,15 @@ static int acl_sDRightsEffective(struct ldb_module *module,
                if (ret == LDB_SUCCESS) {
                        flags |= SECINFO_OWNER | SECINFO_GROUP;
                }
-               ret = acl_check_access_on_attribute(module,
-                                                   msg,
-                                                   sd,
-                                                   sid,
-                                                   SEC_STD_WRITE_DAC,
-                                                   attr,
-                                                   objectclass);
+               ret = acl_check_access_on_attribute_implicit_owner(
+                       module,
+                       msg,
+                       sd,
+                       sid,
+                       SEC_STD_WRITE_DAC,
+                       attr,
+                       objectclass,
+                       IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS);
                if (ret == LDB_SUCCESS) {
                        flags |= SECINFO_DACL;
                }
@@ -2064,6 +2066,9 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
                        uint32_t sd_flags = dsdb_request_sd_flags(req, NULL);
                        uint32_t access_mask = 0;
 
+                       bool block_owner_rights;
+                       enum implicit_owner_rights implicit_owner_rights;
+
                        if (sd_flags & (SECINFO_OWNER|SECINFO_GROUP)) {
                                access_mask |= SEC_STD_WRITE_OWNER;
                        }
@@ -2074,13 +2079,32 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
                                access_mask |= SEC_FLAG_SYSTEM_SECURITY;
                        }
 
-                       ret = acl_check_access_on_attribute(module,
-                                                           tmp_ctx,
-                                                           sd,
-                                                           sid,
-                                                           access_mask,
-                                                           attr,
-                                                           objectclass);
+                       block_owner_rights = !dsdb_module_am_administrator(module);
+
+                       if (block_owner_rights) {
+                               block_owner_rights = dsdb_block_owner_implicit_rights(module,
+                                                                                     req,
+                                                                                     req);
+                       }
+                       if (block_owner_rights) {
+                               block_owner_rights = samdb_find_attribute(ldb,
+                                                                         acl_res->msgs[0],
+                                                                         "objectclass",
+                                                                         "computer");
+                       }
+
+                       implicit_owner_rights = block_owner_rights ?
+                               IMPLICIT_OWNER_READ_CONTROL_RIGHTS :
+                               IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS;
+
+                       ret = acl_check_access_on_attribute_implicit_owner(module,
+                                                                          tmp_ctx,
+                                                                          sd,
+                                                                          sid,
+                                                                          access_mask,
+                                                                          attr,
+                                                                          objectclass,
+                                                                          implicit_owner_rights);
                        if (ret != LDB_SUCCESS) {
                                ldb_asprintf_errstring(ldb_module_get_ctx(module),
                                                       "Object %s has no write dacl access\n",

diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index b221dcde445bb8c3d0b56b8221141bba63ad4370..2ed894a069242c8ac4b08d7631172ed78e257c95 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -406,8 +406,9 @@ static int check_attr_access_rights(TALLOC_CTX *mem_ctx, const char *attr_name,
                return LDB_SUCCESS;
        }
 
-       ret = acl_check_access_on_attribute(ac->module, mem_ctx, sd, sid,
-                                           access_mask, attr, objectclass);
+       ret = acl_check_access_on_attribute_implicit_owner(ac->module, mem_ctx, sd, sid,
+                                                          access_mask, attr, objectclass,
+                                                          IMPLICIT_OWNER_READ_CONTROL_RIGHTS);
 
        if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
                return ret;
@@ -674,13 +675,14 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
                                continue;
                        }
 
-                       ret = acl_check_access_on_attribute(ac->module,
-                                                           tmp_ctx,
-                                                           sd,
-                                                           sid,
-                                                           access_mask,
-                                                           attr,
-                                                           objectclass);
+                       ret = acl_check_access_on_attribute_implicit_owner(ac->module,
+                                                                          tmp_ctx,
+                                                                          sd,
+                                                                          sid,
+                                                                          access_mask,
+                                                                          attr,
+                                                                          objectclass,
+                                                                          IMPLICIT_OWNER_READ_CONTROL_RIGHTS);
 
                        /*
                         * Dirsync control needs the replpropertymetadata attribute

diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c
index 12f00fbff160096ed70b4623ecddedb9b2dc2ade..1545525093d14e8b0680e1eaa5872910433c2631 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/acl_util.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_util.c
@@ -95,13 +95,14 @@ int dsdb_module_check_access_on_dn(struct ldb_module *module,
                                                guid);
 }
 
-int acl_check_access_on_attribute(struct ldb_module *module,
-                                 TALLOC_CTX *mem_ctx,
-                                 struct security_descriptor *sd,
-                                 struct dom_sid *rp_sid,
-                                 uint32_t access_mask,
-                                 const struct dsdb_attribute *attr,
-                                 const struct dsdb_class *objectclass)
+int acl_check_access_on_attribute_implicit_owner(struct ldb_module *module,
+                                                TALLOC_CTX *mem_ctx,
+                                                struct security_descriptor *sd,
+                                                struct dom_sid *rp_sid,
+                                                uint32_t access_mask,
+                                                const struct dsdb_attribute *attr,
+                                                const struct dsdb_class *objectclass,
+                                                enum implicit_owner_rights implicit_owner_rights)
 {
        int ret;
        NTSTATUS status;
@@ -138,11 +139,12 @@ int acl_check_access_on_attribute(struct ldb_module *module,
                goto fail;
        }
 
-       status = sec_access_check_ds(sd, token,
-                                    access_mask,
-                                    &access_granted,
-                                    root,
-                                    rp_sid);
+       status = sec_access_check_ds_implicit_owner(sd, token,
+                                                   access_mask,
+                                                   &access_granted,
+                                                   root,
+                                                   rp_sid,
+                                                   implicit_owner_rights);
        if (!NT_STATUS_IS_OK(status)) {
                ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
        }
@@ -156,6 +158,24 @@ fail:
        return ldb_operr(ldb_module_get_ctx(module));
 }
 
+int acl_check_access_on_attribute(struct ldb_module *module,
+                                 TALLOC_CTX *mem_ctx,
+                                 struct security_descriptor *sd,
+                                 struct dom_sid *rp_sid,
+                                 uint32_t access_mask,
+                                 const struct dsdb_attribute *attr,
+                                 const struct dsdb_class *objectclass)
+{
+       return acl_check_access_on_attribute_implicit_owner(module,
+                                                           mem_ctx,
+                                                           sd,
+                                                           rp_sid,
+                                                           access_mask,
+                                                           attr,
+                                                           objectclass,
+                                                           IMPLICIT_OWNER_READ_CONTROL_RIGHTS);
+}
+
 int acl_check_access_on_objectclass(struct ldb_module *module,
                                    TALLOC_CTX *mem_ctx,
                                    struct security_descriptor *sd,

diff --git a/source4/dsdb/samdb/ldb_modules/util.h b/source4/dsdb/samdb/ldb_modules/util.h
index 937767a9dee7ac75d385f922720167cd53593a87..63726c031de27559a336ca6a129164b49fadbb8f 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/util.h
+++ b/source4/dsdb/samdb/ldb_modules/util.h
@@ -28,6 +28,7 @@ struct dom_sid;
 struct netlogon_samlogon_response;
 
 #include "librpc/gen_ndr/misc.h"
+#include "librpc/gen_ndr/security.h"
 #include "dsdb/samdb/ldb_modules/util_proto.h"
 #include "dsdb/common/util.h"
 #include "../libcli/netlogon/netlogon.h"

diff --git a/source4/dsdb/tests/python/sec_descriptor.py b/source4/dsdb/tests/python/sec_descriptor.py
index 62ba057befffef781e8e10415adafebb82f8064f..5410e9f7246272ef88cdca50300dcaf6018e8950 100755 (executable)
--- a/source4/dsdb/tests/python/sec_descriptor.py
+++ b/source4/dsdb/tests/python/sec_descriptor.py
@@ -1303,7 +1303,10 @@ class DaclDescriptorTests(DescriptorTests):
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertEqual(desc_sddl, sddl)
         sddl = "O:AUG:AUD:AI(D;;CC;;;LG)"
-        self.sd_utils.modify_sd_on_dn(group_dn, sddl)
+        try:
+            self.sd_utils.modify_sd_on_dn(group_dn, sddl)
+        except LdbError as e:
+            self.fail(str(e))
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertEqual(desc_sddl, sddl)
 
@@ -1329,7 +1332,10 @@ class DaclDescriptorTests(DescriptorTests):
         self.assertFalse("ID" in desc_sddl)
         for x in re.findall(r"\(.*?\)", mod):
             self.assertFalse(x in desc_sddl)
-        self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        try:
+            self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        except LdbError as e:
+            self.fail(str(e))
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertFalse("ID" in desc_sddl)
         for x in re.findall(r"\(.*?\)", mod):
@@ -1356,7 +1362,10 @@ class DaclDescriptorTests(DescriptorTests):
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         mod = mod.replace(";CI;", ";CIID;")
         self.assertTrue(mod in desc_sddl)
-        self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        try:
+            self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        except LdbError as e:
+            self.fail(str(e))
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertTrue(moded in desc_sddl)
         self.assertTrue(mod in desc_sddl)
@@ -1382,7 +1391,10 @@ class DaclDescriptorTests(DescriptorTests):
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         mod = mod.replace(";OI;", ";OIIOID;")  # change it how it's gonna look like
         self.assertTrue(mod in desc_sddl)
-        self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        try:
+            self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        except LdbError as e:
+            self.fail(str(e))
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertTrue(moded in desc_sddl)
         self.assertTrue(mod in desc_sddl)
@@ -1408,7 +1420,10 @@ class DaclDescriptorTests(DescriptorTests):
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         mod = mod.replace(";CI;", ";CIID;")  # change it how it's gonna look like
         self.assertTrue(mod in desc_sddl)
-        self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        try:
+            self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        except LdbError as e:
+            self.fail(str(e))
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertTrue(moded in desc_sddl)
         self.assertTrue(mod in desc_sddl)
@@ -1434,7 +1449,10 @@ class DaclDescriptorTests(DescriptorTests):
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         mod = mod.replace(";OI;", ";OIIOID;")  # change it how it's gonna look like
         self.assertTrue(mod in desc_sddl)
-        self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        try:
+            self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        except LdbError as e:
+            self.fail(str(e))
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertTrue(moded in desc_sddl)
         self.assertTrue(mod in desc_sddl)
@@ -1460,7 +1478,10 @@ class DaclDescriptorTests(DescriptorTests):
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         mod = mod.replace(";CI;", ";CIID;")  # change it how it's gonna look like
         self.assertTrue(mod in desc_sddl)
-        self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        try:
+            self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        except LdbError as e:
+            self.fail(str(e))
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertTrue(moded in desc_sddl)
         self.assertTrue(mod in desc_sddl)
@@ -1486,7 +1507,10 @@ class DaclDescriptorTests(DescriptorTests):
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         mod = mod.replace(";OI;", ";OIIOID;")  # change it how it's gonna look like
         self.assertTrue(mod in desc_sddl)
-        self.sd_utils.modify_sd_on_dn(group_dn, "D:(OA;OI;WP;bf967a39-0de6-11d0-a285-00aa003049e2;;DU)" + moded)
+        try:
+            self.sd_utils.modify_sd_on_dn(group_dn, "D:(OA;OI;WP;bf967a39-0de6-11d0-a285-00aa003049e2;;DU)" + moded)
+        except LdbError as e:
+            self.fail(str(e))
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertTrue(moded in desc_sddl)
         self.assertTrue(mod in desc_sddl)
@@ -1512,7 +1536,10 @@ class DaclDescriptorTests(DescriptorTests):
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertTrue("(D;ID;WP;;;AU)" in desc_sddl)
         self.assertTrue("(D;CIIOID;WP;;;CO)" in desc_sddl)
-        self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        try:
+            self.sd_utils.modify_sd_on_dn(group_dn, "D:" + moded)
+        except LdbError as e:
+            self.fail(str(e))
         desc_sddl = self.sd_utils.get_sd_as_sddl(group_dn)
         self.assertTrue(moded in desc_sddl)
         self.assertTrue("(D;ID;WP;;;DA)" in desc_sddl)