[PATCH] samba-tool dbcheck: handle missing objectClass

Stefan (metze) Metzmacher metze at samba.org
Fri Feb 28 06:49:17 MST 2014


Hi Andrew,

please see my inline comments.

metze

> -- Andrew Bartlett http://samba.org/~abartlet/ Authentication Developer,
> Samba Team http://samba.org Samba Developer, Catalyst IT
> http://catalyst.net.nz/services/samba
> 
> 
> 0001-dsdb-Ensure-to-sort-replPropertyMetaData-as-UNSIGNED.patch
> 
> 
> From 428360e2505091abb3095be3b8474f2973ed58bb Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Fri, 28 Feb 2014 22:59:06 +1300
> Subject: [PATCH 1/2] dsdb: Ensure to sort replPropertyMetaData as UNSIGNED,
>  not SIGNED quantities
> 
> enum is an int, and therefore signed.  Some attributes have the high bit set.
> 
> Andrew Bartlett
> ---
>  source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> index f01a09c..593d631 100644
> --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> @@ -682,7 +682,7 @@ static int replmd_replPropertyMetaData1_attid_sort(const struct replPropertyMeta
>  		return -1;
>  	}
>  
> -	return m1->attid > m2->attid ? 1 : -1;
> +	return (uint32_t)m1->attid > (uint32_t)m2->attid ? 1 : -1;
>  }

Can you add uint32_t attid1 and attid2 helper variables?

