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

[Simo Sorce]

On Thu, 2014-01-23 at 15:57 -0500, Nathaniel McCallum wrote:
> On Thu, 2014-01-23 at 15:46 -0500, Nathaniel McCallum wrote:
> > On Thu, 2014-01-23 at 22:12 +0200, Alexander Bokovoy wrote:
> > > 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.
> > 
> > No, extended data from the client side. The client needs to somehow
> > communicate to the server:
> > 1. that it wants the server to resynchronize
> > 2. two tokens codes in a row
> > 
> > Ideally we need some sort of additional metadata in the bind request.
> > Without this, doing all the tests are pretty much impossible. We could
> > refactor the code in ipa-pwd-extop into a library, but ipa-pwd-extop
> > doesn't contain all the tests. Some of them are built into 389ds. The
> > ipa-pwd-extop plugin just performs additional work and then falls
> > through to 389ds.
> > 
> > What we need to do is this:
> > 
> > bool bind_preop() {
> >     if (otp_configured() && !is_sync_request())
> >         if (!validate_otp())
> >             return false;
> > 
> >     // ... fall through to password validation ...
> > }
> > 
> > bool bind_postop() {
> >     return bind_successful()
> >            && (!is_sync_request()
> >                || perform_synchronization());
> > }
> > 
> > At the least, however, we need extra metadata for is_sync_request().
> > 
> > So the question is: is it possible to get some extra metadata in a bind
> > request?
> 
> So from my initial reading it appears we can just embed the sync request
> in a control, is that correct?

Yes, controls are made exactly for this kind of purpose.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York