Drop h and H from randstr, people should use %{hex:%{randstr:b}} instead

- They were only there because the xlat evaluation code wasn't binary safe.  Now it is, there's no reason to keep them.
- Make generating large amounts of random binary data more efficient
- Increase repetition limit to 1024 and warn if we override what the user specified

diff --git a/src/lib/server/xlat_func.c b/src/lib/server/xlat_func.c
index c039d1d82d5aee34bf1d4e85cd69d7bad28b02cb..7f37b434c39506b15e9f322f73230fd1b384a567 100644 (file)
--- a/src/lib/server/xlat_func.c
+++ b/src/lib/server/xlat_func.c
@@ -821,10 +821,13 @@ static xlat_action_t xlat_func_randstr(TALLOC_CTX *ctx, fr_cursor_t *out,
        char            *endptr;
        char            *buff, *buff_p;
        unsigned int    result;
-       unsigned int    number;
+       unsigned int    reps;
        size_t          outlen = 0;
        fr_value_box_t* vb;
 
+       /** Max repetitions of a single character class
+        *
+        */
 #define REPETITION_MAX 1024
 
        /*
@@ -847,12 +850,17 @@ static xlat_action_t xlat_func_randstr(TALLOC_CTX *ctx, fr_cursor_t *out,
         *      Calculate size of output
         */
        while (p < end) {
+               /*
+                *      Repetition modifiers.
+                *
+                *      We limit it to REPETITION_MAX, because we don't want
+                *      utter stupidity.
+                */
                if (isdigit((int) *p)) {
-                       number = strtol(p, &endptr, 10);
-                       if (number > REPETITION_MAX) number = REPETITION_MAX;
+                       reps = strtol(p, &endptr, 10);
+                       if (reps > REPETITION_MAX) reps = REPETITION_MAX;
                        /* hexits take up 2 characters */
-                       if (*endptr == 'h' || *endptr == 'H') number *= 2;
-                       outlen += number;
+                       outlen += reps;
                        p = endptr;
                } else {
                        outlen++;
@@ -866,114 +874,99 @@ static xlat_action_t xlat_func_randstr(TALLOC_CTX *ctx, fr_cursor_t *out,
        p = start;
 
        while (p < end) {
-               number = 0;
+               size_t i;
 
-               /*
-                *      Modifiers are polite.
-                *
-                *      But we limit it to 100, because we don't want
-                *      utter stupidity.
-                */
                if (isdigit((int) *p)) {
-                       number = strtol(p, &endptr, 10);
-                       if (number > REPETITION_MAX) {
-                               number = REPETITION_MAX;
+                       reps = strtol(p, &endptr, 10);
+                       if (reps > REPETITION_MAX) {
+                               reps = REPETITION_MAX;
                                RMARKER(L_WARN, L_DBG_LVL_2, start, start - p,
                                        "Forcing repetition to %u", (unsigned int)REPETITION_MAX);
                        }
                        p = endptr;
+               } else {
+                       reps = 1;
                }
 
-       redo:
-               result = fr_rand();
-
-               switch (*p) {
-               /*
-                *  Lowercase letters
-                */
-               case 'c':
-                       *buff_p++ = 'a' + (result % 26);
-                       break;
-
-               /*
-                *  Uppercase letters
-                */
-               case 'C':
-                       *buff_p++ = 'A' + (result % 26);
-                       break;
-
-               /*
-                *  Numbers
-                */
-               case 'n':
-                       *buff_p++ = '0' + (result % 10);
-                       break;
-
-               /*
-                *  Alpha numeric
-                */
-               case 'a':
-                       *buff_p++ = randstr_salt[result % (sizeof(randstr_salt) - 3)];
-                       break;
+               for (i = 0; i < reps; i++) {
+                       result = fr_rand();
+                       switch (*p) {
+                       /*
+                        *  Lowercase letters
+                        */
+                       case 'c':
+                               *buff_p++ = 'a' + (result % 26);
+                               break;
 
-               /*
-                *  Punctuation
-                */
-               case '!':
-                       *buff_p++ = randstr_punc[result % (sizeof(randstr_punc) - 1)];
-                       break;
+                       /*
+                        *  Uppercase letters
+                        */
+                       case 'C':
+                               *buff_p++ = 'A' + (result % 26);
+                               break;
 
-               /*
-                *  Alpa numeric + punctuation
-                */
-               case '.':
-                       *buff_p++ = '!' + (result % 95);
-                       break;
+                       /*
+                        *  Numbers
+                        */
+                       case 'n':
+                               *buff_p++ = '0' + (result % 10);
+                               break;
 
-               /*
-                *  Alpha numeric + salt chars './'
-                */
-               case 's':
-                       *buff_p++ = randstr_salt[result % (sizeof(randstr_salt) - 1)];
-                       break;
+                       /*
+                        *  Alpha numeric
+                        */
+                       case 'a':
+                               *buff_p++ = randstr_salt[result % (sizeof(randstr_salt) - 3)];
+                               break;
 
-               /*
-                *  Chars suitable for One Time Password tokens.
-                *  Alpha numeric with easily confused char pairs removed.
-                */
-               case 'o':
-                       *buff_p++ = randstr_otp[result % (sizeof(randstr_otp) - 1)];
-                       break;
+                       /*
+                        *  Punctuation
+                        */
+                       case '!':
+                               *buff_p++ = randstr_punc[result % (sizeof(randstr_punc) - 1)];
+                               break;
 
-               /*
-                *  Binary data as hexits (we don't really support
-                *  non printable chars).
-                */
-               case 'h':
-                       snprintf(buff_p, 3, "%02x", result % 256);
+                       /*
+                        *  Alpa numeric + punctuation
+                        */
+                       case '.':
+                               *buff_p++ = '!' + (result % 95);
+                               break;
 
-                       buff_p += 2;
-                       break;
+                       /*
+                        *  Alpha numeric + salt chars './'
+                        */
+                       case 's':
+                               *buff_p++ = randstr_salt[result % (sizeof(randstr_salt) - 1)];
+                               break;
 
-               /*
-                *  Binary data with uppercase hexits
-                */
-               case 'H':
-                       snprintf(buff_p, 3, "%02X", result % 256);
+                       /*
+                        *  Chars suitable for One Time Password tokens.
+                        *  Alpha numeric with easily confused char pairs removed.
+                        */
+                       case 'o':
+                               *buff_p++ = randstr_otp[result % (sizeof(randstr_otp) - 1)];
+                               break;
 
-                       buff_p += 2;
-                       break;
+                       /*
+                        *      Binary data - Copy between 1-4 bytes at a time
+                        */
+                       case 'b':
+                       {
+                               size_t copy = (reps - i) > sizeof(result) ? sizeof(result) : reps - i;
 
-               default:
-                       REDEBUG("Invalid character class '%c'", *p);
-                       talloc_free(buff);
+                               memcpy(buff_p, (uint8_t *)&result, copy);
+                               buff_p += copy;
+                               i += (copy - 1);        /* Loop +1 */
+                       }
+                               break;
 
-                       return XLAT_ACTION_FAIL;
-               }
+                       default:
+                               REDEBUG("Invalid character class '%c'", *p);
+                               talloc_free(buff);
 
-               if (number > 1) {
-                       number--;
-                       goto redo;
+                               return XLAT_ACTION_FAIL;
+                       }
                }
 
                p++;

diff --git a/src/tests/keywords/randstr b/src/tests/keywords/randstr
index f7742508292258d8ba4c4890fc0813a95dc61bbb..8e400000cb4ea64793fd14cbed476081e0063629 100644 (file)
--- a/src/tests/keywords/randstr
+++ b/src/tests/keywords/randstr
@@ -3,9 +3,9 @@ update request {
         &Tmp-String-0           := "%{randstr:%{Tmp-String-0}}"
         &Tmp-String-1           := "%{randstr:nnn}"
         &Tmp-String-2           := "%{randstr:24a}"
-        &Tmp-String-3           := "%{randstr:123aa}"
-        &Tmp-String-4           := "%{randstr:123G}"
-        &Tmp-String-5           := "%{randstr:10H}"
+        &Tmp-String-3           := "%{randstr:1030aa}"
+        &Tmp-String-4           := "%{randstr:G}"
+        &Tmp-String-5           := "%{randstr:10b}"
 }
 
 #
@@ -28,7 +28,7 @@ if ("%{strlen:%{Tmp-String-2}}" != 24) {
 #
 #  Check maximum repitition
 #
-if ("%{strlen:%{Tmp-String-3}}" != 101) {
+if ("%{strlen:%{Tmp-String-3}}" != 1025) {
        test_fail
 }
 
@@ -40,9 +40,9 @@ if (&Tmp-String-4 != "") {
 }
 
 #
-#  Check repitition of hexit
+#  Check repitition of binary output
 #
-if ("%{strlen:%{Tmp-String-5}}" != 20) {
+if ("%{strlen:%{Tmp-String-5}}" != 10) {
        test_fail
 }