Re: [Freeipa-devel] [PATCH 0068] Move OTP synchronization step to after counter writeback
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@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'. thanks thierry Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0068] Move OTP synchronization step to after counter writeback
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@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. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0068] Move OTP synchronization step to after counter writeback
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@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 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0068] Move OTP synchronization step to after counter writeback
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@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³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0068] Move OTP synchronization step to after counter writeback
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@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. 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. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0068] Move OTP synchronization step to after counter writeback
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@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. 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. thanks thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0068] Move OTP synchronization step to after counter writeback
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.
From 4a044ebd32995d6d9792e6e5c5179748b8b7ee90 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Fri, 19 Sep 2014 12:18:34 -0400
Subject: [PATCH] Move OTP synchronization step to after counter writeback
This prevents synchronization when an authentication collision occurs.
https://fedorahosted.org/freeipa/ticket/4493
---
daemons/ipa-slapi-plugins/libotp/libotp.c | 24 ++--
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/libotp/libotp.c b/daemons/ipa-slapi-plugins/libotp/libotp.c
index 41f9e7b4809fbca82452d260b9aa7d1d3059fd2e..083190c1effe84b62f35ccebef6d3ebeda07be5f 100644
--- a/daemons/ipa-slapi-plugins/libotp/libotp.c
+++ b/daemons/ipa-slapi-plugins/libotp/libotp.c
@@ -208,26 +208,22 @@ static bool validate(struct otptoken *token, time_t now, ssize_t step,
if (*second != tmp)
return false;
+}
+/* Write the step value. */
+if (!writeattr(token, attr, step))
+return false;
+
+/* Save our modifications to the object. */
+switch (token-type) {
+case OTPTOKEN_TOTP:
/* Perform optional synchronization steps. */
-switch (token-type) {
-case OTPTOKEN_TOTP:
+if (second != NULL) {
tmp = (step - now / token-totp.step) * token-totp.step;
if (!writeattr(token, T(clockOffset), tmp))
return false;
-break;
-default:
-break;
+token-totp.offset = tmp;
}
-}
-
-/* Write the step value. */
-if (!writeattr(token, attr, step))
-return false;
-
-/* Save our modifications to the object. */
-switch (token-type) {
-case OTPTOKEN_TOTP:
token-totp.watermark = step;
break;
case OTPTOKEN_HOTP:
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
