From dc37fa147a64a3bd8bb9ecbfeec3bc6f85e8117e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 19 Aug 2015 13:26:41 +1200 Subject: [PATCH 1/2] repl: Give an error if we get a secret when not expecting one We should never get a secret from a server when we specify DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING This asserts that this is the case. Signed-off-by: Andrew Bartlett Reviewed-by: Garming Sam (cherry picked from commit 4b25650577cd5c20729f3405c64c20ddf71b0ae3) --- libcli/drsuapi/drsuapi.h | 1 + libcli/drsuapi/repl_decrypt.c | 6 ++++++ source3/libnet/libnet_dssync.c | 1 + source4/dsdb/repl/drepl_out_helpers.c | 3 +++ source4/dsdb/repl/replicated_objects.c | 23 ++++++++++++++++++++--- source4/dsdb/samdb/samdb.h | 1 + source4/libnet/libnet_vampire.c | 7 ++++++- 7 files changed, 38 insertions(+), 4 deletions(-) diff --git a/libcli/drsuapi/drsuapi.h b/libcli/drsuapi/drsuapi.h index a4fb15f..7c6cf2f 100644 --- a/libcli/drsuapi/drsuapi.h +++ b/libcli/drsuapi/drsuapi.h @@ -29,6 +29,7 @@ WERROR drsuapi_decrypt_attribute_value(TALLOC_CTX *mem_ctx, WERROR drsuapi_decrypt_attribute(TALLOC_CTX *mem_ctx, const DATA_BLOB *gensec_skey, uint32_t rid, + uint32_t dsdb_repl_flags, struct drsuapi_DsReplicaAttribute *attr); diff --git a/libcli/drsuapi/repl_decrypt.c b/libcli/drsuapi/repl_decrypt.c index 00b8db8..4a2a28f 100644 --- a/libcli/drsuapi/repl_decrypt.c +++ b/libcli/drsuapi/repl_decrypt.c @@ -28,6 +28,7 @@ #include "../lib/crypto/crypto.h" #include "../libcli/drsuapi/drsuapi.h" #include "libcli/auth/libcli_auth.h" +#include "dsdb/samdb/samdb.h" WERROR drsuapi_decrypt_attribute_value(TALLOC_CTX *mem_ctx, const DATA_BLOB *gensec_skey, @@ -134,6 +135,7 @@ WERROR drsuapi_decrypt_attribute_value(TALLOC_CTX *mem_ctx, WERROR drsuapi_decrypt_attribute(TALLOC_CTX *mem_ctx, const DATA_BLOB *gensec_skey, uint32_t rid, + uint32_t dsdb_repl_flags, struct drsuapi_DsReplicaAttribute *attr) { WERROR status; @@ -164,6 +166,10 @@ WERROR drsuapi_decrypt_attribute(TALLOC_CTX *mem_ctx, return WERR_OK; } + if (dsdb_repl_flags & DSDB_REPL_FLAG_EXPECT_NO_SECRETS) { + return WERR_TOO_MANY_SECRETS; + } + if (attr->value_ctr.num_values > 1) { return WERR_DS_DRA_INVALID_PARAMETER; } diff --git a/source3/libnet/libnet_dssync.c b/source3/libnet/libnet_dssync.c index 94f0628..267709e 100644 --- a/source3/libnet/libnet_dssync.c +++ b/source3/libnet/libnet_dssync.c @@ -113,6 +113,7 @@ static void libnet_dssync_decrypt_attributes(TALLOC_CTX *mem_ctx, drsuapi_decrypt_attribute(mem_ctx, session_key, rid, + 0, attr); } } diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c index a047881..a1e8dcb 100644 --- a/source4/dsdb/repl/drepl_out_helpers.c +++ b/source4/dsdb/repl/drepl_out_helpers.c @@ -740,6 +740,9 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req if (state->op->options & DRSUAPI_DRS_FULL_SYNC_IN_PROGRESS) { dsdb_repl_flags |= DSDB_REPL_FLAG_PRIORITISE_INCOMING; } + if (state->op->options & DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING) { + dsdb_repl_flags |= DSDB_REPL_FLAG_EXPECT_NO_SECRETS; + } status = dsdb_replicated_objects_convert(service->samdb, working_schema ? working_schema : schema, diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c index 97b8b2a..98d5662 100644 --- a/source4/dsdb/repl/replicated_objects.c +++ b/source4/dsdb/repl/replicated_objects.c @@ -347,7 +347,7 @@ WERROR dsdb_convert_object_ex(struct ldb_context *ldb, struct dsdb_extended_replicated_object *out) { NTSTATUS nt_status; - WERROR status; + WERROR status = WERR_OK; uint32_t i; struct ldb_message *msg; struct replPropertyMetaDataBlob *md; @@ -444,8 +444,25 @@ WERROR dsdb_convert_object_ex(struct ldb_context *ldb, } for (j=0; jvalue_ctr.num_values; j++) { - status = drsuapi_decrypt_attribute(a->value_ctr.values[j].blob, gensec_skey, rid, a); - W_ERROR_NOT_OK_RETURN(status); + status = drsuapi_decrypt_attribute(a->value_ctr.values[j].blob, + gensec_skey, rid, + dsdb_repl_flags, a); + if (!W_ERROR_IS_OK(status)) { + break; + } + } + if (W_ERROR_EQUAL(status, WERR_TOO_MANY_SECRETS)) { + WERROR get_name_status = dsdb_attribute_drsuapi_to_ldb(ldb, schema, pfm_remote, + a, msg->elements, e); + if (W_ERROR_IS_OK(get_name_status)) { + DEBUG(0, ("Unxpectedly got secret value %s on %s from DRS server\n", + e->name, ldb_dn_get_linearized(msg->dn))); + } else { + DEBUG(0, ("Unxpectedly got secret value on %s from DRS server", + ldb_dn_get_linearized(msg->dn))); + } + } else if (!W_ERROR_IS_OK(status)) { + return status; } status = dsdb_attribute_drsuapi_to_ldb(ldb, schema, pfm_remote, diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h index 324045a..0a1d90d 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -62,6 +62,7 @@ struct dsdb_control_current_partition { #define DSDB_REPL_FLAG_PRIORITISE_INCOMING 1 #define DSDB_REPL_FLAG_PARTIAL_REPLICA 2 #define DSDB_REPL_FLAG_ADD_NCNAME 4 +#define DSDB_REPL_FLAG_EXPECT_NO_SECRETS 8 #define DSDB_CONTROL_REPLICATED_UPDATE_OID "1.3.6.1.4.1.7165.4.3.3" diff --git a/source4/libnet/libnet_vampire.c b/source4/libnet/libnet_vampire.c index 69195af..01f2487 100644 --- a/source4/libnet/libnet_vampire.c +++ b/source4/libnet/libnet_vampire.c @@ -553,6 +553,7 @@ NTSTATUS libnet_vampire_cb_store_chunk(void *private_data, const struct drsuapi_DsReplicaCursor2CtrEx *uptodateness_vector; struct dsdb_extended_replicated_objects *objs; uint32_t req_replica_flags; + uint32_t dsdb_repl_flags = 0; struct repsFromTo1 *s_dsa; char *tmp_dns_name; uint32_t i; @@ -679,6 +680,10 @@ NTSTATUS libnet_vampire_cb_store_chunk(void *private_data, return NT_STATUS_INTERNAL_ERROR; } + if (req_replica_flags & DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING) { + dsdb_repl_flags |= DSDB_REPL_FLAG_EXPECT_NO_SECRETS; + } + status = dsdb_replicated_objects_convert(s->ldb, schema, c->partition->nc.dn, @@ -690,7 +695,7 @@ NTSTATUS libnet_vampire_cb_store_chunk(void *private_data, s_dsa, uptodateness_vector, c->gensec_skey, - 0, + dsdb_repl_flags, s, &objs); if (!W_ERROR_IS_OK(status)) { DEBUG(0,("Failed to convert objects: %s\n", win_errstr(status))); -- 1.9.1 From cbbde92b7200c7a2f2b8e7a3dcee34aa4023d0bc Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 10 Mar 2016 13:43:15 +1300 Subject: [PATCH 2/2] dsdb/repl: Ensure we use the LOCAL attid value, not the remote one The key here is that while this never was an issue for builtin schema, nor for objects with an msDS-IntID used outside the schema partition, additional attributes added and used in the schema partition were incorrectly using the wrong attributeID value in the replPropertyMetaData. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11783 Signed-off-by: Andrew Bartlett Reviewed-by: Garming Sam Reviewed-by: Stefan Metzmacher Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Sun Mar 13 23:29:14 CET 2016 on sn-devel-144 (cherry picked from commit 6ecfc4cb254f9b2524ec5619ed8cee9db5d959b2) --- source4/dsdb/repl/replicated_objects.c | 18 ++++++++++++++---- source4/dsdb/schema/schema_syntax.c | 12 +++++++++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c index 98d5662..c658fe9 100644 --- a/source4/dsdb/repl/replicated_objects.c +++ b/source4/dsdb/repl/replicated_objects.c @@ -453,7 +453,7 @@ WERROR dsdb_convert_object_ex(struct ldb_context *ldb, } if (W_ERROR_EQUAL(status, WERR_TOO_MANY_SECRETS)) { WERROR get_name_status = dsdb_attribute_drsuapi_to_ldb(ldb, schema, pfm_remote, - a, msg->elements, e); + a, msg->elements, e, NULL); if (W_ERROR_IS_OK(get_name_status)) { DEBUG(0, ("Unxpectedly got secret value %s on %s from DRS server\n", e->name, ldb_dn_get_linearized(msg->dn))); @@ -465,11 +465,21 @@ WERROR dsdb_convert_object_ex(struct ldb_context *ldb, return status; } + /* + * This function also fills in the local attid value, + * based on comparing the remote and local prefixMap + * tables. If we don't convert the value, then we can + * have invalid values in the replPropertyMetaData we + * store on disk, as the prefixMap is per host, not + * per-domain. This may be why Microsoft added the + * msDS-IntID feature, however this is not used for + * extra attributes in the schema partition itself. + */ status = dsdb_attribute_drsuapi_to_ldb(ldb, schema, pfm_remote, - a, msg->elements, e); + a, msg->elements, e, + &m->attid); W_ERROR_NOT_OK_RETURN(status); - m->attid = a->attid; m->version = d->version; m->originating_change_time = d->originating_change_time; m->originating_invocation_id = d->originating_invocation_id; @@ -1001,7 +1011,7 @@ static WERROR dsdb_origin_object_convert(struct ldb_context *ldb, e = &msg->elements[i]; status = dsdb_attribute_drsuapi_to_ldb(ldb, schema, schema->prefixmap, - a, msg->elements, e); + a, msg->elements, e, NULL); W_ERROR_NOT_OK_RETURN(status); } diff --git a/source4/dsdb/schema/schema_syntax.c b/source4/dsdb/schema/schema_syntax.c index f9c50b8..5cf1664 100644 --- a/source4/dsdb/schema/schema_syntax.c +++ b/source4/dsdb/schema/schema_syntax.c @@ -2701,7 +2701,8 @@ WERROR dsdb_attribute_drsuapi_to_ldb(struct ldb_context *ldb, const struct dsdb_schema_prefixmap *pfm_remote, const struct drsuapi_DsReplicaAttribute *in, TALLOC_CTX *mem_ctx, - struct ldb_message_element *out) + struct ldb_message_element *out, + enum drsuapi_DsAttributeId *local_attid_as_enum) { const struct dsdb_attribute *sa; struct dsdb_syntax_ctx syntax_ctx; @@ -2737,6 +2738,15 @@ WERROR dsdb_attribute_drsuapi_to_ldb(struct ldb_context *ldb, return WERR_DS_ATT_NOT_DEF_IN_SCHEMA; } + /* + * We return the same class of attid as we were given. That + * is, we trust the remote server not to use an + * msDS-IntId value in the schema partition + */ + if (local_attid_as_enum != NULL) { + *local_attid_as_enum = (enum drsuapi_DsAttributeId)attid_local; + } + return sa->syntax->drsuapi_to_ldb(&syntax_ctx, sa, in, mem_ctx, out); } -- 1.9.1