On Thu, 23 Jan 2014, Nathaniel McCallum wrote:
On Thu, 2014-01-23 at 14:23 -0500, Simo Sorce wrote:
On Thu, 2014-01-09 at 16:28 -0500, Nathaniel McCallum wrote:
> This plugin adds an extended operation for synchronizing tokens. This
> operation is availalbe both with and without bind. In the latter case,
> the first factor is required. This operation can also be performed
> on a per-token or per-user level. In the latter case, we will attempt
> to find the token automatically.
>
> Thanks to Mark Reynolds for helping me with this patch.


Huge NACK,

otp_sync_request_authenticate() is not ok, it allows an attacker to do
password bruteforcing without any check.

This is the result of trying to re-implement what is basically a bind
but w/o using the ind code.

No checks to see if the user is enabled/disabled/locked
No updates for locking account after N wrong tries, etc...

It also duplicates the places to change should we wish to not depend on
having userPassword as an actual password field anymore (which I
considered multiple times as we already have kerberos keys against which
we could test binds in theory).

You should either extend the bind operation so all checks are used there
and re-sync is done at last if all checks pass, and as a bonus you end
up with an authenticated LDAP connection should you need it, or the bind
code that does all the checks need to be abstracted into helper
functions that can be reused by this plugin.

Simo.

Is it possible to do a post-bind extop which rejects the bind? Because
if synchronization fails, we MUST reject the authentication even if the
first factor was validated correctly. Also, is it possible to add
extended data to a bind operation? Because if not, this is going to get
hairy fast.
Extended data on the server side? You can set what you want in PB in
pre-bind and pick it up in post-bind.

However, the question is about successful binds -- when SIMPLE bind is
successful DS code will send response to the client and will call post
plugins. The response is sent unconditionally.

May be we could add a condition that if a certain flag is set in PB
after pre-bind plugins ran successfully, response is not sent and
post-bind plugins then run. A post-bind plugin then is required to
send out proper result.
--
/ Alexander Bokovoy

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to