[PATCH] s4-drs: replmd_delete implementation

[tridge at samba.org]

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