[Freeipa-devel] [PATCH 0064] Create ipa-otp-decrement 389DS plugin

[thierry bordaz]

On 09/20/2014 09:39 PM, Nathaniel McCallum wrote:
> On Sat, 2014-09-20 at 00:25 +0200, thierry bordaz wrote:
>> Hello Nathaniel,
>>
>>          sanitize_input translates MOD/REPLACE into MOD/DEL+MOD/ADD. It
>>          looks good but difficult to think to all possible cases.
>>          I think to the following corner case:
>>          The initial entry has ipatokenHOTPcounter=5
>>          ldapmodify..
>>          changetype: modify
>>          add: ipatokenHOTPcounter
>>          ipatokenHOTPcounter: 6
>>          -
>>          replace: ipatokenHOTPcounter
>>          ipatokenHOTPcounter: 7
>>          
>>          It translates
>>          add: 6
>>          del: 5
>>          add: 7
>>          
>>          This operation will fail because ipatokenHOTPcounter is
>>          single-valued although IMHO it should succeed.
> No. It should fail. There can only ever be one ipatokenHOTPcounter.
>
>>          This is a so special operation that is may not really be a
>>          concern.
> +1
>
>>          It is important that attribute are single valued. The
>>          replication changelog will replicated MOD/DEL + MOD/ADD for a
>>          MOD/REPL.
>>          That means that if the attributes are updated on several
>>          masters, the number of values can likely increase. Where for
>>          single value it should only keep the most recent value.
> That is a concern, at least for now. Eventually we won't use replication
> for this at all. But for now, we will.
>
> Here is the problem I foresee. You have two servers: A and B.
>
> The user authenticates on A. This triggers a MOD/DEL(0)+MOD/ADD(1).
> Replication is sent to server B.
>
> Before the replication is performed on server B, the user authenticates
> with the next token. This triggers a MOD/DEL(0)+MOD/ADD(2). Replication
> is sent to server A. This replication will fail because A has a value of
> 1, not a value of 0.
>
> The end result is that there will be different values for
> ipatokenHOTPcounter on the two different servers. A will have 1 and B
> will have 2. Once this happens, the replications can never reconcile
> (this is a big problem). I see two options here, both theoretical.

The final value (when all updates has been replicated/applied) will rely 
on the timestamp of the operation (CSN). When a CSN is generated on a 
master it is guaranteed that it is greater (more recent) than any known 
CSNs.  In your scenario "nothing" can guaranty that the CSN of the 
second update (B) is larger than the CSN of the first update. If CSN(B) 
 > CSN(A), the final value will be '2' else it will be '1'.

When the server B will receive the first update ('1') its current value 
is '2'. So it will reject the update, that may break replication.
If we decide to not control replicated update and if CSN(A) > CSN(B), 
then the final value will go backward.


>
> Option #1 is to hook 389 in a different place: before the mods are
> performed by after the replication changelog is generated.
> Alternatively, we could insert a hook after the replication changelog is
> generated, but before it is sent. We could consolidate the MOD/DEL
> +MOD/ADD here into a single MOD/REPLACE operation.
With current replication it would be major change. Update of the 
changelog is done in betxn_postop based on the operations mods.
>
> Option #2 is to have some way to translate the MOD/REPLACE(X) into a
> MOD/DEL(<=X)+MOD/ADD(X).
I think it is already what the plugin is doing. It retrieves the current 
value from the entry and do a mod/del on this value then a mod/add of 
the new value. (at the condition new_value>current_value).
My concern is that in your scenario, due to parallel update with several 
new_values we may select an incorrect new_value.

The 'Dynamic Master proxy' ( 
http://www.freeipa.org/page/V4/OTP_Replay_Prevention#Replication_Counter_Race) 
solution looks as a good one.

>
> Are either of these possible? Is there another option?
>
> Nathaniel
>
>