Bug 1150695

Summary: ldbm_back_modify SLAPI_PLUGIN_BE_PRE_MODIFY_FN does not return even if one of the preop plugins fails.
Product: Red Hat Enterprise Linux 7 Reporter: Noriko Hosoi <nhosoi>
Component: 389-ds-baseAssignee: Noriko Hosoi <nhosoi>
Status: CLOSED ERRATA QA Contact: Viktor Ashirov <vashirov>
Severity: urgent Docs Contact:
Priority: urgent    
Version: 7.0CC: nhosoi, nkinder, rmeggins, tbordaz
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: 389-ds-base-1.3.3.1-6.el7 Doc Type: Bug Fix
Doc Text:
Cause: In ldbm_back_modify, plug in called at the SLAPI_PLUGIN_BE_PRE_MODIFY_FN timing does not return but it continue the modify operation even if one of the preop plugins fails. Consequence: It leaves an incomplete modify result. Fix: Fixed ldbm_back_modify code so that if plug ins called at SLAPI_PLUGIN_BE_PRE_MODIFY_FN fail, it returns the proper error and does not operate modify. Result: Data consistency is assured.
 Story Points: ---
Clone Of: Environment:
Last Closed: 2015-03-05 09:36:14 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Noriko Hosoi 2014-10-08 17:20:00 UTC 
This bug is created as a clone of upstream ticket:
https://fedorahosted.org/389/ticket/47919

If opreturn is SLAPI_PLUGIN_NOOP, it returns at the line 644, but if opreturn is non-zero (e.g., SLAPI_PLUGIN_FAILURE), it goes forward.
{{{
 389 int
 390 ldbm_back_modify( Slapi_PBlock *pb )
  [...]
 630             opreturn = plugin_call_plugins(pb, SLAPI_PLUGIN_BE_PRE_MODIFY_FN);
 631             if (opreturn ||
 632                 (slapi_pblock_get(pb, SLAPI_RESULT_CODE, &ldap_result_code) && ldap_result_code) ||
 633                 (slapi_pblock_get(pb, SLAPI_PLUGIN_OPRETURN, &opreturn) && opreturn)) {
 634                 slapi_pblock_get(pb, SLAPI_RESULT_CODE, &ldap_result_code);
 635                 slapi_pblock_get(pb, SLAPI_PLUGIN_OPRETURN, &opreturn);
 636                 if (!ldap_result_code) {
 637                     LDAPDebug0Args(LDAP_DEBUG_ANY, "ldbm_back_modify: SLAPI_PLUGIN_BE_PRE_MODIFY_FN "
 638                                "returned error but did not set SLAPI_RESULT_CODE\n");
 639                     ldap_result_code = LDAP_OPERATIONS_ERROR;
 640                 }
 641                 if (SLAPI_PLUGIN_NOOP == opreturn) {
 642                     not_an_error = 1;
 643                     rc = opreturn = LDAP_SUCCESS;
 644                     goto error_return;
 645                 } else if (!opreturn) {
 646                     opreturn = -1;
 647                     slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, &opreturn);
 648                 }
 649             }
}}}

Historically, the return value of plugins were just ignored and the process goes forward in the backend.  But it's strictly checked now. (See SLAPI_PLUGIN_BE_PRE_{ADD,DELETE,MODRDN}_FN.)  SLAPI_PLUGIN_BE_PRE_MODIFY_FN should follow the rule.
Comment 1 Sankar Ramalingam 2014-10-09 02:44:29 UTC 
Please add verification steps.
Comment 2 Noriko Hosoi 2014-10-09 03:14:54 UTC 
Sankar,

This issue was found in OTP/IPA development.  I think we could borrow their test case to verify this issue.
Comment 3 thierry bordaz 2014-10-09 08:50:34 UTC 
Noriko, Sankar,

I think it requires to create a test plugin that triggers a failure in BEPREOP_MODIFY.

I looked at existing plugins that register BEPREOP_MODIFY to see if we can trigger a failure. usn plugin there is no possibilty.

dna plugin, if it fails to extend the range of values it returns a failure. But I am not sure it would be easy to trigger.

IMHO, the best option is to write/compile/install/configure a plugin that trigger a failure.

thanks
theirry
Comment 5 Sankar Ramalingam 2014-11-25 11:48:21 UTC 
Configured memberOf plugin with non-existing subtree for nsslapd-pluginConfigArea, and the same got rejected since there is no entry present. As per the design doc... the memberOf plugin configuration should be rejected by Pre-operation plugin if the entry doesn't exist.

[root@vm-idm-035 MMR_WINSYNC]# ldapmodify -x -p 1189 -h localhost -D "cn=Directory Manager" -w Secret123 << EOF
dn: cn=MemberOf Plugin,cn=plugins,cn=config                                                                          
changetype: modify
replace: nsslapd-pluginConfigArea
nsslapd-pluginConfigArea:ou=People,dc=wwwmemofsuff,dc=com
EOF
modifying entry "cn=MemberOf Plugin,cn=plugins,cn=config"
ldap_modify: Server is unwilling to perform (53)
	additional info: The nsslapd-pluginConfigArea configuration attribute points to an entry that  can not be found.  (ou=People,dc=wwwmemofsuff,dc=com)

[root@vm-idm-035 MMR_WINSYNC]# echo $?
53


Marking it as verified since the prompt returned for a pre-operation failure.

Build tested:
[root@vm-idm-035 MMR_WINSYNC]# rpm -qa 389-ds-base
389-ds-base-1.3.3.1-9.el7.x86_64
Comment 7 errata-xmlrpc 2015-03-05 09:36:14 UTC 
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHSA-2015-0416.html