On Thu, 20 Feb 2014, Alexander Bokovoy wrote:
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.
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.
OK, finally I've got it working -- my last patch had error which could
be attributed to the late night time.

New patch is attached to fix libotp to work properly with empty base dn
(such as cn=Directory Manager).

Also I'm attaching the patch that sets precedence of schema-compat
plugin to 49 (less than default 50). With this patch and 389-ds with
patch from ticket 47699 compat tree binds work with OTP.

When updated 389-ds-base will be released, we'll need to add Requires:
to our RPM spec to depend on it. Without the updated 389-ds-base compat
tree binds will not work with OTP but the rest will be working fine.

Finally, ACK to all OTP patches.
/ Alexander Bokovoy
>From de0c56f98b4558a591cc0d416815141c0cbdfbf3 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/17] 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..e6c8eaa 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;
+    const 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);

>From fa4e982f7c424bad9105b283cee34a1758fa6e9d Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Thu, 20 Feb 2014 12:18:16 +0200
Subject: [PATCH 17/17] schema-compat: set precedence to 49 to allow OTP binds
 over compat tree

schema-compat plugin rewrites bind DN to point to the original entry
on LDAP bind operation. To work with OTP tokens this requires that
schema-compat's pre-bind callback is called before pre-bind callback of
the ipa-pwd-extop plugin. Therefore, schema-compat plugin should have
a nsslapd-pluginprecedence value lower than (default) 50 which is used
by the ipa-pwd-extop plugin.

Note that this will only work if ticket 47699 is fixed in 389-ds.
 install/share/schema_compat.uldif       | 4 ++++
 install/updates/10-schema_compat.update | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/install/share/schema_compat.uldif 
index 40b9611..9a9607e 100644
--- a/install/share/schema_compat.uldif
+++ b/install/share/schema_compat.uldif
@@ -13,6 +13,10 @@ default:nsslapd-plugininitfunc: schema_compat_plugin_init
 default:nsslapd-plugintype: object
 default:nsslapd-pluginenabled: on
 default:nsslapd-pluginid: schema-compat-plugin
+# We need to run schema-compat pre-bind callback before
+# other IPA pre-bind callbacks to make sure bind DN is
+# rewritten to the original entry if needed
+default:nsslapd-pluginprecedence: 49
 default:nsslapd-pluginversion: 0.8
 default:nsslapd-pluginbetxn: on
 default:nsslapd-pluginvendor: redhat.com
diff --git a/install/updates/10-schema_compat.update 
index 1199ef3..505bfca 100644
--- a/install/updates/10-schema_compat.update
+++ b/install/updates/10-schema_compat.update
@@ -23,3 +23,10 @@ default:schema-compat-entry-attribute: 
 dn: cn=sudoers,cn=Schema Compatibility,cn=plugins,cn=config
 add:schema-compat-entry-attribute: sudoOrder=%{sudoOrder}
+dn: cn=Schema Compatibility,cn=plugins,cn=config
+# We need to run schema-compat pre-bind callback before
+# other IPA pre-bind callbacks to make sure bind DN is
+# rewritten to the original entry if needed
+add:nsslapd-pluginprecedence: 49

Freeipa-devel mailing list

Reply via email to