[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.

After discussing the details with nkinder, we have three options here.

1. FreeIPA implements all of the bind operation from 389ds in
ipa-pwd-extop. This is, I think, a bad idea.

2. 389DS needs to grow a new plugin type which hooks immediately after
the password validation completes, but before any changes are made to
the connection state/pblock.

3. Manually validate the password like we are doing now, but only for
the synchronization operation. In this case, if the password doesn't
match the normal bind process will fail and no synchronization will be
performed. If the password does match, it will be validated twice: once
for the synchronization operation and once for the bind. This could
potentially allow a user who is locked out, for instance, to synchronize
tokens. It would not, however, permit them to bind. Nor would it permit
brute-forcing.

Thoughts?

Nathaniel