> 0002-dsdb-Further-assert-that-we-always-have-an-objectCla.patch
> 
> 
> From 5c8002c199ee95c292461270340e77efc13eb6e4 Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Fri, 28 Feb 2014 17:49:12 +1300
> Subject: [PATCH 2/2] dsdb: Further assert that we always have an objectClass
>  and an rDN
> 
> We must have these two elements in a replPropertyMetaData for it to be valid.
> 
> We may have to relax this for new partition creation, but for now we
> want to find and isolate the database corruption.
> 
> Based initially on a patch originally by Arvid Requate <requate at univention.de>
> 
> Andrew Bartlett
> ---
>  source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 119 +++++++++++++++++++-----
>  1 file changed, 95 insertions(+), 24 deletions(-)
> 
> diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> index 593d631..8c66acf 100644
> --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> @@ -685,22 +685,45 @@ static int replmd_replPropertyMetaData1_attid_sort(const struct replPropertyMeta
>  	return (uint32_t)m1->attid > (uint32_t)m2->attid ? 1 : -1;
>  }
>  
> -static int replmd_replPropertyMetaDataCtr1_sort(struct replPropertyMetaDataCtr1 *ctr1,
> -						const struct dsdb_schema *schema,
> -						struct ldb_dn *dn)
> +static int replmd_replPropertyMetaDataCtr1_verify(struct ldb_context *ldb, 
> +						  struct replPropertyMetaDataCtr1 *ctr1,
> +						  const struct dsdb_attribute *rdn_sa,
> +						  struct ldb_dn *dn)
> +{
> +	if (ctr1->count == 0) {
> +		ldb_asprintf_errstring(ldb, "No elements found in replPropertyMetaData for %s!\n", ldb_dn_get_linearized(dn));
> +		return LDB_ERR_OPERATIONS_ERROR;
> +	}
> +	if (ctr1->array[ctr1->count - 1].attid != rdn_sa->attributeID_id) {
> +		ldb_asprintf_errstring(ldb, "No rDN found in replPropertyMetaData for %s!\n", ldb_dn_get_linearized(dn));
> +		return LDB_ERR_OPERATIONS_ERROR;
> +	}
> +
> +	if (ctr1->array[0].attid != DRSUAPI_ATTID_objectClass) {
> +		ldb_asprintf_errstring(ldb, "No objectClass found in replPropertyMetaData for %s!\n", ldb_dn_get_linearized(dn));
> +		return LDB_ERR_OPERATIONS_ERROR;
> +	}

Can you add a comment that DRSUAPI_ATTID_objectClass is '0x00000000'?

> +
> +static int replmd_replPropertyMetaDataCtr1_sort_and_verify(struct ldb_context *ldb, 
> +							   struct replPropertyMetaDataCtr1 *ctr1,
> +							   const struct dsdb_schema *schema,
> +							   struct ldb_dn *dn)
>  {
>  	const char *rdn_name;
>  	const struct dsdb_attribute *rdn_sa;
>  
>  	rdn_name = ldb_dn_get_rdn_name(dn);
>  	if (!rdn_name) {
> -		DEBUG(0,(__location__ ": No rDN for %s?\n", ldb_dn_get_linearized(dn)));
> +		ldb_asprintf_errstring(ldb, __location__ ": No rDN for %s?\n", ldb_dn_get_linearized(dn));
>  		return LDB_ERR_OPERATIONS_ERROR;
>  	}
>  
>  	rdn_sa = dsdb_attribute_by_lDAPDisplayName(schema, rdn_name);
>  	if (rdn_sa == NULL) {
> -		DEBUG(0,(__location__ ": No sa found for rDN %s for %s\n", rdn_name, ldb_dn_get_linearized(dn)));
> +		ldb_asprintf_errstring(ldb, ": No sa found for rDN %s for %s\n", rdn_name, ldb_dn_get_linearized(dn));

Do we still see this messages in the log file?
I'd like to keep them!

I think we want ldb_debug_set() here.

>  		return LDB_ERR_OPERATIONS_ERROR;
>  	}
>  
> @@ -708,8 +731,7 @@ static int replmd_replPropertyMetaDataCtr1_sort(struct replPropertyMetaDataCtr1
>  		 rdn_sa->attributeID_id, rdn_name, ldb_dn_get_linearized(dn)));
>  
>  	LDB_TYPESAFE_QSORT(ctr1->array, ctr1->count, &rdn_sa->attributeID_id, replmd_replPropertyMetaData1_attid_sort);
> -
> -	return LDB_SUCCESS;
> +	return replmd_replPropertyMetaDataCtr1_verify(ldb, ctr1, rdn_sa, dn);
>  }
>  
>  static int replmd_ldb_message_element_attid_sort(const struct ldb_message_element *e1,
> @@ -1025,8 +1047,9 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  	/*
>  	 * sort meta data array, and move the rdn attribute entry to the end
>  	 */
> -	ret = replmd_replPropertyMetaDataCtr1_sort(&nmd.ctr.ctr1, ac->schema, msg->dn);
> +	ret = replmd_replPropertyMetaDataCtr1_sort_and_verify(ldb, &nmd.ctr.ctr1, ac->schema, msg->dn);
>  	if (ret != LDB_SUCCESS) {
> +		ldb_asprintf_errstring(ldb, "%s: error during direct ADD: %s", __func__, ldb_errstring(ldb));
>  		talloc_free(ac);
>  		return ret;
>  	}
> @@ -1080,7 +1103,16 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  	 */
>  	replmd_ldb_message_sort(msg, ac->schema);
>  
> +	/*
> +	 * Assert that we do have an objectClass
> +	 */
>  	objectclass_el = ldb_msg_find_element(msg, "objectClass");
> +	if (!objectclass_el) {

if (objectclass_el == NULL) {

> +		DEBUG(0,(__location__ ": objectClass missing on %s\n",
> +			 ldb_dn_get_linearized(msg->dn)));
> +		talloc_free(ac);
> +		return LDB_ERR_OPERATIONS_ERROR;
> +	}
>  	is_urgent = replmd_check_urgent_objectclass(objectclass_el,
>  							REPL_URGENT_ON_CREATE);
>  
> @@ -1398,12 +1430,6 @@ static int replmd_update_rpmd(struct ldb_module *module,
>  			return ret;
>  		}
>  
> -		objectclass_el = ldb_msg_find_element(res->msgs[0], "objectClass");
> -		if (is_urgent && replmd_check_urgent_objectclass(objectclass_el,
> -								situation)) {
> -			*is_urgent = true;
> -		}
> -
>  		db_seq = ldb_msg_find_attr_as_uint64(res->msgs[0], "uSNChanged", 0);
>  		if (*seq_num <= db_seq) {
>  			DEBUG(0,(__location__ ": changereplmetada control provided but max(local_usn)"\
> @@ -1428,12 +1454,6 @@ static int replmd_update_rpmd(struct ldb_module *module,
>  			return ret;
>  		}
>  
> -		objectclass_el = ldb_msg_find_element(res->msgs[0], "objectClass");
> -		if (is_urgent && replmd_check_urgent_objectclass(objectclass_el,
> -								situation)) {
> -			*is_urgent = true;
> -		}
> -
>  		omd_value = ldb_msg_find_ldb_val(res->msgs[0], "replPropertyMetaData");
>  		if (!omd_value) {
>  			DEBUG(0,(__location__ ": Object %s does not have a replPropertyMetaData attribute\n",
> @@ -1464,12 +1484,32 @@ static int replmd_update_rpmd(struct ldb_module *module,
>  				return ret;
>  			}
>  
> -			if (is_urgent && !*is_urgent && (situation == REPL_URGENT_ON_UPDATE)) {
> +			if (!*is_urgent && (situation == REPL_URGENT_ON_UPDATE)) {
>  				*is_urgent = replmd_check_urgent_attribute(&msg->elements[i]);
>  			}
>  
>  		}
>  	}
> +
> +	/*
> +	 * Assert that we have an objectClass attribute - is is major

... it is a major...

> +	 * corruption if we don't have this!
> +	 */
> +	objectclass_el = ldb_msg_find_element(res->msgs[0], "objectClass");
> +	if (!objectclass_el) {

if (objectclass_el == NULL) {

> +		ldb_asprintf_errstring(ldb, __location__ ": objectClass missing on %s\n",
> +				       ldb_dn_get_linearized(msg->dn));
> +		return LDB_ERR_OPERATIONS_ERROR;
> +	}
> +
> +	/*
> +	 * 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;
> +	}
> +
>  	/*
>  	 * replmd_update_rpmd_element has done an update if the
>  	 * seq_num is set
> @@ -1504,8 +1544,9 @@ static int replmd_update_rpmd(struct ldb_module *module,
>  			return LDB_ERR_OPERATIONS_ERROR;
>  		}
>  
> -		ret = replmd_replPropertyMetaDataCtr1_sort(&omd.ctr.ctr1, schema, msg->dn);
> +		ret = replmd_replPropertyMetaDataCtr1_sort_and_verify(ldb, &omd.ctr.ctr1, schema, msg->dn);
>  		if (ret != LDB_SUCCESS) {
> +			ldb_asprintf_errstring(ldb, "%s: %s", __func__, ldb_errstring(ldb));
>  			return ret;
>  		}
>  
> @@ -3831,7 +3872,9 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar)
>  	unsigned int i;
>  	int ret;
>  	bool remote_isDeleted = false;
> -
> +	const struct dsdb_attribute *rdn_sa;
> +	const char *rdn_name;
> +							 
>  	ldb = ldb_module_get_ctx(ar->module);
>  	msg = ar->objs->objects[ar->index_current].msg;
>  	md = ar->objs->objects[ar->index_current].meta_data;
> @@ -3881,6 +3924,25 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar)
>  	/*
>  	 * the meta data array is already sorted by the caller
>  	 */
> +
> +	rdn_name = ldb_dn_get_rdn_name(msg->dn);
> +	if (!rdn_name) {

if (rdn_name == NULL) {

> +		ldb_asprintf_errstring(ldb, __location__ ": No rDN for %s?\n", ldb_dn_get_linearized(msg->dn));
> +		return replmd_replicated_request_error(ar, LDB_ERR_OPERATIONS_ERROR);
> +	}
> +
> +	rdn_sa = dsdb_attribute_by_lDAPDisplayName(ar->schema, rdn_name);
> +	if (rdn_sa == NULL) {
> +		ldb_asprintf_errstring(ldb, ": No schema attribute found for rDN %s for %s\n", rdn_name, ldb_dn_get_linearized(msg->dn));
> +		return replmd_replicated_request_error(ar, LDB_ERR_OPERATIONS_ERROR);
> +	}
> +
> +	ret = replmd_replPropertyMetaDataCtr1_verify(ldb, &md->ctr.ctr1, rdn_sa, msg->dn);
> +	if (ret != LDB_SUCCESS) {
> +		ldb_asprintf_errstring(ldb, "%s: error during DRS repl ADD: %s", __func__, ldb_errstring(ldb));
> +		return replmd_replicated_request_error(ar, ret);
> +	}
> +
>  	for (i=0; i < md->ctr.ctr1.count; i++) {
>  		md->ctr.ctr1.array[i].local_usn = ar->seq_num;
>  	}
> @@ -4390,8 +4452,9 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
>  	 *
>  	 * sort the new meta data array
>  	 */
> -	ret = replmd_replPropertyMetaDataCtr1_sort(&nmd.ctr.ctr1, ar->schema, msg->dn);
> +	ret = replmd_replPropertyMetaDataCtr1_sort_and_verify(ldb, &nmd.ctr.ctr1, ar->schema, msg->dn);
>  	if (ret != LDB_SUCCESS) {
> +		ldb_asprintf_errstring(ldb, "%s: error during DRS repl merge: %s", __func__, ldb_errstring(ldb));
>  		return ret;
>  	}
>  
> @@ -4465,6 +4528,14 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
>  	/* we want to replace the old values */
>  	for (i=0; i < msg->num_elements; i++) {
>  		msg->elements[i].flags = LDB_FLAG_MOD_REPLACE;
> +		if (ldb_attr_cmp(msg->elements[i].name, "objectClass") == 0) {
> +			if (msg->elements[i].num_values == 0) {
> +				ldb_asprintf_errstring(ldb, __location__
> +						       ": objectClass removed on %s, aborting replication\n",
> +						       ldb_dn_get_linearized(msg->dn));
> +				return replmd_replicated_request_error(ar, LDB_ERR_OPERATIONS_ERROR);
> +			}
> +		}
>  	}
>  
>  	if (DEBUGLVL(4)) {
> -- 1.8.5.3


More information about the samba-technical mailing list