[Freeipa-devel] [PATCHES] OTP Patches

[Alexander Bokovoy]

On Wed, 19 Feb 2014, Alexander Bokovoy wrote:
> On Wed, 19 Feb 2014, Alexander Bokovoy wrote:
>> On Mon, 17 Feb 2014, Alexander Bokovoy wrote:
>>> On Thu, 13 Feb 2014, Alexander Bokovoy wrote:
>>>> On Wed, 12 Feb 2014, Nathaniel McCallum wrote:
>>>>> Through the review process, patches are getting shifted around, added,
>>>>> deleted, etc. So I'm now just going to be posting all the patches as an
>>>>> ordered set. The set attached is ordered according to my preferred merge
>>>>> order. It also places easy to review patches up front. I hope this helps
>>>>> reviewers. This format will definitely help me manage the patches.
>>>>> 
>>>>> The first three patches should be very easy reviews and can be merged
>>>>> independently.
>>>>> 
>>>>> All current patch critiques have, to my knowledge, been addressed in
>>>>> this latest series of patches.
>>>> I have tested all the patches altogether, including Web UI patches, and
>>>> everything works.
>>>> 
>>>> I have set up a COPR repo for others to try:
>>>> http://copr.fedoraproject.org/coprs/abbra/freeipa-otp-unstable/
>>>> 
>>>> However, there is one issue which I was not yet able to pin-point in the
>>>> SLAPI plugins. During FreeIPA install and later on actual use I see
>>>> these in the dirsrv error log:
>>>> 
>>>> [13/Feb/2014:14:32:52 +0200] - slapi_search_internal_set_pb: NULL parameter
>>>> [13/Feb/2014:14:32:52 +0200] - allow_operation: component identity is NULL
>>>> [13/Feb/2014:14:32:52 +0200] - slapi_search_internal_set_pb: NULL parameter
>>>> [13/Feb/2014:14:32:52 +0200] - allow_operation: component identity is NULL
>>>> [13/Feb/2014:14:33:01 +0200] - SLAPI_PLUGIN_BE_TXN_POST_DELETE_FN plugin returned error code -1
>>>> [13/Feb/2014:14:33:11 +0200] - slapi_search_internal_set_pb: NULL parameter
>>>> [13/Feb/2014:14:33:11 +0200] - allow_operation: component identity is NULL
>>>> [13/Feb/2014:14:45:53 +0200] - slapi_search_internal_set_pb: NULL parameter
>>>> [13/Feb/2014:14:45:53 +0200] - allow_operation: component identity is NULL
>>>> 
>>>> Additionally, when slapi-nis is enabled, LDAP bind with identity from
>>>> compat tree fails for OTP use and succeeds for password authentication.
>>>> 
>>>> In compat tree we are doing this trick:
>>>> 1731             /* Otherwise force rewrite of the 
>>>> SLAPI_BIND_TARGET_SDN                     1732              * 
>>>> and let other plugins to handle it.
>>>> 1733              * slapi-nis should have plugin ordering set below standard 50 to succeed */
>>>> 1734             slapi_pblock_get(pb, SLAPI_BIND_TARGET_SDN, &sdn);
>>>> 1735             if (sdn != NULL) {
>>>> 1736                 slapi_sdn_free(&sdn);
>>>> 1737             }
>>>> 1738             sdn = slapi_sdn_new_dn_byref(ndn);
>>>> 1739             slapi_pblock_set(pb, SLAPI_BIND_TARGET_SDN, (void*)sdn);
>>>> 1740             ret = 0;
>>>> 1741         }
>>>> 
>>>> I tried to play with plugin precedence and it didn't really help.
>>>> 
>>>> There is definitely a bug (or more) in ipa-pwd-extop in handling
>>>> authentication cases.
>>> Some progress on this investigation.
>>> 
>>> Plugin precedence setting is broken in 389-ds. It is only set once,
>>> before running init function provided by the plugin and does not take
>>> into account all callbacks that the init function may register. As
>>> result, all these functions get classified with default precedence (50)
>>> and no configuration could change this, we get ipa-pwd-extop's pre-bind
>>> callback called before schemacompat's one, thus working on the compat
>>> entry DN instead of the new one. Since that entry has no userPassword
>>> attribute, OTP code refuses to accept any password.
>>> 
>>> When user is allowed to use password auth along with OTP, the fact that
>>> there is no userPassword get ipa-pwd-extop plugin through the failure.
>>> schemacompat plugin rewrites the SLAPI_BIND_TARGET_SDN and the rest of
>>> 389-ds code checks actual password.
>>> 
>>> So we have two issues here: OTP code needs to gracefully ignore entries
>>> without userPassword set, and we need to be able to re-arrange
>>> schemacompat and ipa-pwd-extop precedence for pre-bind operation.
>>> 
>>> I've filed a ticket https://fedorahosted.org/389/ticket/47699 to work on
>>> the latter.
>>> 
>>> The messages from the log are not yet solved...
>> Finally, I have a clue after tracing with debug level 1:
>> [19/Feb/2014:22:57:10 +0200] - Calling plugin 'ipa-otp-lasttoken' #3 type 461
>> [19/Feb/2014:22:57:10 +0200] - slapi_search_internal_set_pb: NULL parameter
>> [19/Feb/2014:22:57:10 +0200] - allow_operation: component identity is NULL
>> [19/Feb/2014:22:57:10 +0200] - Calling plugin 'IPA pwd pre ops betxn' #4 type 461
>> 
>> So I'd say it is somewhere in ipa-otp-lasttoken. I'll dig more.
> There is an error in libotp's find() function which assumes that
> get_basedn() always returns non-NULL value. This is not true for at
> least cn=Directory Manager.
> 
> Patch attached.
More fixes required, now that Thierry produced the fix for 389-ds ticket
47699 which allows to re-arrange schema-compat and ipa-pwd-extop
plugins. I'm getting crash in find() in libotp.c for internal search in
some other conditions but at least user dn now is the correct one.

Stay tuned.

-- 
/ Alexander Bokovoy