diff --git a/README b/README index 2ecc7ef..afa0fe5 100644 --- a/README +++ b/README @@ -1,3 +1,23 @@ +dgamelaunch + security +====================== + +This branch was modified to enable proper password salting, using the +openssl library and pbkdf2 password hashing function. For compatibility +purposes, you can still compile with only sqlite (which salts passwords +with themselves using an MD5 based algorithm, or with neither. + +--enable-pbkdf2 requires --enable-sqlite + +dgamelaunch *should* work with neither pbkdf2 nor sqlite, but I wasn't +able to get it to work with my Dockerfile, where these were tested. + +Tests were carried out on a Debian 8 based Docker container. + +In addition, strings that ever hold raw passwords from the user, are now +cleared out of memory as soon as unnecessary. This is done regardless of +which flags you compile with. + + dgamelaunch =========== @@ -40,6 +60,7 @@ Some options you might want give to autogen: --with-config-file=/absolute/path/to/dgamelaunch.config --enable-shmem --enable-sqlite + --enable-pbkdf2 Dgamelaunch should compile without issue on Linux, Solaris, FreeBSD 4 and 5. diff --git a/configure.ac b/configure.ac index 5b1d3bf..a3c455f 100644 --- a/configure.ac +++ b/configure.ac @@ -26,7 +26,7 @@ fi case "$(uname -s)" in Linux | *BSD) - MY_LIBS="$MY_LIBS -lutil -lcrypt" + MY_LIBS="$MY_LIBS -lutil" AC_DEFINE(NOSTREAMS, 1, [Don't use SVR4 streams support in ttyrec.]) ;; esac @@ -113,6 +113,24 @@ if test "$enable_sqlite" = yes; then fi +AC_ARG_ENABLE(pbkdf2, +[AC_HELP_STRING([--enable-pbkdf2], [Enable pbkdf2 password hashing. Requires sqlite])], +[], []) + +if test "$enable_pbkdf2" = yes; then + AC_MSG_RESULT([Enabling pbkdf2 password hashing.]) + AC_DEFINE(USE_PBKDF2,1,[Enable pbkdf2 password hashing.]) + AC_CHECK_HEADERS([openssl/rand.h], [], [AC_MSG_ERROR([openssl/rand.h not found.])], []) + AC_CHECK_HEADERS([openssl/evp.h], [], [AC_MSG_ERROR([openssl/evp.h not found.])], []) + MY_LIBS="$MY_LIBS -lcrypto" + + if test "$enable_sqlite" = no; then + AC_MSG_ERROR([sqlite must be enabled for pbkdf2 password hashing.]) + fi + + else + MY_LIBS="$MY_LIBS -lcrypt" +fi dgl_rlimit_core_default=157286400 diff --git a/dgamelaunch.c b/dgamelaunch.c index 0b17154..d246d85 100644 --- a/dgamelaunch.c +++ b/dgamelaunch.c @@ -93,6 +93,12 @@ #include #include +/* crypto stuff */ +#ifdef USE_PBKDF2 +# include +# include +#endif + extern FILE* yyin; extern int yyparse (); @@ -157,6 +163,9 @@ cpy_me(struct dg_user *me) if (me->email) tmp->email = strdup(me->email); if (me->env) tmp->env = strdup(me->env); if (me->password) tmp->password = strdup(me->password); +#ifdef USE_PBKDF2 + if (me->salt) tmp->salt = strdup(me->salt); +#endif tmp->flags = me->flags; } return tmp; @@ -1485,6 +1494,155 @@ change_email () } } + +#ifdef USE_PBKDF2 +int +changepw (int dowrite) +{ + int error = 2; + + /* A precondition is that struct `me' exists because we can be not-yet-logged-in. */ + if (!me) { + debug_write("no 'me' in changepw"); + graceful_exit (122); /* Die. */ + } + + if (me->flags & DGLACCT_PASSWD_LOCK) { + clear(); + drawbanner(&banner); + mvprintw(5, 1, "Sorry, you cannot change the password.--More--"); + dgl_getch(); + return 0; + } + + char *buf = (char *)malloc((DGL_PASSWDLEN+1) * sizeof(char)); + char *repeatbuf = (char *)malloc((DGL_PASSWDLEN+1) * sizeof(char)); + + while (error) + { + clear (); + + drawbanner (&banner); + + mvprintw (5, 1, + "Please enter a%s password. Remember that this is sent over the net", + loggedin ? " new" : ""); + mvaddstr (6, 1, + "in plaintext, so make it something new and expect it to be relatively"); + mvaddstr (7, 1, "insecure."); + mvprintw (8, 1, + "%i character max. No ':' characters. Blank line to abort.", DGL_PASSWDLEN); + mvaddstr (10, 1, "=> "); + + if (error == 1) + { + mvaddstr (15, 1, "Sorry, the passwords don't match. Try again."); + move (10, 4); + } + + refresh (); + + if (mygetnstr (buf, DGL_PASSWDLEN, 0) != OK) + { + memset_s(buf, 0, strlen(buf)); + memset_s(repeatbuf, 0, strlen(repeatbuf)); + free(buf); + free(repeatbuf); + return 0; + } + + if (*buf == '\0') + { + memset_s(buf, 0, strlen(buf)); + memset_s(repeatbuf, 0, strlen(repeatbuf)); + free(buf); + free(repeatbuf); + return 0; + } + + if (strchr (buf, ':') != NULL) { + debug_write("cannot have ':' in passwd"); + memset_s(buf, 0, strlen(buf)); + memset_s(repeatbuf, 0, strlen(repeatbuf)); + + free(buf); + free(repeatbuf); + graceful_exit (112); + } + + mvaddstr (12, 1, "And again:"); + mvaddstr (13, 1, "=> "); + + if (mygetnstr (repeatbuf, DGL_PASSWDLEN, 0) != OK) + { + memset_s(buf, 0, strlen(buf)); + memset_s(repeatbuf, 0, strlen(repeatbuf)); + free(buf); + free(repeatbuf); + return 0; + } + + if (!strcmp (buf, repeatbuf)) + { + error = 0; + memset_s(repeatbuf, 0, strlen(repeatbuf)); + free(repeatbuf); + } + else + error = 1; + } + + /* Generate salt */ + unsigned char salt[DGL_SALTLEN]; + unsigned char dk[DGL_KEYLEN]; + + char asalt[2*DGL_SALTLEN+1]; + char adk[2*DGL_KEYLEN+1]; + + if( !RAND_bytes(salt,DGL_SALTLEN) ){ + memset_s(buf, 0, strlen(buf)); + free(buf); + return 0; + } + + if(!PKCS5_PBKDF2_HMAC_SHA1(buf, strlen(buf), salt, DGL_SALTLEN, DGL_ITERATION, DGL_KEYLEN, dk)){ + memset_s(buf, 0, strlen(buf)); + free(buf); + return 0; + } + + clear (); + drawbanner (&banner); + + if(!hex_to_ascii(salt, asalt, DGL_SALTLEN)) + { + return 0; + } + + if(!hex_to_ascii(dk, adk, DGL_KEYLEN)) + { + return 0; + } + + memset_s(buf, 0, strlen(buf)); + free(buf); + + + free(me->salt); + free(me->password); + + me->salt = strdup(asalt); + me->password = strdup(adk); + + + if (dowrite) + writefile (0); + + return 1; +} + + +#else int changepw (int dowrite) { @@ -1531,13 +1689,13 @@ changepw (int dowrite) refresh (); if (mygetnstr (buf, DGL_PASSWDLEN, 0) != OK) - return 0; + return 0; if (*buf == '\0') return 0; if (strchr (buf, ':') != NULL) { - debug_write("cannot have ':' in passwd"); + debug_write("cannot have ':' in passwd"); graceful_exit (112); } @@ -1545,7 +1703,7 @@ changepw (int dowrite) mvaddstr (13, 1, "=> "); if (mygetnstr (repeatbuf, DGL_PASSWDLEN, 0) != OK) - return 0; + return 0; if (!strcmp (buf, repeatbuf)) error = 0; @@ -1562,6 +1720,12 @@ changepw (int dowrite) return 1; } +#endif + + + + + /* ************************************************************* */ void @@ -1793,6 +1957,7 @@ initcurses () void autologin (char* user, char *pass) { + struct dg_user *tmp; tmp = userexist(user, 0); if (tmp) { @@ -1808,7 +1973,9 @@ autologin (char* user, char *pass) void loginprompt (int from_ttyplay) { - char user_buf[DGL_PLAYERNAMELEN+1], pw_buf[DGL_PASSWDLEN+2]; + char user_buf[DGL_PLAYERNAMELEN+1]; + char* pw_buf; + int error = 2; loggedin = 0; @@ -1855,22 +2022,30 @@ loginprompt (int from_ttyplay) drawbanner (&banner); + pw_buf = (char*)malloc((DGL_PASSWDLEN+2)* sizeof(char)); + mvaddstr (5, 1, "Please enter your password."); mvaddstr (7, 1, "=> "); refresh (); - if (mygetnstr (pw_buf, DGL_PASSWDLEN, 0) != OK) + if (mygetnstr (pw_buf, DGL_PASSWDLEN, 0) != OK){ + memset_s(pw_buf, 0, strlen(pw_buf)); + free(pw_buf); return; + } if (passwordgood (pw_buf)) { - if (me->flags & DGLACCT_LOGIN_LOCK) { + memset_s(pw_buf, 0, strlen(pw_buf)); + free(pw_buf); + + if (me->flags & DGLACCT_LOGIN_LOCK) { clear (); mvprintw(5, 1, "Sorry, that account has been banned.--More--"); dgl_getch(); return; - } + } loggedin = 1; if (from_ttyplay) @@ -1881,6 +2056,8 @@ loginprompt (int from_ttyplay) } else { + memset_s(pw_buf, 0, strlen(pw_buf)); + free(pw_buf); me = NULL; if (from_ttyplay == 1) { @@ -1965,9 +2142,7 @@ newuser () error = 1; } - if (strlen (buf) < 2) - error = 1; - + if (strlen (buf) < 2) error = 1; if (strlen (buf) == 0) { free(me); @@ -2048,6 +2223,33 @@ newuser () /* ************************************************************* */ +/* crypto changes */ +#ifdef USE_PBKDF2 +int +passwordgood (char *cpw) +{ + assert (me != NULL); + + unsigned char testdk[DGL_KEYLEN+1]; + unsigned char dk[DGL_KEYLEN+1]; + unsigned char salt[DGL_SALTLEN+1]; + + ascii_to_hex(me->salt, salt, DGL_SALTLEN); + ascii_to_hex(me->password, dk, DGL_KEYLEN); + + if(!PKCS5_PBKDF2_HMAC_SHA1(cpw, strlen(cpw), salt, DGL_SALTLEN, DGL_ITERATION, DGL_KEYLEN, testdk)){ + /* proper error handling needed */ + return 0; + } + + if(!strncmp(dk,testdk,DGL_KEYLEN)){ + return 1; + } + + return 0; +} + +#else int passwordgood (char *cpw) { @@ -2058,19 +2260,21 @@ passwordgood (char *cpw) if (crypted == NULL) return 0; -#ifdef USE_SQLITE3 +# ifdef USE_SQLITE3 if (!strncmp (crypted, me->password, DGL_PASSWDLEN)) return 1; -#else +# else if (!strncmp (cpw, me->password, DGL_PASSWDLEN)) return 1; -#endif +# endif return 0; } +#endif + /* ************************************************************* */ int @@ -2244,6 +2448,10 @@ userexist_callback(void *NotUsed, int argc, char **argv, char **colname) userexist_tmp_me->email = strdup(argv[i]); else if (!strcmp(colname[i], "env")) userexist_tmp_me->env = strdup(argv[i]); +# ifdef USE_PBKDF2 + else if (!strcmp(colname[i], "salt")) + userexist_tmp_me->salt = strdup(argv[i]); +# endif else if (!strcmp(colname[i], "password")) userexist_tmp_me->password = strdup(argv[i]); else if (!strcmp(colname[i], "flags")) @@ -2286,6 +2494,9 @@ userexist (char *cname, int isnew) free(userexist_tmp_me->email); free(userexist_tmp_me->env); free(userexist_tmp_me->password); +# ifdef USE_PBKDF2 + free(userexist_tmp_me->salt); +# endif free(userexist_tmp_me); userexist_tmp_me = NULL; } @@ -2455,13 +2666,24 @@ writefile (int requirenew) int ret, retry = 10; char *qbuf; + + # ifdef USE_PBKDF2 + if (requirenew) { + qbuf = sqlite3_mprintf("insert into dglusers (username, email, env, salt, password, flags) values ('%q', '%q', '%q', '%q', '%q', %li)", me->username, me->email, me->env, me->salt, me->password, me->flags); + } else { + qbuf = sqlite3_mprintf("update dglusers set username='%q', email='%q', env='%q', salt='%q', password='%q', flags=%li where id=%i", me->username, me->email, me->env, me->salt, me->password, me->flags, me->id); + } + # else if (requirenew) { - qbuf = sqlite3_mprintf("insert into dglusers (username, email, env, password, flags) values ('%q', '%q', '%q', '%q', %li)", me->username, me->email, me->env, me->password, me->flags); + qbuf = sqlite3_mprintf("insert into dglusers (username, email, env, password, flags) values ('%q', '%q', '%q', '%q', %li)", me->username, me->email, me->env, me->password, me->flags); } else { - qbuf = sqlite3_mprintf("update dglusers set username='%q', email='%q', env='%q', password='%q', flags=%li where id=%i", me->username, me->email, me->env, me->password, me->flags, me->id); + qbuf = sqlite3_mprintf("update dglusers set username='%q', email='%q', env='%q', password='%q', flags=%li where id=%i", me->username, me->email, me->env, me->password, me->flags, me->id); } + + # endif + ret = sqlite3_open(globalconfig.passwd, &db); if (ret) { sqlite3_close(db); @@ -2976,3 +3198,60 @@ main (int argc, char** argv) return 1; } + + +//converts ascii representation of hash to bytes +//in unsigned char array +int ascii_to_hex(char *input, unsigned char* output, int keyLen) +{ + + char chunk[2]; + int i; + + if(strlen(input) < keyLen*2) + { + return 0; + } + + for (i =0; i < keyLen; i++) + { + + chunk[0] = input[2*i]; + chunk[1] = input[2*i+1]; + sprintf(output + i, "%c", (unsigned char)strtol(chunk, NULL, 16)); + + } + + return 1; + +} + +//converts byte representation of hash to ascii +//representation in char array +int hex_to_ascii(unsigned char* input, char* output, int keyLen) +{ + + int i; + + for(i = 0; i < keyLen; i++) + { + sprintf(output + (i * 2), "%02x", input[i]); + + } + output[2*keyLen] = '\0'; + + return 1; + +} + + +int memset_s(void *v, int c, size_t n) { + if (v == NULL) return EINVAL; + + volatile unsigned char *p = v; + while (n--) { + *p++ = c; + } + + return 0; +} diff --git a/dgamelaunch.h b/dgamelaunch.h index e298adf..46e5843 100644 --- a/dgamelaunch.h +++ b/dgamelaunch.h @@ -20,9 +20,16 @@ #define dglsign(x) (x < 0 ? -1 : (x > 0 ? 1 : 0)) #define DGL_PLAYERNAMELEN 30 /* max. length of player name */ -#define DGL_PASSWDLEN 20 /* max. length of passwords */ +#define DGL_PASSWDLEN 32 /* max. length of passwords */ #define DGL_MAILMSGLEN 80 /* max. length of mail message */ +/* crypto stuff */ +#ifdef USE_PBKDF2 +# define DGL_SALTLEN 32 +# define DGL_ITERATION 200000 +# define DGL_KEYLEN 32 +#endif + #define DGL_MAXWATCHCOLS 10 #define DGL_BANNER_LINELEN 256 /* max. length of banner lines*/ @@ -131,6 +138,9 @@ struct dg_user char *email; char *env; char *password; +#ifdef USE_PBKDF2 + char *salt; +#endif int flags; /* dgl_acct_flag bitmask */ }; @@ -364,3 +374,8 @@ extern size_t strlcat (char *dst, const char *src, size_t siz); extern int mygetnstr(char *buf, int maxlen, int doecho); #endif + +/* crypto stuff */ +extern int ascii_to_byte(char *input, unsigned char* output, int keyLen); +extern int byte_to_ascii(unsigned char* input, char* output, int keyLen); +extern int memset_s(void *v, int c, size_t n); diff --git a/dgl-create-chroot b/dgl-create-chroot index a24a19d..86155de 100755 --- a/dgl-create-chroot +++ b/dgl-create-chroot @@ -104,7 +104,7 @@ if [ -n "$SQLITE_DBFILE" ]; then echo "Creating SQLite database at $SQLITE_DBFILE" SQLITE_DBFILE="`echo ${SQLITE_DBFILE%/}`" SQLITE_DBFILE="`echo ${SQLITE_DBFILE#/}`" - sqlite3 "$CHROOT/$SQLITE_DBFILE" "create table dglusers (id integer primary key, username text, email text, env text, password text, flags integer);" + sqlite3 "$CHROOT/$SQLITE_DBFILE" "create table dglusers (id integer primary key, username text, email text, env text, salt text, password text, flags integer);" chown "$USRGRP" "$CHROOT/$SQLITE_DBFILE" fi fi