From: Stefan Metzmacher Date: Thu, 4 Aug 2016 08:03:14 +0000 (+0200) Subject: s4:dsdb/schema: store struct dsdb_schema_info instead of a hexstring X-Git-Tag: tevent-0.9.30~163 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=386dbc428b447bbdf9c500191273658b92b13f7a;p=thirdparty%2Fsamba.git 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 --- diff --git a/source4/dsdb/schema/schema.h b/source4/dsdb/schema/schema.h index 6c3581bb5e4..6543edace80 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 e113033a52a..76cd90da13a 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 16f213d2fe2..c9c55cabc29 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 270e6bebd98..c5acf9c4ce5 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 29941ebec6d..969d641af29 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 2fb5308f741..3b492b80a09 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 */