Commit 3783235 Ticket 47925 - Move add and delete operation aci checks to be before plugins.

2 files Authored and Committed by firstyear 5 days ago
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: ???

    
 1 @@ -315,6 +315,15 @@
 2   »       »       »       »       »       ldap_result_code = get_copy_of_entry(pb, &addr, &txn, SLAPI_ADD_PARENT_ENTRY, !is_replicated_operation);
 3   »       »       »       »       }
 4   
 5 + »       »       »       »       ldap_result_code = plugin_call_acl_plugin(pb, e, NULL, NULL, SLAPI_ACL_ADD, 
 6 + »       »       »       »                                                 ACLPLUGIN_ACCESS_DEFAULT, &errbuf);
 7 + »       »       »       »       if ( ldap_result_code != LDAP_SUCCESS )
 8 + »       »       »       »       {
 9 + »       »       »       »       »       slapi_log_err(SLAPI_LOG_TRACE, "ldbm_back_add", "no access to parent, pdn = %s\n",
10 + »       »       »       »       »                     slapi_sdn_get_dn(&parentsdn));
11 + »       »       »       »       »       ldap_result_message= errbuf;
12 + »       »       »       »       »       goto error_return;
13 + »       »       »       »       }
14   »       »       »       »       /* Call the Backend Pre Add plugins */
15   »       »       »       »       ldap_result_code = LDAP_SUCCESS;
16   »       »       »       »       slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code);
17 @@ -734,15 +743,6 @@
18   »       »       »       »       »       slapi_sdn_done(&ancestorsdn);
19   »       »       »       »       »       goto error_return;
20   »       »       »       »       }
21 - »       »       »       »       ldap_result_code = plugin_call_acl_plugin(pb, e, NULL, NULL, SLAPI_ACL_ADD, 
22 - »       »       »       »                                                 ACLPLUGIN_ACCESS_DEFAULT, &errbuf);
23 - »       »       »       »       if ( ldap_result_code != LDAP_SUCCESS )
24 - »       »       »       »       {
25 - »       »       »       »       »       slapi_log_err(SLAPI_LOG_TRACE, "ldbm_back_add", "no access to parent, pdn = %s\n",
26 - »       »       »       »       »                     slapi_sdn_get_dn(&parentsdn));
27 - »       »       »       »       »       ldap_result_message= errbuf;
28 - »       »       »       »       »       goto error_return;
29 - »       »       »       »       }
30   »       »       »       »       pid = parententry->ep_id;
31   
32   »       »       »       »       /* We may need to adjust the DN since parent could be a resurrected conflict entry... */
 1 @@ -270,12 +270,25 @@
 2   »       »       »        */
 3   »       »       »       if ((e = find_entry2modify(pb, be, addr, &txn, &result_sent)) == NULL)
 4   »       »       »       {
 5 - »       »       »       »       ldap_result_code= LDAP_NO_SUCH_OBJECT; 
 6 + »       »       »       »       ldap_result_code= LDAP_NO_SUCH_OBJECT;
 7   »       »       »       »       retval = -1;
 8   »       »       »       »       slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_delete", "Deleting entry is already deleted.\n");
 9   »       »       »       »       goto error_return; /* error result sent by find_entry2modify() */
10   »       »       »       }
11   »       »       »       ep_id = e->ep_id;
12 + 
13 + »       »       »       /* JCMACL - Shouldn't the access check be before the has children check...
14 + »       »       »        * otherwise we're revealing the fact that an entry exists and has children */
15 + »       »       »       /* Before has children to mask the presence of children disclosure. */
16 + »       »       »       ldap_result_code = plugin_call_acl_plugin (pb, e->ep_entry, NULL, NULL, SLAPI_ACL_DELETE, 
17 + »       »       »       »       »       »       »       ACLPLUGIN_ACCESS_DEFAULT, &errbuf );
18 + »       »       »       if ( ldap_result_code != LDAP_SUCCESS )
19 + »       »       »       {
20 + »       »       »       »       ldap_result_message= errbuf;
21 + »       »       »       »       retval = -1;
22 + »       »       »       »       goto error_return;
23 + »       »       »       }
24 + 
25   »       »       »       retval = slapi_entry_has_children(e->ep_entry);
26   »       »       »       if (retval) {
27   »       »       »       »       ldap_result_code= LDAP_NOT_ALLOWED_ON_NONLEAF;
28 @@ -285,7 +298,7 @@
29   »       »       »       »       retval = -1;
30   »       »       »       »       goto error_return;
31   »       »       »       }
32 - »       »       
33 + 
34   »       »       »       /* Don't call pre-op for Tombstone entries */
35   »       »       »       if (!delete_tombstone_entry)
36   »       »       »       {
37 @@ -439,17 +452,6 @@
38   »       »       »       /* Save away a copy of the entry, before modifications */
39   »       »       »       slapi_pblock_set( pb, SLAPI_ENTRY_PRE_OP, slapi_entry_dup( e->ep_entry ));
40   
41 - »       »       »       /* JCMACL - Shouldn't the access check be before the has children check...
42 - »       »       »        * otherwise we're revealing the fact that an entry exists and has children */
43 - »       »       »       ldap_result_code = plugin_call_acl_plugin (pb, e->ep_entry, NULL, NULL, SLAPI_ACL_DELETE, 
44 - »       »       »       »       »       »       »       ACLPLUGIN_ACCESS_DEFAULT, &errbuf );
45 - »       »       »       if ( ldap_result_code != LDAP_SUCCESS )
46 - »       »       »       {
47 - »       »       »       »       ldap_result_message= errbuf;
48 - »       »       »       »       retval = -1;
49 - »       »       »       »       goto error_return;
50 - »       »       »       }
51 - 
52   »       »       »       /*
53   »       »       »        * Get the entry's parent. We do this here because index_read
54   »       »       »        * seems to deadlock the database when dblayer_txn_begin is