From 0f1e5309ceda026f12ad5657b386afd7ca7f4287 Mon Sep 17 00:00:00 2001 From: William Brown Date: Fri, 10 Feb 2017 13:44:00 +1000 Subject: [PATCH] Ticket 47925 - Move add and delete operation aci checks to be before plugins. Bug Description: Add, delete and modify had their aci checks in different orders. This meant that you had different behaviour in all of them. Fix Description: Move the add and delete checks to be before calling plugins to match the modify behaviour. This should allow certain plugins to do entry transformations that the user is not able to do, allowing proxy or complex internal operations to be allowed. https://fedorahosted.org/389/ticket/47925 Author: wibrown Review by: ??? --- ldap/servers/slapd/back-ldbm/ldbm_add.c | 18 +++++++++--------- ldap/servers/slapd/back-ldbm/ldbm_delete.c | 28 +++++++++++++++------------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c index 20f3df9..9e80479 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c @@ -315,6 +315,15 @@ ldbm_back_add( Slapi_PBlock *pb ) ldap_result_code = get_copy_of_entry(pb, &addr, &txn, SLAPI_ADD_PARENT_ENTRY, !is_replicated_operation); } + ldap_result_code = plugin_call_acl_plugin(pb, e, NULL, NULL, SLAPI_ACL_ADD, + ACLPLUGIN_ACCESS_DEFAULT, &errbuf); + if ( ldap_result_code != LDAP_SUCCESS ) + { + slapi_log_err(SLAPI_LOG_TRACE, "ldbm_back_add", "no access to parent, pdn = %s\n", + slapi_sdn_get_dn(&parentsdn)); + ldap_result_message= errbuf; + goto error_return; + } /* Call the Backend Pre Add plugins */ ldap_result_code = LDAP_SUCCESS; slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); @@ -734,15 +743,6 @@ ldbm_back_add( Slapi_PBlock *pb ) slapi_sdn_done(&ancestorsdn); goto error_return; } - ldap_result_code = plugin_call_acl_plugin(pb, e, NULL, NULL, SLAPI_ACL_ADD, - ACLPLUGIN_ACCESS_DEFAULT, &errbuf); - if ( ldap_result_code != LDAP_SUCCESS ) - { - slapi_log_err(SLAPI_LOG_TRACE, "ldbm_back_add", "no access to parent, pdn = %s\n", - slapi_sdn_get_dn(&parentsdn)); - ldap_result_message= errbuf; - goto error_return; - } pid = parententry->ep_id; /* We may need to adjust the DN since parent could be a resurrected conflict entry... */ diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index 72b2d29..9a9bce6 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -270,12 +270,25 @@ ldbm_back_delete( Slapi_PBlock *pb ) */ if ((e = find_entry2modify(pb, be, addr, &txn, &result_sent)) == NULL) { - ldap_result_code= LDAP_NO_SUCH_OBJECT; + ldap_result_code= LDAP_NO_SUCH_OBJECT; retval = -1; slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_delete", "Deleting entry is already deleted.\n"); goto error_return; /* error result sent by find_entry2modify() */ } ep_id = e->ep_id; + + /* JCMACL - Shouldn't the access check be before the has children check... + * otherwise we're revealing the fact that an entry exists and has children */ + /* Before has children to mask the presence of children disclosure. */ + ldap_result_code = plugin_call_acl_plugin (pb, e->ep_entry, NULL, NULL, SLAPI_ACL_DELETE, + ACLPLUGIN_ACCESS_DEFAULT, &errbuf ); + if ( ldap_result_code != LDAP_SUCCESS ) + { + ldap_result_message= errbuf; + retval = -1; + goto error_return; + } + retval = slapi_entry_has_children(e->ep_entry); if (retval) { ldap_result_code= LDAP_NOT_ALLOWED_ON_NONLEAF; @@ -285,7 +298,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) retval = -1; goto error_return; } - + /* Don't call pre-op for Tombstone entries */ if (!delete_tombstone_entry) { @@ -439,17 +452,6 @@ ldbm_back_delete( Slapi_PBlock *pb ) /* Save away a copy of the entry, before modifications */ slapi_pblock_set( pb, SLAPI_ENTRY_PRE_OP, slapi_entry_dup( e->ep_entry )); - /* JCMACL - Shouldn't the access check be before the has children check... - * otherwise we're revealing the fact that an entry exists and has children */ - ldap_result_code = plugin_call_acl_plugin (pb, e->ep_entry, NULL, NULL, SLAPI_ACL_DELETE, - ACLPLUGIN_ACCESS_DEFAULT, &errbuf ); - if ( ldap_result_code != LDAP_SUCCESS ) - { - ldap_result_message= errbuf; - retval = -1; - goto error_return; - } - /* * Get the entry's parent. We do this here because index_read * seems to deadlock the database when dblayer_txn_begin is -- 1.8.3.1