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

[thierry bordaz]

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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/0a910926/attachment.htm>