Change OTPSyncRequest structure to use OctetString

This change has two motivations:
  1. Clients don't have to parse the string.
  2. Future token types may have new formats.

Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

5 files changed, 79 insertions, 56 deletions

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
index 23c7cb18c..60ceaaa7a 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
@@ -1157,8 +1157,8 @@ static bool ipapwd_do_otp_auth(const char *dn, Slapi_Entry *bind_entry,
     /* Loop through each token. */
     for (int i = 0; tokens[i] && !success; i++) {
         /* Attempt authentication. */
-        success = otptoken_validate_string(tokens[i], OTP_VALIDATE_STEPS,
-                                           creds->bv_val, creds->bv_len, true);
+        success = otptoken_validate_berval(tokens[i], OTP_VALIDATE_STEPS,
+                                           creds, true);
 
         /* Truncate the password to remove the OTP code at the end. */
         if (success) {
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c
index 27878776f..2bfcf10a2 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c
@@ -58,10 +58,11 @@ bool sync_request_handle(Slapi_ComponentId *plugin_id, Slapi_PBlock *pb,
 {
     struct otptoken **tokens = NULL;
     LDAPControl **controls = NULL;
+    struct berval *second = NULL;
+    struct berval *first = NULL;
     BerElement *ber = NULL;
     char *token_dn = NULL;
-    int second = 0;
-    int first = 0;
+    bool success;
 
     if (slapi_pblock_get(pb, SLAPI_REQCONTROLS, &controls) != 0)
         return false;
@@ -79,32 +80,30 @@ bool sync_request_handle(Slapi_ComponentId *plugin_id, Slapi_PBlock *pb,
             return false;
 
         /* Decode the token codes. */
-        if (ber_scanf(ber, "{ii", &first, &second) == LBER_ERROR) {
+        if (ber_scanf(ber, "{OO", &first, &second) == LBER_ERROR) {
             ber_free(ber, 1);
             return false;
         }
 
         /* Decode the optional token DN. */
         ber_scanf(ber, "a", &token_dn);
-        if (ber_scanf(ber, "}") == LBER_ERROR) {
-            ber_free(ber, 1);
-            return false;
-        }
-        ber_free(ber, 1);
 
-        /* Find all the tokens. */
-        tokens = otptoken_find(plugin_id, user_dn, token_dn, true, NULL);
-        ber_memfree(token_dn);
-        if (tokens == NULL)
-            return false;
-
-        /* Synchronize the token. */
-        if (!otptoken_sync(tokens, OTP_SYNC_MAX_STEPS, first, second)) {
-            otptoken_free_array(tokens);
-            return false;
+        /* Process the synchronization. */
+        success = false;
+        if (ber_scanf(ber, "}") != LBER_ERROR) {
+            tokens = otptoken_find(plugin_id, user_dn, token_dn, true, NULL);
+            if (tokens != NULL) {
+                success = otptoken_sync_berval(tokens, OTP_SYNC_MAX_STEPS, first, second);
+                otptoken_free_array(tokens);
+            }
         }
 
-        otptoken_free_array(tokens);
+        ber_memfree(token_dn); token_dn = NULL;
+        ber_bvfree(second);
+        ber_bvfree(first);
+        ber_free(ber, 1);
+        if (!success)
+            return false;
     }
 
     return true;
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.h b/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.h
index 049a62102..34235901b 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.h
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.h
@@ -48,8 +48,8 @@
  * The ASN.1 encoding of the request structure:
  *
  *     OTPSyncRequest ::= SEQUENCE {
- *         firstCode   INTEGER,
- *         secondCode  INTEGER,
+ *         firstCode   OCTET STRING,
+ *         secondCode  OCTET STRING,
  *         tokenDN     OCTET STRING OPTIONAL
  *     }
  */
diff --git a/daemons/ipa-slapi-plugins/libotp/libotp.c b/daemons/ipa-slapi-plugins/libotp/libotp.c
index e6c8eaade..eeead43e3 100644
--- a/daemons/ipa-slapi-plugins/libotp/libotp.c
+++ b/daemons/ipa-slapi-plugins/libotp/libotp.c
@@ -449,7 +449,8 @@ const Slapi_DN *otptoken_get_sdn(struct otptoken *token)
     return token->sdn;
 }
 
-bool otptoken_validate(struct otptoken *token, size_t steps, uint32_t code)
+static bool otptoken_validate(struct otptoken *token, size_t steps,
+                              uint32_t code)
 {
     time_t now = 0;
 
@@ -477,44 +478,53 @@ bool otptoken_validate(struct otptoken *token, size_t steps, uint32_t code)
     return false;
 }
 
-bool otptoken_validate_string(struct otptoken *token, size_t steps,
-                              const char *code, ssize_t len, bool tail)
+
+/*
+ *  Convert code berval to decimal.
+ *
+ *  NOTE: We can't use atol() or strtoul() because:
+ *    1. If we have leading zeros, atol() fails.
+ *    2. Neither support limiting conversion by length.
+ */
+static bool bvtod(const struct berval *code, uint32_t *out)
 {
+    *out = 0;
+
+    for (ber_len_t i = 0; i < code->bv_len; i++) {
+        if (code->bv_val[i] < '0' || code->bv_val[i] > '9')
+            return false;
+        *out *= 10;
+        *out += code->bv_val[i] - '0';
+    }
+
+    return code->bv_len != 0;
+}
+
+bool otptoken_validate_berval(struct otptoken *token, size_t steps,
+                              const struct berval *code, bool tail)
+{
+    struct berval tmp;
     uint32_t otp;
 
     if (token == NULL || code == NULL)
         return false;
+    tmp = *code;
 
-    if (len < 0)
-        len = strlen(code);
-
-    if (len < token->token.digits)
+    if (tmp.bv_len < token->token.digits)
         return false;
 
     if (tail)
-        code = &code[len - token->token.digits];
-    len = token->token.digits;
-
-    /*
-     *  Convert code string to decimal.
-     *
-     *  NOTE: We can't use atol() or strtoul() because:
-     *  1. We may have leading zeros (atol() fails here).
-     *  2. Neither support limiting conversion by length.
-     */
-    otp = 0;
-    for (ssize_t i = 0; i < len; i++) {
-        if (code[i] < '0' || code[i] > '9')
-            return false;
-        otp *= 10;
-        otp += code[i] - '0';
-    }
+        tmp.bv_val = &tmp.bv_val[tmp.bv_len - token->token.digits];
+    tmp.bv_len = token->token.digits;
+
+    if (!bvtod(&tmp, &otp))
+        return false;
 
     return otptoken_validate(token, steps, otp);
 }
 
-bool otptoken_sync(struct otptoken * const *tokens, size_t steps,
-                   uint32_t first_code, uint32_t second_code)
+static bool otptoken_sync(struct otptoken * const *tokens, size_t steps,
+                          uint32_t first_code, uint32_t second_code)
 {
     time_t now = 0;
 
@@ -542,3 +552,19 @@ bool otptoken_sync(struct otptoken * const *tokens, size_t steps,
 
     return false;
 }
+
+bool otptoken_sync_berval(struct otptoken * const *tokens, size_t steps,
+                          const struct berval *first_code,
+                          const struct berval *second_code)
+{
+    uint32_t second = 0;
+    uint32_t first = 0;
+
+    if (!bvtod(first_code, &first))
+        return false;
+
+    if (!bvtod(second_code, &second))
+        return false;
+
+    return otptoken_sync(tokens, steps, first, second);
+}
diff --git a/daemons/ipa-slapi-plugins/libotp/libotp.h b/daemons/ipa-slapi-plugins/libotp/libotp.h
index 7ed729337..24915f866 100644
--- a/daemons/ipa-slapi-plugins/libotp/libotp.h
+++ b/daemons/ipa-slapi-plugins/libotp/libotp.h
@@ -80,16 +80,14 @@ int otptoken_get_digits(struct otptoken *token);
 /* Get the SDN of the token. */
 const Slapi_DN *otptoken_get_sdn(struct otptoken *token);
 
-/* Validate the token code within a range of steps. */
-bool otptoken_validate(struct otptoken *token, size_t steps, uint32_t code);
-
 /* Validate the token code within a range of steps. If tail is true,
  * it will be assumed that the token is specified at the end of the string. */
-bool otptoken_validate_string(struct otptoken *token, size_t steps,
-                       const char *code, ssize_t len, bool tail);
+bool otptoken_validate_berval(struct otptoken *token, size_t steps,
+                              const struct berval *code, bool tail);
 
 /* Synchronize the token within a range of steps. */
-bool otptoken_sync(struct otptoken * const *tokens, size_t steps,
-                   uint32_t first_code, uint32_t second_code);
+bool otptoken_sync_berval(struct otptoken * const *tokens, size_t steps,
+                          const struct berval *first_code,
+                          const struct berval *second_code);
 
 #endif /* LIBOTP_H_ */