[PATCH] s4-drs: replmd_delete implementation

Tue Dec 15 12:00:57 MST 2009

Hi Tridge,

The relax control now seems to be working. Before calling
ldb_modify_control, I create a control and pass it to the ldb_modify. In
samldb.c I verify if there is a LDB_CONTROL_RELAX_OID flag only in the place
that was failing the sAMAccountType deletion. Is this ok? (
http://repo.or.cz/w/Samba/eduardoll.git/commit/8272ccad865562f37ca641144db256289eb6ce0b
)

Now I have started working on creating a function to modify/remove the
elements. And I still need to change dsdb_get_deleted_objects_dn to use the
wellKnownObjects.

Thanks.

--
Eduardo Lima
Sent from Campinas, SP, Brazil

On Mon, Dec 7, 2009 at 15:48, Eduardo Lima <eduardoll at gmail.com> wrote:

> Hi Tridge,
>
> Sorry for the late answer. The two last weeks were very difficult here. Now
> I'm working hard on this task to delivery it faster.
>
>
> >I think though that dsdb_get_deleted_objects_dn() needs to be redone
> > to take a DN argument, as the correct Deleted Objects container is
> > different depending on what DN is being deleted. So I think you should
> > probably first write a function which finds the root of a NC given a
> > DN, and then construct the Deleted Objects DN from that. The function
> > to find the root of a NC might look like this:
> >
> >  struct ldb_dn *dsdb_find_nc_root(struct ldb_context *samdb, const struct
> ldb_dn *dn);
> >
> > it could operate in a couple of different ways. One possibility is to
> > get the list of naming contexts from the rootDSE and then find the NC
> > that matches the DN that is passed in. To make this easy you would
> > sort the DNs from the list of naming contexts using a function similar
> > to partition_sort_compare(). You'd then find the matching one by
> > calling ldb_dn_compare_base().
>
> I've updated my repository to a new version of replmd_delete that uses
> dsdb_find_nc_root (that code in the repository is currently not working..
> it's just there to show it to you). I also created two new functions inside
> util.c to help in the sorting of namingContexts. Is there a better place to
> put these functions?
>
> From what I understood, now I need to find de Deleted Objects container for
> de root of NC using the wellKnownObjects, right? (Currently I'm just
> inserting CN=Deleted Objects to the root of NC). But I can't find a good
> example to understand how to use a wellKnownObject.
>
> Right before when I return the deleted_dn (in function
> dsdb_get_deleted_objects_dn) and base_dn (in function dsdb_find_nc_root) I
> do a talloc_free(tmp_ctx). Is this right? Do I need to pass a tmp_ctx
> created in the replmd_delete to the other functions (I think I'll lose the
> returned DN if I don't do this)
>
> I also changed function samldb_modify() in samldb.c to test if there is a
> relax control. This is currently not working because I could not make
> ldb_modify_ctrl work:
>
> ctrl = talloc(tmp_ctx, struct ldb_control);
> ctrl->oid = LDB_CONTROL_RELAX_OID;
> ctrl->critical = 1;
>
> ret = ldb_modify_ctrl(ldb, msg, &ctrl);
>
> >Would you like me to help you by creating some of the helper functions
> >you will need? For example, I could add the dsdb_module_modify(),
> >dsdb_module_rename() and dsdb_find_nc_root() functions. I could also
> >do the "relax" change needed in samldb.c. That would just leave you
> > with the core logic of replmd_delete(). Or if you prefer to do all the
> >functions yourself let me know.
>
> I'm trying to implement these functions but I will need some help. If you
> think that creating the helper functions will be easier to make me
> understand, please, feel free to do it. You can also modify the functions
> I've created so far.
>
>
> Thanks.
>
> --
> Eduardo Lima
> Sent from Campinas, SP, Brazil
>
> On Sat, Nov 28, 2009 at 03:24, <tridge at samba.org> wrote:
>
>> Hi Eduardo,
>>
>>  > My git tree is updated to these latest changes: git://
>>  > repo.or.cz/Samba/eduardoll.git
>>
>> Thanks, I've looked through your changes and I have some comments
>> below.
>>
>>  > It will fail whenever sAMAccountType is specified in the message to be
>>  > modified. Just for testing purpose, I modified the code above to not
>> fail
>>  > when el->flag == LDB_FLAG_MOD_DELETE:
>>  >
>>  > el = ldb_msg_find_element(req->op.mod.message, "sAMAccountType");
>>  >     if ((el != NULL) && (el->flags != LDB_FLAG_MOD_DELETE)) {
>>  >         ldb_asprintf_errstring(ldb,
>>  >             "sAMAccountType must not be specified!");
>>  >         return LDB_ERR_UNWILLING_TO_PERFORM;
>>  > }
>>
>> I think a better approach would be to set the "relax" control on the
>> modify operation. The "relax" control can be set for internal
>> operations to say "don't do strict checking, I know what I'm doing".
>>
>> If you run "git grep RELAX_OID" you'll find some existing places that
>> use this control for similar purposes. You'll need to check for the
>> relax control in samldb.c and if it is set then allow for modifies on
>> sAMAccountType. Then in repl_meta_data.c you'll need to make sure the
>> modify is done with the relax control set.
>>
>>  > Regarding the delete test on Windows 2008, I could not find out an easy
>> way
>>  > to insert a new object into the CN=Schema,CN=Configuration,DC=x
>> partition.
>>
>> If you look in lib/ldb/tests/python/ldap.py you'll see some examples
>> of adding a new schema entry.
>>
>> I looked into this a bit further, and it looks to me like deleting
>> schema objects is not something we should be allowing anyway.
>>
>> I think the simplest thing to do is to not do the rename/modify and
>> instead do a normal delete if the NC doesn't have a Deleted Objects
>> container.
>>
>>  > Another thing I noticed is that all the deleted objects in
>>  > *
>>  > CN=Deleted Objects,CN=Configuration,DC=x*
>>  >
>>  > have lastKnownParent:
>>  >
>>  > CN=NTDS
>>  >
>> Settings,CN=W2K8,CN=Servers,CN=Default-First-Site-Name,CN=Sites,CN=Configuration,DC=adsamba,DC=ltc,DC=inovasoft,DC=unicamp,DC=br.
>>
>> that's just because those are the objects that have been deleted on
>> your system.
>>
>>  > When browsing my DC using LDP I also noticed that shows a
>> wellKnownObjects
>>  > list that has de deleted container:
>>  >
>>  > wellKnownObjects:
>>  > B:32:18E2EA80684F11D2B9AA00C04F79F805:CN=Deleted
>>  > Objects,DC=adsamba,DC=ltc,DC=inovasoft,DC=unicamp,DC=br;
>>
>> right
>>
>>  >
>>  > Does samba has this wellKnownObjects? If so, should I create a entry in
>> the
>>  > samba's wellKnownObjects list?
>>
>> Samba already creates this wellKnownObjects entry (try "git grep
>> wellKnownObject" and you'll see it in setup/*.ldif).
>>
>> I think the correct way of finding the Deleted Objects container for a
>> NC is to fetch the wellKnownObjects attribute for the root of the NC,
>> and then use the GUID embedded in that to find the Deleted Objects
>> container. You could modify dsdb_get_deleted_objects_dn() to do this
>> (thought you will have to pass the DN of the object being deleted).
>>
>> Now for some more specific comments on the code in your tree.
>>
>> In dsdb_get_deleted_objects_dn() you do a search for the
>> defaultNamingContext. It would have been much simpler to just call
>> samdb_base_dn() which already returns this.
>>
>> I think though that dsdb_get_deleted_objects_dn() needs to be redone
>> to take a DN argument, as the correct Deleted Objects container is
>> different depending on what DN is being deleted. So I think you should
>> probably first write a function which finds the root of a NC given a
>> DN, and then construct the Deleted Objects DN from that. The function
>> to find the root of a NC might look like this:
>>
>>  struct ldb_dn *dsdb_find_nc_root(struct ldb_context *samdb, const struct
>> ldb_dn *dn);
>>
>> it could operate in a couple of different ways. One possibility is to
>> get the list of naming contexts from the rootDSE and then find the NC
>> that matches the DN that is passed in. To make this easy you would
>> sort the DNs from the list of naming contexts using a function similar
>> to partition_sort_compare(). You'd then find the matching one by
>> calling ldb_dn_compare_base().
>>
>> Another alternative is to write a function that walks up the tree,
>> looking for a DN with instanceType containing
>> INSTANCE_TYPE_IS_NC_HEAD.
>>
>> In your replmd_delete() function you have the same bug that we
>> discussed in the tutorial last week where you use
>> ldb_dn_add_child_fmt() on a DN that came from a cache, thus modifying
>> that DN. The current dsdb_get_deleted_objects_dn() function stores and
>> returns values from cache.deleted_dn. When you call
>> ldb_dn_add_child_fmt() you are modifying that DN directly, which means
>> the 2nd time you call dsdb_get_deleted_objects_dn() you will not get
>> the Deleted Objects DN, but instead you will get the DN you last
>> modified.
>>
>> Further down your replmd_delete() code you directly call
>> ldb_rename(). That is OK for now, but later we will need to add a
>> dsdb_module_rename() call in dsdb/samdb/ldb_modules/util.c to allow
>> you to do a rename on the correct part of the module stack.
>>
>> For the modify part of the replmd_delete() code, you need to read
>> section 3.1.1.5.5.1.1 of [MS-ADTS].pdf to see how you should be
>> handling which attributes need to be added and removed from the object
>> when it is deleted. You should probably also read the other parts of
>> [MS-ADTS] that deal with deleted objects.
>>
>> If you read that doc, you'll see that you will need to do something
>> like this:
>>
>>  1) fetch the existing object from the database
>>
>>  2) for each of the attributes of the object, get the schema attribute
>>  structure using dsdb_attribute_by_lDAPDisplayName(). That will
>>  return a struct dsdb_attribute, which contains a searchFlags element,
>>  which says whether the attribute should be preserved on deletion.
>>
>>  3) you'll also need to check the attribute against the list of
>>  attributes specified in 3.1.1.5.5.1.1 as being always preserved.
>>
>> Then for each attribute in the existing object that should be removed,
>> you'll need to build up the msg ready for the modify. You need to make
>> sure that you only specify an attribute for deletion if it is in the
>> object currently (other the ldb_modify will give an error).
>>
>> Finally, we will need a dsdb_module_modify() function, similar to what
>> I mentioned about dsdb_module_rename() above.
>>
>> Would you like me to help you by creating some of the helper functions
>> you will need? For example, I could add the dsdb_module_modify(),
>> dsdb_module_rename() and dsdb_find_nc_root() functions. I could also
>> do the "relax" change needed in samldb.c. That would just leave you
>> with the core logic of replmd_delete(). Or if you prefer to do all the
>> functions yourself let me know.
>>
>> Cheers, Tridge
>>
>
>