kadmin: generate stronger random passwords

Generate 12 random bytes and encode them with Heimdal base64url, producing
16-character random passwords with 96 bits of entropy.

This avoids punctuation and quoting issues while still raising generated
password entropy substantially from the old roughly 55-bit generator.

Fixes #1145

kadmin/Makefile.am

@@ -54,7 +54,7 @@ kadmind_SOURCES =				\
 
 add_random_users_SOURCES = add-random-users.c
 
-test_util_SOURCES = test_util.c util.c
+test_util_SOURCES = test_util.c util.c random_password.c
 
 TESTS = test_util
 

kadmin/random_password.c

@@ -32,131 +32,30 @@
  */
 
 #include "kadmin_locl.h"
+#include <base64.h>
 
-/* This file defines some a function that generates a random password,
-   that can be used when creating a large amount of principals (such
-   as for a batch of students). Since this is a political matter, you
-   should think about how secure generated passwords has to be.
-
-   Both methods defined here will give you at least 55 bits of
-   entropy.
-   */
-
-/* If you want OTP-style passwords, define OTP_STYLE */
-
-#ifdef OTP_STYLE
-#include <otp.h>
-#else
-static void generate_password(char **pw, int num_classes, ...);
-#endif
+#define RANDOM_PASSWORD_BYTES 12
 
 void
 random_password(char *pw, size_t len)
 {
-#ifdef OTP_STYLE
-    {
-	OtpKey newkey;
+    unsigned char random[RANDOM_PASSWORD_BYTES];
+    char *password = NULL;
+    size_t password_len;
 
-	krb5_generate_random_block(&newkey, sizeof(newkey));
-	otp_print_stddict (newkey, pw, len);
-	strlwr(pw);
-    }
-#else
-    char *pass;
-    generate_password(&pass, 3,
-		      "abcdefghijklmnopqrstuvwxyz", 7,
-		      "ABCDEFGHIJKLMNOPQRSTUVWXYZ", 2,
-		      "@$%&*()-+=:,/<>1234567890", 1);
-    strlcpy(pw, pass, len);
-    len = strlen(pass);
-    memset_s(pass, len, 0, len);
-    free(pass);
-#endif
-}
-
-/* some helper functions */
-
-#ifndef OTP_STYLE
-/* return a random value in range 0-127 */
-static int
-RND(unsigned char *key, int keylen, int *left)
-{
-    if(*left == 0){
-	krb5_generate_random_block(key, keylen);
-	*left = keylen;
-    }
-    (*left)--;
-    return ((unsigned char*)key)[*left];
-}
-
-/* This a helper function that generates a random password with a
-   number of characters from a set of character classes.
-
-   If there are n classes, and the size of each class is Pi, and the
-   number of characters from each class is Ni, the number of possible
-   passwords are (given that the character classes are disjoint):
-
-     n             n
-   -----        /  ----  \
-   |   |  Ni    |  \     |
-   |   | Pi     |   \  Ni| !
-   |   | ---- * |   /    |
-   |   | Ni!    |  /___  |
-    i=1          \  i=1  /
-
-    Since it uses the RND function above, neither the size of each
-    class, nor the total length of the generated password should be
-    larger than 127 (without fixing RND).
-
-   */
-static void
-generate_password(char **pw, int num_classes, ...)
-{
-    struct {
-	const char *str;
-	int len;
-	int freq;
-    } *classes;
-    va_list ap;
-    int len, i;
-    unsigned char rbuf[8]; /* random buffer */
-    int rleft = 0;
-
-    *pw = NULL;
-
-    classes = malloc(num_classes * sizeof(*classes));
-    if(classes == NULL)
+    if (len == 0)
 	return;
-    va_start(ap, num_classes);
-    len = 0;
-    for(i = 0; i < num_classes; i++){
-	classes[i].str = va_arg(ap, const char*);
-	classes[i].len = strlen(classes[i].str);
-	classes[i].freq = va_arg(ap, int);
-	len += classes[i].freq;
+
+    krb5_generate_random_block(random, sizeof(random));
+    if (rk_base64url_encode(random, sizeof(random), &password) < 0 ||
+	password == NULL) {
+	memset_s(random, sizeof(random), 0, sizeof(random));
+	krb5_abortx(NULL, "Failed to encode random password");
     }
-    va_end(ap);
-    *pw = malloc(len + 1);
-    if(*pw == NULL) {
-	free(classes);
-	return;
-    }
-    for(i = 0; i < len; i++) {
-	int j;
-	int x = RND(rbuf, sizeof(rbuf), &rleft) % (len - i);
-	int t = 0;
-	for(j = 0; j < num_classes; j++) {
-	    if(x < t + classes[j].freq) {
-		(*pw)[i] = classes[j].str[RND(rbuf, sizeof(rbuf), &rleft)
-					 % classes[j].len];
-		classes[j].freq--;
-		break;
-	    }
-	    t += classes[j].freq;
-	}
-    }
-    (*pw)[len] = '\0';
-    memset_s(rbuf, sizeof(rbuf), 0, sizeof(rbuf));
-    free(classes);
+
+    password_len = strlen(password);
+    strlcpy(pw, password, len);
+    memset_s(random, sizeof(random), 0, sizeof(random));
+    memset_s(password, password_len, 0, password_len);
+    free(password);
 }
-#endif

kadmin/test_util.c

@@ -68,6 +68,49 @@ test_time(void)
     return errors;
 }
 
+static int
+test_random_password(void)
+{
+    static const char alphabet[] =
+	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
+    char pw[128];
+    char small[5];
+    char zero[1] = { 'x' };
+    size_t i, j;
+    int errors = 0;
+
+    for (i = 0; i < 128; i++) {
+	random_password(pw, sizeof(pw));
+	if (strlen(pw) != 16) {
+	    printf("wrong random password length: %lu\n",
+		   (unsigned long)strlen(pw));
+	    errors++;
+	}
+	for (j = 0; pw[j] != '\0'; j++) {
+	    if (strchr(alphabet, pw[j]) == NULL) {
+		printf("random password contains invalid byte: 0x%02x\n",
+		       (unsigned char)pw[j]);
+		errors++;
+	    }
+	}
+    }
+
+    random_password(small, sizeof(small));
+    if (strlen(small) != sizeof(small) - 1) {
+	printf("wrong short random password length: %lu\n",
+	       (unsigned long)strlen(small));
+	errors++;
+    }
+
+    random_password(zero, 0);
+    if (zero[0] != 'x') {
+	printf("zero-length random password buffer was modified\n");
+	errors++;
+    }
+
+    return errors;
+}
+
 
 int
 main(int argc, char **argv)
@@ -82,9 +125,9 @@ main(int argc, char **argv)
 
     ret = 0;
     ret += test_time();
+    ret += test_random_password();
 
     krb5_free_context(context);
 
     return ret;
 }
-