From fb70ad418a81ce37ff33c150adbd7c49616c6c41 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 4 Aug 2016 11:09:21 -0700 Subject: [PATCH 01/20] s4: repl: Ensure all error paths in dreplsrv_op_pull_source_get_changes_trigger() are protected with tevent returns. Otherwise dreplsrv_op_pull_source_get_changes_trigger() could infinitely recurse. Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Sat Aug 6 01:24:05 CEST 2016 on sn-devel-144 BUG: https://bugzilla.samba.org/show_bug.cgi?id=12115 (cherry picked from commit 1ddd01dd21d5db9440dd382190e203c1e756fa3a) --- source4/dsdb/repl/drepl_out_helpers.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c index bf8a372..2a74cb7 100644 --- a/source4/dsdb/repl/drepl_out_helpers.c +++ b/source4/dsdb/repl/drepl_out_helpers.c @@ -446,6 +446,8 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req) if (!W_ERROR_IS_OK(werr)) { DEBUG(0,(__location__ ": Failed to convert UDV for %s : %s\n", ldb_dn_get_linearized(partition->dn), win_errstr(werr))); + tevent_req_nterror(req, werror_to_ntstatus(werr)); + return; } } @@ -470,6 +472,7 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req) status = dreplsrv_get_gc_partial_attribute_set(service, r, &pas); if (!NT_STATUS_IS_OK(status)) { DEBUG(0,(__location__ ": Failed to construct GC partial attribute set : %s\n", nt_errstr(status))); + tevent_req_nterror(req, status); return; } replica_flags &= ~DRSUAPI_DRS_WRIT_REP; @@ -482,6 +485,7 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req) status = dreplsrv_get_rodc_partial_attribute_set(service, r, &pas, for_schema); if (!NT_STATUS_IS_OK(status)) { DEBUG(0,(__location__ ": Failed to construct RODC partial attribute set : %s\n", nt_errstr(status))); + tevent_req_nterror(req, status); return; } replica_flags &= ~DRSUAPI_DRS_WRIT_REP; -- 1.9.1 From 7322f6fc2d5b5e59d1be18967679428b971c74c1 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 4 Aug 2016 23:04:32 +0200 Subject: [PATCH 02/20] s4:dsdb/schema: don't change schema->schema_info on originating schema changes. The next reload will take care of it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12114 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 7143aed4e428f52bf1381b38a27412fd198060ab) --- source4/dsdb/samdb/ldb_modules/schema_util.c | 38 +++++++++++++++------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/schema_util.c b/source4/dsdb/samdb/ldb_modules/schema_util.c index 7402c04..47d2411 100644 --- a/source4/dsdb/samdb/ldb_modules/schema_util.c +++ b/source4/dsdb/samdb/ldb_modules/schema_util.c @@ -276,9 +276,7 @@ int dsdb_module_schema_info_update(struct ldb_module *ldb_module, { int ret; const struct GUID *invocation_id; - DATA_BLOB ndr_blob; struct dsdb_schema_info *schema_info; - const char *schema_info_str; WERROR werr; TALLOC_CTX *temp_ctx = talloc_new(schema); if (temp_ctx == NULL) { @@ -321,22 +319,26 @@ int dsdb_module_schema_info_update(struct ldb_module *ldb_module, return ret; } - /* finally, update schema_info in the cache */ - werr = dsdb_blob_from_schema_info(schema_info, temp_ctx, &ndr_blob); - if (!W_ERROR_IS_OK(werr)) { - ldb_asprintf_errstring(ldb_module_get_ctx(ldb_module), "Failed to get schema info"); - talloc_free(temp_ctx); - return ldb_operr(ldb_module_get_ctx(ldb_module)); - } - - schema_info_str = hex_encode_talloc(schema, ndr_blob.data, ndr_blob.length); - if (!schema_info_str) { - talloc_free(temp_ctx); - return ldb_module_oom(ldb_module); - } - - talloc_unlink(schema, discard_const(schema->schema_info)); - schema->schema_info = schema_info_str; + /* + * We don't update the schema->schema_info! + * as that would not represent the other information + * in schema->* + * + * We're not sure if the current transaction will go through! + * E.g. schema changes are only allowed on the schema master, + * otherwise they result in a UNWILLING_TO_PERFORM and a + * + * Note that schema might a global variable shared between + * multiple ldb_contexts. With process model "single" it + * means the drsuapi server also uses it. + * + * We keep it simple and just try to update the + * stored value. + * + * The next schema reload will pick it up, which + * then works for originating and replicated changes + * in the same way. + */ talloc_free(temp_ctx); return LDB_SUCCESS; -- 1.9.1 From add100181e9af25d0e12ca9d5109fa2680852059 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 5 Aug 2016 11:05:37 +0200 Subject: [PATCH 03/20] s4:dsdb/repl: avoid recursion after fetching schema changes. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12115 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit ab63866e258c1afbe60663f2a36bea58e5e80d53) --- source4/dsdb/repl/drepl_out_helpers.c | 35 +++++++++++++++++++++++++---------- source4/dsdb/repl/drepl_service.h | 7 ------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c index 2a74cb7..9fe8c3b 100644 --- a/source4/dsdb/repl/drepl_out_helpers.c +++ b/source4/dsdb/repl/drepl_out_helpers.c @@ -241,6 +241,13 @@ struct dreplsrv_op_pull_source_state { struct tevent_context *ev; struct dreplsrv_out_operation *op; void *ndr_struct_ptr; + /* + * Used when we have to re-try with a different NC, eg for + * EXOP retry or to get a current schema first + */ + struct dreplsrv_partition_source_dsa *source_dsa_retry; + enum drsuapi_DsExtendedOperation extended_op_retry; + bool retry_started; }; static void dreplsrv_op_pull_source_connect_done(struct tevent_req *subreq); @@ -785,10 +792,17 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req dsdb_repl_flags, state, &objects); - if (W_ERROR_EQUAL(status, WERR_DS_DRA_SCHEMA_MISMATCH) - && state->op->source_dsa_retry == NULL) { + if (W_ERROR_EQUAL(status, WERR_DS_DRA_SCHEMA_MISMATCH)) { struct dreplsrv_partition *p; + if (state->retry_started) { + nt_status = werror_to_ntstatus(WERR_BAD_NET_RESP); + DEBUG(0,("Failed to convert objects after retry: %s/%s\n", + win_errstr(status), nt_errstr(nt_status))); + tevent_req_nterror(req, nt_status); + return; + } + /* * Change info sync or extended operation into a fetch * of the schema partition, so we get all the schema @@ -802,14 +816,14 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req if (state->op->extended_op == DRSUAPI_EXOP_REPL_SECRET) { - state->op->extended_op_retry = state->op->extended_op; + state->extended_op_retry = state->op->extended_op; } else { - state->op->extended_op_retry = DRSUAPI_EXOP_NONE; + state->extended_op_retry = DRSUAPI_EXOP_NONE; } state->op->extended_op = DRSUAPI_EXOP_NONE; if (ldb_dn_compare(nc_root, partition->dn) == 0) { - state->op->source_dsa_retry = state->op->source_dsa; + state->source_dsa_retry = state->op->source_dsa; } else { status = dreplsrv_partition_find_for_nc(service, NULL, NULL, @@ -825,7 +839,7 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req } status = dreplsrv_partition_source_dsa_by_guid(p, &state->op->source_dsa->repsFrom1->source_dsa_obj_guid, - &state->op->source_dsa_retry); + &state->source_dsa_retry); if (!W_ERROR_IS_OK(status)) { struct GUID_txt_buf str; @@ -867,6 +881,7 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req } DEBUG(4,("Wrong schema when applying reply GetNCChanges, retrying\n")); + state->retry_started = true; dreplsrv_op_pull_source_get_changes_trigger(req); return; @@ -930,10 +945,10 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req * pulling the schema, then go back and do the original * operation once we are done. */ - if (state->op->source_dsa_retry != NULL) { - state->op->source_dsa = state->op->source_dsa_retry; - state->op->extended_op = state->op->extended_op_retry; - state->op->source_dsa_retry = NULL; + if (state->source_dsa_retry != NULL) { + state->op->source_dsa = state->source_dsa_retry; + state->op->extended_op = state->extended_op_retry; + state->source_dsa_retry = NULL; dreplsrv_op_pull_source_get_changes_trigger(req); return; } diff --git a/source4/dsdb/repl/drepl_service.h b/source4/dsdb/repl/drepl_service.h index 317fa87..edba4c4 100644 --- a/source4/dsdb/repl/drepl_service.h +++ b/source4/dsdb/repl/drepl_service.h @@ -130,13 +130,6 @@ struct dreplsrv_out_operation { enum drsuapi_DsExtendedError extended_ret; dreplsrv_extended_callback_t callback; void *cb_data; - - /* - * Used when we have to re-try with a different NC, eg for - * EXOP retry or to get a current schema first - */ - struct dreplsrv_partition_source_dsa *source_dsa_retry; - enum drsuapi_DsExtendedOperation extended_op_retry; }; struct dreplsrv_notify_operation { -- 1.9.1 From 7db128424f29da830c4e6bb014b5a75aada193a2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 4 Aug 2016 10:03:14 +0200 Subject: [PATCH 04/20] s4:dsdb/schema: store struct dsdb_schema_info instead of a hexstring This will simplify the schema checking in future. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12115 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 386dbc428b447bbdf9c500191273658b92b13f7a) --- source4/dsdb/schema/schema.h | 2 +- source4/dsdb/schema/schema_info_attr.c | 27 ++++++++++------ source4/dsdb/schema/schema_init.c | 31 +++++++++--------- source4/dsdb/schema/schema_prefixmap.c | 22 +++++++------ source4/torture/drs/unit/prefixmap_tests.c | 49 ++++++++++++++--------------- source4/torture/drs/unit/schemainfo_tests.c | 12 ++++--- 6 files changed, 79 insertions(+), 64 deletions(-) diff --git a/source4/dsdb/schema/schema.h b/source4/dsdb/schema/schema.h index 6c3581b..6543eda 100644 --- a/source4/dsdb/schema/schema.h +++ b/source4/dsdb/schema/schema.h @@ -212,7 +212,7 @@ struct dsdb_schema { * this is the content of the schemaInfo attribute of the * Schema-Partition head object. */ - const char *schema_info; + struct dsdb_schema_info *schema_info; struct dsdb_attribute *attributes; struct dsdb_class *classes; diff --git a/source4/dsdb/schema/schema_info_attr.c b/source4/dsdb/schema/schema_info_attr.c index e113033..76cd90d 100644 --- a/source4/dsdb/schema/schema_info_attr.c +++ b/source4/dsdb/schema/schema_info_attr.c @@ -175,10 +175,11 @@ WERROR dsdb_blob_from_schema_info(const struct dsdb_schema_info *schema_info, WERROR dsdb_schema_info_cmp(const struct dsdb_schema *schema, const struct drsuapi_DsReplicaOIDMapping_Ctr *ctr) { - bool bres; - DATA_BLOB blob; - char *schema_info_str; - struct drsuapi_DsReplicaOIDMapping *mapping; + TALLOC_CTX *frame = NULL; + DATA_BLOB blob = data_blob_null; + struct dsdb_schema_info *schema_info = NULL; + const struct drsuapi_DsReplicaOIDMapping *mapping = NULL; + WERROR werr; /* we should have at least schemaInfo element */ if (ctr->num_mappings < 1) { @@ -196,13 +197,21 @@ WERROR dsdb_schema_info_cmp(const struct dsdb_schema *schema, return WERR_INVALID_PARAMETER; } - schema_info_str = hex_encode_talloc(NULL, blob.data, blob.length); - W_ERROR_HAVE_NO_MEMORY(schema_info_str); + frame = talloc_stackframe(); + werr = dsdb_schema_info_from_blob(&blob, frame, &schema_info); + if (!W_ERROR_IS_OK(werr)) { + TALLOC_FREE(frame); + return werr; + } - bres = strequal(schema->schema_info, schema_info_str); - talloc_free(schema_info_str); + if (schema->schema_info->revision != schema_info->revision) { + werr = WERR_DS_DRA_SCHEMA_MISMATCH; + } else { + werr = WERR_OK; + } - return bres ? WERR_OK : WERR_DS_DRA_SCHEMA_MISMATCH; + TALLOC_FREE(frame); + return werr; } diff --git a/source4/dsdb/schema/schema_init.c b/source4/dsdb/schema/schema_init.c index 16f213d..c9c55ca 100644 --- a/source4/dsdb/schema/schema_init.c +++ b/source4/dsdb/schema/schema_init.c @@ -64,7 +64,11 @@ struct dsdb_schema *dsdb_schema_copy_shallow(TALLOC_CTX *mem_ctx, goto failed; } - schema_copy->schema_info = talloc_strdup(schema_copy, schema->schema_info); + schema_copy->schema_info = talloc(schema_copy, struct dsdb_schema_info); + if (!schema_copy->schema_info) { + goto failed; + } + *schema_copy->schema_info = *schema->schema_info; /* copy classes and attributes*/ for (cls = schema->classes; cls; cls = cls->next) { @@ -107,8 +111,8 @@ WERROR dsdb_load_prefixmap_from_drsuapi(struct dsdb_schema *schema, const struct drsuapi_DsReplicaOIDMapping_Ctr *ctr) { WERROR werr; - const char *schema_info; - struct dsdb_schema_prefixmap *pfm; + struct dsdb_schema_info *schema_info = NULL; + struct dsdb_schema_prefixmap *pfm = NULL; werr = dsdb_schema_pfm_from_drsuapi_pfm(ctr, true, schema, &pfm, &schema_info); W_ERROR_NOT_OK_RETURN(werr); @@ -117,7 +121,7 @@ WERROR dsdb_load_prefixmap_from_drsuapi(struct dsdb_schema *schema, talloc_free(schema->prefixmap); schema->prefixmap = pfm; - talloc_free(discard_const(schema->schema_info)); + talloc_free(schema->schema_info); schema->schema_info = schema_info; return WERR_OK; @@ -169,8 +173,8 @@ WERROR dsdb_load_oid_mappings_ldb(struct dsdb_schema *schema, const struct ldb_val *schemaInfo) { WERROR werr; - const char *schema_info; - struct dsdb_schema_prefixmap *pfm; + struct dsdb_schema_info *schema_info = NULL; + struct dsdb_schema_prefixmap *pfm = NULL; TALLOC_CTX *mem_ctx; /* verify schemaInfo blob is valid one */ @@ -192,19 +196,18 @@ WERROR dsdb_load_oid_mappings_ldb(struct dsdb_schema *schema, } /* decode schema_info */ - schema_info = hex_encode_talloc(mem_ctx, - schemaInfo->data, - schemaInfo->length); - if (!schema_info) { + werr = dsdb_schema_info_from_blob(schemaInfo, mem_ctx, &schema_info); + if (!W_ERROR_IS_OK(werr)) { + DEBUG(0, (__location__ " dsdb_schema_info_from_blob failed: %s\n", win_errstr(werr))); talloc_free(mem_ctx); - return WERR_NOMEM; + return werr; } /* store prefixMap and schema_info into cached Schema */ talloc_free(schema->prefixmap); schema->prefixmap = talloc_steal(schema, pfm); - talloc_free(discard_const(schema->schema_info)); + talloc_free(schema->schema_info); schema->schema_info = talloc_steal(schema, schema_info); /* clean up locally allocated mem */ @@ -257,8 +260,8 @@ WERROR dsdb_get_oid_mappings_ldb(const struct dsdb_schema *schema, talloc_free(ctr); W_ERROR_NOT_OK_RETURN(status); - *schemaInfo = strhex_to_data_blob(mem_ctx, schema->schema_info); - W_ERROR_HAVE_NO_MEMORY(schemaInfo->data); + status = dsdb_blob_from_schema_info(schema->schema_info, mem_ctx, schemaInfo); + W_ERROR_NOT_OK_RETURN(status); return WERR_OK; } diff --git a/source4/dsdb/schema/schema_prefixmap.c b/source4/dsdb/schema/schema_prefixmap.c index 270e6be..c5acf9c 100644 --- a/source4/dsdb/schema/schema_prefixmap.c +++ b/source4/dsdb/schema/schema_prefixmap.c @@ -510,7 +510,7 @@ WERROR dsdb_schema_pfm_from_drsuapi_pfm(const struct drsuapi_DsReplicaOIDMapping bool have_schema_info, TALLOC_CTX *mem_ctx, struct dsdb_schema_prefixmap **_pfm, - const char **_schema_info) + struct dsdb_schema_info **_schema_info) { WERROR werr; uint32_t i; @@ -561,12 +561,12 @@ WERROR dsdb_schema_pfm_from_drsuapi_pfm(const struct drsuapi_DsReplicaOIDMapping * but set it here for clarity */ i = ctr->num_mappings - 1; - *_schema_info = hex_encode_talloc(mem_ctx, - ctr->mappings[i].oid.binary_oid, - ctr->mappings[i].oid.length); - if (!*_schema_info) { + blob = data_blob_const(ctr->mappings[i].oid.binary_oid, + ctr->mappings[i].oid.length); + werr = dsdb_schema_info_from_blob(&blob, mem_ctx, _schema_info); + if (!W_ERROR_IS_OK(werr)) { talloc_free(pfm); - return WERR_NOMEM; + return werr; } } @@ -585,7 +585,7 @@ WERROR dsdb_schema_pfm_from_drsuapi_pfm(const struct drsuapi_DsReplicaOIDMapping * \param _ctr Out pointer to drsuapi_DsReplicaOIDMapping_Ctr prefix map structure */ WERROR dsdb_drsuapi_pfm_from_schema_pfm(const struct dsdb_schema_prefixmap *pfm, - const char *schema_info, + const struct dsdb_schema_info *schema_info, TALLOC_CTX *mem_ctx, struct drsuapi_DsReplicaOIDMapping_Ctr **_ctr) { @@ -628,14 +628,16 @@ WERROR dsdb_drsuapi_pfm_from_schema_pfm(const struct dsdb_schema_prefixmap *pfm, /* make schema_info entry if needed */ if (schema_info) { + WERROR werr; + /* by this time, i should have this value, * but set it here for clarity */ i = ctr->num_mappings - 1; - blob = strhex_to_data_blob(ctr, schema_info); - if (!blob.data) { + werr = dsdb_blob_from_schema_info(schema_info, ctr, &blob); + if (!W_ERROR_IS_OK(werr)) { talloc_free(ctr); - return WERR_NOMEM; + return werr; } ctr->mappings[i].id_prefix = 0; diff --git a/source4/torture/drs/unit/prefixmap_tests.c b/source4/torture/drs/unit/prefixmap_tests.c index 29941eb..969d641 100644 --- a/source4/torture/drs/unit/prefixmap_tests.c +++ b/source4/torture/drs/unit/prefixmap_tests.c @@ -26,7 +26,7 @@ #include "torture/rpc/drsuapi.h" #include "torture/drs/proto.h" #include "param/param.h" - +#include "librpc/ndr/libndr.h" /** * Private data to be shared among all test in Test case @@ -36,7 +36,6 @@ struct drsut_prefixmap_data { struct dsdb_schema_prefixmap *pfm_full; /* default schemaInfo value to test with */ - const char *schi_default_str; struct dsdb_schema_info *schi_default; struct ldb_context *ldb_ctx; @@ -539,7 +538,8 @@ static bool torture_drs_unit_pfm_oid_from_attid_check_attid(struct torture_conte static bool torture_drs_unit_pfm_to_from_drsuapi(struct torture_context *tctx, struct drsut_prefixmap_data *priv) { WERROR werr; - const char *schema_info; + struct dsdb_schema_info *schema_info; + DATA_BLOB schema_info_blob; struct dsdb_schema_prefixmap *pfm; struct drsuapi_DsReplicaOIDMapping_Ctr *ctr; TALLOC_CTX *mem_ctx; @@ -548,19 +548,20 @@ static bool torture_drs_unit_pfm_to_from_drsuapi(struct torture_context *tctx, s torture_assert(tctx, mem_ctx, "Unexpected: Have no memory!"); /* convert Schema_prefixMap to drsuapi_prefixMap */ - werr = dsdb_drsuapi_pfm_from_schema_pfm(priv->pfm_full, priv->schi_default_str, mem_ctx, &ctr); + werr = dsdb_drsuapi_pfm_from_schema_pfm(priv->pfm_full, priv->schi_default, mem_ctx, &ctr); torture_assert_werr_ok(tctx, werr, "dsdb_drsuapi_pfm_from_schema_pfm() failed"); torture_assert(tctx, ctr && ctr->mappings, "drsuapi_prefixMap not constructed correctly"); torture_assert_int_equal(tctx, ctr->num_mappings, priv->pfm_full->length + 1, "drs_mappings count does not match"); /* look for schema_info entry - it should be the last one */ - schema_info = hex_encode_talloc(mem_ctx, - ctr->mappings[ctr->num_mappings - 1].oid.binary_oid, - ctr->mappings[ctr->num_mappings - 1].oid.length); - torture_assert_str_equal(tctx, - schema_info, - priv->schi_default_str, - "schema_info not stored correctly or not last entry"); + schema_info_blob = data_blob_const(ctr->mappings[ctr->num_mappings - 1].oid.binary_oid, + ctr->mappings[ctr->num_mappings - 1].oid.length); + werr = dsdb_schema_info_from_blob(&schema_info_blob, tctx, &schema_info); + torture_assert_werr_ok(tctx, werr, "dsdb_schema_info_from_blob failed"); + torture_assert_int_equal(tctx, schema_info->revision, priv->schi_default->revision, + "schema_info (revision) not stored correctly or not last entry"); + torture_assert(tctx, GUID_equal(&schema_info->invocation_id, &priv->schi_default->invocation_id), + "schema_info (invocation_id) not stored correctly or not last entry"); /* compare schema_prefixMap and drsuapi_prefixMap */ werr = dsdb_schema_pfm_contains_drsuapi_pfm(priv->pfm_full, ctr); @@ -569,7 +570,10 @@ static bool torture_drs_unit_pfm_to_from_drsuapi(struct torture_context *tctx, s /* convert back drsuapi_prefixMap to schema_prefixMap */ werr = dsdb_schema_pfm_from_drsuapi_pfm(ctr, true, mem_ctx, &pfm, &schema_info); torture_assert_werr_ok(tctx, werr, "dsdb_schema_pfm_from_drsuapi_pfm() failed"); - torture_assert_str_equal(tctx, schema_info, priv->schi_default_str, "Fetched schema_info is different"); + torture_assert_int_equal(tctx, schema_info->revision, priv->schi_default->revision, + "Fetched schema_info is different (revision)"); + torture_assert(tctx, GUID_equal(&schema_info->invocation_id, &priv->schi_default->invocation_id), + "Fetched schema_info is different (invocation_id)"); /* compare against the original */ if (!_torture_drs_pfm_compare_same(tctx, priv->pfm_full, pfm, true)) { @@ -599,7 +603,6 @@ static bool torture_drs_unit_pfm_to_from_drsuapi(struct torture_context *tctx, s static bool torture_drs_unit_pfm_to_from_ldb_val(struct torture_context *tctx, struct drsut_prefixmap_data *priv) { WERROR werr; - const char *schema_info; struct dsdb_schema *schema; struct ldb_val pfm_ldb_val; struct ldb_val schema_info_ldb_val; @@ -613,7 +616,7 @@ static bool torture_drs_unit_pfm_to_from_ldb_val(struct torture_context *tctx, s /* set priv->pfm_full as prefixMap for new schema object */ schema->prefixmap = priv->pfm_full; - schema->schema_info = priv->schi_default_str; + schema->schema_info = priv->schi_default; /* convert schema_prefixMap to ldb_val blob */ werr = dsdb_get_oid_mappings_ldb(schema, mem_ctx, &pfm_ldb_val, &schema_info_ldb_val); @@ -622,14 +625,6 @@ static bool torture_drs_unit_pfm_to_from_ldb_val(struct torture_context *tctx, s "pfm_ldb_val not constructed correctly"); torture_assert(tctx, schema_info_ldb_val.data && schema_info_ldb_val.length, "schema_info_ldb_val not constructed correctly"); - /* look for schema_info entry - it should be the last one */ - schema_info = hex_encode_talloc(mem_ctx, - schema_info_ldb_val.data, - schema_info_ldb_val.length); - torture_assert_str_equal(tctx, - schema_info, - priv->schi_default_str, - "schema_info not stored correctly or not last entry"); /* convert pfm_ldb_val back to schema_prefixMap */ schema->prefixmap = NULL; @@ -641,6 +636,10 @@ static bool torture_drs_unit_pfm_to_from_ldb_val(struct torture_context *tctx, s talloc_free(mem_ctx); return false; } + torture_assert_int_equal(tctx, schema->schema_info->revision, priv->schi_default->revision, + "Fetched schema_info is different (revision)"); + torture_assert(tctx, GUID_equal(&schema->schema_info->invocation_id, &priv->schi_default->invocation_id), + "Fetched schema_info is different (invocation_id)"); talloc_free(mem_ctx); return true; @@ -664,7 +663,7 @@ static bool torture_drs_unit_pfm_read_write_ldb(struct torture_context *tctx, st torture_assert(tctx, schema, "Unexpected: failed to allocate schema object"); /* set priv->pfm_full as prefixMap for new schema object */ schema->prefixmap = priv->pfm_full; - schema->schema_info = priv->schi_default_str; + schema->schema_info = priv->schi_default; /* write prfixMap to ldb */ werr = dsdb_write_prefixes_from_schema_to_ldb(mem_ctx, priv->ldb_ctx, schema); @@ -701,7 +700,7 @@ static bool torture_drs_unit_dsdb_create_prefix_mapping(struct torture_context * schema = dsdb_new_schema(mem_ctx); torture_assert(tctx, schema, "Unexpected: failed to allocate schema object"); /* set priv->pfm_full as prefixMap for new schema object */ - schema->schema_info = priv->schi_default_str; + schema->schema_info = priv->schi_default; werr = _drsut_prefixmap_new(_prefixmap_test_new_data, ARRAY_SIZE(_prefixmap_test_new_data), schema, &schema->prefixmap); torture_assert_werr_ok(tctx, werr, "_drsut_prefixmap_new() failed"); @@ -827,8 +826,6 @@ static bool torture_drs_unit_prefixmap_setup(struct torture_context *tctx, struc werr = dsdb_blob_from_schema_info(priv->schi_default, priv, &blob); torture_assert_werr_ok(tctx, werr, "dsdb_blob_from_schema_info() failed"); - priv->schi_default_str = data_blob_hex_string_upper(priv, &blob); - /* create temporary LDB and populate with data */ if (!torture_drs_unit_ldb_setup(tctx, priv)) { return false; diff --git a/source4/torture/drs/unit/schemainfo_tests.c b/source4/torture/drs/unit/schemainfo_tests.c index 2fb5308..3b492b8 100644 --- a/source4/torture/drs/unit/schemainfo_tests.c +++ b/source4/torture/drs/unit/schemainfo_tests.c @@ -304,6 +304,7 @@ static bool test_dsdb_schema_info_cmp(struct torture_context *tctx, { DATA_BLOB blob; struct drsuapi_DsReplicaOIDMapping_Ctr *ctr; + struct dsdb_schema_info schema_info; ctr = talloc_zero(priv, struct drsuapi_DsReplicaOIDMapping_Ctr); torture_assert(tctx, ctr, "Not enough memory!"); @@ -354,8 +355,10 @@ static bool test_dsdb_schema_info_cmp(struct torture_context *tctx, "dsdb_schema_info_cmp(): unexpected result"); /* test with correct schemaInfo, but invalid ATTID */ - blob = strhex_to_data_blob(ctr, priv->schema->schema_info); - torture_assert(tctx, blob.data, "Not enough memory!"); + schema_info = *priv->schema->schema_info; + torture_assert_werr_ok(tctx, + dsdb_blob_from_schema_info(&schema_info, tctx, &blob), + "dsdb_blob_from_schema_info() failed"); ctr->mappings[0].id_prefix = 1; ctr->mappings[0].oid.length = blob.length; ctr->mappings[0].oid.binary_oid = blob.data; @@ -365,7 +368,6 @@ static bool test_dsdb_schema_info_cmp(struct torture_context *tctx, "dsdb_schema_info_cmp(): unexpected result"); /* test with valid schemaInfo */ - blob = strhex_to_data_blob(ctr, priv->schema->schema_info); ctr->mappings[0].id_prefix = 0; torture_assert_werr_ok(tctx, dsdb_schema_info_cmp(priv->schema, ctr), @@ -595,7 +597,9 @@ static bool torture_drs_unit_schemainfo_setup(struct torture_context *tctx, priv->schema = dsdb_new_schema(priv); /* set schema_info in dsdb_schema for testing */ - priv->schema->schema_info = talloc_strdup(priv->schema, SCHEMA_INFO_DEFAULT_STR); + torture_assert(tctx, + _drsut_schemainfo_new(tctx, SCHEMA_INFO_DEFAULT_STR, &priv->schema->schema_info), + "Failed to create schema_info test object"); /* pre-cache invocationId for samdb_ntds_invocation_id() * to work with our mock ldb */ -- 1.9.1 From 8df411447ad7b93d78de78915f3d30eddbc76a97 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 4 Aug 2016 11:00:34 +0200 Subject: [PATCH 05/20] s4:dsdb/schema: don't treat an older remote schema as SCHEMA_MISMATCH It's perfectly valid to replicate from a partner with an older schema version, otherwise schema changes would block any other replication until every dc in the forest has the schema changes. The avoids an endless loop trying to get schema in sync with the partner. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12115 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 0a1627de6d7c70f2462a4d4db717ea50c8aefc2f) --- source4/dsdb/schema/schema_info_attr.c | 11 ++++- source4/torture/drs/unit/schemainfo_tests.c | 71 ++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/schema/schema_info_attr.c b/source4/dsdb/schema/schema_info_attr.c index 76cd90d..0d54012 100644 --- a/source4/dsdb/schema/schema_info_attr.c +++ b/source4/dsdb/schema/schema_info_attr.c @@ -204,8 +204,17 @@ WERROR dsdb_schema_info_cmp(const struct dsdb_schema *schema, return werr; } - if (schema->schema_info->revision != schema_info->revision) { + if (schema->schema_info->revision > schema_info->revision) { + /* + * It's ok if our schema is newer than the remote one + */ + werr = WERR_OK; + } else if (schema->schema_info->revision < schema_info->revision) { werr = WERR_DS_DRA_SCHEMA_MISMATCH; + } else if (!GUID_equal(&schema->schema_info->invocation_id, + &schema_info->invocation_id)) + { + werr = WERR_DS_DRA_SCHEMA_CONFLICT; } else { werr = WERR_OK; } diff --git a/source4/torture/drs/unit/schemainfo_tests.c b/source4/torture/drs/unit/schemainfo_tests.c index 3b492b8..e5a1c87 100644 --- a/source4/torture/drs/unit/schemainfo_tests.c +++ b/source4/torture/drs/unit/schemainfo_tests.c @@ -344,14 +344,14 @@ static bool test_dsdb_schema_info_cmp(struct torture_context *tctx, WERR_INVALID_PARAMETER, "dsdb_schema_info_cmp(): unexpected result"); - /* test with valid schemaInfo, but not correct one */ + /* test with valid schemaInfo, but older one should be ok */ blob = strhex_to_data_blob(ctr, "FF0000000000000000000000000000000000000000"); torture_assert(tctx, blob.data, "Not enough memory!"); ctr->mappings[0].oid.length = blob.length; ctr->mappings[0].oid.binary_oid = blob.data; torture_assert_werr_equal(tctx, dsdb_schema_info_cmp(priv->schema, ctr), - WERR_DS_DRA_SCHEMA_MISMATCH, + WERR_OK, "dsdb_schema_info_cmp(): unexpected result"); /* test with correct schemaInfo, but invalid ATTID */ @@ -373,6 +373,73 @@ static bool test_dsdb_schema_info_cmp(struct torture_context *tctx, dsdb_schema_info_cmp(priv->schema, ctr), "dsdb_schema_info_cmp(): unexpected result"); + /* test with valid schemaInfo, but older revision */ + schema_info = *priv->schema->schema_info; + schema_info.revision -= 1; + torture_assert_werr_ok(tctx, + dsdb_blob_from_schema_info(&schema_info, tctx, &blob), + "dsdb_blob_from_schema_info() failed"); + ctr->mappings[0].oid.length = blob.length; + ctr->mappings[0].oid.binary_oid = blob.data; + torture_assert_werr_equal(tctx, + dsdb_schema_info_cmp(priv->schema, ctr), + WERR_OK, + "dsdb_schema_info_cmp(): unexpected result"); + + /* test with valid schemaInfo, but newer revision */ + schema_info = *priv->schema->schema_info; + schema_info.revision += 1; + torture_assert_werr_ok(tctx, + dsdb_blob_from_schema_info(&schema_info, tctx, &blob), + "dsdb_blob_from_schema_info() failed"); + ctr->mappings[0].oid.length = blob.length; + ctr->mappings[0].oid.binary_oid = blob.data; + torture_assert_werr_equal(tctx, + dsdb_schema_info_cmp(priv->schema, ctr), + WERR_DS_DRA_SCHEMA_MISMATCH, + "dsdb_schema_info_cmp(): unexpected result"); + + /* test with valid schemaInfo, but newer revision and other invocationId */ + schema_info = *priv->schema->schema_info; + schema_info.revision += 1; + schema_info.invocation_id.time_mid += 1; + torture_assert_werr_ok(tctx, + dsdb_blob_from_schema_info(&schema_info, tctx, &blob), + "dsdb_blob_from_schema_info() failed"); + ctr->mappings[0].oid.length = blob.length; + ctr->mappings[0].oid.binary_oid = blob.data; + torture_assert_werr_equal(tctx, + dsdb_schema_info_cmp(priv->schema, ctr), + WERR_DS_DRA_SCHEMA_MISMATCH, + "dsdb_schema_info_cmp(): unexpected result"); + + /* test with valid schemaInfo, but older revision and other invocationId */ + schema_info = *priv->schema->schema_info; + schema_info.revision -= 1; + schema_info.invocation_id.time_mid += 1; + torture_assert_werr_ok(tctx, + dsdb_blob_from_schema_info(&schema_info, tctx, &blob), + "dsdb_blob_from_schema_info() failed"); + ctr->mappings[0].oid.length = blob.length; + ctr->mappings[0].oid.binary_oid = blob.data; + torture_assert_werr_equal(tctx, + dsdb_schema_info_cmp(priv->schema, ctr), + WERR_OK, + "dsdb_schema_info_cmp(): unexpected result"); + + /* test with valid schemaInfo, but same revision and other invocationId */ + schema_info = *priv->schema->schema_info; + schema_info.invocation_id.time_mid += 1; + torture_assert_werr_ok(tctx, + dsdb_blob_from_schema_info(&schema_info, tctx, &blob), + "dsdb_blob_from_schema_info() failed"); + ctr->mappings[0].oid.length = blob.length; + ctr->mappings[0].oid.binary_oid = blob.data; + torture_assert_werr_equal(tctx, + dsdb_schema_info_cmp(priv->schema, ctr), + WERR_DS_DRA_SCHEMA_CONFLICT, + "dsdb_schema_info_cmp(): unexpected result"); + talloc_free(ctr); return true; } -- 1.9.1 From aa0b8d8eb57185bc05d5af9190cace38f021aae1 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 4 Aug 2016 15:20:27 +1200 Subject: [PATCH 06/20] s4:dsdb/repl: Improve memory handling in replicated schema code This attempts to make it clear what memory is short term and what memory is long term BUG: https://bugzilla.samba.org/show_bug.cgi?id=12115 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit c533b60ceb761b609e78dfe270930cb99fdac848) --- source4/dsdb/repl/replicated_objects.c | 14 ++++++++++---- source4/libnet/libnet_vampire.c | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c index fb8083d..17b68a4 100644 --- a/source4/dsdb/repl/replicated_objects.c +++ b/source4/dsdb/repl/replicated_objects.c @@ -103,7 +103,6 @@ static WERROR dsdb_repl_merge_working_schema(struct ldb_context *ldb, } WERROR dsdb_repl_resolve_working_schema(struct ldb_context *ldb, - TALLOC_CTX *mem_ctx, struct dsdb_schema_prefixmap *pfm_remote, uint32_t cycle_before_switching, struct dsdb_schema *initial_schema, @@ -129,10 +128,11 @@ WERROR dsdb_repl_resolve_working_schema(struct ldb_context *ldb, DRSUAPI_ATTID_systemPossSuperiors, DRSUAPI_ATTID_INVALID }; + TALLOC_CTX *frame = talloc_stackframe(); /* create a list of objects yet to be converted */ for (cur = first_object; cur; cur = cur->next_object) { - schema_list_item = talloc(mem_ctx, struct schema_list); + schema_list_item = talloc(frame, struct schema_list); if (schema_list_item == NULL) { return WERR_NOMEM; } @@ -164,6 +164,7 @@ WERROR dsdb_repl_resolve_working_schema(struct ldb_context *ldb, working_schema, resulting_schema); if (!W_ERROR_IS_OK(werr)) { + talloc_free(frame); return werr; } } @@ -242,6 +243,7 @@ WERROR dsdb_repl_resolve_working_schema(struct ldb_context *ldb, "all %d remaining of %d objects " "failed to convert\n", failed_obj_count, object_count)); + talloc_free(frame); return WERR_INTERNAL_ERROR; } @@ -257,12 +259,14 @@ WERROR dsdb_repl_resolve_working_schema(struct ldb_context *ldb, ret = dsdb_setup_sorted_accessors(ldb, working_schema); if (LDB_SUCCESS != ret) { DEBUG(0,("Failed to create schema-cache indexes!\n")); + talloc_free(frame); return WERR_INTERNAL_ERROR; } } pass_no++; } + talloc_free(frame); return WERR_OK; } @@ -298,14 +302,15 @@ WERROR dsdb_repl_make_working_schema(struct ldb_context *ldb, /* we are going to need remote prefixMap for decoding */ werr = dsdb_schema_pfm_from_drsuapi_pfm(mapping_ctr, true, - mem_ctx, &pfm_remote, NULL); + working_schema, &pfm_remote, NULL); if (!W_ERROR_IS_OK(werr)) { DEBUG(0,(__location__ ": Failed to decode remote prefixMap: %s", win_errstr(werr))); + talloc_free(working_schema); return werr; } - werr = dsdb_repl_resolve_working_schema(ldb, mem_ctx, + werr = dsdb_repl_resolve_working_schema(ldb, pfm_remote, 0, /* cycle_before_switching */ working_schema, @@ -315,6 +320,7 @@ WERROR dsdb_repl_make_working_schema(struct ldb_context *ldb, if (!W_ERROR_IS_OK(werr)) { DEBUG(0, ("%s: dsdb_repl_resolve_working_schema() failed: %s", __location__, win_errstr(werr))); + talloc_free(working_schema); return werr; } diff --git a/source4/libnet/libnet_vampire.c b/source4/libnet/libnet_vampire.c index 0186caf..60bfa41 100644 --- a/source4/libnet/libnet_vampire.c +++ b/source4/libnet/libnet_vampire.c @@ -332,7 +332,7 @@ static NTSTATUS libnet_vampire_cb_apply_schema(struct libnet_vampire_cb_state *s "become dc", "schema convert retrial", 1); - status = dsdb_repl_resolve_working_schema(s->ldb, s, + status = dsdb_repl_resolve_working_schema(s->ldb, pfm_remote, cycle_before_switching, provision_schema, -- 1.9.1 From c969d20b2d5b2343c3db338043af080038bb49f2 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 4 Aug 2016 15:25:52 +1200 Subject: [PATCH 07/20] s4:dsdb/schema: Remove unused old schema from memory This avoids confusion when reading the talloc dump from a ldb context that has been the target of replication, as the dsdb_schema_copy_shallow() memory was still around, if unused. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12115 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 29caafaf2888b25408fcafdb9767a003a910c1d7) --- source4/dsdb/schema/schema_set.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c index 1b29c4d..a404e0a 100644 --- a/source4/dsdb/schema/schema_set.c +++ b/source4/dsdb/schema/schema_set.c @@ -566,6 +566,8 @@ int dsdb_set_global_schema(struct ldb_context *ldb) { int ret; void *use_global_schema = (void *)1; + struct dsdb_schema *old_schema = ldb_get_opaque(ldb, "dsdb_schema"); + ret = ldb_set_opaque(ldb, "dsdb_use_global_schema", use_global_schema); if (ret != LDB_SUCCESS) { return ret; @@ -575,6 +577,16 @@ int dsdb_set_global_schema(struct ldb_context *ldb) return LDB_SUCCESS; } + /* Remove any pointer to a previous schema */ + ret = ldb_set_opaque(ldb, "dsdb_schema", NULL); + if (ret != LDB_SUCCESS) { + return ret; + } + + /* Remove the reference to the schema we just overwrote - if there was + * none, NULL is harmless here */ + talloc_unlink(ldb, old_schema); + /* Set the new attributes based on the new schema */ ret = dsdb_schema_set_indices_and_attributes(ldb, global_schema, false /* Don't write indices and attributes, it's expensive */); if (ret == LDB_SUCCESS) { -- 1.9.1 From e0f570485b67e00a735d4ac8261904593db22424 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2016 11:00:02 +0200 Subject: [PATCH 08/20] s4:dsdb/schema: make dsdb_schema_pfm_add_entry() public and more useful We allow a hint for the id from the remote prefix map. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit f905ddc104bb130a63e714d16b6da41bc92d6864) --- source4/dsdb/schema/schema_prefixmap.c | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/schema/schema_prefixmap.c b/source4/dsdb/schema/schema_prefixmap.c index c5acf9c..dc4e617 100644 --- a/source4/dsdb/schema/schema_prefixmap.c +++ b/source4/dsdb/schema/schema_prefixmap.c @@ -148,7 +148,10 @@ struct dsdb_schema_prefixmap *dsdb_schema_pfm_copy_shallow(TALLOC_CTX *mem_ctx, * \param bin_oid OID prefix to be added to prefixMap * \param pfm_id Location where to store prefixMap entry ID */ -static WERROR _dsdb_schema_pfm_add_entry(struct dsdb_schema_prefixmap *pfm, DATA_BLOB bin_oid, uint32_t *_idx) +WERROR dsdb_schema_pfm_add_entry(struct dsdb_schema_prefixmap *pfm, + DATA_BLOB bin_oid, + const uint32_t *remote_id, + uint32_t *_idx) { uint32_t i; struct dsdb_schema_prefixmap_oid * pfm_entry; @@ -170,15 +173,34 @@ static WERROR _dsdb_schema_pfm_add_entry(struct dsdb_schema_prefixmap *pfm, DATA pfm_entry = &pfm->prefixes[pfm->length]; pfm_entry->id = 0; for (i = 0; i < pfm->length; i++) { - if (pfm_entry->id < pfm->prefixes[i].id) + if (pfm_entry->id < pfm->prefixes[i].id) { pfm_entry->id = pfm->prefixes[i].id; + } + + if (remote_id == NULL) { + continue; + } + + if (pfm->prefixes[i].id == *remote_id) { + /* + * We can't use the remote id. + * it's already in use. + */ + remote_id = NULL; + } } /* add new bin-oid prefix */ - pfm_entry->id++; + if (remote_id != NULL) { + pfm_entry->id = *remote_id; + } else { + pfm_entry->id++; + } pfm_entry->bin_oid = bin_oid; - *_idx = pfm->length; + if (_idx != NULL) { + *_idx = pfm->length; + } pfm->length++; return WERR_OK; @@ -316,7 +338,7 @@ static WERROR dsdb_schema_pfm_make_attid_impl(struct dsdb_schema_prefixmap *pfm, } /* entry does not exists, add it */ - werr = _dsdb_schema_pfm_add_entry(pfm, bin_oid, &idx); + werr = dsdb_schema_pfm_add_entry(pfm, bin_oid, NULL, &idx); W_ERROR_NOT_OK_RETURN(werr); } -- 1.9.1 From 34cebc9fe0983abe23e222db0b0e58764c094d74 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2016 11:07:18 +0200 Subject: [PATCH 09/20] s4:dsdb/repl: make sure the working_schema prefix map is populated with the remote prefix map We should create the working_schema prefix map before we try to resolve the schema. This allows getting the same mapping (if there's not already a conflict) and allows us to remove the implicit prefix mapping creation in the prefix mapping lookup functions. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit edeb577a59ea88d2a22559ffb8cbe994ea28f3f1) --- source4/dsdb/repl/replicated_objects.c | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c index 17b68a4..288bcc1 100644 --- a/source4/dsdb/repl/replicated_objects.c +++ b/source4/dsdb/repl/replicated_objects.c @@ -291,6 +291,7 @@ WERROR dsdb_repl_make_working_schema(struct ldb_context *ldb, { WERROR werr; struct dsdb_schema_prefixmap *pfm_remote; + uint32_t r; struct dsdb_schema *working_schema; /* make a copy of the iniatial_scheam so we don't mess with it */ @@ -310,6 +311,40 @@ WERROR dsdb_repl_make_working_schema(struct ldb_context *ldb, return werr; } + for (r=0; r < pfm_remote->length; r++) { + const struct dsdb_schema_prefixmap_oid *rm = &pfm_remote->prefixes[r]; + bool found_oid = false; + uint32_t l; + + for (l=0; l < working_schema->prefixmap->length; l++) { + const struct dsdb_schema_prefixmap_oid *lm = &working_schema->prefixmap->prefixes[l]; + int cmp; + + cmp = data_blob_cmp(&rm->bin_oid, &lm->bin_oid); + if (cmp == 0) { + found_oid = true; + break; + } + } + + if (found_oid) { + continue; + } + + /* + * We prefer the same is as we got from the remote peer + * if there's no conflict. + */ + werr = dsdb_schema_pfm_add_entry(working_schema->prefixmap, + rm->bin_oid, &rm->id, NULL); + if (!W_ERROR_IS_OK(werr)) { + DEBUG(0,(__location__ ": Failed to merge remote prefixMap: %s", + win_errstr(werr))); + talloc_free(working_schema); + return werr; + } + } + werr = dsdb_repl_resolve_working_schema(ldb, pfm_remote, 0, /* cycle_before_switching */ -- 1.9.1 From 47bd5d4b2a025338d0ab3fbec7605673b3f183ff Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2016 13:02:21 +0200 Subject: [PATCH 10/20] s4:dsdb/objectclass_attrs: call dsdb_attribute_from_ldb() without a prefixmap We may not have a prefix mapping for the new attribute definition, it will be added later. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit fa580f255ca0878b99946cda3f95913ff37f6d54) --- source4/dsdb/samdb/ldb_modules/objectclass_attrs.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c index e051913..f739c40 100644 --- a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c +++ b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c @@ -437,13 +437,25 @@ static int attr_handler2(struct oc_context *ac) struct dsdb_attribute *att = talloc(ac, struct dsdb_attribute); const struct dsdb_syntax *attrSyntax; WERROR status; - - status= dsdb_attribute_from_ldb(ac->schema, msg, att); + struct dsdb_schema *tmp_schema = NULL; + + /* + * We temporary remove the prefix map from the schema, + * a new prefix map is added by dsdb_create_prefix_mapping() + * via the "schema_data" module. + */ + tmp_schema = dsdb_schema_copy_shallow(ac, ldb, ac->schema); + if (tmp_schema == NULL) { + return ldb_module_oom(ac->module); + } + TALLOC_FREE(tmp_schema->prefixmap); + status= dsdb_attribute_from_ldb(tmp_schema, msg, att); if (!W_ERROR_IS_OK(status)) { ldb_set_errstring(ldb, "objectclass: failed to translate the schemaAttribute to a dsdb_attribute"); return LDB_ERR_UNWILLING_TO_PERFORM; } + TALLOC_FREE(tmp_schema); attrSyntax = dsdb_syntax_for_attribute(att); if (!attrSyntax) { -- 1.9.1 From 35762dcd285c07eb8c390d1c484189980476a892 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2016 11:11:20 +0200 Subject: [PATCH 11/20] s4:dsdb/schema: avoid an implicit prefix map creation in lookup functions dsdb_create_prefix_mapping() should be the only place that calls dsdb_schema_pfm_make_attid(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit b755ec74e17e9a5ed746af0a2716effe3176965a) --- source4/dsdb/schema/schema_init.c | 8 ++++---- source4/dsdb/schema/schema_syntax.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/schema/schema_init.c b/source4/dsdb/schema/schema_init.c index c9c55ca..c1cf466 100644 --- a/source4/dsdb/schema/schema_init.c +++ b/source4/dsdb/schema/schema_init.c @@ -639,7 +639,7 @@ WERROR dsdb_attribute_from_ldb(const struct dsdb_schema *schema, /* set an invalid value */ attr->attributeID_id = DRSUAPI_ATTID_INVALID; } else { - status = dsdb_schema_pfm_make_attid(schema->prefixmap, + status = dsdb_schema_pfm_attid_from_oid(schema->prefixmap, attr->attributeID_oid, &attr->attributeID_id); if (!W_ERROR_IS_OK(status)) { @@ -793,9 +793,9 @@ WERROR dsdb_set_class_from_ldb_dups(struct dsdb_schema *schema, /* set an invalid value */ obj->governsID_id = DRSUAPI_ATTID_INVALID; } else { - status = dsdb_schema_pfm_make_attid(schema->prefixmap, - obj->governsID_oid, - &obj->governsID_id); + status = dsdb_schema_pfm_attid_from_oid(schema->prefixmap, + obj->governsID_oid, + &obj->governsID_id); if (!W_ERROR_IS_OK(status)) { DEBUG(0,("%s: '%s': unable to map governsID %s: %s\n", __location__, obj->lDAPDisplayName, obj->governsID_oid, diff --git a/source4/dsdb/schema/schema_syntax.c b/source4/dsdb/schema/schema_syntax.c index 5b7c8b1..e3f1421 100644 --- a/source4/dsdb/schema/schema_syntax.c +++ b/source4/dsdb/schema/schema_syntax.c @@ -97,7 +97,7 @@ static bool dsdb_syntax_attid_from_remote_attid(const struct dsdb_syntax_ctx *ct return false; } - werr = dsdb_schema_pfm_make_attid(ctx->schema->prefixmap, oid, id_local); + werr = dsdb_schema_pfm_attid_from_oid(ctx->schema->prefixmap, oid, id_local); if (!W_ERROR_IS_OK(werr)) { DEBUG(0,("OID->ATTID failed (%s) for: %s\n", win_errstr(werr), oid)); return false; -- 1.9.1 From 10dafc52da511df3b4abadbe74cf9a4b16430fd0 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2016 11:22:13 +0200 Subject: [PATCH 12/20] s4:dsdb/schema: don't update the in memory schema->prefixmap without reloading the schema! BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 2e6860df7149559df84ed3995ed36332ccb0f649) --- source4/dsdb/schema/schema_init.c | 15 ++++++-- source4/torture/drs/unit/prefixmap_tests.c | 57 +++++++++++++++++++----------- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/source4/dsdb/schema/schema_init.c b/source4/dsdb/schema/schema_init.c index c1cf466..3fac3c1 100644 --- a/source4/dsdb/schema/schema_init.c +++ b/source4/dsdb/schema/schema_init.c @@ -276,6 +276,7 @@ WERROR dsdb_create_prefix_mapping(struct ldb_context *ldb, struct dsdb_schema *s uint32_t attid; TALLOC_CTX *mem_ctx; struct dsdb_schema_prefixmap *pfm; + struct dsdb_schema_prefixmap *orig_pfm = NULL; mem_ctx = talloc_new(ldb); W_ERROR_HAVE_NO_MEMORY(mem_ctx); @@ -312,8 +313,11 @@ WERROR dsdb_create_prefix_mapping(struct ldb_context *ldb, struct dsdb_schema *s return status; } - talloc_unlink(schema, schema->prefixmap); - schema->prefixmap = talloc_steal(schema, pfm); + /* + * We temporary replcate schema->prefixmap. + */ + orig_pfm = schema->prefixmap; + schema->prefixmap = pfm; /* Update prefixMap in ldb*/ status = dsdb_write_prefixes_from_schema_to_ldb(mem_ctx, ldb, schema); @@ -327,6 +331,13 @@ WERROR dsdb_create_prefix_mapping(struct ldb_context *ldb, struct dsdb_schema *s DEBUG(2,(__location__ " Added prefixMap %s - now have %u prefixes\n", full_oid, schema->prefixmap->length)); + /* + * We restore the original prefix map. + * + * The next schema reload should get an updated prefix map! + */ + schema->prefixmap = orig_pfm; + talloc_free(mem_ctx); return status; } diff --git a/source4/torture/drs/unit/prefixmap_tests.c b/source4/torture/drs/unit/prefixmap_tests.c index 969d641..ec5fec5 100644 --- a/source4/torture/drs/unit/prefixmap_tests.c +++ b/source4/torture/drs/unit/prefixmap_tests.c @@ -692,6 +692,7 @@ static bool torture_drs_unit_dsdb_create_prefix_mapping(struct torture_context * uint32_t i; struct dsdb_schema *schema; TALLOC_CTX *mem_ctx; + struct dsdb_schema_prefixmap *pfm_ldb = NULL; mem_ctx = talloc_new(tctx); torture_assert(tctx, mem_ctx, "Unexpected: Have no memory!"); @@ -708,44 +709,58 @@ static bool torture_drs_unit_dsdb_create_prefix_mapping(struct torture_context * werr = dsdb_write_prefixes_from_schema_to_ldb(mem_ctx, priv->ldb_ctx, schema); torture_assert_werr_ok(tctx, werr, "dsdb_write_prefixes_from_schema_to_ldb() failed"); + /* read from ldb what we have written */ + werr = dsdb_read_prefixes_from_ldb(priv->ldb_ctx, mem_ctx, &pfm_ldb); + torture_assert_werr_ok(tctx, werr, "dsdb_read_prefixes_from_ldb() failed"); + /* compare data written/read */ + if (!_torture_drs_pfm_compare_same(tctx, schema->prefixmap, pfm_ldb, true)) { + torture_fail(tctx, "pfm in LDB is different"); + } + TALLOC_FREE(pfm_ldb); + for (i = 0; i < ARRAY_SIZE(_prefixmap_test_data); i++) { - struct dsdb_schema_prefixmap *pfm_ldb; struct dsdb_schema_prefixmap *pfm_prev; + struct dsdb_schema_prefixmap *pfm_new; + + pfm_prev = schema->prefixmap; - /* add ref to prefixMap so we can use it later */ - pfm_prev = talloc_reference(schema, schema->prefixmap); + pfm_new = dsdb_schema_pfm_copy_shallow(schema, pfm_prev); + torture_assert(tctx, pfm_new != NULL, "dsdb_schema_pfm_copy_shallow() failed"); + + if (!_prefixmap_test_data[i].exists) { + uint32_t attid; + + werr = dsdb_schema_pfm_make_attid(pfm_new, + _prefixmap_test_data[i].oid, + &attid); + torture_assert_werr_ok(tctx, werr, "dsdb_schema_pfm_make_attid() failed"); + } /* call dsdb_create_prefix_mapping() and check result accordingly */ werr = dsdb_create_prefix_mapping(priv->ldb_ctx, schema, _prefixmap_test_data[i].oid); torture_assert_werr_ok(tctx, werr, "dsdb_create_prefix_mapping() failed"); - /* verify pfm has been altered or not if needed */ - if (_prefixmap_test_data[i].exists) { - torture_assert(tctx, pfm_prev == schema->prefixmap, - "schema->prefixmap has been reallocated!"); - if (!_torture_drs_pfm_compare_same(tctx, pfm_prev, schema->prefixmap, true)) { - torture_fail(tctx, "schema->prefixmap has changed"); - } - } else { - torture_assert(tctx, pfm_prev != schema->prefixmap, - "schema->prefixmap should be reallocated!"); - if (_torture_drs_pfm_compare_same(tctx, pfm_prev, schema->prefixmap, true)) { - torture_fail(tctx, "schema->prefixmap should be changed"); - } + /* + * The prefix should not change, only on reload + */ + torture_assert(tctx, pfm_prev == schema->prefixmap, + "schema->prefixmap has been reallocated!"); + if (!_torture_drs_pfm_compare_same(tctx, pfm_prev, schema->prefixmap, true)) { + torture_fail(tctx, "schema->prefixmap has changed"); } /* read from ldb what we have written */ werr = dsdb_read_prefixes_from_ldb(priv->ldb_ctx, mem_ctx, &pfm_ldb); torture_assert_werr_ok(tctx, werr, "dsdb_read_prefixes_from_ldb() failed"); /* compare data written/read */ - if (!_torture_drs_pfm_compare_same(tctx, schema->prefixmap, pfm_ldb, true)) { - torture_fail(tctx, "schema->prefixmap and pfm in LDB are different"); + if (!_torture_drs_pfm_compare_same(tctx, pfm_new, pfm_ldb, true)) { + torture_fail(tctx, talloc_asprintf(tctx, "%u: pfm in LDB is different", i)); } /* free mem for pfm read from LDB */ - talloc_free(pfm_ldb); + TALLOC_FREE(pfm_ldb); - /* release prefixMap pointer */ - talloc_unlink(schema, pfm_prev); + /* prepare for the next round */ + schema->prefixmap = pfm_new; } talloc_free(mem_ctx); -- 1.9.1 From 6d2eb275e08c9b17d0561554e70c375ec8e5e0ea Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2016 09:10:13 +0200 Subject: [PATCH 13/20] s4:dsdb/schema: split out a dsdb_attribute_drsuapi_remote_to_local() function BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 5ee6f9371570f968122c6eb1bceeb9bb0518bb4c) --- source4/dsdb/schema/schema_syntax.c | 70 ++++++++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 17 deletions(-) diff --git a/source4/dsdb/schema/schema_syntax.c b/source4/dsdb/schema/schema_syntax.c index e3f1421..03251dc 100644 --- a/source4/dsdb/schema/schema_syntax.c +++ b/source4/dsdb/schema/schema_syntax.c @@ -2697,45 +2697,49 @@ const struct dsdb_syntax *dsdb_syntax_for_attribute(const struct dsdb_attribute return NULL; } -WERROR dsdb_attribute_drsuapi_to_ldb(struct ldb_context *ldb, - const struct dsdb_schema *schema, - const struct dsdb_schema_prefixmap *pfm_remote, - const struct drsuapi_DsReplicaAttribute *in, - TALLOC_CTX *mem_ctx, - struct ldb_message_element *out, - enum drsuapi_DsAttributeId *local_attid_as_enum) +WERROR dsdb_attribute_drsuapi_remote_to_local(const struct dsdb_syntax_ctx *ctx, + enum drsuapi_DsAttributeId remote_attid_as_enum, + enum drsuapi_DsAttributeId *local_attid_as_enum, + const struct dsdb_attribute **_sa) { + TALLOC_CTX *frame = talloc_stackframe(); const struct dsdb_attribute *sa; - struct dsdb_syntax_ctx syntax_ctx; uint32_t attid_local; + bool ok; - /* use default syntax conversion context */ - dsdb_syntax_ctx_init(&syntax_ctx, ldb, schema); - syntax_ctx.pfm_remote = pfm_remote; + if (ctx->pfm_remote == NULL) { + smb_panic(__location__); + } - switch (dsdb_pfm_get_attid_type(in->attid)) { + switch (dsdb_pfm_get_attid_type(remote_attid_as_enum)) { case DSDB_ATTID_TYPE_PFM: /* map remote ATTID to local ATTID */ - if (!dsdb_syntax_attid_from_remote_attid(&syntax_ctx, mem_ctx, in->attid, &attid_local)) { + ok = dsdb_syntax_attid_from_remote_attid(ctx, frame, + remote_attid_as_enum, + &attid_local); + if (!ok) { DEBUG(0,(__location__ ": Can't find local ATTID for 0x%08X\n", - in->attid)); + remote_attid_as_enum)); + TALLOC_FREE(frame); return WERR_DS_ATT_NOT_DEF_IN_SCHEMA; } break; case DSDB_ATTID_TYPE_INTID: /* use IntId value directly */ - attid_local = in->attid; + attid_local = remote_attid_as_enum; break; default: /* we should never get here */ DEBUG(0,(__location__ ": Invalid ATTID type passed for conversion - 0x%08X\n", - in->attid)); + remote_attid_as_enum)); + TALLOC_FREE(frame); return WERR_INVALID_PARAMETER; } - sa = dsdb_attribute_by_attributeID_id(schema, attid_local); + sa = dsdb_attribute_by_attributeID_id(ctx->schema, attid_local); if (!sa) { DEBUG(1,(__location__ ": Unknown attributeID_id 0x%08X\n", in->attid)); + TALLOC_FREE(frame); return WERR_DS_ATT_NOT_DEF_IN_SCHEMA; } @@ -2748,6 +2752,38 @@ WERROR dsdb_attribute_drsuapi_to_ldb(struct ldb_context *ldb, *local_attid_as_enum = (enum drsuapi_DsAttributeId)attid_local; } + if (_sa != NULL) { + *_sa = sa; + } + + TALLOC_FREE(frame); + return WERR_OK; +} + +WERROR dsdb_attribute_drsuapi_to_ldb(struct ldb_context *ldb, + const struct dsdb_schema *schema, + const struct dsdb_schema_prefixmap *pfm_remote, + const struct drsuapi_DsReplicaAttribute *in, + TALLOC_CTX *mem_ctx, + struct ldb_message_element *out, + enum drsuapi_DsAttributeId *local_attid_as_enum) +{ + struct dsdb_syntax_ctx syntax_ctx; + const struct dsdb_attribute *sa = NULL; + WERROR werr; + + /* use default syntax conversion context */ + dsdb_syntax_ctx_init(&syntax_ctx, ldb, schema); + syntax_ctx.pfm_remote = pfm_remote; + + werr = dsdb_attribute_drsuapi_remote_to_local(&syntax_ctx, + in->attid, + local_attid_as_enum, + &sa); + if (!W_ERROR_IS_OK(werr)) { + return werr; + } + return sa->syntax->drsuapi_to_ldb(&syntax_ctx, sa, in, mem_ctx, out); } -- 1.9.1 From 1fb0220eae376befae4048639665f5070d8ef1ad Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2016 09:10:13 +0200 Subject: [PATCH 14/20] s4:dsdb/schema: move messages for unknown attids to higher debug levels during resolving BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 6bc007a9143a57525732792bb1fcb934fcb2c91c) --- source4/dsdb/schema/schema.h | 8 ++++++++ source4/dsdb/schema/schema_syntax.c | 25 ++++++++++++++++++------- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/source4/dsdb/schema/schema.h b/source4/dsdb/schema/schema.h index 6543eda..ee2b850 100644 --- a/source4/dsdb/schema/schema.h +++ b/source4/dsdb/schema/schema.h @@ -256,6 +256,14 @@ struct dsdb_schema { /* Should the syntax handlers in this case handle all incoming OIDs automatically, assigning them as an OID if no text name is known? */ bool relax_OID_conversions; + + /* + * we're currently trying to construct a working_schema + * in order to replicate the schema partition. + * + * We use this in order to avoid temporary failure DEBUG messages + */ + bool resolving_in_progress; }; enum dsdb_attr_list_query { diff --git a/source4/dsdb/schema/schema_syntax.c b/source4/dsdb/schema/schema_syntax.c index 03251dc..2e85a4ef 100644 --- a/source4/dsdb/schema/schema_syntax.c +++ b/source4/dsdb/schema/schema_syntax.c @@ -996,7 +996,7 @@ static WERROR _dsdb_syntax_OID_obj_drsuapi_to_ldb(const struct dsdb_syntax_ctx * W_ERROR_HAVE_NO_MEMORY(out->values); for (i=0; i < out->num_values; i++) { - uint32_t v; + uint32_t v, vo; const struct dsdb_class *c; const char *str; @@ -1009,6 +1009,7 @@ static WERROR _dsdb_syntax_OID_obj_drsuapi_to_ldb(const struct dsdb_syntax_ctx * } v = IVAL(in->value_ctr.values[i].blob->data, 0); + vo = v; /* convert remote ATTID to local ATTID */ if (!dsdb_syntax_attid_from_remote_attid(ctx, mem_ctx, v, &v)) { @@ -1018,8 +1019,11 @@ static WERROR _dsdb_syntax_OID_obj_drsuapi_to_ldb(const struct dsdb_syntax_ctx * c = dsdb_class_by_governsID_id(ctx->schema, v); if (!c) { - DEBUG(1,(__location__ ": Unknown governsID 0x%08X\n", v)); - return WERR_FOOBAR; + int dbg_level = ctx->schema->resolving_in_progress ? 10 : 0; + DEBUG(dbg_level,(__location__ ": %s unknown local governsID 0x%08X remote 0x%08X%s\n", + attr->lDAPDisplayName, v, vo, + ctx->schema->resolving_in_progress ? "resolving in progress" : "")); + return WERR_DS_OBJ_CLASS_NOT_DEFINED; } str = talloc_strdup(out->values, c->lDAPDisplayName); @@ -1049,7 +1053,7 @@ static WERROR _dsdb_syntax_OID_attr_drsuapi_to_ldb(const struct dsdb_syntax_ctx W_ERROR_HAVE_NO_MEMORY(out->values); for (i=0; i < out->num_values; i++) { - uint32_t v; + uint32_t v, vo; const struct dsdb_attribute *a; const char *str; @@ -1064,6 +1068,7 @@ static WERROR _dsdb_syntax_OID_attr_drsuapi_to_ldb(const struct dsdb_syntax_ctx } v = IVAL(in->value_ctr.values[i].blob->data, 0); + vo = v; /* convert remote ATTID to local ATTID */ if (!dsdb_syntax_attid_from_remote_attid(ctx, mem_ctx, v, &v)) { @@ -1073,8 +1078,11 @@ static WERROR _dsdb_syntax_OID_attr_drsuapi_to_ldb(const struct dsdb_syntax_ctx a = dsdb_attribute_by_attributeID_id(ctx->schema, v); if (!a) { - DEBUG(1,(__location__ ": Unknown attributeID_id 0x%08X\n", v)); - return WERR_FOOBAR; + int dbg_level = ctx->schema->resolving_in_progress ? 10 : 0; + DEBUG(dbg_level,(__location__ ": %s unknown local attributeID_id 0x%08X remote 0x%08X%s\n", + attr->lDAPDisplayName, v, vo, + ctx->schema->resolving_in_progress ? "resolving in progress" : "")); + return WERR_DS_ATT_NOT_DEF_IN_SCHEMA; } str = talloc_strdup(out->values, a->lDAPDisplayName); @@ -2738,7 +2746,10 @@ WERROR dsdb_attribute_drsuapi_remote_to_local(const struct dsdb_syntax_ctx *ctx, sa = dsdb_attribute_by_attributeID_id(ctx->schema, attid_local); if (!sa) { - DEBUG(1,(__location__ ": Unknown attributeID_id 0x%08X\n", in->attid)); + int dbg_level = ctx->schema->resolving_in_progress ? 10 : 0; + DEBUG(dbg_level,(__location__ ": Unknown local attributeID_id 0x%08X remote 0x%08X%s\n", + attid_local, remote_attid_as_enum, + ctx->schema->resolving_in_progress ? "resolving in progress" : "")); TALLOC_FREE(frame); return WERR_DS_ATT_NOT_DEF_IN_SCHEMA; } -- 1.9.1 From c8db070118dde0e176c1e46ec582b22b9cba6762 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2016 12:11:53 +0200 Subject: [PATCH 15/20] s4:dsdb/repl: set working_schema->resolving_in_progress during schema creation BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit cff6111d2fe5b3999447eaa870c5ddee158226aa) --- source4/dsdb/repl/replicated_objects.c | 3 +++ source4/libnet/libnet_vampire.c | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c index 288bcc1..f3aade2 100644 --- a/source4/dsdb/repl/replicated_objects.c +++ b/source4/dsdb/repl/replicated_objects.c @@ -300,6 +300,7 @@ WERROR dsdb_repl_make_working_schema(struct ldb_context *ldb, DEBUG(0,(__location__ ": schema copy failed!\n")); return WERR_NOMEM; } + working_schema->resolving_in_progress = true; /* we are going to need remote prefixMap for decoding */ werr = dsdb_schema_pfm_from_drsuapi_pfm(mapping_ctr, true, @@ -359,6 +360,8 @@ WERROR dsdb_repl_make_working_schema(struct ldb_context *ldb, return werr; } + working_schema->resolving_in_progress = false; + *_schema_out = working_schema; return WERR_OK; diff --git a/source4/libnet/libnet_vampire.c b/source4/libnet/libnet_vampire.c index 60bfa41..8d68f8f 100644 --- a/source4/libnet/libnet_vampire.c +++ b/source4/libnet/libnet_vampire.c @@ -332,6 +332,9 @@ static NTSTATUS libnet_vampire_cb_apply_schema(struct libnet_vampire_cb_state *s "become dc", "schema convert retrial", 1); + provision_schema->resolving_in_progress = true; + s->self_made_schema->resolving_in_progress = true; + status = dsdb_repl_resolve_working_schema(s->ldb, pfm_remote, cycle_before_switching, @@ -348,6 +351,8 @@ static NTSTATUS libnet_vampire_cb_apply_schema(struct libnet_vampire_cb_state *s /* free temp objects for 1st conversion phase */ talloc_unlink(s, provision_schema); + s->self_made_schema->resolving_in_progress = false; + /* * attach the schema we just brought over DRS to the ldb, * so we can use it in dsdb_convert_object_ex below -- 1.9.1 From d710f64b17fd7f61e6459f4b8dbf19cb197986bb Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2016 09:10:48 +0200 Subject: [PATCH 16/20] s4:dsdb/repl: let dsdb_replicated_objects_convert() change remote to local attid for linked attributes We already do that for objects in dsdb_convert_object_ex(). We need to be consistent and do the same for linked attributes. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 111c5fd83fa3d0f2e1ae3af0767d8717edab484a) --- source4/dsdb/repl/replicated_objects.c | 62 +++++++++++++++++++++++++++++++--- source4/dsdb/samdb/samdb.h | 2 +- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c index f3aade2..89d288a 100644 --- a/source4/dsdb/repl/replicated_objects.c +++ b/source4/dsdb/repl/replicated_objects.c @@ -649,6 +649,7 @@ WERROR dsdb_replicated_objects_convert(struct ldb_context *ldb, struct dsdb_schema_prefixmap *pfm_remote; struct dsdb_extended_replicated_objects *out; const struct drsuapi_DsReplicaObjectListItemEx *cur; + struct dsdb_syntax_ctx syntax_ctx; uint32_t i; out = talloc_zero(mem_ctx, struct dsdb_extended_replicated_objects); @@ -672,6 +673,10 @@ WERROR dsdb_replicated_objects_convert(struct ldb_context *ldb, return status; } + /* use default syntax conversion context */ + dsdb_syntax_ctx_init(&syntax_ctx, ldb, schema); + syntax_ctx.pfm_remote = pfm_remote; + if (ldb_dn_compare(partition_dn, ldb_get_schema_basedn(ldb)) != 0) { /* * check for schema changes in case @@ -697,11 +702,6 @@ WERROR dsdb_replicated_objects_convert(struct ldb_context *ldb, object_count); W_ERROR_HAVE_NO_MEMORY_AND_FREE(out->objects, out); - /* pass the linked attributes down to the repl_meta_data - module */ - out->linked_attributes_count = linked_attributes_count; - out->linked_attributes = linked_attributes; - for (i=0, cur = first_object; cur; cur = cur->next_object, i++) { if (i == object_count) { talloc_free(out); @@ -750,6 +750,58 @@ WERROR dsdb_replicated_objects_convert(struct ldb_context *ldb, return WERR_FOOBAR; } + out->linked_attributes = talloc_array(out, + struct drsuapi_DsReplicaLinkedAttribute, + linked_attributes_count); + W_ERROR_HAVE_NO_MEMORY_AND_FREE(out->linked_attributes, out); + + for (i=0; i < linked_attributes_count; i++) { + const struct drsuapi_DsReplicaLinkedAttribute *ra = &linked_attributes[i]; + struct drsuapi_DsReplicaLinkedAttribute *la = &out->linked_attributes[i]; + + if (ra->identifier == NULL) { + talloc_free(out); + return WERR_BAD_NET_RESP; + } + + *la = *ra; + + la->identifier = talloc_zero(out->linked_attributes, + struct drsuapi_DsReplicaObjectIdentifier); + W_ERROR_HAVE_NO_MEMORY_AND_FREE(la->identifier, out); + + /* + * We typically only get the guid filled + * and the repl_meta_data module only cares abouf + * the guid. + */ + la->identifier->guid = ra->identifier->guid; + + if (ra->value.blob != NULL) { + la->value.blob = talloc_zero(out->linked_attributes, + DATA_BLOB); + W_ERROR_HAVE_NO_MEMORY_AND_FREE(la->value.blob, out); + + if (ra->value.blob->length != 0) { + *la->value.blob = data_blob_dup_talloc(la->value.blob, + *ra->value.blob); + W_ERROR_HAVE_NO_MEMORY_AND_FREE(la->value.blob->data, out); + } + } + + status = dsdb_attribute_drsuapi_remote_to_local(&syntax_ctx, + ra->attid, + &la->attid, + NULL); + if (!W_ERROR_IS_OK(status)) { + DEBUG(0,(__location__": linked_attribute[%u] attid 0x%08X not found: %s\n", + i, ra->attid, win_errstr(status))); + return status; + } + } + + out->linked_attributes_count = linked_attributes_count; + /* free pfm_remote, we won't need it anymore */ talloc_free(pfm_remote); diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h index 0acffa1..c7260d0 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -215,7 +215,7 @@ struct dsdb_extended_replicated_objects { struct dsdb_extended_replicated_object *objects; uint32_t linked_attributes_count; - const struct drsuapi_DsReplicaLinkedAttribute *linked_attributes; + struct drsuapi_DsReplicaLinkedAttribute *linked_attributes; WERROR error; -- 1.9.1 From b7b221779ec5ac1dd071811a7234249287de022e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 2 Aug 2016 14:28:12 +1200 Subject: [PATCH 17/20] s4:dsdb/repl_meta_data: Add more info on which DN we failed to find an attid on BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Andrew Bartlett Reviewed-by: Jeremy Allison (cherry picked from commit 108697402c9e39511f9ea1b1e21cae9322586f6b) --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 6 +++++- 1 file changed, 5 insertions(+), 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 4989a4b..f3573f6 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -6096,7 +6096,11 @@ linked_attributes[0]: /* find the attribute being modified */ attr = dsdb_attribute_by_attributeID_id(schema, la->attid); if (attr == NULL) { - DEBUG(0, (__location__ ": Unable to find attributeID 0x%x\n", la->attid)); + struct GUID_txt_buf guid_str; + ldb_asprintf_errstring(ldb, "Unable to find attributeID 0x%x for link on ", + la->attid, + GUID_buf_string(&la->identifier->guid, + &guid_str)); talloc_free(tmp_ctx); return LDB_ERR_OPERATIONS_ERROR; } -- 1.9.1 From c577bc705d4a6e360f202bd7dbc60cc9fe378862 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 2 Aug 2016 15:45:18 +1200 Subject: [PATCH 18/20] selftest: Move repl_schema test to a distinct OID prefix We also take the chance to make it clearer that the number being passed in should be unique. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12128 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 3ce5ad1e6ca25fb91f0a0dcd7e389713913a35b0) --- source4/setup/schema_samba4.ldif | 2 ++ source4/torture/drs/python/repl_schema.py | 26 +++++++++++++------------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/source4/setup/schema_samba4.ldif b/source4/setup/schema_samba4.ldif index 604e115..9e3ff91 100644 --- a/source4/setup/schema_samba4.ldif +++ b/source4/setup/schema_samba4.ldif @@ -18,12 +18,14 @@ ## 1.3.6.1.4.1.7165.4.6.1.1.x - ldap_syntaxes.py ## 1.3.6.1.4.1.7165.4.6.1.2.x - ldap_syntaxes.py ## 1.3.6.1.4.1.7165.4.6.1.4.x - urgent_replication.py +## 1.3.6.1.4.1.7165.4.6.1.5.x - repl_schema.py ## 1.3.6.1.4.1.7165.4.6.2.x - SELFTEST random classes ## 1.3.6.1.4.1.7165.4.6.2.1.x - ldap_syntaxes.py ## 1.3.6.1.4.1.7165.4.6.2.2.x - ldap_syntaxes.py ## 1.3.6.1.4.1.7165.4.6.2.3.x - sec_descriptor.py ## 1.3.6.1.4.1.7165.4.6.2.4.x - urgent_replication.py +## 1.3.6.1.4.1.7165.4.6.2.5.x - repl_schema.py ## 1.3.6.1.4.1.7165.4.255.x - mapped OIDs due to conflicts between AD and standards-track # diff --git a/source4/torture/drs/python/repl_schema.py b/source4/torture/drs/python/repl_schema.py index ff22adb..865260c 100644 --- a/source4/torture/drs/python/repl_schema.py +++ b/source4/torture/drs/python/repl_schema.py @@ -120,7 +120,7 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase): "objectClass": ["top", "classSchema"], "cn": class_name, "lDAPDisplayName": class_ldn, - "governsId": "1.3.6.1.4.1.7165.4.6.2." \ + "governsId": "1.3.6.1.4.1.7165.4.6.2.5." \ + str((100000 * base_int) + random.randint(1,100000)) + ".1.5.13", "instanceType": "4", "objectClassCategory": "%d" % oc_cat, @@ -144,7 +144,7 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase): "objectClass": ["top", "attributeSchema"], "cn": attr_name, "lDAPDisplayName": attr_ldn, - "attributeId": "1.3.6.1.4.1.7165.4.6.1." \ + "attributeId": "1.3.6.1.4.1.7165.4.6.1.5." \ + str((100000 * base_int) + random.randint(1,100000)) + ".1.5.13", "attributeSyntax": "2.5.5.12", "omSyntax": "64", @@ -217,15 +217,15 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase): This should check code path that searches for AttributeID_id in Schema cache""" # add new attributeSchema object - (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-A", 1) + (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-A", 7) # add a base classSchema class so we can use our new # attribute in class definition in a sibling class - (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-A", 7, + (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-A", 8, 1, {"systemMayContain": a_ldn, "subClassOf": "classSchema"}) # add new classSchema object with value for a_ldb attribute - (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-B", 8, + (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-B", 9, 1, {"objectClass": ["top", "classSchema", c_ldn], a_ldn: "test_classWithCustomAttribute"}) @@ -241,18 +241,18 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase): This should check code path that searches for AttributeID_id in Schema cache""" # add new attributeSchema object - (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-Link-X", 1, + (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-Link-X", 10, attrs={'linkID':"99990", "attributeSyntax": "2.5.5.1", "omSyntax": "127"}) # add a base classSchema class so we can use our new # attribute in class definition in a sibling class - (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-Link-Y", 7, + (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-Link-Y", 11, 1, {"systemMayContain": a_ldn, "subClassOf": "classSchema"}) # add new classSchema object with value for a_ldb attribute - (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-Link-Z", 8, + (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-Link-Z", 12, 1, {"objectClass": ["top", "classSchema", c_ldn], a_ldn: self.schema_dn}) @@ -290,7 +290,7 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase): def test_attribute(self): """Simple test for attributeSchema replication""" # add new attributeSchema object - (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-S", 2) + (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-S", 13) # force replication from DC1 to DC2 self._net_drs_replicate(DC=self.dnsname_dc2, fromDC=self.dnsname_dc1, nc_dn=self.schema_dn, forced=True) # check object is replicated @@ -306,8 +306,8 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase): self._disable_all_repl(self.dnsname_dc2) # add new attributeSchema object - (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-OU-S", 3) - (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-OU-A", 8, + (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-OU-S", 14) + (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-OU-A", 15, 3, {"mayContain": a_ldn}) ou_dn = ldb.Dn(self.ldb_dc1, "ou=X") @@ -333,9 +333,9 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase): and then check all objects are replicated correctly""" # add new classSchema object - (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-A", 9) + (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-A", 16) # add new attributeSchema object - (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-A", 3) + (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-A", 17) # add attribute to the class we have m = Message.from_dict(self.ldb_dc1, -- 1.9.1 From 7c85abafc63a659b08a5de5363c39e67e788620c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 5 Aug 2016 22:06:41 +0200 Subject: [PATCH 19/20] Revert "s4: tests: Skip drs tests." This reverts commit 08d03f79de49826dc5dff3bc09193f1404e5f549. Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 119e7d094da26029e16d72598a55377d8010f28f) --- selftest/skip | 4 ---- 1 file changed, 4 deletions(-) diff --git a/selftest/skip b/selftest/skip index 879608d..ba6718a 100644 --- a/selftest/skip +++ b/selftest/skip @@ -138,7 +138,3 @@ bench # don't run benchmarks in our selftest ^samba4.rpc.unixinfo # This contains a server-side getpwuid call which hangs the server when nss_winbindd is in use ^samba.tests.dcerpc.unix # This contains a server-side getpwuid call which hangs the server when nss_winbindd is in use GETADDRINFO # socket wrapper doesn't support threads -# -# drs tests are currently broken. -# Revert this when they are working again. -^samba.*drs.* -- 1.9.1 From fb9d8cd7a089c8dc3b1a396f1b6026d1a37360eb Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sat, 6 Aug 2016 10:33:49 +0200 Subject: [PATCH 20/20] selftest/flapping: add some samba3.blackbox.smbclient_s3 tests BUG: https://bugzilla.samba.org/show_bug.cgi?id=12108 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Thu Aug 11 04:42:30 CEST 2016 on sn-devel-144 (cherry picked from commit c064357428799ebb9c8ac5e9bf3a081bdad45072) --- selftest/flapping | 2 ++ 1 file changed, 2 insertions(+) diff --git a/selftest/flapping b/selftest/flapping index 4ea0a44..50fdf1e 100644 --- a/selftest/flapping +++ b/selftest/flapping @@ -32,3 +32,5 @@ # ^samba4.ldap.notification.python\(.*\).__main__.LDAPNotificationTest.test_max_search ^samba3.blackbox.smbclient_tar.* # fails very, very often on sn-devel +^samba3.blackbox.smbclient_s3.*.sending a message to the remote server # flakey on sn-devel-104 and sn-devel-144 +^samba3.blackbox.smbclient_s3.*.creating a good symlink and deleting it by path # flakey on sn-devel-104 and sn-devel-144 -- 1.9.1