From d5153422a12a6099da0925436f4a28370256801d Mon Sep 17 00:00:00 2001 From: Ludwig Krispenz Date: Tue, 10 Sep 2013 10:09:30 +0200 Subject: [PATCH] Ticket 601 - multi master replication allows schema violation Bug Description: If a required attribute has two values, it is possible to delete them concurrently on different masters. The delete of a single value is valid, but after the deletes are replicated all values are gone. Fix Description: In the state of update resoultion it is too late to reject any of the deletes (and it would require schema check in the update resolution process), they have been acked to the client. What could an dshould be done is to detect this situation log an error and flag the entry with a replConflict. To do this schema checking for mods has to be done for replicated operations and the slapi_enty_schema_check function needs to be extended. https://fedorahosted.org/389/ticket/601 Reviewed by: ? --- ldap/servers/slapd/back-ldbm/ldbm_modify.c | 47 +++++++++++++++++++++++++----- ldap/servers/slapd/schema.c | 13 +++++++-- ldap/servers/slapd/slapi-plugin.h | 10 +++++++ 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index d2302fe..6b4246f 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -272,17 +272,46 @@ modify_apply_check_expand( goto done; } - /* if this is a replicated op, we don't need to perform these checks */ - if(!repl_op){ - /* check that the entry still obeys the schema */ - if ((operation_is_flag_set(operation,OP_FLAG_ACTION_SCHEMA_CHECK)) && - slapi_entry_schema_check( pb, ec->ep_entry ) != 0 ) { + /* multimaster replication can result in a schema violation, + * although the individual operations on each master were valid + * It is too late to resolve this. But we can check schema and + * add a replication conflict attribute. + */ + /* check that the entry still obeys the schema */ + if ((operation_is_flag_set(operation,OP_FLAG_ACTION_SCHEMA_CHECK)) && + slapi_entry_schema_check_ext( pb, ec->ep_entry, 1 ) != 0 ) { + if(repl_op){ + Slapi_Attr *attr; + Slapi_Mods smods; + LDAPMod **lmods; + if (slapi_entry_attr_find (ec->ep_entry, ATTR_NSDS5_REPLCONFLICT, &attr) == 0) + { + /* add value */ + Slapi_Value *val = slapi_value_new_string("Schema violation"); + slapi_attr_add_value(attr,val); + slapi_value_free(&val); + } else { + /* Add new attribute */ + slapi_entry_add_string (ec->ep_entry, ATTR_NSDS5_REPLCONFLICT, "Schema violation"); + } + /* the replconflict attribute is indexed and the index is built from the mods, + * so we need to extend the mods */ + slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &lmods); + slapi_mods_init_passin(&smods, mods); + slapi_mods_add_string (&smods, LDAP_MOD_ADD, ATTR_NSDS5_REPLCONFLICT, "Schema violation"); + lmods = slapi_mods_get_ldapmods_passout(&smods); + slapi_pblock_set(pb, SLAPI_MODIFY_MODS, lmods); + slapi_mods_done(&smods); + + } else { *ldap_result_code = LDAP_OBJECT_CLASS_VIOLATION; slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, ldap_result_message); rc = -1; goto done; } + } + if(!repl_op){ /* check attribute syntax for the new values */ if (slapi_mods_syntax_check(pb, mods, 0) != 0) { *ldap_result_code = LDAP_INVALID_SYNTAX; @@ -557,15 +586,17 @@ ldbm_back_modify( Slapi_PBlock *pb ) } /* The Plugin may have messed about with some of the PBlock parameters... ie. mods */ slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &mods ); - slapi_mods_init_byref(&smods,mods); - mod_count = slapi_mods_get_num_mods(&smods); /* apply the mods, check for syntax, schema problems, etc. */ if (modify_apply_check_expand(pb, operation, mods, e, ec, &postentry, &ldap_result_code, &ldap_result_message)) { goto error_return; } - + /* the schema check could have added a repl conflict mod + * get the mods again */ + slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &mods ); + slapi_mods_init_byref(&smods,mods); + mod_count = slapi_mods_get_num_mods(&smods); /* * Grab a copy of the mods and the entry in case the be_txn_preop changes * the them. If we have a failure, then we need to reset the mods to their diff --git a/ldap/servers/slapd/schema.c b/ldap/servers/slapd/schema.c index 7a91aa1..290e754 100644 --- a/ldap/servers/slapd/schema.c +++ b/ldap/servers/slapd/schema.c @@ -458,12 +458,21 @@ free_qdlist(char **vals, int n) /* * slapi_entry_schema_check - check that entry e conforms to the schema * required by its object class(es). returns 0 if so, non-zero otherwise. - * [ the pblock is only used to check if this is a replicated operation. + * [ the pblock is used to check if this is a replicated operation. * you may pass in NULL if this isn't part of an operation. ] + * the pblock is also used to return a reason why schema checking failed. + * it is also used to get schema flags + * if replicated operations should be checked use slapi_entry_schema_check_ext */ int slapi_entry_schema_check( Slapi_PBlock *pb, Slapi_Entry *e ) { + return (slapi_entry_schema_check_ext(pb, e, 0)); +} + +int +slapi_entry_schema_check_ext( Slapi_PBlock *pb, Slapi_Entry *e, int repl_check ) +{ struct objclass **oclist; struct objclass *oc; const char *ocname; @@ -486,7 +495,7 @@ slapi_entry_schema_check( Slapi_PBlock *pb, Slapi_Entry *e ) slapi_pblock_get(pb, SLAPI_IS_REPLICATED_OPERATION, &is_replicated_operation); slapi_pblock_get(pb, SLAPI_SCHEMA_FLAGS, &schema_flags); } - if ( schemacheck == 0 || is_replicated_operation ) { + if ( schemacheck == 0 || (is_replicated_operation && !repl_check)) { return( 0 ); } diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 6cb6d74..6adecb9 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -1505,6 +1505,16 @@ void slapi_entry_set_uniqueid( Slapi_Entry *e, char *uniqueid ); int slapi_entry_schema_check( Slapi_PBlock *pb, Slapi_Entry *e ); /** + * Determines whether the specified entry complies with the schema for its object + * class. + * + * Like slapi_entry_schema_check() with one additional parameter to enforce schema + * checking for replicated operations. + * \param check_repl Set to 1 if replicted operations should be checked + */ +int slapi_entry_schema_check_ext( Slapi_PBlock *pb, Slapi_Entry *e, int check_repl ); + +/** * Determines whether the specified entry complies with the syntax rules imposed * by it's attribute types. * -- 1.7.11.7