[PATCH] samba-tool dbcheck: handle missing objectClass

Wed Mar 26 02:07:38 MDT 2014

Am 26.03.2014 01:09, schrieb Andrew Bartlett:
> On Sat, 2014-03-22 at 22:10 +1300, Andrew Bartlett wrote:
>> > On Fri, 2014-03-21 at 08:58 +0100, Stefan (metze) Metzmacher wrote:
>>> > > Hi Felix,
>>> > > 
>>>> > > > +    def err_missing_objectclass(self, dn):
>>>> > > > +        """handle object without objectclass"""
>>>> > > > +        self.report("ERROR: missing objectclass in object %s" % (dn))
>>>> > > > +        if not self.confirm_all("Delete object'%s'?" % dn, 'fix_all_missing_objectclass'):
>>>> > > > +            self.report("Not deleting object with missing objectclass '%s'" % dn)
>>>> > > > +            return
>>>> > > > +        if self.do_delete(dn, ["local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK],
>>>> > > > +                          "Failed to remove DN %s" % dn):
>>>> > > > +            self.report("Removed DN %s" % dn)
>>> > > 
>>> > > do_delete aleady adds DSDB_CONTROL_DBCHECK.
>> > 
>> > OK, I'll drop that.  I'll also put some more thought into the message
>> > and proposed action.  I'm hoping Felix can confirm that a samba-tool drs
>> > replicate --local --sync-all can fix the objects, and if so we might
>> > wish to suggest that (and if that isn't possible, say no other replica
>> > any longer, we should set a control to just purge the object). 
>> > 
>>> > > I'm wondering if it would be possible to reconstruct the objectClass
>>> > > based on the objectCategory?
>> > 
>> > I asked the same thing.  The issue is that these objects only have
>> > 'recently changed' attributes, not the whole object.  The bug that
>> > created them was due to a misunderstanding between the source and
>> > destination replica DC about which USNs had been sent to the destination
>> > replica. 
>> > 
>> > objectCategory would be in the same class - added at the time of add,
>> > and so not replicated.  Additionally, this seems to happen most often
>> > with deleted objects, which also don't have the objectCategory.
> I don't have any way to test this, so what I've done is as much as I
> think is reasonable in the hope that when we next see a corrupt database
> we can work to make this recoverable. 
> 
> In the meantime, at least it detects this condition, and suggests a
> solution.
> 
> Felix:  This changes your patch a little, please give your OK on that. 
> 
> Thanks,
> 
> Andrew Bartlett
> 
> -- Andrew Bartlett http://samba.org/~abartlet/ Authentication Developer,
> Samba Team http://samba.org Samba Developer, Catalyst IT
> http://catalyst.net.nz/services/samba
> 
> 
> 0001-samba-tool-dbcheck-handle-missing-objectClass.patch
> 
> 
> From 85c1e5f8a4327852bee1a368db6f509ea59101b2 Mon Sep 17 00:00:00 2001
> From: Felix Botner <botner at univention.de>
> Date: Mon, 24 Feb 2014 14:08:25 +0100
> Subject: [PATCH 1/3] samba-tool dbcheck: handle missing objectClass
> 
> In several cases we have seen objects without the objectClass attribute.
> Here the suggestion for a patch to find such objects in "samba-tool dbcheck"
> with the option to delete them.
> 
> (patch improved by Andrew Bartlett to suggest DRS re-replication)
> 
> Signed-off-by: Felix Botner <botner at univention.de>
> 
> Change-Id: I8eb0d191a2089271a9af5884d6bfbf173a5c85c6
> Reviewed-by: Andrew Bartlett <abartlet at samba.org>

Reviewed-by: Stefan Metzmacher <metze at samba.org>
But this should be the 2nd patch.

> 
> 
> 0002-dsdb-Improve-missing-objectClass-handling.patch
> 
> 
> From 75f3b6074b45ee87a30aa1b88fb717ecc0ebaf20 Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Wed, 26 Mar 2014 12:46:57 +1300
> Subject: [PATCH 2/3] dsdb: Improve missing objectClass handling
> 
> This attempts to permit deletion of objects that have no objectClass
> to allow dbcheck to clean up a corrupt database.  It is not complete,
> the replmd_replPropertyMetaDataCtr1_sort_and_verify() call will still
> fail, but this is as much as is safe to do without a way to replicate
> the original issue.
> 
> Andrew Bartlett
> 
> Change-Id: If0b6c7f18e8aee587e6b3b4af878a0145f5eac37
> Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> ---
>  source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> index 6abd27e..d5b7a70 100644
> --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> @@ -1523,19 +1523,14 @@ static int replmd_update_rpmd(struct ldb_module *module,
>  	 * corruption if we don't have this!
>  	 */
>  	objectclass_el = ldb_msg_find_element(res->msgs[0], "objectClass");
> -	if (objectclass_el == NULL) {
> -		ldb_debug_set(ldb, LDB_DEBUG_FATAL,
> -			      __location__ ": objectClass missing on %s\n",
> -			      ldb_dn_get_linearized(msg->dn));
> -		return LDB_ERR_OPERATIONS_ERROR;
> -	}

Can you keep this unless the DBCHECK control is specified ?

> -	/*
> -	 * Now check if this objectClass means we need to do urgent replication
> -	 */
> -	if (!*is_urgent && replmd_check_urgent_objectclass(objectclass_el,
> -							   situation)) {
> -		*is_urgent = true;
> +	if (objectclass_el != NULL) {
> +		/*
> +		 * Now check if this objectClass means we need to do urgent replication
> +		 */
> +		if (!*is_urgent && replmd_check_urgent_objectclass(objectclass_el,
> +								   situation)) {
> +			*is_urgent = true;
> +		}
>  	}
>  
>  	/*
> -- 1.9.0
> 
> 
> 0003-dsdb-Improve-errors-and-checks-for-missing-objectCla.patch
> 
> 
> From 417ec4717fc6e7fe34788e88ca47fc144fdbda5f Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Wed, 26 Mar 2014 12:48:17 +1300
> Subject: [PATCH 3/3] dsdb: Improve errors and checks for missing objectClass
>  values
> 
> Change-Id: I8c4ac679accc90748d20c9c86986b127c939fa75
> Signed-off-by: Andrew Bartlett <abartlet at samba.org>

Reviewed-by: Stefan Metzmacher <metze at samba.org>

metze