[Freeipa-devel] [PATCH 0030] Add OTP sync plugin

[Nathaniel McCallum]

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.

Nathaniel