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

[Alexander Bokovoy]

On Thu, 23 Jan 2014, 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
BIND operation is 
   BindRequest ::= SEQUENCE {                                                         
       version     INTEGER,         -- version
       name        DistinguishedName,   -- dn
       authentication  CHOICE {
           simple      [0] OCTET STRING, -- passwd
           krbv42ldap  [1] OCTET STRING, -- not used
           krbv42dsa   [2] OCTET STRING, -- not used
           sasl        [3] SaslCredentials -- v3 only
       }
   }

And there could be messge controls attached.

So, we have a choice of 

- embedding this information into passwd in LDAP_AUTH_SIMPLE and then
   announcing a special password scheme plugin that would split the
   password and sync codes.
- embedding it in SASL mech name
- attaching a control

I'd suggest using a control, pretty much like we already do with
extop/extdom. Checking the control is more or less trivial. 

>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?
Yes, in LDAPv3 which is what we are supporting here anyway.


-- 
/ Alexander Bokovoy