[PATCH] Avoid replication corruption on the RODC

[Douglas Bagnall]

On 14/02/18 13:48, Garming Sam via samba-technical wrote:
> From 83058d6002a3b7531e185fede1bc73303f9c7294 Mon Sep 17 00:00:00 2001
> From: Garming Sam <garming at catalyst.net.nz>
> Date: Wed, 14 Feb 2018 13:32:33 +1300
> Subject: [PATCH 6/6] repl_metadata: Avoid silent skipping an object during DRS
>  (due to RODC rename collisions)
> 
> No error code was being set in this case, and so, we would commit the
> HWM and UDV without actually having all the updates.

> --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> @@ -5559,6 +5559,7 @@ static int replmd_replicated_handle_rename(struct replmd_replicated_request *ar,
>  				       "Conflict adding object '%s' from incoming replication but we are read only for the partition.  \n"
>  				       " - We must fail the operation until a master for this partition resolves the conflict",
>  				       ldb_dn_get_linearized(conflict_dn));
> +		ret = LDB_ERR_OPERATIONS_ERROR;
>  		goto failed;
>  	}

That's in master, and fixed a case where an LDB_SUCCESS was being
returned on error. Unfortunately there are about a dozen similar "goto
failed"s with no error set in replmd_replicated_handle_rename() and
replmd_op_possible_conflict_callback(). They go on and on like this:

	omd_value = ldb_msg_find_ldb_val(res->msgs[0], "replPropertyMetaData");
	if (omd_value == NULL) {
		DEBUG(0,(__location__ ": Unable to find replPropertyMetaData for conflicting record '%s'\n",
			 ldb_dn_get_linearized(conflict_dn)));
		goto failed;
	}

	ndr_err = ndr_pull_struct_blob(omd_value, res->msgs[0], &omd,
				       (ndr_pull_flags_fn_t)ndr_pull_replPropertyMetaDataBlob);
	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
		DEBUG(0,(__location__ ": Failed to parse old replPropertyMetaData for %s\n",
			 ldb_dn_get_linearized(conflict_dn)));
		goto failed;
	}

and at failed we return ret, which is preset to LDB_SUCCESS.

Rather than changing each one, this patch makes the code at failed
set an error code if necessary. Do we make this mistake elsewhere, 
you wonder? I couldn't find any cases.

These errors are quite unlikely.

Douglas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-repl_md-avoid-returning-LDB_SUCCESS-on-failure.patch
Type: text/x-patch
Size: 1551 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180223/8cd2a7ca/0001-repl_md-avoid-returning-LDB_SUCCESS-on-failure.bin>