[Freeipa-devel] [PATCH 0068] Move OTP synchronization step to after counter writeback

[Petr Viktorin]

On 09/30/2014 02:53 PM, thierry bordaz wrote:
> On 09/30/2014 02:41 PM, Nathaniel McCallum wrote:
>> On Tue, 2014-09-30 at 13:42 +0200, thierry bordaz wrote:
>>> On 09/29/2014 08:38 PM, Nathaniel McCallum wrote:
>>>> On Thu, 2014-09-25 at 15:15 +0200, thierry bordaz wrote:
>>>>> On 09/19/2014 07:53 PM, Nathaniel McCallum wrote:
>>>>>
>>>>>> This prevents synchronization when an authentication collision occurs.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/4493
>>>>>>
>>>>>> NOTE: this patch is related to the above ticket, but does not solve it.
>>>>>> For the solution, please see patch 0064. This behavior fix is from patch
>>>>>> 0062 (rescinded) and is worth keeping.
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Freeipa-devel mailing list
>>>>>> Freeipa-devel at redhat.com
>>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>> Hello Nathaniel,
>>>>> .
>>>>>           My understanding is that during a pre_bind, the plugins
>>>>>           validates token codes (for example "HOTP") checking that step
>>>>>           ranges [-25..+25].
>>>>>           As soon as the token is valid, the new HOTPcounter is written
>>>>>           in the entry.
>>>>>           But in case of negative steps,I believe the otp-decrement
>>>>>           plugin will reject this update.
>>>> HOTP never goes backwards. See line 188 in libotp.c. Even if this check
>>>> weren't present, we would *want* the decrement plugin to reject the
>>>> update.
>>> Ok.
>>>>>           If TOTPwatermark is updated and there is a second token code,
>>>>>           then clockOffset is also updated.
>>>>>           This update is done during a pre_bind, so if there are
>>>>>           parallel binds on the server, there is a possibility that
>>>>>           TOTPwatermark is updated from a bind and 'clockOffset' updated
>>>>>           from an other bind.
>>>>>           An option is to do a single internal modify to update both.
>>>> We don't care about atomicity in this case. If two TOTP synchronizations
>>>> occur at almost the same time, the value of clockOffset will be written
>>>> twice with the same value. Since the values are the same, we don't care
>>>> which value gets written first.
>>> I was just wondering if parallel binds with different 'step' values
>>> could occur.
>>> Because different 'step' values can lead to different 'clockOffset'.
>> It is possible, but this isn't a concern. This code path is reached only
>> when synchronization is being performed (not regular binds). This means
>> that the clockOffset value is currently wildly incorrect. We are trying
>> to move this value to something roughly correct. For various reasons, we
>> can't get this value exactly. So if parallel authentication has
>> succeeded, either of the resulting step values is correct.
>>
> Thanks Nathaniel for the explanations.
> The fix is fine for me and for me it is a ACK.
>
> thanks
> thierry
>

Pushed to:
master: 915837c14af5f0839d1d08683ea8332334e284ba
ipa-4-1: 98debb7fb1b1e998d48806702ad4d950b6dd9f23



-- 
Petr³