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(-)
New commits:
commit d9f25b7bed11ea64daf5d3250f2e168a85b4c999
Author: Ludwig Krispenz <lkrispen(a)redhat.com>
Date: Tue Sep 24 15:18:07 2013 +0200
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: ?
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c
b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
index d2302fe..def314e 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, lmods);
+ 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 97fb81f..e02a125 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.
*