logons, crypt(), randomization and portability pain

  • Krell
  • Krell's Avatar Topic Author
  • Offline
  • Gold Boarder
  • Gold Boarder
More
1 year 3 months ago #6632 by Krell
Some time back I had an issue where the mud would crash upon logging in. It turned out to be an issue with the values that crypt() on my system was returning to strmcmp(). I resolved the crashing issue, at least for that instance, but I never was able to get my system to encrypt the users' passwords.

MUD Crash on logon - crypt() strncmp()

This past weekend, when I should have been working on my essay, I found myself instead obsessed with this problem. I guess it must have been bugging me subconciously the whole time. The version of tbaMUD I'm running is 3.67 though I see that 3.68 has now been released so I don't know if what I'm about to talk about still applies.

Anyhow, I realized that my fix above doesn't give crypt() what it needs to hash a user's password. After some time perusing OpenBSD's man crypt(3) , and reading the source code in /usr/src/lib/libc/crypt/crypt.c and /usr/src/lib/libc/crypt/bcrypt.c, I was able to piece together what the correct arguments were to pass to crypt(). Then I had to figure out a reliable way to generate a salt using OpenBSD's CSPRNG tools, like arc4random() , and put it all together! Below is the results of my labours and they work...on OpenBSD.

I've been poking around with getting my changes to work on my Debian testing machine, but there are obvious portability issues. Debian's version of gcc complains about strlcat()/strlcpy() even though the libs are on my system, I've included them in my source code and even pointed gcc to the libs. Similarly for arc4random(). I can see why the CircleMUD/tbaMUD devs decided to go about making their own randomizer for the in-game dice rolls. So I'll have to leave the build on Debian alone, at least until I get this paper handed in and probably some training modules for work as well. XD

Changes I made to interpreter.c
@@ -1318,6 +1318,8 @@
 {
   int load_result;     /* Overloaded variable */
   int player_i;
+  const char *setting = "$2b$12$";
 
   /* OasisOLC states */
   struct {
@@ -1553,7 +1554,8 @@
       write_to_output(d, "\r\nIllegal password.\r\nPassword: ");
       return;
     }
-    strncpy(GET_PASSWD(d->character), CRYPT(arg, GET_PC_NAME(d->character)), MAX_PWD_LENGTH);  /* strncpy: OK (G_P:MAX_PWD_LENGTH+1) */
+    strncpy(GET_PASSWD(d->character), CRYPT(arg, salt(setting)), MAX_PWD_LENGTH);      /* strncpy: OK (G_P:MAX_PWD_LENGTH+1) */
     *(GET_PASSWD(d->character) + MAX_PWD_LENGTH) = '\0';
 
     write_to_output(d, "\r\nPlease retype password: ");

My changes to utils.c
@@ -1533,3 +1534,76 @@
   parse_tab(buf);
   return(buf);
 }
+
+#if defined(CIRCLE_CRYPT)
+/* 
+ * Function to make a string of randomized characters
+ * sourced from another string of characters and then
+ * concatonate that to the end of a string passed to
+ * the function as a parameter.
+ *
+ * Pre: give must be a valid string of type const char that
+ * tells the server's encryption software the appropriate 
+ * settings to use.
+ */
+
+const char * salt(const char *given)
+{
+        uint32_t pwr = 6, i, size; 
+        char *rtext;
+       static char buf[MAX_STRING_LENGTH];
+        const char *letters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJ\
+                              KLMNOPQRSTUVWXYZ0123456789./";
+
+       size = strlen(letters);
+
+       CREATE(rtext, char*, size);
+
+       /* Create the random salt using a CSPRNG. */
+        for (i = 0; i < size; i++)
+                rtext[i] = letters[arc4random_uniform(pwr)];
+       rtext[i + 1] = '\0';
+
+       /* Copy string from *given to buf[]. */
+       if (strlcpy(buf, given, strlen(given) + 1) > strlen(given)) {
+               perror("Syserror: buf longer than given!");
+               exit(1);
+       }
+
+       /* Concatonate the salt to *given which will be used to
+        * create the new hash in the server's cryptographic 
+        * software. */
+       if (strlcat(buf, rtext, strlen(given) + strlen(rtext) + 1) > \
+                       (strlen(given) + strlen(rtext) + 1)) {
+               perror("Syserror: *given larger than buffer!");
+               exit(1);
+       }
+
+        DISPOSE(rtext);
+
+       return(buf);
+}
+
+/* 
+ * NULL pointers are undefined in strncmp() for many os's and 
+ * architectures. This function returns a valid pointer to the
+ * user entered string rather than an NULL pointer since NULL
+ * pointers can crash some versions of strncmp().
+ *
+ * Pre: kstr must be a valid char pointer to user input.
+ *      pstr must be a valid char pointer to password database
+ *      or setting to create new password.
+ */
+
+const char * wrap_crypt(const char *kstr, const char *pstr)
+{
+        const char *retstr;
+
+        retstr = crypt(kstr, pstr);
+
+        if (retstr)
+                return(retstr);
+        else
+                return(kstr);
+}
+#endif

My changes to utils.h
@@ -70,6 +70,12 @@
 int get_class_by_name(char *classname);
 char * convert_from_tabs(char * string);
 int count_non_protocol_chars(char * str);
+#if defined(CIRCLE_CRYPT)
+/* salt generator */
+const char * salt(const char *);
+/* crypt wrapper utility */
+const char * wrap_crypt(const char *, const char *);
+#endif
 
 /* Public functions made available form weather.c */
 void weather_and_time(int mode);
The following user(s) said Thank You: thomas

Please Log in or Create an account to join the conversation.

More
1 year 2 months ago #6708 by cunning
I still see your using crypt, which has 8 character limitation. I have been pondering the same thing generating a unique salt for each character and saving it out to the plrfiles. I was going to use mcrypt to do this, i am not thrilled about the 8 character limitation crypt has. But this is an outstanding effort.

Please Log in or Create an account to join the conversation.

  • Krell
  • Krell's Avatar Topic Author
  • Offline
  • Gold Boarder
  • Gold Boarder
More
1 year 2 months ago #6710 by Krell
Hi Cunning,

Thank you for the complement. In my local copy of the code I've already refactored a few things, including using sizeof() where appropriate in place of strlen() for example. I've also fixed up the error messages to accurately phrase the problem for the exit.

Since the local implementation of crypt() uses the Blowfish algorithm with the setting I used I don't think that the logon password is restricted to 8 characters. I have a logon password for my Imp with a password length of 12 characters and I had been testing it since. Unless I put all 12 characters in, no more and no less, I cannot logon.

Krell

Please Log in or Create an account to join the conversation.

Time to create page: 1.225 seconds