This prevents any local attempt at rapid token code replay. If two
token codes hit the system at roughly the same moment, only the
first write will succeed. All subsequent authentications will fail.

This obviates the need for an OTP authentication lock.

https://fedorahosted.org/freeipa/ticket/4493
From a414a5873e47880328ef578c3ea5ff078e096cd5 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Thu, 28 Aug 2014 22:45:18 -0400
Subject: [PATCH] Use delete/add for OTP counter/watermark updates

This prevents any local attempt at rapid token code replay. If two
token codes hit the system at roughly the same moment, only the
first write will succeed. All subsequent authentications will fail.

This obviates the need for an OTP authentication lock.

https://fedorahosted.org/freeipa/ticket/4493
---
 daemons/ipa-slapi-plugins/libotp/libotp.c | 67 +++++++++++++++++--------------
 1 file changed, 36 insertions(+), 31 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/libotp/libotp.c b/daemons/ipa-slapi-plugins/libotp/libotp.c
index 41f9e7b4809fbca82452d260b9aa7d1d3059fd2e..d41fc2886987b0ab2d69bcb1bbb9986d6935774e 100644
--- a/daemons/ipa-slapi-plugins/libotp/libotp.c
+++ b/daemons/ipa-slapi-plugins/libotp/libotp.c
@@ -124,31 +124,39 @@ static const struct berval *entry_attr_get_berval(const Slapi_Entry* e,
     return slapi_value_get_berval(v);
 }
 
+/**
+ * Changes an attribute from oldval to newval.
+ * If oldval has changed on disk since it was read, fail.
+ */
 static bool writeattr(const struct otptoken *token, const char *attr,
-                      int value)
+                      long long oldval, long long newval)
 {
-    Slapi_Value *svals[] = { NULL, NULL };
     Slapi_PBlock *pb = NULL;
-    Slapi_Mods *mods = NULL;
     bool success = false;
+    char oldvalue[32];
+    char newvalue[32];
     int ret;
 
-    /* Create the value. */
-    svals[0] = slapi_value_new();
-    if (slapi_value_set_int(svals[0], value) != 0) {
-        slapi_value_free(&svals[0]);
-        return false;
-    }
+    LDAPMod *mods[] = {
+        &(LDAPMod) {
+            LDAP_MOD_DELETE, (char *) attr,
+            .mod_values = (char *[]) { oldvalue, NULL }
+        },
+        &(LDAPMod) {
+            LDAP_MOD_ADD, (char *) attr,
+            .mod_values = (char *[]) { newvalue, NULL }
+        },
+        NULL
+    };
 
-    /* Create the mods. */
-    mods = slapi_mods_new();
-    slapi_mods_add_mod_values(mods, LDAP_MOD_REPLACE, attr, svals);
+    /* Convert the values to strings. */
+    snprintf(oldvalue, sizeof(oldvalue), "%lld", oldval);
+    snprintf(newvalue, sizeof(newvalue), "%lld", newval);
 
     /* Perform the modification. */
     pb = slapi_pblock_new();
     slapi_modify_internal_set_pb(pb, slapi_sdn_get_dn(token->sdn),
-                                 slapi_mods_get_ldapmods_byref(mods),
-                                 NULL, NULL, token->plugin_id, 0);
+                                 mods, NULL, NULL, token->plugin_id, 0);
     if (slapi_modify_internal_pb(pb) != 0)
         goto error;
     if (slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, &ret) != 0)
@@ -160,9 +168,7 @@ static bool writeattr(const struct otptoken *token, const char *attr,
 
 error:
     slapi_pblock_destroy(pb);
-    slapi_mods_free(&mods);
     return success;
-
 }
 
 /**
@@ -174,12 +180,14 @@ static bool validate(struct otptoken *token, time_t now, ssize_t step,
                      uint32_t first, const uint32_t *second)
 {
     const char *attr;
+    long long last;
     uint32_t tmp;
 
     /* Calculate the absolute step. */
     switch (token->type) {
     case OTPTOKEN_TOTP:
         attr = T("watermark");
+        last = token->totp.watermark;
         step = (now + token->totp.offset) / token->totp.step + step;
         if (token->totp.watermark > 0 && step < token->totp.watermark)
             return false;
@@ -188,6 +196,7 @@ static bool validate(struct otptoken *token, time_t now, ssize_t step,
         if (step < 0) /* NEVER go backwards! */
             return false;
         attr = H("counter");
+        last = token->hotp.counter;
         step = token->hotp.counter + step;
         break;
     default:
@@ -208,26 +217,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, last, 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))
+            if (!writeattr(token, T("clockOffset"), token->totp.offset, 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

Reply via email to