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

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:

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 

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.
/ Alexander Bokovoy
>From c91c69fb05f5411ce2a583fc4678ce10cb31e894 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Wed, 19 Feb 2014 23:24:29 +0200
Subject: [PATCH 16/16] libotp: do not call internal search for NULL dn

 daemons/ipa-slapi-plugins/libotp/libotp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-slapi-plugins/libotp/libotp.c 
index 31cc591..e7119f0 100644
--- a/daemons/ipa-slapi-plugins/libotp/libotp.c
+++ b/daemons/ipa-slapi-plugins/libotp/libotp.c
@@ -332,6 +332,7 @@ static struct otptoken **find(Slapi_ComponentId *id, const 
char *user_dn,
     Slapi_PBlock *pb = NULL;
     Slapi_DN *sdn = NULL;
     char *filter = NULL;
+    char basedn = NULL;
     size_t count = 0;
     int result = -1;
@@ -362,8 +363,12 @@ static struct otptoken **find(Slapi_ComponentId *id, const 
char *user_dn,
         if (sdn == NULL)
             goto error;
+        basedn = get_basedn(sdn);
+        if (basedn == NULL)
+            goto error;
         /* Find all user tokens. */
-        slapi_search_internal_set_pb(pb, get_basedn(sdn),
+        slapi_search_internal_set_pb(pb, basedn,
                                      LDAP_SCOPE_SUBTREE, filter, NULL,
                                      0, NULL, NULL, id, 0);

Freeipa-devel mailing list

Reply via email to