On 02/01/2012 02:48 PM, Mark Reynolds wrote:
On 02/01/2012 04:28 PM, Rich Megginson wrote:
On 02/01/2012 02:16 PM, Mark Reynolds wrote:
Hi Everyone,
There is an issue with the PAM plugin, that when it performs a
successful bind we actually return error 1 to plugins_call_func(),
which essentially causes the abort of the all plugin processing:
the rest of pre-op, the backend call, and all of post-op. PAM has
completed the bind and already returned the result, so it returns 1
to stop the DS from doing the rest of bind op. Makes sense...
Where is the code that calls the preop?
However, with the Account Policy plugin, when tracking the "last
bind time", binding thru PAM won't update the entry, even though the
bind was successful. This is because the successful PAM bind
essentially aborted all the pre-op and post-op plugins. I feel that
we should still call the post op plugins in this scenario. The
pre-op plugins should still be aborted, because the operation was
already completed - there's nothing to reject at that point.
So to get around plugins like this, I am proposing a new plugin
pre-op return code(either use 1, or -2). This return code implies
that the operation was fully completed, and that we should also
process the post op plugins - but do not send the operation to the
backend.
So to the recap, the new return code says the operation was fully
completed by the plugin, but we still want to process just the
post-op plugins.
I know this will impact documentation, there might be unforeseen
issues, and this is also a rare situation(only PAM plugin seems to
behave like this). Saying that, I still think its the valid
solution to this type of problem. It could also allow future
plugins for be more versatile/robust.
Please let me know your thoughts.
I don't think we need a new return code - I just think we need to
diff --git a/ldap/servers/slapd/bind.c b/ldap/servers/slapd/bind.c
index 1d860b6..bd7df3d 100644
--- a/ldap/servers/slapd/bind.c
+++ b/ldap/servers/slapd/bind.c
@@ -796,6 +796,10 @@ do_bind( Slapi_PBlock *pb )
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &rc );
plugin_call_plugins( pb, SLAPI_PLUGIN_POST_BIND_FN );
+ } else {
+ /* even if the pre-op plugins returned an error, we
still need
+ to call the post-op plugins */
+ plugin_call_plugins( pb, SLAPI_PLUGIN_POST_BIND_FN );
}
} else {
send_ldap_result( pb, LDAP_UNWILLING_TO_PERFORM, NULL,
I thought about this, but I thought we needed a new code because isn't
it by design: if pre-op fails, it should all fail?
I don't think so. In every other case in bind.c, when something fails,
the POST_BIND functions are called. I don't see why this particular
case should be any different. IMHO a POST_BIND plugin will always want
to know if the bind succeeded or failed for any reason. Note that IPA
has a POST_BIND plugin - ipa_lockout - which will want to be called in
this case.
PAM really isn't failing, it just doesn't have a better mechanism to
handle its behaviour.
The above code would work fine, so maybe this could be a discussion
for a later time. I was just looking for a more "global" solution.
So, I will use this approach for now so I can get the bug resolved.
If there's time in the future maybe we can come back to this. Like I
said it's not something that is needed, but it would allow for more
creative plugins :-)
Thanks,
Mark
Looking at bind.c - there is a lot of special case code that makes
sure to call the POST_BIND plugins in various error cases. This
seems like just another error case that we should handle by calling
the POST_BIND plugins.
Thanks,
Mark
--
389-devel mailing list
389-de...@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel
--
389-devel mailing list
389-de...@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel
